From 70b4b320ae1d04dd087ae9336bdd962a9615fbfe Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Fri, 11 Jun 2010 14:51:59 +0200 Subject: [PATCH] check if the list encoding needs to be changed on LPUSHX, RPUSHX, LINSERT --- redis.c | 21 +++++- tests/unit/type/list.tcl | 135 ++++++++++++++++++++++++--------------- 2 files changed, 102 insertions(+), 54 deletions(-) diff --git a/redis.c b/redis.c index ca4d9f870..efaeb0f1c 100644 --- a/redis.c +++ b/redis.c @@ -4926,7 +4926,7 @@ static void listTypePush(robj *subject, robj *value, int where) { /* Check if we need to convert the ziplist */ listTypeTryConversion(subject,value); if (subject->encoding == REDIS_ENCODING_ZIPLIST && - ziplistLen(subject->ptr) > server.list_max_ziplist_entries) + ziplistLen(subject->ptr) >= server.list_max_ziplist_entries) listTypeConvert(subject,REDIS_ENCODING_LIST); if (subject->encoding == REDIS_ENCODING_ZIPLIST) { @@ -5214,6 +5214,7 @@ static void pushxGenericCommand(redisClient *c, robj *refval, robj *val, int whe robj *subject; listTypeIterator *iter; listTypeEntry entry; + int inserted = 0; if ((subject = lookupKeyReadOrReply(c,c->argv[1],shared.czero)) == NULL || checkType(c,subject,REDIS_LIST)) return; @@ -5227,20 +5228,36 @@ static void pushxGenericCommand(redisClient *c, robj *refval, robj *val, int whe * last argument of the multi-bulk LINSERT. */ redisAssert(refval->encoding == REDIS_ENCODING_RAW); + /* We're not sure if this value can be inserted yet, but we cannot + * convert the list inside the iterator. We don't want to loop over + * the list twice (once to see if the value can be inserted and once + * to do the actual insert), so we assume this value can be inserted + * and convert the ziplist to a regular list if necessary. */ + listTypeTryConversion(subject,val); + /* Seek refval from head to tail */ iter = listTypeInitIterator(subject,0,REDIS_TAIL); while (listTypeNext(iter,&entry)) { if (listTypeEqual(&entry,refval)) { listTypeInsert(&entry,val,where); + inserted = 1; break; } } listTypeReleaseIterator(iter); + + if (inserted) { + /* Check if the length exceeds the ziplist length threshold. */ + if (subject->encoding == REDIS_ENCODING_ZIPLIST && + ziplistLen(subject->ptr) > server.list_max_ziplist_entries) + listTypeConvert(subject,REDIS_ENCODING_LIST); + server.dirty++; + } } else { listTypePush(subject,val,where); + server.dirty++; } - server.dirty++; addReplyUlong(c,listTypeLength(subject)); } diff --git a/tests/unit/type/list.tcl b/tests/unit/type/list.tcl index 81eabd3a9..052cde0af 100644 --- a/tests/unit/type/list.tcl +++ b/tests/unit/type/list.tcl @@ -52,58 +52,6 @@ start_server { assert_equal $large_value [r lindex mylist2 2] } - test {LPUSHX, RPUSHX, LPUSHXAFTER, RPUSHXAFTER - ziplist} { - r del xlist - assert_equal 0 [r lpushx xlist a] - assert_equal 0 [r rpushx xlist a] - assert_equal 1 [r rpush xlist b] - assert_equal 2 [r rpush xlist c] - assert_equal 3 [r rpushx xlist d] - assert_equal 4 [r lpushx xlist a] - assert_encoding ziplist xlist - assert_equal {a b c d} [r lrange xlist 0 10] - - assert_equal 5 [r linsert xlist before c zz] - assert_equal {a b zz c d} [r lrange xlist 0 10] - assert_equal 6 [r linsert xlist after c yy] - assert_equal {a b zz c yy d} [r lrange xlist 0 10] - assert_equal 7 [r linsert xlist after d dd] - assert_equal 7 [r linsert xlist after bad ddd] - assert_equal {a b zz c yy d dd} [r lrange xlist 0 10] - assert_equal 8 [r linsert xlist before a aa] - assert_equal 8 [r linsert xlist before bad aaa] - assert_equal {aa a b zz c yy d dd} [r lrange xlist 0 10] - assert_equal 9 [r linsert xlist before aa 42] - assert_equal 42 [r lrange xlist 0 0] - } - - test {LPUSHX, RPUSHX, LPUSHXAFTER, RPUSHXAFTER - regular list} { - set large_value "aaaaaaaaaaaaaaaaa" - - r del xlist - assert_equal 0 [r lpushx xlist a] - assert_equal 0 [r rpushx xlist a] - assert_equal 1 [r rpush xlist $large_value] - assert_equal 2 [r rpush xlist c] - assert_equal 3 [r rpushx xlist d] - assert_equal 4 [r lpushx xlist a] - assert_encoding list xlist - assert_equal {a aaaaaaaaaaaaaaaaa c d} [r lrange xlist 0 10] - - assert_equal 5 [r linsert xlist before c zz] - assert_equal {a aaaaaaaaaaaaaaaaa zz c d} [r lrange xlist 0 10] - assert_equal 6 [r linsert xlist after c yy] - assert_equal {a aaaaaaaaaaaaaaaaa zz c yy d} [r lrange xlist 0 10] - assert_equal 7 [r linsert xlist after d dd] - assert_equal 7 [r linsert xlist after bad ddd] - assert_equal {a aaaaaaaaaaaaaaaaa zz c yy d dd} [r lrange xlist 0 10] - assert_equal 8 [r linsert xlist before a aa] - assert_equal 8 [r linsert xlist before bad aaa] - assert_equal {aa a aaaaaaaaaaaaaaaaa zz c yy d dd} [r lrange xlist 0 10] - assert_equal 9 [r linsert xlist before aa 42] - assert_equal 42 [r lrange xlist 0 0] - } - test {DEL a list - ziplist} { assert_equal 1 [r del myziplist2] assert_equal 0 [r exists myziplist2] @@ -130,6 +78,89 @@ start_server { assert_encoding list $key } + test {LPUSHX, RPUSHX - generic} { + r del xlist + assert_equal 0 [r lpushx xlist a] + assert_equal 0 [r llen xlist] + assert_equal 0 [r rpushx xlist a] + assert_equal 0 [r llen xlist] + } + + foreach type {ziplist list} { + test "LPUSHX, RPUSHX - $type" { + create_$type xlist {b c} + assert_equal 3 [r rpushx xlist d] + assert_equal 4 [r lpushx xlist a] + assert_equal {a b c d} [r lrange xlist 0 -1] + } + + test "LINSERT - $type" { + create_$type xlist {a b c d} + assert_equal 5 [r linsert xlist before c zz] + assert_equal {a b zz c d} [r lrange xlist 0 10] + assert_equal 6 [r linsert xlist after c yy] + assert_equal {a b zz c yy d} [r lrange xlist 0 10] + assert_equal 7 [r linsert xlist after d dd] + assert_equal 7 [r linsert xlist after bad ddd] + assert_equal {a b zz c yy d dd} [r lrange xlist 0 10] + assert_equal 8 [r linsert xlist before a aa] + assert_equal 8 [r linsert xlist before bad aaa] + assert_equal {aa a b zz c yy d dd} [r lrange xlist 0 10] + + # check inserting integer encoded value + assert_equal 9 [r linsert xlist before aa 42] + assert_equal 42 [r lrange xlist 0 0] + } + } + + test {LPUSHX, RPUSHX convert from ziplist to list} { + set large_value "aaaaaaaaaaaaaaaaa" + + # convert when a large value is pushed + create_ziplist xlist a + assert_equal 2 [r rpushx xlist $large_value] + assert_encoding list xlist + create_ziplist xlist a + assert_equal 2 [r lpushx xlist $large_value] + assert_encoding list xlist + + # convert when the length threshold is exceeded + create_ziplist xlist [lrepeat 256 a] + assert_equal 257 [r rpushx xlist b] + assert_encoding list xlist + create_ziplist xlist [lrepeat 256 a] + assert_equal 257 [r lpushx xlist b] + assert_encoding list xlist + } + + test {LINSERT convert from ziplist to list} { + set large_value "aaaaaaaaaaaaaaaaa" + + # convert when a large value is inserted + create_ziplist xlist a + assert_equal 2 [r linsert xlist before a $large_value] + assert_encoding list xlist + create_ziplist xlist a + assert_equal 2 [r linsert xlist after a $large_value] + assert_encoding list xlist + + # convert when the length threshold is exceeded + create_ziplist xlist [lrepeat 256 a] + assert_equal 257 [r linsert xlist before a a] + assert_encoding list xlist + create_ziplist xlist [lrepeat 256 a] + assert_equal 257 [r linsert xlist after a a] + assert_encoding list xlist + + # don't convert when the value could not be inserted + create_ziplist xlist [lrepeat 256 a] + assert_equal 256 [r linsert xlist before foo a] + assert_encoding ziplist xlist + create_ziplist xlist [lrepeat 256 a] + assert_equal 256 [r linsert xlist after foo a] + assert_encoding ziplist xlist + } + foreach {type num} {ziplist 250 list 500} { proc check_numbered_list_consistency {key} { set len [r llen $key] -- GitLab