Determine the large limit of the quicklist node based on fill#12659
Determine the large limit of the quicklist node based on fill#12659oranagra merged 16 commits intoredis:unstablefrom
Conversation
|
@imchuncai Feel free to share your thoughts. |
oranagra
left a comment
oranagra
left a comment
There was a problem hiding this comment.
concept LGTM.
indeed there's no reason nowadays to create a listpack for an isolated item, we can just as well store it as plain.
didn't review the tests yet.
i don't understand the comment above about 2GB.
AFAICT, isolated nodes creates when > 8k (when the fill is positive), or 64k (when fill is negative)
|
@oranagra When fill is -5, we first add 32k 2byte elements to a quicklist node, and then replace them one by one with (64k -1) bytes elements, since these elements do not exceed the 64k limit, they can still be replaced successfully, and we will end up with a 32k*(64k-1) quicklist node(2GB). |
|
@sundb what's the status here? let me know when you consider it ready. |
i still need double checking it, please wait me for a while. |
After redis#11303, list will exit with listpack when the number of quicklist nodes are less than 1, cause some list related tests to be always run in listpack encoding. Now let them being run in both encoding.
|
@oranagra It's ready, pelase have a look. |
…12659) Following redis#12568 In issue redis#9357, when inserting an element larger than 1GB, we currently store it in a plain node instead of a listpack. Presently, when we insert an element that exceeds the maximum size of a packed node, it cannot be accommodated in any other nodes, thus ending up isolated like a large element. I.e. it's a node with only one element, but it's listpack encoded rather than a plain buffer. This PR lowers the threshold for considering an element as 'large' from 1GB to the maximum size of a node. While this change doesn't completely resolve the bug mentioned in the previous PR, it does mitigate its potential impact. As a result of this change, we can now only use LSET to replace an element with another element that falls below the maximum size threshold. In the worst-case scenario, with a fill of -5, the largest packed node we can create is 2GB (32k * 64k): * 32k: The smallest element in a listpack is 2 bytes, which allows us to store up to 32k elements. * 64k: This is the maximum size for a single quicklist node. ## Others To fully fix redis#9357, we need more work, as discussed in redis#12568, when we insert an element into a quicklistNode, it may be created in a new node, put into another node, or merged, and we can't correctly delete the node that was supposed to be deleted. I'm not sure it's worth it, since it involves a lot of modifications.
2 participants
Following #12568
In issue #9357, when inserting an element larger than 1GB, we currently store it in a plain node instead of a listpack.
Presently, when we insert an element that exceeds the maximum size of a packed node, it cannot be accommodated in any other nodes, thus ending up isolated like a large element.
I.e. it's a node with only one element, but it's listpack encoded rather than a plain buffer.
This PR lowers the threshold for considering an element as 'large' from 1GB to the maximum size of a node.
While this change doesn't completely resolve the bug mentioned in the previous PR, it does mitigate its potential impact.
As a result of this change, we can now only use LSET to replace an element with another element that falls below the maximum size threshold.
In the worst-case scenario, with a fill of -5, the largest packed node we can create is 2GB (32k * 64k):
Others
To fully fix #9357, we need more work, as discussed in #12568, when we insert an element into a quicklistNode, it may be created in a new node, put into another node, or merged, and we can't correctly delete the node that was supposed to be deleted.
I'm not sure it's worth it, since it involves a lot of modifications.