gvsafronov/futriix
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

__quicklistCompress may compress more node than required (#8311)

Browse Source 
When a quicklist has quicklist->compress * 2 nodes, then call
__quicklistCompress, all nodes will be decompressed and the middle
two nodes will be recompressed again. This violates the fact that
quicklist->compress * 2 nodes are uncompressed. It's harmless
because when visit a node, we always try to uncompress node first.
This only happened when a quicklist has quicklist->compress * 2 + 1
nodes, then delete a node. For other scenarios like insert node and
iterate this will not happen.
This commit is contained in:
Huang Zhw 2021-03-09 03:43:09 +08:00 committed by GitHub
parent 817894c012
commit 9b4edfdf08
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 43 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
src/quicklist.c
View File

@ -315,7 +315,9 @@ REDIS_STATIC void __quicklistCompress(const quicklist *quicklist,
if (forward == node || reverse == node) if (forward == node || reverse == node)
in_depth = 1; in_depth = 1;
if (forward == reverse) /* We passed into compress depth of opposite side of the quicklist
* so there's no need to compress anything and we can exit. */
if (forward == reverse || forward->next == reverse)
return; return;
forward = forward->next; forward = forward->next;
@ -325,11 +327,9 @@ REDIS_STATIC void __quicklistCompress(const quicklist *quicklist,
if (!in_depth) if (!in_depth)
quicklistCompressNode(node); quicklistCompressNode(node);
if (depth > 2) {
/* At this point, forward and reverse are one node beyond depth */ /* At this point, forward and reverse are one node beyond depth */
quicklistCompressNode(forward); quicklistCompressNode(forward);
quicklistCompressNode(reverse); quicklistCompressNode(reverse);
}
} }
#define quicklistCompress(_ql, _node) \ #define quicklistCompress(_ql, _node) \
@ -380,10 +380,11 @@ REDIS_STATIC void __quicklistInsertNode(quicklist *quicklist,
quicklist->head = quicklist->tail = new_node; quicklist->head = quicklist->tail = new_node;
} }
/* Update len first, so in __quicklistCompress we know exactly len */
quicklist->len++;
if (old_node) if (old_node)
quicklistCompress(quicklist, old_node); quicklistCompress(quicklist, old_node);
quicklist->len++;
} }
/* Wrappers for node inserting around existing node. */ /* Wrappers for node inserting around existing node. */
@ -602,15 +603,16 @@ REDIS_STATIC void __quicklistDelNode(quicklist *quicklist,
quicklist->head = node->next; quicklist->head = node->next;
} }
/* Update len first, so in __quicklistCompress we know exactly len */
quicklist->len--;
quicklist->count -= node->count;
/* If we deleted a node within our compress depth, we /* If we deleted a node within our compress depth, we
* now have compressed nodes needing to be decompressed. */ * now have compressed nodes needing to be decompressed. */
__quicklistCompress(quicklist, NULL); __quicklistCompress(quicklist, NULL);
quicklist->count -= node->count;
zfree(node->zl); zfree(node->zl);
zfree(node); zfree(node);
quicklist->len--;
} }
/* Delete one entry from list given the node for the entry and a pointer /* Delete one entry from list given the node for the entry and a pointer
@ -2706,6 +2708,14 @@ int quicklistTest(int argc, char *argv[]) {
quicklistPushHead(ql, genstr("hello HEAD", i + 1), 64); quicklistPushHead(ql, genstr("hello HEAD", i + 1), 64);
} }
for (int step = 0; step < 2; step++) {
/* test remove node */
if (step == 1) {
for (int i = 0; i < list_sizes[list] / 2; i++) {
quicklistPop(ql, QUICKLIST_HEAD, NULL, NULL, NULL);
quicklistPop(ql, QUICKLIST_TAIL, NULL, NULL, NULL);
}
}
quicklistNode *node = ql->head; quicklistNode *node = ql->head;
unsigned int low_raw = ql->compress; unsigned int low_raw = ql->compress;
unsigned int high_raw = ql->len - ql->compress; unsigned int high_raw = ql->len - ql->compress;
@ -2730,6 +2740,8 @@ int quicklistTest(int argc, char *argv[]) {
} }
} }
} }
}
quicklistRelease(ql); quicklistRelease(ql);
} }
} }