From f31af3bacffbde874d7ca9a52fdd36338b9fcc8d Mon Sep 17 00:00:00 2001 From: John Sully Date: Tue, 28 Jan 2020 21:39:56 -0500 Subject: [PATCH 1/4] CRON jobs were running half as often as required Former-commit-id: 1069d0b4bf39818cbd72d0bcbd168769244cf18f --- src/cron.cpp | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/cron.cpp b/src/cron.cpp index 72d26f5c3..fd777d43d 100644 --- a/src/cron.cpp +++ b/src/cron.cpp @@ -110,12 +110,20 @@ void executeCronJobExpireHook(const char *key, robj *o) if (job->startTime < (uint64_t)g_pserver->mstime) { // If we are more than one interval in the past then fast forward to - // the first interval still in the future - auto delta = g_pserver->mstime - job->startTime; - auto multiple = (delta / job->interval)+1; - job->startTime += job->interval * multiple; + // the first interval still in the future. If startTime wasn't zero align + // this to the original startTime, if it was zero align to now + if (job->startTime == job->interval) + { // startTime was 0 + job->startTime = g_pserver->mstime + job->interval; + } + else + { + auto delta = g_pserver->mstime - job->startTime; + auto multiple = (delta / job->interval)+1; + job->startTime += job->interval * multiple; + } } - setExpire(cFake, cFake->db, keyobj, keyobj, job->startTime + job->interval); + setExpire(cFake, cFake->db, keyobj, keyobj, job->startTime); } notifyKeyspaceEvent(NOTIFY_KEYEVENT, "CRON Executed", keyobj, dbId); From 4352a26a5bf51d1b38c980fab5c4a2f3453a8557 Mon Sep 17 00:00:00 2001 From: John Sully Date: Thu, 30 Jan 2020 17:55:48 -0500 Subject: [PATCH 2/4] Fix memory leak in cron Former-commit-id: a9667e84ad44a3f2c08df0d95caeb6364f3f3509 --- src/cron.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cron.cpp b/src/cron.cpp index 2c20a5c0e..58f709a2d 100644 --- a/src/cron.cpp +++ b/src/cron.cpp @@ -67,6 +67,7 @@ void cronCommand(client *c) robj *o = createObject(OBJ_CRON, spjob.release()); setKey(c->db, c->argv[ARG_NAME], o); + decrRefCount(o); // use an expire to trigger execution. Note: We use a subkey expire here so legacy clients don't delete it. setExpire(c, c->db, c->argv[ARG_NAME], c->argv[ARG_NAME], base + interval); addReply(c, shared.ok); From a7b70405771536a75d57a43000d835bcf22b2913 Mon Sep 17 00:00:00 2001 From: John Sully Date: Thu, 30 Jan 2020 17:56:24 -0500 Subject: [PATCH 3/4] replication memory leaks Former-commit-id: 73020b6a939f241ade7512d58a4ddf17f5a803c5 --- src/replication.cpp | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/replication.cpp b/src/replication.cpp index 853aaa4a3..b6d988dae 100644 --- a/src/replication.cpp +++ b/src/replication.cpp @@ -2628,7 +2628,6 @@ void syncWithMaster(connection *conn) { serverLog(LL_WARNING,"Opening the temp file needed for MASTER <-> REPLICA synchronization: %s",strerror(errno)); goto error; } - mi->repl_transfer_tmpfile = zstrdup(tmpfile); mi->repl_transfer_fd = dfd; } @@ -2648,6 +2647,8 @@ void syncWithMaster(connection *conn) { mi->repl_transfer_read = 0; mi->repl_transfer_last_fsync_off = 0; mi->repl_transfer_lastio = g_pserver->unixtime; + if (mi->repl_transfer_tmpfile) + zfree(mi->repl_transfer_tmpfile); mi->repl_transfer_tmpfile = zstrdup(tmpfile); return; @@ -2805,7 +2806,13 @@ void freeMasterInfo(redisMaster *mi) { zfree(mi->masterauth); zfree(mi->masteruser); + if (mi->repl_transfer_tmpfile) + zfree(mi->repl_transfer_tmpfile); delete mi->staleKeyMap; + if (mi->cached_master != nullptr) + freeClientAsync(mi->cached_master); + if (mi->master != nullptr) + freeClientAsync(mi->master); zfree(mi); } @@ -3098,6 +3105,12 @@ void replicationCacheMaster(redisMaster *mi, client *c) { * current offset if no data was lost during the failover. So we use our * current replication ID and offset in order to synthesize a cached master. */ void replicationCacheMasterUsingMyself(redisMaster *mi) { + if (mi->cached_master != nullptr) + { + // This can happen on first load of the RDB, the master we created in config load is stale + freeClient(mi->cached_master); + } + /* The master client we create can be set to any DBID, because * the new master will start its replication stream with SELECT. */ mi->master_initial_offset = g_pserver->master_repl_offset; From 958b86ddbbf0e5104fc2aacbb49c9e5e04e5760a Mon Sep 17 00:00:00 2001 From: John Sully Date: Thu, 30 Jan 2020 17:57:10 -0500 Subject: [PATCH 4/4] RDB memory leaks Former-commit-id: 6208118b133c7f4209fd0a55d2a75341407e3e2c --- src/rdb.cpp | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/src/rdb.cpp b/src/rdb.cpp index 8ae4a0651..a3351fe59 100644 --- a/src/rdb.cpp +++ b/src/rdb.cpp @@ -1632,9 +1632,17 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, robj *key, uint64_t mvcc_tstamp) { len--; /* Load raw strings */ if ((field = (sds)rdbGenericLoadStringObject(rdb,RDB_LOAD_SDS,NULL)) - == NULL) return NULL; + == NULL) + { + decrRefCount(o); + return NULL; + } if ((value = (sds)rdbGenericLoadStringObject(rdb,RDB_LOAD_SDS,NULL)) - == NULL) return NULL; + == NULL) + { + decrRefCount(o); + return NULL; + } /* Add pair to ziplist */ o->m_ptr = ziplistPush((unsigned char*)ptrFromObj(o), (unsigned char*)field, @@ -1663,9 +1671,17 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, robj *key, uint64_t mvcc_tstamp) { len--; /* Load encoded strings */ if ((field = (sds)rdbGenericLoadStringObject(rdb,RDB_LOAD_SDS,NULL)) - == NULL) return NULL; + == NULL) + { + decrRefCount(o); + return NULL; + } if ((value = (sds)rdbGenericLoadStringObject(rdb,RDB_LOAD_SDS,NULL)) - == NULL) return NULL; + == NULL) + { + decrRefCount(o); + return NULL; + } /* Add pair to hash table */ ret = dictAdd((dict*)ptrFromObj(o), field, value);