Unverified Commit 32e7b46a authored by sundb's avatar sundb Committed by GitHub
Browse files

Fix quicklist node not being recompressed correctly after inserting a new node...

Fix quicklist node not being recompressed correctly after inserting a new node before or after it (#10120)

### Describe
Fix crash found by CI, Introduced by #9849.
When we do any operation on the quicklist, we should make sure that all nodes
of the quicklist should not be in the recompressed state.

### Issues
This PR fixes two issues with incorrect recompression.
1. The current quicklist node is full and the previous node isn't full,
    the current node is not recompressed correctly after inserting elements into the previous node.
2. The current quicklist node is full and the next node isn't full,
    the current node is not recompressed correctly after inserting elements into the next node.

### Test
Add two tests to cover incorrect compression issues.

### Other
Fix unittest test failure caused by assertion introduced by #9849.
parent 7da7d2aa
......@@ -1018,6 +1018,7 @@ REDIS_STATIC void _quicklistInsert(quicklistIter *iter, quicklistEntry *entry,
new_node->count++;
quicklistNodeUpdateSz(new_node);
quicklistRecompressOnly(new_node);
quicklistRecompressOnly(node);
} else if (full && at_head && avail_prev && !after) {
/* If we are: at head, previous has free space, and inserting before:
* - insert entry at tail of previous node. */
......@@ -1028,6 +1029,7 @@ REDIS_STATIC void _quicklistInsert(quicklistIter *iter, quicklistEntry *entry,
new_node->count++;
quicklistNodeUpdateSz(new_node);
quicklistRecompressOnly(new_node);
quicklistRecompressOnly(node);
} else if (full && ((at_tail && !avail_next && after) ||
(at_head && !avail_prev && !after))) {
/* If we are: full, and our prev/next has no available space, then:
......@@ -3166,6 +3168,11 @@ int quicklistTest(int argc, char *argv[], int flags) {
TEST("compress and decompress quicklist listpack node") {
quicklistNode *node = quicklistCreateNode();
node->entry = lpNew(0);
/* Just to avoid triggering the assertion in __quicklistCompressNode(),
* it disables the passing of quicklist head or tail node. */
node->prev = quicklistCreateNode();
node->next = quicklistCreateNode();
/* Create a rand string */
size_t sz = (1 << 25); /* 32MB per one entry */
......@@ -3185,6 +3192,8 @@ int quicklistTest(int argc, char *argv[], int flags) {
}
zfree(s);
zfree(node->prev);
zfree(node->next);
zfree(node->entry);
zfree(node);
}
......@@ -3199,6 +3208,11 @@ int quicklistTest(int argc, char *argv[], int flags) {
quicklistNode *node = __quicklistCreatePlainNode(s, sz);
/* Just to avoid triggering the assertion in __quicklistCompressNode(),
* it disables the passing of quicklist head or tail node. */
node->prev = quicklistCreateNode();
node->next = quicklistCreateNode();
long long start = mstime();
assert(__quicklistCompressNode(node));
assert(__quicklistDecompressNode(node));
......@@ -3207,6 +3221,8 @@ int quicklistTest(int argc, char *argv[], int flags) {
assert(memcmp(node->entry, "helloworld", 10) == 0);
assert(memcmp(node->entry + sz - 10, "1234567890", 10) == 0);
zfree(node->prev);
zfree(node->next);
zfree(node->entry);
zfree(node);
}
......
......@@ -93,6 +93,32 @@ start_server {
r ping
}
test {LINSERT correctly recompress full quicklistNode after inserting a element before it} {
r del key
config_set list-compress-depth 1
r rpush key b
r rpush key c
r lset key -1 [string repeat x 8192]"969"
r lpush key a
r rpush key d
r linsert key before b f
r rpop key
r ping
}
test {LINSERT correctly recompress full quicklistNode after inserting a element after it} {
r del key
config_set list-compress-depth 1
r rpush key b
r rpush key c
r lset key 0 [string repeat x 8192]"969"
r lpush key a
r rpush key d
r linsert key after c f
r lpop key
r ping
}
foreach comp {2 1 0} {
set cycles 1000
if {$::accurate} { set cycles 10000 }
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment