[redis]github-547-1 Report

1. Symptom

Assertion failed because a ziplist hash contains duplicate elements:

void hashTypeConvertZiplist(robj *o, int enc) {

    if (enc == REDIS_ENCODING_ZIPLIST) {

        /* Nothing to do... */

    } else if (enc == REDIS_ENCODING_HT) {

      .. .. ..

        while (hashTypeNext(hi) != REDIS_ERR) {

            .. .. ..

            ret = dictAdd(dict, field, value);

            if (ret != DICT_OK) {

                redisLogHexDump(REDIS_WARNING,"ziplist with dup elements dump",

                    o->ptr,ziplistBlobLen(o->ptr));

                redisAssert(ret == DICT_OK);

            }

        }

    } else {

        redisPanic("Unknown hash encoding");

    }

}

1.1 Severity

Severe

1.2 Was there exception thrown?

Yes

1.2.1 Were there multiple exceptions?

No

1.3 Scope of the failure

Single/a few clients when accessing the particular data. Only one RDB data will cause the crash.

2. How to reproduce this failure

2.0 Version

2.5.10

2.1 Configuration

Standard

2.2 Reproduction procedure

1. Generate an RDB file with a 2.4 version of Redis, including a ziplist key with 0-255 value.

2. Load the RDF file into the buggy version (2.5.10), read that key, observe the crash

2.2.1 Timing order

In this order

2.2.2 Events order externally controllable?

Yes

2.3 Can the logs tell how to reproduce the failure?

No (not completely). It can tell the client command that caused the Redis to crash, but not the key and the source of the RDB (resulted in lots of discussion).

2.4 How many machines needed?

1.

3. Diagnosis procedure

3.1 Detailed Symptom (where you start)

The crash dump, and you realize there is duplicated keys in ziplist (but you don’t know why).

3.2 Backward inference

However, it is extremely hard to understand where the duplicated keys are from. The developer had to ask for the RDB file in order to reproduce it.

4. Root cause

This is because Redis 2.6 introduces new integer encodings, so it’s no longer true that if two entries have a different encoding they are not equal. In this case, the RDB contains an old ziplist (generated from Redis 2.4) that contains a small unsigned integer encoded in 16-bit encoding, and now Redis 2.6 loads it twice: once using the original encoding and the 2nd time using an 8 bit encoding format and treat it as different values --- now the ziplist contains two identical values (with different encodings).

4.1 Category:

Semantic

5. Fix

5.1 How?

Not loading the element twice:

                     return p;

                 }

             } else {

-                /* Find out if the specified entry can be encoded */

+                /* Find out if the searched field can be encoded. Note that

+                 * we do it only the first time, once done vencoding is set

+                 * to non-zero and vll is set to the integer value. */

                 if (vencoding == 0) {

-                    /* UINT_MAX when the entry CANNOT be encoded */

                     if (!zipTryEncoding(vstr, vlen, &vll, &vencoding)) {

+                        /* If the entry can't be encoded we set it to

+                         * UCHAR_MAX so that we don't retry again the next

+                         * time. */

                         vencoding = UCHAR_MAX;

                     }

-

                     /* Must be non-zero by now */

                     assert(vencoding);

                 }

 

-                /* Compare current entry with specified entry */

-                if (encoding == vencoding) {

+                /* Compare current entry with specified entry, do it only

+                 * if vencoding != UCHAR_MAX because if there is no encoding

+                 * possible for the field it can't be a valid integer. */

+                if (vencoding != UCHAR_MAX) {

                     long long ll = zipLoadInteger(q, encoding);

                     if (ll == vll) {

                         return p;