summaryrefslogtreecommitdiff
path: root/src/t_list.c
diff options
context:
space:
mode:
authorsundb <sundbcn@gmail.com>2021-11-29 13:57:01 +0800
committerGitHub <noreply@github.com>2021-11-29 07:57:01 +0200
commit494ee2f1fc5cf1687b302a95e49003573dc375d5 (patch)
tree0b7869db90fd5d1d4662bd9913482e8fdb257bed /src/t_list.c
parent8759c1e14bafd026ddc8a097b3fbe2aa914b7578 (diff)
downloadredis-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.c15
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");
}