diff options
author | sundb <sundbcn@gmail.com> | 2021-11-29 13:57:01 +0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-11-29 07:57:01 +0200 |
commit | 494ee2f1fc5cf1687b302a95e49003573dc375d5 (patch) | |
tree | 0b7869db90fd5d1d4662bd9913482e8fdb257bed /src/t_list.c | |
parent | 8759c1e14bafd026ddc8a097b3fbe2aa914b7578 (diff) | |
download | redis-494ee2f1fc5cf1687b302a95e49003573dc375d5.tar.gz |
Fix abnormal compression due to out-of-control recompress (#9849)
This pr is following #9779 .
## Describe of feature
Now when we turn on the `list-compress-depth` configuration, the list will compress
the ziplist between `[list-compress-depth, -list-compress-depth]`.
When we need to use the compressed data, we will first decompress it, then use it,
and finally compress it again.
It's controlled by `quicklistNode->recompress`, which is designed to avoid the need to
re-traverse the entire quicklist for compression after each decompression, we only need
to recompress the quicklsitNode being used.
In order to ensure the correctness of recompressing, we should normally let
quicklistDecompressNodeForUse and quicklistCompress appear in pairs, otherwise,
it may lead to the head and tail being compressed or the middle ziplist not being
compressed correctly, which is exactly the problem this pr needs to solve.
## Solution
1. Reset `quicklistIter` after insert and replace.
The quicklist node will be compressed in `quicklistInsertAfter`, `quicklistInsertBefore`,
`quicklistReplaceAtIndex`, so we can safely reset the quicklistIter to avoid it being used again
2. `quicklistIndex` will return an iterator that can be used to recompress the current node after use.
## Test
1. In the `Stress Tester for #3343-Similar Errors` test, when the server crashes or when
`valgrind` or `asan` error is detected, print violating commands.
2. Add a crash test due to wrongly recompressing after `lrem`.
3. Remove `insert before with 0 elements` and `insert after with 0 elements`,
Now we forbid any operation on an NULL quicklistIter.
Diffstat (limited to 'src/t_list.c')
-rw-r--r-- | src/t_list.c | 15 |
1 files changed, 7 insertions, 8 deletions
diff --git a/src/t_list.c b/src/t_list.c index 7a1733bfc..84855ebe2 100644 --- a/src/t_list.c +++ b/src/t_list.c @@ -112,7 +112,7 @@ void listTypeSetIteratorDirection(listTypeIterator *li, unsigned char direction) /* Clean up the iterator. */ void listTypeReleaseIterator(listTypeIterator *li) { - zfree(li->iter); + quicklistReleaseIterator(li->iter); zfree(li); } @@ -154,11 +154,9 @@ void listTypeInsert(listTypeEntry *entry, robj *value, int where) { sds str = value->ptr; size_t len = sdslen(str); if (where == LIST_TAIL) { - quicklistInsertAfter((quicklist *)entry->entry.quicklist, - &entry->entry, str, len); + quicklistInsertAfter(entry->li->iter, &entry->entry, str, len); } else if (where == LIST_HEAD) { - quicklistInsertBefore((quicklist *)entry->entry.quicklist, - &entry->entry, str, len); + quicklistInsertBefore(entry->li->iter, &entry->entry, str, len); } decrRefCount(value); } else { @@ -172,8 +170,7 @@ void listTypeReplace(listTypeEntry *entry, robj *value) { value = getDecodedObject(value); sds str = value->ptr; size_t len = sdslen(str); - quicklistReplaceEntry((quicklist *)entry->entry.quicklist, - &entry->entry, str, len); + quicklistReplaceEntry(entry->li->iter, &entry->entry, str, len); decrRefCount(value); } else { serverPanic("Unknown list encoding"); @@ -348,7 +345,8 @@ void lindexCommand(client *c) { if (o->encoding == OBJ_ENCODING_QUICKLIST) { quicklistEntry entry; - if (quicklistIndex(o->ptr, index, &entry)) { + quicklistIter *iter = quicklistGetIteratorEntryAtIdx(o->ptr, index, &entry); + if (iter) { if (entry.value) { addReplyBulkCBuffer(c, entry.value, entry.sz); } else { @@ -357,6 +355,7 @@ void lindexCommand(client *c) { } else { addReplyNull(c); } + quicklistReleaseIterator(iter); } else { serverPanic("Unknown list encoding"); } |