Expireset should do a deep hash

Former-commit-id: cdcdf57dd1419ec09eab6579caf5e5ff9a5a242c

src/Makefile

@@ -208,7 +208,7 @@ endif
 
 REDIS_SERVER_NAME=keydb-server
 REDIS_SENTINEL_NAME=keydb-sentinel
-REDIS_SERVER_OBJ=adlist.o quicklist.o ae.o anet.o dict.o server.o sds.o zmalloc.o lzf_c.o lzf_d.o pqsort.o zipmap.o sha1.o ziplist.o release.o networking.o util.o object.o db.o replication.o rdb.o t_string.o t_list.o t_set.o t_zset.o t_hash.o config.o aof.o pubsub.o multi.o debug.o sort.o intset.o syncio.o cluster.o crc16.o endianconv.o slowlog.o scripting.o bio.o rio.o rand.o memtest.o crc64.o bitops.o sentinel.o notify.o setproctitle.o blocked.o hyperloglog.o latency.o sparkline.o redis-check-rdb.o redis-check-aof.o geo.o lazyfree.o module.o evict.o expire.o geohash.o geohash_helper.o childinfo.o defrag.o siphash.o rax.o t_stream.o listpack.o localtime.o acl.o storage.o rdb-s3.o fastlock.o new.o tracking.o AsyncWorkQueue.o snapshot.o storage/rocksdb.o storage/rocksdbfactory.o $(ASM_OBJ)
+REDIS_SERVER_OBJ=adlist.o quicklist.o ae.o anet.o dict.o server.o sds.o zmalloc.o lzf_c.o lzf_d.o pqsort.o zipmap.o sha1.o ziplist.o release.o networking.o util.o object.o db.o replication.o rdb.o t_string.o t_list.o t_set.o t_zset.o t_hash.o config.o aof.o pubsub.o multi.o debug.o sort.o intset.o syncio.o cluster.o crc16.o endianconv.o slowlog.o scripting.o bio.o rio.o rand.o memtest.o crc64.o bitops.o sentinel.o notify.o setproctitle.o blocked.o hyperloglog.o latency.o sparkline.o redis-check-rdb.o redis-check-aof.o geo.o lazyfree.o module.o evict.o expire.o geohash.o geohash_helper.o childinfo.o defrag.o siphash.o rax.o t_stream.o listpack.o localtime.o acl.o storage.o rdb-s3.o fastlock.o new.o tracking.o AsyncWorkQueue.o snapshot.o storage/rocksdb.o storage/rocksdbfactory.o keydbutils.o $(ASM_OBJ)
 REDIS_CLI_NAME=keydb-cli
 REDIS_CLI_OBJ=anet.o adlist.o dict.o redis-cli.o redis-cli-cpphelper.o zmalloc.o release.o anet.o ae.o crc64.o siphash.o crc16.o storage-lite.o fastlock.o new.o $(ASM_OBJ)
 REDIS_BENCHMARK_NAME=keydb-benchmark

src/db.cpp

@@ -1319,7 +1319,6 @@ int redisDbPersistentData::removeExpire(robj *key, dict_iter itr) {
     trackkey(key);
     auto itrExpire = m_setexpire->find(itr.key());
     serverAssert(itrExpire != m_setexpire->end());
-    serverAssert(itrExpire->key() == itr.key());
     m_setexpire->erase(itrExpire);
     val->SetFExpires(false);
     return 1;
@@ -1415,12 +1414,12 @@ void setExpire(client *c, redisDb *db, robj *key, expireEntry &&e)
 
 /* Return the expire time of the specified key, or null if no expire
  * is associated with this key (i.e. the key is non volatile) */
-expireEntry *redisDbPersistentData::getExpire(robj_roptr key) {
+expireEntry *redisDbPersistentDataSnapshot::getExpire(robj_roptr key) {
     /* No expire? return ASAP */
     if (expireSize() == 0)
         return nullptr;
 
-    auto itr = find(szFromObj(key));
+    auto itr = find_threadsafe(szFromObj(key));
     if (itr == nullptr)
         return nullptr;
     if (!itr.val()->FExpires())
@@ -1430,9 +1429,9 @@ expireEntry *redisDbPersistentData::getExpire(robj_roptr key) {
     return itrExpire.operator->();
 }
 
-const expireEntry *redisDbPersistentData::getExpire(robj_roptr key) const
+const expireEntry *redisDbPersistentDataSnapshot::getExpire(robj_roptr key) const
 {
-    return const_cast<redisDbPersistentData*>(this)->getExpire(key);
+    return const_cast<redisDbPersistentDataSnapshot*>(this)->getExpire(key);
 }
 
 /* Propagate expires into slaves and the AOF file.

src/sds.c

@@ -787,7 +787,7 @@ void sdstoupper(sds s) {
  * If two strings share exactly the same prefix, but one of the two has
  * additional characters, the longer string is considered to be greater than
  * the smaller one. */
-int sdscmp(const sds s1, const sds s2) {
+int sdscmp(const char *s1, const char *s2) {
     size_t l1, l2, minlen;
     int cmp;
 

src/sds.h

@@ -265,7 +265,7 @@ sds sdstrim(sds s, const char *cset);
 void sdsrange(sds s, ssize_t start, ssize_t end);
 void sdsupdatelen(sds s);
 void sdsclear(sds s);
-int sdscmp(const sds s1, const sds s2);
+int sdscmp(const char *s1, const char *s2);
 sds *sdssplitlen(const char *s, ssize_t len, const char *sep, int seplen, int *count);
 void sdsfreesplitres(sds *tokens, int count);
 void sdstolower(sds s);
@@ -298,6 +298,48 @@ int sdsTest(int argc, char *argv[]);
 
 #ifdef __cplusplus
 }
+
+class sdsview
+{
+    const char *m_str;
+
+public:
+    sdsview(sds str)
+        : m_str((const char*) str)
+    {}
+
+    sdsview(const char *str)
+        : m_str(str)
+    {}
+
+    bool operator<(const sdsview &other) const
+    {
+        return sdscmp(m_str, other.m_str) < 0;
+    }
+
+    bool operator==(const sdsview &other) const
+    {
+        return sdscmp(m_str, other.m_str) == 0;
+    }
+
+    bool operator==(const char *other) const
+    {
+        return sdscmp(m_str, other) == 0;
+    }
+
+    char operator[](size_t idx) const
+    {
+        return m_str[idx];
+    }
+
+    size_t size() const
+    {
+        return sdslen(m_str);
+    }
+
+    explicit operator const char*() const { return m_str; }
+};
+
 #endif
 
 #endif

src/semiorderedset.h

@@ -15,6 +15,18 @@
 
 extern uint64_t dictGenHashFunction(const void *key, int len);
 
+namespace keydbutils
+{
+    template<typename T>
+    size_t hash(const T& key)
+    {
+        return (size_t)dictGenHashFunction(&key, sizeof(key));
+    }
+
+    template<>
+    size_t hash(const sdsview &);
+}
+
 template<typename T, typename T_KEY = T, bool MEMMOVE_SAFE = false>
 class semiorderedset
 {
@@ -281,7 +293,7 @@ private:
 
     size_t idxFromObj(const T_KEY &key)
     {
-        size_t v = (size_t)dictGenHashFunction(&key, sizeof(key));
+        size_t v = keydbutils::hash(key);
         return v & hashmask();
     }
 

src/server.h

@@ -1088,9 +1088,9 @@ public:
     const expireEntryFat *pfatentry() const { assert(FFat()); return u.m_pfatentry; }
 
 
-    bool operator==(const char *key) const noexcept
+    bool operator==(const sdsview &key) const noexcept
     { 
-        return this->key() == key; 
+        return key == this->key(); 
     }
 
     bool operator<(const expireEntry &e) const noexcept
@@ -1168,10 +1168,10 @@ public:
         return false;
     }
 
-    explicit operator const char*() const noexcept { return key(); }
+    explicit operator sdsview() const noexcept { return key(); }
     explicit operator long long() const noexcept { return when(); }
 };
-typedef semiorderedset<expireEntry, const char *, true /*expireEntry can be memmoved*/> expireset;
+typedef semiorderedset<expireEntry, sdsview, true /*expireEntry can be memmoved*/> expireset;
 
 /* The a string name for an object's type as listed above
  * Native types are checked against the OBJ_STRING, OBJ_LIST, OBJ_* defines,
@@ -1310,8 +1310,6 @@ public:
     bool iterate(std::function<bool(const char*, robj*)> fn);
     void setExpire(robj *key, robj *subkey, long long when);
     void setExpire(expireEntry &&e);
-    expireEntry *getExpire(robj_roptr key);
-    const expireEntry *getExpire(robj_roptr key) const;
     void initialize();
 
     void setStorageProvider(IStorage *pstorage);
@@ -1389,10 +1387,12 @@ public:
     dict_iter random_threadsafe() const;
     dict_iter find_threadsafe(const char *key) const;
 
+    expireEntry *getExpire(robj_roptr key);
+    const expireEntry *getExpire(robj_roptr key) const;
+
     // These need to be fixed
     using redisDbPersistentData::size;
     using redisDbPersistentData::expireSize;
-    using redisDbPersistentData::getExpire;
 };
 
 /* Redis database representation. There are multiple databases identified
@@ -1455,7 +1455,6 @@ typedef struct redisDb : public redisDbPersistentDataSnapshot
     using redisDbPersistentData::emptyDbAsync;
     using redisDbPersistentData::iterate;
     using redisDbPersistentData::setExpire;
-    using redisDbPersistentData::getExpire;
     using redisDbPersistentData::trackChanges;
     using redisDbPersistentData::processChanges;
     using redisDbPersistentData::commitChanges;