From 3e3e7ceb9b75622f28f093974656d3eb19c6abb0 Mon Sep 17 00:00:00 2001 From: Binbin Date: Thu, 8 Feb 2024 17:23:50 +0800 Subject: [PATCH] Fix SORT STORE quicklist with the right options We forgot to call quicklistSetOptions after createQuicklistObject, in the sort store scenario, we will create a quicklist with incorrect fill or compress options. This PR adds fill and depth parameters to createQuicklistObject to specify that options need to be set after creating a quicklist. This closes #12871. --- src/object.c | 3 ++- src/rdb.c | 8 ++------ src/server.h | 2 +- src/sort.c | 5 ++--- tests/unit/sort.tcl | 12 ++++++++++++ 5 files changed, 19 insertions(+), 11 deletions(-) diff --git a/src/object.c b/src/object.c index 054f7d348b0..169e7ada40f 100644 --- a/src/object.c +++ b/src/object.c @@ -232,10 +232,11 @@ robj *dupStringObject(const robj *o) { } } -robj *createQuicklistObject(void) { +robj *createQuicklistObject(int fill, int depth) { quicklist *l = quicklistCreate(); robj *o = createObject(OBJ_LIST,l); o->encoding = OBJ_ENCODING_QUICKLIST; + quicklistSetOptions(o->ptr, fill, depth); return o; } diff --git a/src/rdb.c b/src/rdb.c index 8bd630bf5ac..48a0c61c186 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -1862,9 +1862,7 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error) { if ((len = rdbLoadLen(rdb,NULL)) == RDB_LENERR) return NULL; if (len == 0) goto emptykey; - o = createQuicklistObject(); - quicklistSetOptions(o->ptr, server.list_max_listpack_size, - server.list_compress_depth); + o = createQuicklistObject(server.list_max_listpack_size, server.list_compress_depth); /* Load every single element of the list */ while(len--) { @@ -2174,9 +2172,7 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error) { if ((len = rdbLoadLen(rdb,NULL)) == RDB_LENERR) return NULL; if (len == 0) goto emptykey; - o = createQuicklistObject(); - quicklistSetOptions(o->ptr, server.list_max_listpack_size, - server.list_compress_depth); + o = createQuicklistObject(server.list_max_listpack_size, server.list_compress_depth); uint64_t container = QUICKLIST_NODE_CONTAINER_PACKED; while (len--) { unsigned char *lp; diff --git a/src/server.h b/src/server.h index edc93087426..3e352491b89 100644 --- a/src/server.h +++ b/src/server.h @@ -2771,7 +2771,7 @@ robj *createStringObjectFromLongLong(long long value); robj *createStringObjectFromLongLongForValue(long long value); robj *createStringObjectFromLongLongWithSds(long long value); robj *createStringObjectFromLongDouble(long double value, int humanfriendly); -robj *createQuicklistObject(void); +robj *createQuicklistObject(int fill, int depth); robj *createListListpackObject(void); robj *createSetObject(void); robj *createIntsetObject(void); diff --git a/src/sort.c b/src/sort.c index bef260555a5..bd1f10064da 100644 --- a/src/sort.c +++ b/src/sort.c @@ -304,8 +304,7 @@ void sortCommandGeneric(client *c, int readonly) { if (sortval) incrRefCount(sortval); else - sortval = createQuicklistObject(); - + sortval = createQuicklistObject(server.list_max_listpack_size, server.list_compress_depth); /* When sorting a set with no sort specified, we must sort the output * so the result is consistent across scripting and replication. @@ -557,7 +556,7 @@ void sortCommandGeneric(client *c, int readonly) { } else { /* We can't predict the size and encoding of the stored list, we * assume it's a large list and then convert it at the end if needed. */ - robj *sobj = createQuicklistObject(); + robj *sobj = createQuicklistObject(server.list_max_listpack_size, server.list_compress_depth); /* STORE option specified, set the sorting result as a List object */ for (j = start; j <= end; j++) { diff --git a/tests/unit/sort.tcl b/tests/unit/sort.tcl index eade6ea341f..ce209410729 100644 --- a/tests/unit/sort.tcl +++ b/tests/unit/sort.tcl @@ -356,6 +356,18 @@ foreach command {SORT SORT_RO} { } } } + + test {SORT STORE quicklist with the right options} { + set origin_config [config_get_set list-max-listpack-size -1] + r del lst lst_dst + r config set list-max-listpack-size -1 + r config set list-compress-depth 12 + r lpush lst {*}[split [string repeat "1" 6000] ""] + r sort lst store lst_dst + assert_encoding quicklist lst_dst + assert_match "*ql_listpack_max:-1 ql_compressed:1*" [r debug object lst_dst] + config_set list-max-listpack-size $origin_config + } {} {needs:debug} } start_cluster 1 0 {tags {"external:skip cluster sort"}} {