From 5def65008ff92519a828e1ba403e9a46836ca802 Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Wed, 21 Feb 2018 11:04:13 +0200 Subject: [PATCH 01/42] Fix zrealloc to behave similarly to je_realloc when size is 0 According to C11, the behavior of realloc with size 0 is now deprecated. it can either behave as free(ptr) and return NULL, or return a valid pointer. but in zmalloc it can lead to zmalloc_oom_handler and panic. and that can affect modules that use it. It looks like both glibc allocator and jemalloc behave like so: realloc(malloc(32),0) returns NULL realloc(NULL,0) returns a valid pointer This commit changes zmalloc to behave the same --- src/zmalloc.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/zmalloc.c b/src/zmalloc.c index 094dd80fa..01ac8c797 100644 --- a/src/zmalloc.c +++ b/src/zmalloc.c @@ -147,6 +147,10 @@ void *zrealloc(void *ptr, size_t size) { size_t oldsize; void *newptr; + if (size == 0 && ptr!=NULL) { + zfree(ptr); + return NULL; + } if (ptr == NULL) return zmalloc(size); #ifdef HAVE_MALLOC_SIZE oldsize = zmalloc_size(ptr); From 17c5f17686354b28c715b6f16c9c4e8eb2239df4 Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Mon, 13 Aug 2018 17:43:29 +0300 Subject: [PATCH 02/42] Add log when server dies of SIGTERM during loading this is very confusing to see the server disappears as if it got SIGKILL when it was not the case. --- src/server.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/server.c b/src/server.c index b537ee04a..0c665033a 100644 --- a/src/server.c +++ b/src/server.c @@ -3780,6 +3780,7 @@ static void sigShutdownHandler(int sig) { rdbRemoveTempFile(getpid()); exit(1); /* Exit with an error since this was not a clean shutdown. */ } else if (server.loading) { + serverLogFromHandler(LL_WARNING, "Received shutdown signal during loading, exiting now."); exit(0); } From a88264d934744b23c02d92a3ba3fccbe070af0b4 Mon Sep 17 00:00:00 2001 From: Yossi Gottlieb Date: Wed, 30 Nov 2016 21:47:02 +0200 Subject: [PATCH 03/42] Add RedisModule_GetKeyNameFromIO(). --- src/aof.c | 2 +- src/cluster.c | 10 +++++----- src/module.c | 9 +++++++++ src/rdb.c | 14 +++++++------- src/rdb.h | 4 ++-- src/redis-check-rdb.c | 2 +- src/redismodule.h | 2 ++ src/server.h | 4 +++- 8 files changed, 30 insertions(+), 17 deletions(-) diff --git a/src/aof.c b/src/aof.c index cafcf961c..615eebd01 100644 --- a/src/aof.c +++ b/src/aof.c @@ -1239,7 +1239,7 @@ int rewriteModuleObject(rio *r, robj *key, robj *o) { RedisModuleIO io; moduleValue *mv = o->ptr; moduleType *mt = mv->type; - moduleInitIOContext(io,mt,r); + moduleInitIOContext(io,mt,r,key); mt->aof_rewrite(&io,key,mv->value); if (io.ctx) { moduleFreeContext(io.ctx); diff --git a/src/cluster.c b/src/cluster.c index 50a9ae687..c85e3791d 100644 --- a/src/cluster.c +++ b/src/cluster.c @@ -4776,7 +4776,7 @@ NULL /* Generates a DUMP-format representation of the object 'o', adding it to the * io stream pointed by 'rio'. This function can't fail. */ -void createDumpPayload(rio *payload, robj *o) { +void createDumpPayload(rio *payload, robj *o, robj *key) { unsigned char buf[2]; uint64_t crc; @@ -4784,7 +4784,7 @@ void createDumpPayload(rio *payload, robj *o) { * byte followed by the serialized object. This is understood by RESTORE. */ rioInitWithBuffer(payload,sdsempty()); serverAssert(rdbSaveObjectType(payload,o)); - serverAssert(rdbSaveObject(payload,o)); + serverAssert(rdbSaveObject(payload,o,key)); /* Write the footer, this is how it looks like: * ----------------+---------------------+---------------+ @@ -4842,7 +4842,7 @@ void dumpCommand(client *c) { } /* Create the DUMP encoded representation. */ - createDumpPayload(&payload,o); + createDumpPayload(&payload,o,c->argv[1]); /* Transfer to the client */ dumpobj = createObject(OBJ_STRING,payload.io.buffer.ptr); @@ -4915,7 +4915,7 @@ void restoreCommand(client *c) { rioInitWithBuffer(&payload,c->argv[3]->ptr); if (((type = rdbLoadObjectType(&payload)) == -1) || - ((obj = rdbLoadObject(type,&payload)) == NULL)) + ((obj = rdbLoadObject(type,&payload,c->argv[1])) == NULL)) { addReplyError(c,"Bad data format"); return; @@ -5203,7 +5203,7 @@ void migrateCommand(client *c) { /* Emit the payload argument, that is the serialized object using * the DUMP format. */ - createDumpPayload(&payload,ov[j]); + createDumpPayload(&payload,ov[j],kv[j]); serverAssertWithInfo(c,NULL, rioWriteBulkString(&cmd,payload.io.buffer.ptr, sdslen(payload.io.buffer.ptr))); diff --git a/src/module.c b/src/module.c index e69d3dc61..e1ffd7313 100644 --- a/src/module.c +++ b/src/module.c @@ -3438,6 +3438,14 @@ RedisModuleCtx *RM_GetContextFromIO(RedisModuleIO *io) { return io->ctx; } +/* Returns a RedisModuleString with the name of the key currently saving or + * loading, when an IO data type callback is called. There is no guarantee + * that the key name is always available, so this may return NULL. + */ +const RedisModuleString *RM_GetKeyNameFromIO(RedisModuleIO *io) { + return io->key; +} + /* -------------------------------------------------------------------------- * Logging * -------------------------------------------------------------------------- */ @@ -5164,6 +5172,7 @@ void moduleRegisterCoreAPI(void) { REGISTER_API(RetainString); REGISTER_API(StringCompare); REGISTER_API(GetContextFromIO); + REGISTER_API(GetKeyNameFromIO); REGISTER_API(BlockClient); REGISTER_API(UnblockClient); REGISTER_API(IsBlockedReplyRequest); diff --git a/src/rdb.c b/src/rdb.c index 52dddf210..95e4766ea 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -751,7 +751,7 @@ size_t rdbSaveStreamConsumers(rio *rdb, streamCG *cg) { /* Save a Redis object. * Returns -1 on error, number of bytes written on success. */ -ssize_t rdbSaveObject(rio *rdb, robj *o) { +ssize_t rdbSaveObject(rio *rdb, robj *o, robj *key) { ssize_t n = 0, nwritten = 0; if (o->type == OBJ_STRING) { @@ -966,7 +966,7 @@ ssize_t rdbSaveObject(rio *rdb, robj *o) { RedisModuleIO io; moduleValue *mv = o->ptr; moduleType *mt = mv->type; - moduleInitIOContext(io,mt,rdb); + moduleInitIOContext(io,mt,rdb,key); /* Write the "module" identifier as prefix, so that we'll be able * to call the right module during loading. */ @@ -996,7 +996,7 @@ ssize_t rdbSaveObject(rio *rdb, robj *o) { * this length with very little changes to the code. In the future * we could switch to a faster solution. */ size_t rdbSavedObjectLen(robj *o) { - ssize_t len = rdbSaveObject(NULL,o); + ssize_t len = rdbSaveObject(NULL,o,NULL); serverAssertWithInfo(NULL,o,len != -1); return len; } @@ -1038,7 +1038,7 @@ int rdbSaveKeyValuePair(rio *rdb, robj *key, robj *val, long long expiretime) { /* Save type, key, value */ if (rdbSaveObjectType(rdb,val) == -1) return -1; if (rdbSaveStringObject(rdb,key) == -1) return -1; - if (rdbSaveObject(rdb,val) == -1) return -1; + if (rdbSaveObject(rdb,val,key) == -1) return -1; return 1; } @@ -1380,7 +1380,7 @@ robj *rdbLoadCheckModuleValue(rio *rdb, char *modulename) { /* Load a Redis object of the specified type from the specified file. * On success a newly allocated object is returned, otherwise NULL. */ -robj *rdbLoadObject(int rdbtype, rio *rdb) { +robj *rdbLoadObject(int rdbtype, rio *rdb, robj *key) { robj *o = NULL, *ele, *dec; uint64_t len; unsigned int i; @@ -1767,7 +1767,7 @@ robj *rdbLoadObject(int rdbtype, rio *rdb) { exit(1); } RedisModuleIO io; - moduleInitIOContext(io,mt,rdb); + moduleInitIOContext(io,mt,rdb,key); io.ver = (rdbtype == RDB_TYPE_MODULE) ? 1 : 2; /* Call the rdb_load method of the module providing the 10 bit * encoding version in the lower 10 bits of the module ID. */ @@ -2023,7 +2023,7 @@ int rdbLoadRio(rio *rdb, rdbSaveInfo *rsi, int loading_aof) { /* Read key */ if ((key = rdbLoadStringObject(rdb)) == NULL) goto eoferr; /* Read value */ - if ((val = rdbLoadObject(type,rdb)) == NULL) goto eoferr; + if ((val = rdbLoadObject(type,rdb,key)) == NULL) goto eoferr; /* 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 * received from the master. In the latter case, the master is diff --git a/src/rdb.h b/src/rdb.h index 7b9486169..0acddf9ab 100644 --- a/src/rdb.h +++ b/src/rdb.h @@ -140,9 +140,9 @@ int rdbSaveBackground(char *filename, rdbSaveInfo *rsi); int rdbSaveToSlavesSockets(rdbSaveInfo *rsi); void rdbRemoveTempFile(pid_t childpid); int rdbSave(char *filename, rdbSaveInfo *rsi); -ssize_t rdbSaveObject(rio *rdb, robj *o); +ssize_t rdbSaveObject(rio *rdb, robj *o, robj *key); size_t rdbSavedObjectLen(robj *o); -robj *rdbLoadObject(int type, rio *rdb); +robj *rdbLoadObject(int type, rio *rdb, robj *key); void backgroundSaveDoneHandler(int exitcode, int bysignal); int rdbSaveKeyValuePair(rio *rdb, robj *key, robj *val, long long expiretime); robj *rdbLoadStringObject(rio *rdb); diff --git a/src/redis-check-rdb.c b/src/redis-check-rdb.c index 8de1d8f48..ec00ee71c 100644 --- a/src/redis-check-rdb.c +++ b/src/redis-check-rdb.c @@ -285,7 +285,7 @@ int redis_check_rdb(char *rdbfilename, FILE *fp) { rdbstate.keys++; /* Read value */ rdbstate.doing = RDB_CHECK_DOING_READ_OBJECT_VALUE; - if ((val = rdbLoadObject(type,&rdb)) == NULL) goto eoferr; + if ((val = rdbLoadObject(type,&rdb,key)) == NULL) goto eoferr; /* Check if the key already expired. */ if (expiretime != -1 && expiretime < now) rdbstate.already_expired++; diff --git a/src/redismodule.h b/src/redismodule.h index 272da08df..02941aa96 100644 --- a/src/redismodule.h +++ b/src/redismodule.h @@ -278,6 +278,7 @@ int REDISMODULE_API_FUNC(RedisModule_StringAppendBuffer)(RedisModuleCtx *ctx, Re void REDISMODULE_API_FUNC(RedisModule_RetainString)(RedisModuleCtx *ctx, RedisModuleString *str); int REDISMODULE_API_FUNC(RedisModule_StringCompare)(RedisModuleString *a, RedisModuleString *b); RedisModuleCtx *REDISMODULE_API_FUNC(RedisModule_GetContextFromIO)(RedisModuleIO *io); +const RedisModuleString *REDISMODULE_API_FUNC(RedisModule_GetKeyNameFromIO)(RedisModuleIO *io); long long REDISMODULE_API_FUNC(RedisModule_Milliseconds)(void); void REDISMODULE_API_FUNC(RedisModule_DigestAddStringBuffer)(RedisModuleDigest *md, unsigned char *ele, size_t len); void REDISMODULE_API_FUNC(RedisModule_DigestAddLongLong)(RedisModuleDigest *md, long long ele); @@ -442,6 +443,7 @@ static int RedisModule_Init(RedisModuleCtx *ctx, const char *name, int ver, int REDISMODULE_GET_API(RetainString); REDISMODULE_GET_API(StringCompare); REDISMODULE_GET_API(GetContextFromIO); + REDISMODULE_GET_API(GetKeyNameFromIO); REDISMODULE_GET_API(Milliseconds); REDISMODULE_GET_API(DigestAddStringBuffer); REDISMODULE_GET_API(DigestAddLongLong); diff --git a/src/server.h b/src/server.h index 56c3b67d3..b888266a4 100644 --- a/src/server.h +++ b/src/server.h @@ -578,16 +578,18 @@ typedef struct RedisModuleIO { int ver; /* Module serialization version: 1 (old), * 2 (current version with opcodes annotation). */ struct RedisModuleCtx *ctx; /* Optional context, see RM_GetContextFromIO()*/ + struct redisObject *key; /* Optional name of key processed */ } RedisModuleIO; /* Macro to initialize an IO context. Note that the 'ver' field is populated * inside rdb.c according to the version of the value to load. */ -#define moduleInitIOContext(iovar,mtype,rioptr) do { \ +#define moduleInitIOContext(iovar,mtype,rioptr,keyptr) do { \ iovar.rio = rioptr; \ iovar.type = mtype; \ iovar.bytes = 0; \ iovar.error = 0; \ iovar.ver = 0; \ + iovar.key = keyptr; \ iovar.ctx = NULL; \ } while(0); From 6aa162f6ff528951894b1af80f5f7b81562e1b37 Mon Sep 17 00:00:00 2001 From: John Date: Tue, 19 Mar 2019 22:08:20 +0000 Subject: [PATCH 04/42] Fix linker errors on some compilers due to sdslen() --- src/sds.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sds.h b/src/sds.h index 1985fb263..4ae1e4dfd 100644 --- a/src/sds.h +++ b/src/sds.h @@ -98,7 +98,7 @@ struct __attribute__ ((__packed__)) sdshdr64 { #define SDS_HDR(T,s) ((struct sdshdr##T *)((s)-(sizeof(struct sdshdr##T)))) #define SDS_TYPE_5_LEN(f) ((f)>>SDS_TYPE_BITS) -inline size_t sdslen(const sds s) { +static inline size_t sdslen(const sds s) { unsigned char flags = s[-1]; int type = flags & SDS_TYPE_MASK; From 4e70215307e0a898d03454687d4ca2534925aef3 Mon Sep 17 00:00:00 2001 From: John Sully Date: Tue, 19 Mar 2019 18:27:54 -0400 Subject: [PATCH 05/42] Let's try travis-ci --- .travis.yml | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 .travis.yml diff --git a/.travis.yml b/.travis.yml new file mode 100644 index 000000000..5501fd8ba --- /dev/null +++ b/.travis.yml @@ -0,0 +1,26 @@ +install: make nasm get-deps + +compiler: + - clang + - gcc + +language: generic +os: osx +matrix: + include: + - os: linux + env: COMPILER_NAME=gcc CXX=g++-5 CC=gcc-5 + addons: + apt: + packages: + - g++-5 + sources: &sources + - llvm-toolchain-precise-3.8 + - ubuntu-toolchain-r-test + - os: linux + env: COMPILER_NAME=clang CXX=clang++-3.8 CC=clang-3.8 + addons: + apt: + packages: + - clang-3.8 + sources: *sources From da7e54819afd78ff58a1f03a7e4aee0a12b94bbb Mon Sep 17 00:00:00 2001 From: John Sully Date: Tue, 19 Mar 2019 18:31:45 -0400 Subject: [PATCH 06/42] Fix issues with travis config --- .travis.yml | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/.travis.yml b/.travis.yml index 5501fd8ba..788028ac2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,9 +1,3 @@ -install: make nasm get-deps - -compiler: - - clang - - gcc - language: generic os: osx matrix: @@ -14,6 +8,7 @@ matrix: apt: packages: - g++-5 + - nasm sources: &sources - llvm-toolchain-precise-3.8 - ubuntu-toolchain-r-test @@ -23,4 +18,5 @@ matrix: apt: packages: - clang-3.8 + - nasm sources: *sources From 0dd8e682f40356f465565bc14405735b2073cdd4 Mon Sep 17 00:00:00 2001 From: John Sully Date: Tue, 19 Mar 2019 18:37:53 -0400 Subject: [PATCH 07/42] It would be nice if our CI actually called make... --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 788028ac2..03e0721f9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,5 +1,6 @@ language: generic os: osx +scipt: make matrix: include: - os: linux From 344cca75c6cf46fbf5fa82769ab6a0b74c0f1603 Mon Sep 17 00:00:00 2001 From: John Sully Date: Tue, 19 Mar 2019 18:40:50 -0400 Subject: [PATCH 08/42] typo --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 03e0721f9..977ac01b3 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,6 @@ language: generic os: osx -scipt: make +script: make matrix: include: - os: linux From 267e84f97d61f8aa416f88870c21bdf2a0cc635a Mon Sep 17 00:00:00 2001 From: John Sully Date: Tue, 19 Mar 2019 18:55:42 -0400 Subject: [PATCH 09/42] Feature gate SO_INCOMING_CPU so we compile on older kernels --- src/fmacros.h | 5 +++++ src/networking.cpp | 2 ++ 2 files changed, 7 insertions(+) diff --git a/src/fmacros.h b/src/fmacros.h index a56bb9331..ea663e1a3 100644 --- a/src/fmacros.h +++ b/src/fmacros.h @@ -35,7 +35,12 @@ #if defined(__linux__) #define _GNU_SOURCE 1 #define _DEFAULT_SOURCE 1 + +#include +#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,19,0) +#define HAVE_SO_INCOMING_CPU 1 #endif +#endif // __linux__ #if defined(_AIX) #define _ALL_SOURCE diff --git a/src/networking.cpp b/src/networking.cpp index b608370fb..4a6794d98 100644 --- a/src/networking.cpp +++ b/src/networking.cpp @@ -1035,6 +1035,7 @@ static void acceptCommonHandler(int fd, int flags, char *ip, int iel) { return; } +#ifdef HAVE_SO_INCOMING_CPU // Set thread affinity if (server.fThreadAffinity) { @@ -1044,6 +1045,7 @@ static void acceptCommonHandler(int fd, int flags, char *ip, int iel) { serverLog(LL_WARNING, "Failed to set socket affinity"); } } +#endif /* If maxclient directive is set and this is one client more... close the * connection. Note that we create the client instead to check before From 0c5fa59221ada0639047dd16aed49cec3e467b26 Mon Sep 17 00:00:00 2001 From: John Sully Date: Tue, 19 Mar 2019 19:12:15 -0400 Subject: [PATCH 10/42] Maybe this will make travis-ci stop using GCC headers when compiling with clang --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 977ac01b3..d3db55772 100644 --- a/.travis.yml +++ b/.travis.yml @@ -14,7 +14,7 @@ matrix: - llvm-toolchain-precise-3.8 - ubuntu-toolchain-r-test - os: linux - env: COMPILER_NAME=clang CXX=clang++-3.8 CC=clang-3.8 + env: COMPILER_NAME=clang CXX=clang++-3.8 CC=clang-3.8 EXTRA_FLAGS="-stdlib=libc++" addons: apt: packages: From 30dab7ad14a473be3368b8dbc281bb4522d0a53b Mon Sep 17 00:00:00 2001 From: John Sully Date: Tue, 19 Mar 2019 19:22:27 -0400 Subject: [PATCH 11/42] No underscore --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index d3db55772..9db52849a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -14,7 +14,7 @@ matrix: - llvm-toolchain-precise-3.8 - ubuntu-toolchain-r-test - os: linux - env: COMPILER_NAME=clang CXX=clang++-3.8 CC=clang-3.8 EXTRA_FLAGS="-stdlib=libc++" + env: COMPILER_NAME=clang CXX=clang++-3.8 CC=clang-3.8 CXXFLAGS="-stdlib=libc++" addons: apt: packages: From 0ab29f7394be270f958c01cc66d26da14eb32f2c Mon Sep 17 00:00:00 2001 From: John Sully Date: Tue, 19 Mar 2019 19:29:14 -0400 Subject: [PATCH 12/42] Makefile ignores env CXXFLAGS --- src/Makefile | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Makefile b/src/Makefile index d9b986e71..33d0959f9 100644 --- a/src/Makefile +++ b/src/Makefile @@ -56,6 +56,10 @@ endif endif endif +ifeq ($(COMPILER_NAME),clang) + CXXFLAGS+= -stdlib=libc++ +endif + # To get ARM stack traces if Redis crashes we need a special C flag. ifneq (,$(filter aarch64 armv,$(uname_M))) CFLAGS+=-funwind-tables @@ -86,7 +90,7 @@ endif -include .make-settings FINAL_CFLAGS=$(STD) $(WARN) $(OPT) $(DEBUG) $(CFLAGS) $(REDIS_CFLAGS) -FINAL_CXXFLAGS=$(CXX_STD) $(WARN) $(OPT) $(DEBUG) $(CFLAGS) $(REDIS_CFLAGS) +FINAL_CXXFLAGS=$(CXX_STD) $(WARN) $(OPT) $(DEBUG) $(CFLAGS) $(CXXFLAGS) $(REDIS_CFLAGS) FINAL_LDFLAGS=$(LDFLAGS) $(REDIS_LDFLAGS) $(DEBUG) FINAL_LIBS=-lm DEBUG=-g -ggdb From c548431611bad9bcfdb3c348ab7bad61151a27a2 Mon Sep 17 00:00:00 2001 From: John Sully Date: Tue, 19 Mar 2019 19:50:28 -0400 Subject: [PATCH 13/42] Travis clang dependencies --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index 9db52849a..009ba2dda 100644 --- a/.travis.yml +++ b/.travis.yml @@ -19,5 +19,7 @@ matrix: apt: packages: - clang-3.8 + - libc++-dev + - libc++abi-dev - nasm sources: *sources From 3d93ca1bf699cbd399faf4a37e78d8d5d9f1b253 Mon Sep 17 00:00:00 2001 From: John Sully Date: Tue, 19 Mar 2019 19:55:03 -0400 Subject: [PATCH 14/42] clang build failure --- src/server.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/server.cpp b/src/server.cpp index 9d7127b5d..8707b07f5 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -57,6 +57,7 @@ #include #include #include +#include /* Our shared "common" objects */ From 2bf5c1324a9ff7739ee2eb90d7a722065d2d8a98 Mon Sep 17 00:00:00 2001 From: John Sully Date: Tue, 19 Mar 2019 20:00:52 -0400 Subject: [PATCH 15/42] Will clang build now with travis? --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 009ba2dda..9180e2593 100644 --- a/.travis.yml +++ b/.travis.yml @@ -14,7 +14,7 @@ matrix: - llvm-toolchain-precise-3.8 - ubuntu-toolchain-r-test - os: linux - env: COMPILER_NAME=clang CXX=clang++-3.8 CC=clang-3.8 CXXFLAGS="-stdlib=libc++" + env: COMPILER_NAME=clang CXX=clang++-3.8 CC=clang-3.8 CXXFLAGS="-stdlib=libc++" LDFLAGS="-stlib=libc++" addons: apt: packages: From 04679b3b3b53c87c85c2d64c9f6d34a73969af2c Mon Sep 17 00:00:00 2001 From: John Sully Date: Tue, 19 Mar 2019 20:25:30 -0400 Subject: [PATCH 16/42] Fix race condition building dependencies with multithreaded builds --- src/Makefile | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/Makefile b/src/Makefile index 33d0959f9..2ef54aa10 100644 --- a/src/Makefile +++ b/src/Makefile @@ -233,17 +233,15 @@ persist-settings: distclean .PHONY: persist-settings # Prerequisites target -.make-prerequisites: - @touch $@ - # Clean everything, persist settings and build dependencies if anything changed ifneq ($(strip $(PREV_FINAL_CFLAGS)), $(strip $(FINAL_CFLAGS))) .make-prerequisites: persist-settings -endif - -ifneq ($(strip $(PREV_FINAL_LDFLAGS)), $(strip $(FINAL_LDFLAGS))) +else ifneq ($(strip $(PREV_FINAL_LDFLAGS)), $(strip $(FINAL_LDFLAGS))) .make-prerequisites: persist-settings +else +.make-prerequisites: endif + @touch $@ # keydb-server $(REDIS_SERVER_NAME): $(REDIS_SERVER_OBJ) From ea520027b99582a78ee829fa6c0d470566a7425f Mon Sep 17 00:00:00 2001 From: John Sully Date: Tue, 19 Mar 2019 20:26:35 -0400 Subject: [PATCH 17/42] more clang travis fixes --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 9180e2593..88b6bdede 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,4 +22,5 @@ matrix: - libc++-dev - libc++abi-dev - nasm + - libjemalloc-dev sources: *sources From 0900edf776d5b6c7c28c7ed9462732178738862d Mon Sep 17 00:00:00 2001 From: John Sully Date: Tue, 19 Mar 2019 20:34:09 -0400 Subject: [PATCH 18/42] Clang build with libc malloc --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 88b6bdede..51936a2c2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,9 +1,9 @@ language: generic os: osx -script: make matrix: include: - os: linux + script: make env: COMPILER_NAME=gcc CXX=g++-5 CC=gcc-5 addons: apt: @@ -14,6 +14,7 @@ matrix: - llvm-toolchain-precise-3.8 - ubuntu-toolchain-r-test - os: linux + script: make MALLOC=libc env: COMPILER_NAME=clang CXX=clang++-3.8 CC=clang-3.8 CXXFLAGS="-stdlib=libc++" LDFLAGS="-stlib=libc++" addons: apt: @@ -22,5 +23,4 @@ matrix: - libc++-dev - libc++abi-dev - nasm - - libjemalloc-dev sources: *sources From 9ad229f663dc4aff0772c26c3e1ac7090eaca19e Mon Sep 17 00:00:00 2001 From: John Sully Date: Tue, 19 Mar 2019 20:40:17 -0400 Subject: [PATCH 19/42] Getting close! Maybe there was a better way to do this than checking in travis.yml files repeatedly but alas it works and doesn't affect people building locally --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 51936a2c2..c3b5c0f42 100644 --- a/.travis.yml +++ b/.travis.yml @@ -15,7 +15,7 @@ matrix: - ubuntu-toolchain-r-test - os: linux script: make MALLOC=libc - env: COMPILER_NAME=clang CXX=clang++-3.8 CC=clang-3.8 CXXFLAGS="-stdlib=libc++" LDFLAGS="-stlib=libc++" + env: COMPILER_NAME=clang CXX=clang++-3.8 CC=clang-3.8 CXXFLAGS="-stdlib=libc++" LDFLAGS="-stdlib=libc++" addons: apt: packages: From 03379106d6ce85c63389a73e78788a3aed589a6f Mon Sep 17 00:00:00 2001 From: John Sully Date: Tue, 19 Mar 2019 20:46:25 -0400 Subject: [PATCH 20/42] Add CI badge --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index b1a02eef2..3d930b1b4 100644 --- a/README.md +++ b/README.md @@ -1,3 +1,5 @@ +[![Build Status](https://travis-ci.org/JohnSully/KeyDB.svg?branch=unstable)](https://travis-ci.org/JohnSully/KeyDB) + What is KeyDB? -------------- From c74100a552021b7b3af236349fc30c389b5ea4c2 Mon Sep 17 00:00:00 2001 From: John Sully Date: Tue, 19 Mar 2019 20:54:36 -0400 Subject: [PATCH 21/42] Add release badge --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 3d930b1b4..bcfbb423b 100644 --- a/README.md +++ b/README.md @@ -1,3 +1,4 @@ +![Current Release](https://img.shields.io/github/release/JohnSully/KeyDB.svg) [![Build Status](https://travis-ci.org/JohnSully/KeyDB.svg?branch=unstable)](https://travis-ci.org/JohnSully/KeyDB) What is KeyDB? From ac80a5c6a6676f45ac7d460a9cfb02fef8b48d78 Mon Sep 17 00:00:00 2001 From: John Sully Date: Tue, 19 Mar 2019 22:04:33 -0400 Subject: [PATCH 22/42] Add debugging stats to the INFO command --- src/fastlock.cpp | 10 ++++++++++ src/fastlock.h | 2 ++ src/fastlock_x64.asm | 3 +++ src/networking.cpp | 3 +++ src/server.cpp | 15 +++++++++++++-- src/server.h | 1 + 6 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/fastlock.cpp b/src/fastlock.cpp index f265f3908..1a8d51165 100644 --- a/src/fastlock.cpp +++ b/src/fastlock.cpp @@ -43,6 +43,13 @@ ****************************************************/ static_assert(sizeof(pid_t) <= sizeof(fastlock::m_pidOwner), "fastlock::m_pidOwner not large enough"); +uint64_t g_longwaits = 0; + +uint64_t fastlock_getlongwaitcount() +{ + return g_longwaits; +} + extern "C" pid_t gettid() { @@ -75,7 +82,10 @@ extern "C" void fastlock_lock(struct fastlock *lock) while (__atomic_load_2(&lock->m_ticket.m_active, __ATOMIC_ACQUIRE) != myticket) { if ((++cloops % 1024*1024) == 0) + { sched_yield(); + ++g_longwaits; + } #if defined(__i386__) || defined(__amd64__) __asm__ ("pause"); #endif diff --git a/src/fastlock.h b/src/fastlock.h index b5a70c530..b8027de54 100644 --- a/src/fastlock.h +++ b/src/fastlock.h @@ -13,6 +13,8 @@ int fastlock_trylock(struct fastlock *lock); void fastlock_unlock(struct fastlock *lock); void fastlock_free(struct fastlock *lock); +uint64_t fastlock_getlongwaitcount(); // this is a global value + /* End C API */ #ifdef __cplusplus } diff --git a/src/fastlock_x64.asm b/src/fastlock_x64.asm index 1b876350f..7645d3baa 100644 --- a/src/fastlock_x64.asm +++ b/src/fastlock_x64.asm @@ -2,6 +2,7 @@ section .text extern gettid extern sched_yield +extern g_longwaits ; This is the first use of assembly in this codebase, a valid question is WHY? ; The spinlock we implement here is performance critical, and simply put GCC @@ -49,6 +50,8 @@ ALIGN 16 syscall ; give up our timeslice we'll be here a while pop rax pop rsi + mov rcx, g_longwaits + inc qword [rcx] ; increment our long wait counter mov rdi, [rsp] ; our struct pointer is on the stack already xor ecx, ecx ; Reset our loop counter jmp .LLoop ; Get back in the game diff --git a/src/networking.cpp b/src/networking.cpp index 4a6794d98..b4f14f238 100644 --- a/src/networking.cpp +++ b/src/networking.cpp @@ -138,6 +138,7 @@ void linkClient(client *c) { * this way removing the client in unlinkClient() will not require * a linear scan, but just a constant time operation. */ c->client_list_node = listLast(server.clients); + if (c->fd != -1) atomicIncr(server.rgthreadvar[c->iel].cclients, 1); uint64_t id = htonu64(c->id); raxInsert(server.clients_index,(unsigned char*)&id,sizeof(id),c,NULL); } @@ -1208,6 +1209,8 @@ void unlinkClient(client *c) { aeDeleteFileEvent(server.rgthreadvar[c->iel].el,c->fd,AE_WRITABLE); close(c->fd); c->fd = -1; + + atomicDecr(server.rgthreadvar[c->iel].cclients, 1); } /* Remove from the list of pending writes if needed. */ diff --git a/src/server.cpp b/src/server.cpp index 8707b07f5..72a589c73 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -2815,6 +2815,7 @@ static void initServerThread(struct redisServerThreadVars *pvar, int fMain) pvar->unblocked_clients = listCreate(); pvar->clients_pending_asyncwrite = listCreate(); pvar->ipfd_count = 0; + pvar->cclients = 0; pvar->el = aeCreateEventLoop(server.maxclients+CONFIG_FDSET_INCR); if (pvar->el == NULL) { serverLog(LL_WARNING, @@ -3997,6 +3998,12 @@ extern "C" sds genRedisInfoString(const char *section) { listLength(server.clients)-listLength(server.slaves), maxin, maxout, server.blocked_clients); + for (int ithread = 0; ithread < server.cthreads; ++ithread) + { + info = sdscatprintf(info, + "thread_%d_clients:%d\r\n", + ithread, server.rgthreadvar[ithread].cclients); + } } /* Memory */ @@ -4401,11 +4408,15 @@ extern "C" sds genRedisInfoString(const char *section) { "used_cpu_sys:%ld.%06ld\r\n" "used_cpu_user:%ld.%06ld\r\n" "used_cpu_sys_children:%ld.%06ld\r\n" - "used_cpu_user_children:%ld.%06ld\r\n", + "used_cpu_user_children:%ld.%06ld\r\n" + "server_threads:%d\r\n" + "long_lock_waits:%" PRIu64 "\r\n", (long)self_ru.ru_stime.tv_sec, (long)self_ru.ru_stime.tv_usec, (long)self_ru.ru_utime.tv_sec, (long)self_ru.ru_utime.tv_usec, (long)c_ru.ru_stime.tv_sec, (long)c_ru.ru_stime.tv_usec, - (long)c_ru.ru_utime.tv_sec, (long)c_ru.ru_utime.tv_usec); + (long)c_ru.ru_utime.tv_sec, (long)c_ru.ru_utime.tv_usec, + server.cthreads, + fastlock_getlongwaitcount()); } /* Command statistics */ diff --git a/src/server.h b/src/server.h index 3fa5f90a3..3417d9696 100644 --- a/src/server.h +++ b/src/server.h @@ -1053,6 +1053,7 @@ struct redisServerThreadVars { list *clients_pending_write; /* There is to write or install handler. */ list *unblocked_clients; /* list of clients to unblock before next loop NOT THREADSAFE */ list *clients_pending_asyncwrite; + int cclients; struct fastlock lockPendingWrite; }; From b775ebb382240876234e179b73866c7d63a209d6 Mon Sep 17 00:00:00 2001 From: John Sully Date: Wed, 20 Mar 2019 04:14:33 +0000 Subject: [PATCH 23/42] ARM build fix: Don't use X64 asm here --- src/Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Makefile b/src/Makefile index 2ef54aa10..7ae04a2fa 100644 --- a/src/Makefile +++ b/src/Makefile @@ -45,8 +45,9 @@ endif endif USEASM?=true -# Do we use our assembly spinlock? +# Do we use our assembly spinlock? X64 only ifeq ($(uname_S),Linux) +ifeq ($(uname_M),x86_64) ifneq ($(@@),32bit) ifeq ($(USEASM),true) ASM_OBJ+= fastlock_x64.o @@ -55,6 +56,7 @@ ifeq ($(USEASM),true) endif endif endif +endif ifeq ($(COMPILER_NAME),clang) CXXFLAGS+= -stdlib=libc++ From 385f6190a3a9f8d2d5775bd058aaa2173dc05c8c Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Thu, 27 Sep 2018 18:12:31 +0300 Subject: [PATCH 24/42] getKeysFromCommand for TOUCH only extracted the first key. also, airty for COMMAND command was wrong. --- src/server.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/server.c b/src/server.c index 712cda1bd..1341ab405 100644 --- a/src/server.c +++ b/src/server.c @@ -715,7 +715,7 @@ struct redisCommand redisCommandTable[] = { {"touch",touchCommand,-2, "read-only fast @keyspace", - 0,NULL,1,1,1,0,0,0}, + 0,NULL,1,-1,1,0,0,0}, {"pttl",pttlCommand,2, "read-only fast random @keyspace", @@ -863,7 +863,7 @@ struct redisCommand redisCommandTable[] = { "no-script @keyspace", 0,NULL,0,0,0,0,0,0}, - {"command",commandCommand,0, + {"command",commandCommand,-1, "ok-loading ok-stale random @connection", 0,NULL,0,0,0,0,0,0}, From 747174388f305148b0832dd97b9754e2a64bdfef Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Thu, 27 Sep 2018 18:03:47 +0300 Subject: [PATCH 25/42] change SORT and SPOP to use lookupKeyWrite rather than lookupKeyRead like in SUNIONSTORE etc, commands that perform writes are expected to open all keys, even input keys, with lookupKeyWrite --- src/sort.c | 55 ++++++++++++++++++++++++++++++----------------------- src/t_set.c | 2 +- 2 files changed, 32 insertions(+), 25 deletions(-) diff --git a/src/sort.c b/src/sort.c index 8608cd8b3..db26da158 100644 --- a/src/sort.c +++ b/src/sort.c @@ -58,7 +58,7 @@ redisSortOperation *createSortOperation(int type, robj *pattern) { * * The returned object will always have its refcount increased by 1 * when it is non-NULL. */ -robj *lookupKeyByPattern(redisDb *db, robj *pattern, robj *subst) { +robj *lookupKeyByPattern(redisDb *db, robj *pattern, robj *subst, int writeflag) { char *p, *f, *k; sds spat, ssub; robj *keyobj, *fieldobj = NULL, *o; @@ -106,7 +106,10 @@ robj *lookupKeyByPattern(redisDb *db, robj *pattern, robj *subst) { decrRefCount(subst); /* Incremented by decodeObject() */ /* Lookup substituted key */ - o = lookupKeyRead(db,keyobj); + if (!writeflag) + o = lookupKeyRead(db,keyobj); + else + o = lookupKeyWrite(db,keyobj); if (o == NULL) goto noobj; if (fieldobj) { @@ -198,30 +201,12 @@ void sortCommand(client *c) { robj *sortval, *sortby = NULL, *storekey = NULL; redisSortObject *vector; /* Resulting vector to sort */ - /* Lookup the key to sort. It must be of the right types */ - sortval = lookupKeyRead(c->db,c->argv[1]); - if (sortval && sortval->type != OBJ_SET && - sortval->type != OBJ_LIST && - sortval->type != OBJ_ZSET) - { - addReply(c,shared.wrongtypeerr); - return; - } - /* Create a list of operations to perform for every sorted element. * Operations can be GET */ operations = listCreate(); listSetFreeMethod(operations,zfree); j = 2; /* options start at argv[2] */ - /* Now we need to protect sortval incrementing its count, in the future - * SORT may have options able to overwrite/delete keys during the sorting - * and the sorted key itself may get destroyed */ - if (sortval) - incrRefCount(sortval); - else - sortval = createQuicklistObject(); - /* The SORT command has an SQL-alike syntax, parse it */ while(j < c->argc) { int leftargs = c->argc-j-1; @@ -280,11 +265,33 @@ void sortCommand(client *c) { /* Handle syntax errors set during options parsing. */ if (syntax_error) { - decrRefCount(sortval); listRelease(operations); return; } + /* Lookup the key to sort. It must be of the right types */ + if (storekey) + sortval = lookupKeyRead(c->db,c->argv[1]); + else + sortval = lookupKeyWrite(c->db,c->argv[1]); + if (sortval && sortval->type != OBJ_SET && + sortval->type != OBJ_LIST && + sortval->type != OBJ_ZSET) + { + listRelease(operations); + addReply(c,shared.wrongtypeerr); + return; + } + + /* Now we need to protect sortval incrementing its count, in the future + * SORT may have options able to overwrite/delete keys during the sorting + * and the sorted key itself may get destroyed */ + if (sortval) + incrRefCount(sortval); + else + sortval = createQuicklistObject(); + + /* When sorting a set with no sort specified, we must sort the output * so the result is consistent across scripting and replication. * @@ -452,7 +459,7 @@ void sortCommand(client *c) { robj *byval; if (sortby) { /* lookup value to sort by */ - byval = lookupKeyByPattern(c->db,sortby,vector[j].obj); + byval = lookupKeyByPattern(c->db,sortby,vector[j].obj,storekey!=NULL); if (!byval) continue; } else { /* use object itself to sort by */ @@ -515,7 +522,7 @@ void sortCommand(client *c) { while((ln = listNext(&li))) { redisSortOperation *sop = ln->value; robj *val = lookupKeyByPattern(c->db,sop->pattern, - vector[j].obj); + vector[j].obj,storekey!=NULL); if (sop->type == SORT_OP_GET) { if (!val) { @@ -545,7 +552,7 @@ void sortCommand(client *c) { while((ln = listNext(&li))) { redisSortOperation *sop = ln->value; robj *val = lookupKeyByPattern(c->db,sop->pattern, - vector[j].obj); + vector[j].obj,storekey!=NULL); if (sop->type == SORT_OP_GET) { if (!val) val = createStringObject("",0); diff --git a/src/t_set.c b/src/t_set.c index cbe55aaa4..05d9ee243 100644 --- a/src/t_set.c +++ b/src/t_set.c @@ -415,7 +415,7 @@ void spopWithCountCommand(client *c) { /* Make sure a key with the name inputted exists, and that it's type is * indeed a set. Otherwise, return nil */ - if ((set = lookupKeyReadOrReply(c,c->argv[1],shared.null[c->resp])) + if ((set = lookupKeyWriteOrReply(c,c->argv[1],shared.null[c->resp])) == NULL || checkType(c,set,OBJ_SET)) return; /* If count is zero, serve an empty multibulk ASAP to avoid special From c9e2900efc1ed33727356df114fb716442ae2ce6 Mon Sep 17 00:00:00 2001 From: oranagra Date: Thu, 23 Feb 2017 03:13:44 -0800 Subject: [PATCH 26/42] bugfix to restartAOF, exit will never happen since retry will get negative. also reduce an excess sleep --- src/replication.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/replication.c b/src/replication.c index f2adc7995..59e42e561 100644 --- a/src/replication.c +++ b/src/replication.c @@ -1091,12 +1091,13 @@ void replicationCreateMasterClient(int fd, int dbid) { } void restartAOF() { - int retry = 10; - while (retry-- && startAppendOnly() == C_ERR) { + unsigned int tries, max_tries = 10; + for (tries = 0; tries < max_tries; ++tries) { + if (tries) sleep(1); + if (startAppendOnly() == C_OK) break; serverLog(LL_WARNING,"Failed enabling the AOF after successful master synchronization! Trying it again in one second."); - sleep(1); } - if (!retry) { + if (tries == max_tries) { serverLog(LL_WARNING,"FATAL: this replica instance finished the synchronization with its master, but the AOF can't be turned on. Exiting now."); exit(1); } From 9dabbd1ab072f3abced48b4995d9ef3e745f0608 Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 21 Mar 2019 12:18:55 +0100 Subject: [PATCH 27/42] Alter coding style in #4696 to conform to Redis code base. --- src/zmalloc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/zmalloc.c b/src/zmalloc.c index 4c40a7782..5e6010278 100644 --- a/src/zmalloc.c +++ b/src/zmalloc.c @@ -148,7 +148,7 @@ void *zrealloc(void *ptr, size_t size) { size_t oldsize; void *newptr; - if (size == 0 && ptr!=NULL) { + if (size == 0 && ptr != NULL) { zfree(ptr); return NULL; } From 9588fd52ac3333d0bf3243523ec9a165fa18f87e Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 21 Mar 2019 17:18:24 +0100 Subject: [PATCH 28/42] Mostly aesthetic changes to restartAOF(). See #3829. --- src/replication.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/replication.c b/src/replication.c index 59e42e561..c25e7fa6f 100644 --- a/src/replication.c +++ b/src/replication.c @@ -1093,12 +1093,16 @@ void replicationCreateMasterClient(int fd, int dbid) { void restartAOF() { unsigned int tries, max_tries = 10; for (tries = 0; tries < max_tries; ++tries) { - if (tries) sleep(1); if (startAppendOnly() == C_OK) break; - serverLog(LL_WARNING,"Failed enabling the AOF after successful master synchronization! Trying it again in one second."); + serverLog(LL_WARNING, + "Failed enabling the AOF after successful master synchronization! " + "Trying it again in one second."); + sleep(1); } if (tries == max_tries) { - serverLog(LL_WARNING,"FATAL: this replica instance finished the synchronization with its master, but the AOF can't be turned on. Exiting now."); + serverLog(LL_WARNING, + "FATAL: this replica instance finished the synchronization with " + "its master, but the AOF can't be turned on. Exiting now."); exit(1); } } From b3408e9a9b1bdf8ea59bf80d715c695a113820f3 Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 21 Mar 2019 17:21:25 +0100 Subject: [PATCH 29/42] More sensible name for function: restartAOFAfterSYNC(). Related to #3829. --- src/replication.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/replication.c b/src/replication.c index c25e7fa6f..a27c29a3b 100644 --- a/src/replication.c +++ b/src/replication.c @@ -1090,7 +1090,11 @@ void replicationCreateMasterClient(int fd, int dbid) { if (dbid != -1) selectDb(server.master,dbid); } -void restartAOF() { +/* This function will try to re-enable the AOF file after the + * master-replica synchronization: if it fails after multiple attempts + * the replica cannot be considered reliable and exists with an + * error. */ +void restartAOFAfterSYNC() { unsigned int tries, max_tries = 10; for (tries = 0; tries < max_tries; ++tries) { if (startAppendOnly() == C_OK) break; @@ -1289,7 +1293,7 @@ void readSyncBulkPayload(aeEventLoop *el, int fd, void *privdata, int mask) { cancelReplicationHandshake(); /* Re-enable the AOF if we disabled it earlier, in order to restore * the original configuration. */ - if (aof_is_enabled) restartAOF(); + if (aof_is_enabled) restartAOFAfterSYNC(); return; } /* Final setup of the connected slave <- master link */ @@ -1314,7 +1318,7 @@ void readSyncBulkPayload(aeEventLoop *el, int fd, void *privdata, int mask) { /* Restart the AOF subsystem now that we finished the sync. This * will trigger an AOF rewrite, and when done will start appending * to the new file. */ - if (aof_is_enabled) restartAOF(); + if (aof_is_enabled) restartAOFAfterSYNC(); } return; From 27a420fbc2ada275376982b880dc8f6da935d53e Mon Sep 17 00:00:00 2001 From: John Sully Date: Thu, 21 Mar 2019 21:57:18 +0000 Subject: [PATCH 30/42] Compile issues at O0 optimization --- deps/hiredis/sds.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deps/hiredis/sds.h b/deps/hiredis/sds.h index 404f246c4..6b46297c2 100644 --- a/deps/hiredis/sds.h +++ b/deps/hiredis/sds.h @@ -83,7 +83,7 @@ struct __attribute__ ((__packed__)) sdshdr64 { #define SDS_HDR(T,s) ((struct sdshdr##T *)((s)-(sizeof(struct sdshdr##T)))) #define SDS_TYPE_5_LEN(f) ((f)>>SDS_TYPE_BITS) -inline size_t sdslen(const sds s) { +static inline size_t sdslen(const sds s) { unsigned char flags = s[-1]; switch(__builtin_expect((flags&SDS_TYPE_MASK), SDS_TYPE_5)) { From e62c6b0ed05811d204637d9cd679d7ef731f8fc7 Mon Sep 17 00:00:00 2001 From: Richard Silver Date: Thu, 21 Mar 2019 15:15:11 -0700 Subject: [PATCH 31/42] Added build Dockerfile --- Dockerfile | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 Dockerfile diff --git a/Dockerfile b/Dockerfile new file mode 100644 index 000000000..5f2c43011 --- /dev/null +++ b/Dockerfile @@ -0,0 +1,7 @@ +FROM ubuntu:18.04 + +RUN apt-get update \ + && apt-get install -qqy build-essential nasm autotools-dev autoconf libjemalloc-dev \ + && apt-get clean + +CMD make \ No newline at end of file From ab9122f2cb910bb5560a9fde16bc3c096ecfb510 Mon Sep 17 00:00:00 2001 From: John Sully Date: Thu, 21 Mar 2019 22:17:04 +0000 Subject: [PATCH 32/42] Polarity of the weak flag was wrong. We want to use the strong variant of atomic_compare_exchange always --- src/fastlock.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/fastlock.cpp b/src/fastlock.cpp index 1a8d51165..6dd2893bb 100644 --- a/src/fastlock.cpp +++ b/src/fastlock.cpp @@ -113,7 +113,7 @@ extern "C" int fastlock_trylock(struct fastlock *lock) struct ticket ticket_expect { active, active }; struct ticket ticket_setiflocked { active, next }; - if (__atomic_compare_exchange(&lock->m_ticket, &ticket_expect, &ticket_setiflocked, true /*strong*/, __ATOMIC_ACQUIRE, __ATOMIC_ACQUIRE)) + if (__atomic_compare_exchange(&lock->m_ticket, &ticket_expect, &ticket_setiflocked, false /*strong*/, __ATOMIC_ACQUIRE, __ATOMIC_ACQUIRE)) { lock->m_depth = 1; __atomic_store_4(&lock->m_pidOwner, gettid(), __ATOMIC_RELEASE); @@ -147,4 +147,4 @@ extern "C" void fastlock_free(struct fastlock *lock) bool fastlock::fOwnLock() { return gettid() == m_pidOwner; -} \ No newline at end of file +} From 19956b3807aa6895b438162f39d17db17ef726f4 Mon Sep 17 00:00:00 2001 From: John Sully Date: Thu, 21 Mar 2019 22:18:48 +0000 Subject: [PATCH 33/42] comment issue --- src/fastlock.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fastlock.cpp b/src/fastlock.cpp index 6dd2893bb..dcbcff688 100644 --- a/src/fastlock.cpp +++ b/src/fastlock.cpp @@ -113,7 +113,7 @@ extern "C" int fastlock_trylock(struct fastlock *lock) struct ticket ticket_expect { active, active }; struct ticket ticket_setiflocked { active, next }; - if (__atomic_compare_exchange(&lock->m_ticket, &ticket_expect, &ticket_setiflocked, false /*strong*/, __ATOMIC_ACQUIRE, __ATOMIC_ACQUIRE)) + if (__atomic_compare_exchange(&lock->m_ticket, &ticket_expect, &ticket_setiflocked, false /*weak*/, __ATOMIC_ACQUIRE, __ATOMIC_ACQUIRE)) { lock->m_depth = 1; __atomic_store_4(&lock->m_pidOwner, gettid(), __ATOMIC_RELEASE); From 677ae5487f2b68ddd045e5cf2af3c49d625adef6 Mon Sep 17 00:00:00 2001 From: Richard Silver Date: Thu, 21 Mar 2019 16:18:37 -0700 Subject: [PATCH 34/42] updated readme and Dockerfile to allow for tests to be run in container --- Dockerfile | 5 ++++- README.md | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 5f2c43011..36c5621af 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,7 +1,10 @@ FROM ubuntu:18.04 + + RUN apt-get update \ - && apt-get install -qqy build-essential nasm autotools-dev autoconf libjemalloc-dev \ + && DEBIAN_FRONTEND=noninteractive apt-get install -qqy \ + build-essential nasm autotools-dev autoconf libjemalloc-dev tcl tcl-dev \ && apt-get clean CMD make \ No newline at end of file diff --git a/README.md b/README.md index bcfbb423b..5afe9a394 100644 --- a/README.md +++ b/README.md @@ -204,6 +204,26 @@ Future work: - Allow rebalancing of connections to different threads after the connection - Allow multiple readers access to the hashtable concurrently +Docker Build +------------ + +Run the following commands for a full source download and build: + +``` +git clone git@github.com:JohnSully/KeyDB.git +docker run -it --rm `pwd`/KeyDB:/build -w /build devopsdood/keydb-builder make +``` + +Then you have fresh binaries built, you can also pass any other options to the make command above after the word make. E.g. + +```docker run -it --rm `pwd`/KeyDB:/build -w /build devopsdood/keydb-builder make MAllOC=memkind``` + +The above commands will build you binaries in the src directory. Standard `make install` without Docker command will work after if you wish to install + +If you'd prefer you can build the Dockerfile in the repo instead of pulling the above container for use: + +`docker build -t KeyDB .` + Code contributions ----------------- From ef7e89352de93331c8ef05504f1d7c5fbae6edaa Mon Sep 17 00:00:00 2001 From: The Gitter Badger Date: Fri, 22 Mar 2019 01:24:32 +0000 Subject: [PATCH 35/42] Add Gitter badge --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 5afe9a394..6b28f1c60 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,5 @@ ![Current Release](https://img.shields.io/github/release/JohnSully/KeyDB.svg) -[![Build Status](https://travis-ci.org/JohnSully/KeyDB.svg?branch=unstable)](https://travis-ci.org/JohnSully/KeyDB) +[![Build Status](https://travis-ci.org/JohnSully/KeyDB.svg?branch=unstable)](https://travis-ci.org/JohnSully/KeyDB) [![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) What is KeyDB? -------------- From 550b3a3a99228d2b6a0f9b8691d28121719663e9 Mon Sep 17 00:00:00 2001 From: John Sully Date: Thu, 21 Mar 2019 21:25:49 -0400 Subject: [PATCH 36/42] Update README.md --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 5afe9a394..239544bd9 100644 --- a/README.md +++ b/README.md @@ -11,6 +11,7 @@ On the same hardware KeyDB can perform twice as many queries per second as Redis KeyDB has full compatibility with the Redis protocol, modules, and scripts. This includes full support for transactions, and atomic execution of scripts. For more information see our architecture section below. Try our docker container: https://hub.docker.com/r/eqalpha/keydb +Talk on Gitter: https://gitter.im/KeyDB Why fork Redis? --------------- From 4c98bc690efce0a7afc4d0fe35a67275f2b3629f Mon Sep 17 00:00:00 2001 From: John Sully Date: Thu, 21 Mar 2019 21:26:00 -0400 Subject: [PATCH 37/42] Update README.md --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 239544bd9..9547bb61c 100644 --- a/README.md +++ b/README.md @@ -11,6 +11,7 @@ On the same hardware KeyDB can perform twice as many queries per second as Redis KeyDB has full compatibility with the Redis protocol, modules, and scripts. This includes full support for transactions, and atomic execution of scripts. For more information see our architecture section below. Try our docker container: https://hub.docker.com/r/eqalpha/keydb + Talk on Gitter: https://gitter.im/KeyDB Why fork Redis? From 7b914c253166bc91bbe0852a8f737a7ef2306c6a Mon Sep 17 00:00:00 2001 From: John Sully Date: Thu, 21 Mar 2019 23:11:19 -0400 Subject: [PATCH 38/42] Fix kqueue build error --- src/ae_kqueue.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ae_kqueue.c b/src/ae_kqueue.c index 19ac9ffc1..200bbd09e 100644 --- a/src/ae_kqueue.c +++ b/src/ae_kqueue.c @@ -39,7 +39,7 @@ typedef struct aeApiState { } aeApiState; static int aeApiCreate(aeEventLoop *eventLoop) { - aeApiState *state = zmalloc(sizeof(aeApiState)); + aeApiState *state = zmalloc(sizeof(aeApiState), MALLOC_LOCAL); if (!state) return -1; state->events = zmalloc(sizeof(struct kevent)*eventLoop->setsize, MALLOC_LOCAL); From a7aa2b074049a130761bc0a98d47130b6a0ff817 Mon Sep 17 00:00:00 2001 From: John Sully Date: Sun, 24 Mar 2019 15:39:10 -0400 Subject: [PATCH 39/42] Active Replica Support --- redis.conf | 5 ++ src/Makefile | 2 +- src/config.c | 11 +++++ src/networking.cpp | 3 +- src/replication.cpp | 110 ++++++++++++++++++++++++++++++++++++++++---- src/server.cpp | 6 +++ src/server.h | 36 +++++++++++---- 7 files changed, 153 insertions(+), 20 deletions(-) diff --git a/redis.conf b/redis.conf index c37989ece..99f66a667 100644 --- a/redis.conf +++ b/redis.conf @@ -1562,3 +1562,8 @@ server-threads 2 # Should KeyDB pin threads to CPUs? By default this is disabled, and KeyDB will not bind threads. # When enabled threads are bount to cores sequentially starting at core 0. # server-thread-affinity true + +# Uncomment the option below to enable Active Active support. Note that +# replicas will still sync in the normal way and incorrect ordering when +# bringing up replicas can result in data loss (the first master will win). +# active-replica yes diff --git a/src/Makefile b/src/Makefile index 7ae04a2fa..70e297929 100644 --- a/src/Makefile +++ b/src/Makefile @@ -94,7 +94,7 @@ endif FINAL_CFLAGS=$(STD) $(WARN) $(OPT) $(DEBUG) $(CFLAGS) $(REDIS_CFLAGS) FINAL_CXXFLAGS=$(CXX_STD) $(WARN) $(OPT) $(DEBUG) $(CFLAGS) $(CXXFLAGS) $(REDIS_CFLAGS) FINAL_LDFLAGS=$(LDFLAGS) $(REDIS_LDFLAGS) $(DEBUG) -FINAL_LIBS=-lm +FINAL_LIBS=-lm -luuid DEBUG=-g -ggdb ifeq ($(uname_S),SunOS) diff --git a/src/config.c b/src/config.c index fe563e7ed..dd8bdfdf8 100644 --- a/src/config.c +++ b/src/config.c @@ -849,6 +849,16 @@ void loadServerConfigFromString(char *config) { err = "Unknown argument: server-thread-affinity expects either true or false"; goto loaderr; } + } else if (!strcasecmp(argv[0], "active-replica") && argc == 2) { + server.fActiveReplica = yesnotoi(argv[1]); + if (server.repl_slave_ro) { + server.repl_slave_ro = FALSE; + serverLog(LL_NOTICE, "Notice: \"active-replica yes\" implies \"replica-read-only no\""); + } + if (server.fActiveReplica == -1) { + server.fActiveReplica = CONFIG_DEFAULT_ACTIVE_REPLICA; + err = "argument must be 'yes' or 'no'"; goto loaderr; + } } else { err = "Bad directive or wrong number of arguments"; goto loaderr; } @@ -2350,6 +2360,7 @@ int rewriteConfig(char *path) { rewriteConfigYesNoOption(state,"lazyfree-lazy-server-del",server.lazyfree_lazy_server_del,CONFIG_DEFAULT_LAZYFREE_LAZY_SERVER_DEL); rewriteConfigYesNoOption(state,"replica-lazy-flush",server.repl_slave_lazy_flush,CONFIG_DEFAULT_SLAVE_LAZY_FLUSH); rewriteConfigYesNoOption(state,"dynamic-hz",server.dynamic_hz,CONFIG_DEFAULT_DYNAMIC_HZ); + rewriteConfigYesNoOption(state,"active-replica",server.fActiveReplica,CONFIG_DEFAULT_ACTIVE_REPLICA); /* Rewrite Sentinel config if in Sentinel mode. */ if (server.sentinel_mode) rewriteConfigSentinelOption(state); diff --git a/src/networking.cpp b/src/networking.cpp index b4f14f238..550f84b9f 100644 --- a/src/networking.cpp +++ b/src/networking.cpp @@ -226,6 +226,7 @@ client *createClient(int fd, int iel) { c->bufAsync = NULL; c->buflenAsync = 0; c->bufposAsync = 0; + memset(c->uuid, 0, UUID_BINARY_LEN); listSetFreeMethod(c->pubsub_patterns,decrRefCountVoid); listSetMatchMethod(c->pubsub_patterns,listMatchObjects); @@ -2135,7 +2136,7 @@ void readQueryFromClient(aeEventLoop *el, int fd, void *privdata, int mask) { * corresponding part of the replication stream, will be propagated to * the sub-slaves and to the replication backlog. */ processInputBufferAndReplicate(c); - aelock.arm(nullptr); + aelock.arm(c); ProcessPendingAsyncWrites(); } diff --git a/src/replication.cpp b/src/replication.cpp index 23989428f..5c885659d 100644 --- a/src/replication.cpp +++ b/src/replication.cpp @@ -38,6 +38,7 @@ #include #include #include +#include void replicationDiscardCachedMaster(void); void replicationResurrectCachedMaster(int newfd); @@ -74,6 +75,21 @@ char *replicationGetSlaveName(client *c) { return buf; } +static bool FSameHost(client *clientA, client *clientB) +{ + const unsigned char *a = clientA->uuid; + const unsigned char *b = clientB->uuid; + + unsigned char zeroCheck = 0; + for (int i = 0; i < UUID_BINARY_LEN; ++i) + { + if (a[i] != b[i]) + return false; + zeroCheck |= a[i]; + } + return (zeroCheck != 0); // if the UUID is nil then it is never equal +} + /* ---------------------------------- MASTER -------------------------------- */ void createReplicationBacklog(void) { @@ -117,7 +133,13 @@ void resizeReplicationBacklog(long long newsize) { void freeReplicationBacklog(void) { serverAssert(GlobalLocksAcquired()); - serverAssert(listLength(server.slaves) == 0); + listIter li; + listNode *ln; + listRewind(server.slaves, &li); + while ((ln = listNext(&li))) { + // server.slaves should be empty, or filled with clients pending close + serverAssert(((client*)listNodeValue(ln))->flags & CLIENT_CLOSE_ASAP); + } zfree(server.repl_backlog); server.repl_backlog = NULL; } @@ -186,7 +208,7 @@ void replicationFeedSlaves(list *slaves, int dictid, robj **argv, int argc) { * propagate *identical* replication stream. In this way this slave can * advertise the same replication ID as the master (since it shares the * master replication history and has the same backlog and offsets). */ - if (server.masterhost != NULL) return; + if (!server.fActiveReplica && server.masterhost != NULL) return; /* If there aren't slaves, and there is no backlog buffer to populate, * we can return ASAP. */ @@ -226,6 +248,7 @@ void replicationFeedSlaves(list *slaves, int dictid, robj **argv, int argc) { while((ln = listNext(&li))) { client *slave = (client*)ln->value; if (slave->replstate == SLAVE_STATE_WAIT_BGSAVE_START) continue; + if (server.current_client && FSameHost(server.current_client, slave)) continue; addReplyAsync(slave,selectcmd); } @@ -268,6 +291,7 @@ void replicationFeedSlaves(list *slaves, int dictid, robj **argv, int argc) { /* Don't feed slaves that are still waiting for BGSAVE to start */ if (slave->replstate == SLAVE_STATE_WAIT_BGSAVE_START) continue; + if (server.current_client && FSameHost(server.current_client, slave)) continue; /* Feed slaves that are waiting for the initial SYNC (so these commands * are queued in the output buffer until the initial SYNC completes), @@ -282,10 +306,10 @@ void replicationFeedSlaves(list *slaves, int dictid, robj **argv, int argc) { addReplyBulkAsync(slave,argv[j]); } - /* Release the lock on all slaves */ listRewind(slaves,&li); while((ln = listNext(&li))) { - ((client*)ln->value)->lock.unlock(); + client *slave = (client*)ln->value; + slave->lock.unlock(); } } @@ -311,6 +335,8 @@ void replicationFeedSlavesFromMasterStream(list *slaves, char *buf, size_t bufle while((ln = listNext(&li))) { client *slave = (client*)ln->value; std::lock_guardlock)> ulock(slave->lock); + if (FSameHost(slave, server.master)) + continue; // Active Active case, don't feed back /* Don't feed slaves that are still waiting for BGSAVE to start */ if (slave->replstate == SLAVE_STATE_WAIT_BGSAVE_START) continue; @@ -658,9 +684,11 @@ void syncCommand(client *c) { /* Refuse SYNC requests if we are a slave but the link with our master * is not ok... */ - if (server.masterhost && server.repl_state != REPL_STATE_CONNECTED) { - addReplySds(c,sdsnew("-NOMASTERLINK Can't SYNC while not connected with my master\r\n")); - return; + if (!server.fActiveReplica) { + if (server.masterhost && server.repl_state != REPL_STATE_CONNECTED) { + addReplySds(c,sdsnew("-NOMASTERLINK Can't SYNC while not connected with my master\r\n")); + return; + } } /* SYNC can't be issued when the server has pending data to send to @@ -790,6 +818,33 @@ void syncCommand(client *c) { return; } +void processReplconfUuid(client *c, robj *arg) +{ + try + { + if (arg->type != OBJ_STRING) + throw "Invalid UUID"; + + const char *remoteUUID = (const char*)ptrFromObj(arg); + if (strlen(remoteUUID) != 36) + throw "Invalid UUID"; + + if (uuid_parse(remoteUUID, c->uuid) != 0) + throw "Invalid UUID"; + + char szServerUUID[36 + 2]; // 1 for the '+', another for '\0' + szServerUUID[0] = '+'; + uuid_unparse(server.uuid, szServerUUID+1); + addReplyProto(c, szServerUUID, 37); + addReplyProto(c, "\r\n", 2); + } + catch (const char *szErr) + { + addReplyError(c, szErr); + return; + } +} + /* REPLCONF