From 1653e5d990e9963106eaea0f1c121e34a217810c Mon Sep 17 00:00:00 2001 From: John Sully Date: Mon, 13 Jul 2020 21:14:03 -0400 Subject: [PATCH 1/3] Remove gitter, we don't check it often enough Former-commit-id: 119797014c09c9330e473b904f98353b32d549ab --- README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/README.md b/README.md index 88f5f0f7f..fe0030dff 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,5 @@ ![Current Release](https://img.shields.io/github/release/JohnSully/KeyDB.svg) ![CI](https://github.com/JohnSully/KeyDB/workflows/CI/badge.svg?branch=unstable) -[![Join the chat at https://gitter.im/KeyDB/community](https://badges.gitter.im/KeyDB/community.svg)](https://gitter.im/KeyDB/community?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge) [![StackShare](http://img.shields.io/badge/tech-stack-0690fa.svg?style=flat)](https://stackshare.io/eq-alpha-technology-inc/eq-alpha-technology-inc) ##### New! Want to extend KeyDB with Javascript? Try [ModJS](https://github.com/JohnSully/ModJS) From e06152eb13fb4f8f25b485a3b6171f32957c9132 Mon Sep 17 00:00:00 2001 From: John Sully Date: Fri, 17 Jul 2020 21:02:51 +0000 Subject: [PATCH 2/3] Fix lock after free in module API Former-commit-id: d88fd1588d292bffc0aa53c299cb52e7a4e91015 --- src/ae.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/ae.cpp b/src/ae.cpp index 44f302e69..789a6888b 100644 --- a/src/ae.cpp +++ b/src/ae.cpp @@ -258,9 +258,11 @@ int aeCreateRemoteFileEvent(aeEventLoop *eventLoop, int fd, int mask, if (fSynchronous) { + { std::unique_lock ulock(cmd.pctl->mutexcv, std::adopt_lock); cmd.pctl->cv.wait(ulock); ret = cmd.pctl->rval; + } delete cmd.pctl; } @@ -300,7 +302,7 @@ int aePostFunction(aeEventLoop *eventLoop, std::function fn, bool fSynch cmd.fLock = fLock; if (fSynchronous) { - cmd.pctl = new (MALLOC_LOCAL) aeCommandControl(); + cmd.pctl = new (MALLOC_LOCAL) aeCommandControl; cmd.pctl->mutexcv.lock(); } @@ -311,9 +313,11 @@ int aePostFunction(aeEventLoop *eventLoop, std::function fn, bool fSynch int ret = AE_OK; if (fSynchronous) { + { std::unique_lock ulock(cmd.pctl->mutexcv, std::adopt_lock); cmd.pctl->cv.wait(ulock); ret = cmd.pctl->rval; + } delete cmd.pctl; } return ret; From e87dee8dc7a045359e8047f12c227d281f1c1b70 Mon Sep 17 00:00:00 2001 From: John Sully Date: Mon, 10 Aug 2020 06:10:06 +0000 Subject: [PATCH 3/3] Add build flag to disable MVCC tstamps Former-commit-id: f17d178d03f44abcdaddd851a313dd3f7ec87ed5 --- src/Makefile | 5 +++++ src/db.cpp | 10 +++++++++- src/object.cpp | 13 ++++++++++++- src/rdb.cpp | 8 ++++++++ src/server.h | 6 ++++++ src/storage.h | 2 -- 6 files changed, 40 insertions(+), 4 deletions(-) diff --git a/src/Makefile b/src/Makefile index 038ac51f8..f3c72db27 100644 --- a/src/Makefile +++ b/src/Makefile @@ -47,6 +47,11 @@ endif USEASM?=true +ifeq ($(NOMVCC),) + CFLAGS+= -DENABLE_MVCC + CXXFLAGS+= -DENABLE_MVCC +endif + ifneq ($(SANITIZE),) CFLAGS+= -fsanitize=$(SANITIZE) -DSANITIZE CXXFLAGS+= -fsanitize=$(SANITIZE) -DSANITIZE diff --git a/src/db.cpp b/src/db.cpp index 11816262b..33eae9fc3 100644 --- a/src/db.cpp +++ b/src/db.cpp @@ -92,9 +92,11 @@ static robj *lookupKey(redisDb *db, robj *key, int flags) { updateDbValAccess(de, flags); +#ifdef ENABLE_MVCC if (flags & LOOKUP_UPDATEMVCC) { val->mvcc_tstamp = getMvccTstamp(); } +#endif return val; } else { return NULL; @@ -206,7 +208,9 @@ int dbAddCore(redisDb *db, robj *key, robj *val) { serverAssert(!val->FExpires()); sds copy = sdsdup(szFromObj(key)); int retval = dictAdd(db->pdict, copy, val); +#ifdef ENABLE_MVCC val->mvcc_tstamp = key->mvcc_tstamp = getMvccTstamp(); +#endif if (retval == DICT_OK) { @@ -256,7 +260,9 @@ void dbOverwriteCore(redisDb *db, dictEntry *de, robj *key, robj *val, bool fUpd if (fUpdateMvcc) { if (val->getrefcount(std::memory_order_relaxed) == OBJ_SHARED_REFCOUNT) val = dupStringObject(val); +#ifdef ENABLE_MVCC val->mvcc_tstamp = getMvccTstamp(); +#endif } dictSetVal(db->pdict, de, val); @@ -290,13 +296,15 @@ int dbMerge(redisDb *db, robj *key, robj *val, int fReplace) if (de == nullptr) return (dbAddCore(db, key, val) == DICT_OK); +#ifdef ENABLE_MVCC robj *old = (robj*)dictGetVal(de); if (old->mvcc_tstamp <= val->mvcc_tstamp) { dbOverwriteCore(db, de, key, val, false, true); return true; } - +#endif + return false; } else diff --git a/src/object.cpp b/src/object.cpp index 091e3d4da..3befa73d7 100644 --- a/src/object.cpp +++ b/src/object.cpp @@ -46,7 +46,9 @@ robj *createObject(int type, void *ptr) { o->encoding = OBJ_ENCODING_RAW; o->m_ptr = ptr; o->setrefcount(1); +#ifdef ENABLE_MVCC o->mvcc_tstamp = OBJ_MVCC_INVALID; +#endif /* Set the LRU to the current lruclock (minutes resolution), or * alternatively the LFU counter. */ @@ -101,7 +103,9 @@ robj *createEmbeddedStringObject(const char *ptr, size_t len) { o->type = OBJ_STRING; o->encoding = OBJ_ENCODING_EMBSTR; o->setrefcount(1); +#ifdef ENABLE_MVCC o->mvcc_tstamp = OBJ_MVCC_INVALID; +#endif if (g_pserver->maxmemory_policy & MAXMEMORY_FLAG_LFU) { o->lru = (LFUGetTimeInMinutes()<<8) | LFU_INIT_VAL; @@ -129,8 +133,13 @@ robj *createEmbeddedStringObject(const char *ptr, size_t len) { * * The current limit of 52 is chosen so that the biggest string object * we allocate as EMBSTR will still fit into the 64 byte arena of jemalloc. */ +#ifdef ENABLE_MVCC #define OBJ_ENCODING_EMBSTR_SIZE_LIMIT 48 -static_assert((sizeof(redisObject)+OBJ_ENCODING_EMBSTR_SIZE_LIMIT-8) == 64, "Max EMBSTR obj should be 64 bytes total"); +#else +#define OBJ_ENCODING_EMBSTR_SIZE_LIMIT 256 +#endif + +//static_assert((sizeof(redisObject)+OBJ_ENCODING_EMBSTR_SIZE_LIMIT-8) == 64, "Max EMBSTR obj should be 64 bytes total"); robj *createStringObject(const char *ptr, size_t len) { if (len <= OBJ_ENCODING_EMBSTR_SIZE_LIMIT) return createEmbeddedStringObject(ptr,len); @@ -1317,10 +1326,12 @@ NULL * because we update the access time only * when the key is read or overwritten. */ addReplyLongLong(c,LFUDecrAndReturn(o)); +#ifdef ENABLE_MVCC } else if (!strcasecmp(szFromObj(c->argv[1]), "lastmodified") && c->argc == 3) { if ((o = objectCommandLookupOrReply(c,c->argv[2],shared.null[c->resp])) == NULL) return; addReplyLongLong(c, (g_pserver->mstime - (o->mvcc_tstamp >> MVCC_MS_SHIFT)) / 1000); +#endif } else { addReplySubcommandSyntaxError(c); } diff --git a/src/rdb.cpp b/src/rdb.cpp index 5ee6baa25..c6d39a010 100644 --- a/src/rdb.cpp +++ b/src/rdb.cpp @@ -1089,8 +1089,10 @@ int rdbSaveKeyValuePair(rio *rdb, robj *key, robj *val, expireEntry *pexpire) { } char szT[32]; +#ifdef ENABLE_MVCC snprintf(szT, 32, "%" PRIu64, val->mvcc_tstamp); if (rdbSaveAuxFieldStrStr(rdb,"mvcc-tstamp", szT) == -1) return -1; +#endif /* Save type, key, value */ if (rdbSaveObjectType(rdb,val) == -1) return -1; @@ -2046,7 +2048,9 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, uint64_t mvcc_tstamp) { return NULL; } +#ifdef ENABLE_MVCC o->mvcc_tstamp = mvcc_tstamp; +#endif serverAssert(!o->FExpires()); return o; } @@ -2398,7 +2402,11 @@ int rdbLoadRio(rio *rdb, int rdbflags, rdbSaveInfo *rsi) { key = nullptr; goto eoferr; } +#ifdef ENABLE_MVCC bool fStaleMvccKey = (rsi) ? val->mvcc_tstamp < rsi->mvccMinThreshold : false; +#else + bool fStaleMvccKey = false; +#endif /* Check if the key already expired. This function is used when loading * an RDB file from disk, either at startup, or when an RDB was diff --git a/src/server.h b/src/server.h index 7479b4557..4ed40339f 100644 --- a/src/server.h +++ b/src/server.h @@ -805,7 +805,9 @@ typedef struct redisObject { private: mutable std::atomic refcount {0}; public: +#ifdef ENABLE_MVCC uint64_t mvcc_tstamp; +#endif void *m_ptr; inline bool FExpires() const { return refcount.load(std::memory_order_relaxed) >> 31; } @@ -816,7 +818,11 @@ public: void addref() const { refcount.fetch_add(1, std::memory_order_relaxed); } unsigned release() const { return refcount.fetch_sub(1, std::memory_order_seq_cst) & ~(1U << 31); } } robj; +#ifdef ENABLE_MVCC static_assert(sizeof(redisObject) == 24, "object size is critical, don't increase"); +#else +static_assert(sizeof(redisObject) == 16, "object size is critical, don't increase"); +#endif __attribute__((always_inline)) inline const void *ptrFromObj(robj_roptr &o) { diff --git a/src/storage.h b/src/storage.h index e9106aca2..2d46c0223 100644 --- a/src/storage.h +++ b/src/storage.h @@ -1,8 +1,6 @@ #ifndef __STORAGE_H__ #define __STORAGE_H__ -#define OBJ_ENCODING_EMBSTR_SIZE_LIMIT 48 // Note: also defined in object.c - should always match - #ifdef __cplusplus extern "C" { #endif