From 234e6e7b0940db9484e99dd8180dfd18ed61c046 Mon Sep 17 00:00:00 2001 From: Guy Benoish Date: Mon, 3 Feb 2020 17:19:00 +0530 Subject: [PATCH] Diskless-load emptyDb-related fixes 1. Call emptyDb even in case of diskless-load: We want modules to get the same FLUSHDB event as disk-based replication. 2. Do not fire any module events when flushing the backups array. 3. Delete redundant call to signalFlushedDb (Called from emptyDb). Former-commit-id: aa8a3077a2d20e66e34f72f2860d0cc3daad496e --- src/db.cpp | 55 +++++++++++++++++++++++++++------------------ src/replication.cpp | 14 +++++++----- src/server.h | 1 + 3 files changed, 43 insertions(+), 27 deletions(-) diff --git a/src/db.cpp b/src/db.cpp index 2a1df37ac..b1d9cad8e 100644 --- a/src/db.cpp +++ b/src/db.cpp @@ -443,7 +443,10 @@ robj *dbUnshareStringValue(redisDb *db, robj *key, robj *o) { * DB number if we want to flush only a single Redis database number. * * Flags are be EMPTYDB_NO_FLAGS if no special flags are specified or - * EMPTYDB_ASYNC if we want the memory to be freed in a different thread + * 1. EMPTYDB_ASYNC if we want the memory to be freed in a different thread. + * 2. EMPTYDB_BACKUP if we want to empty the backup dictionaries created by + * disklessLoadMakeBackups. In that case we only free memory and avoid + * firing module events. * and the function to return ASAP. * * On success the fuction returns the number of keys removed from the @@ -451,6 +454,8 @@ robj *dbUnshareStringValue(redisDb *db, robj *key, robj *o) { * DB number is out of range, and errno is set to EINVAL. */ long long emptyDbGeneric(redisDb *dbarray, int dbnum, int flags, void(callback)(void*)) { int async = (flags & EMPTYDB_ASYNC); + int backup = (flags & EMPTYDB_BACKUP); /* Just free the memory, nothing else */ + RedisModuleFlushInfoV1 fi = {REDISMODULE_FLUSHINFO_VERSION,!async,dbnum}; long long removed = 0; if (dbnum < -1 || dbnum >= cserver.dbnum) { @@ -458,16 +463,18 @@ long long emptyDbGeneric(redisDb *dbarray, int dbnum, int flags, void(callback)( return -1; } - /* Fire the flushdb modules event. */ - RedisModuleFlushInfoV1 fi = {REDISMODULE_FLUSHINFO_VERSION,!async,dbnum}; - moduleFireServerEvent(REDISMODULE_EVENT_FLUSHDB, - REDISMODULE_SUBEVENT_FLUSHDB_START, - &fi); + /* Pre-flush actions */ + if (!backup) { + /* Fire the flushdb modules event. */ + moduleFireServerEvent(REDISMODULE_EVENT_FLUSHDB, + REDISMODULE_SUBEVENT_FLUSHDB_START, + &fi); - /* Make sure the WATCHed keys are affected by the FLUSH* commands. - * Note that we need to call the function while the keys are still - * there. */ - signalFlushedDb(dbnum); + /* Make sure the WATCHed keys are affected by the FLUSH* commands. + * Note that we need to call the function while the keys are still + * there. */ + signalFlushedDb(dbnum); + } int startdb, enddb; if (dbnum == -1) { @@ -486,20 +493,24 @@ long long emptyDbGeneric(redisDb *dbarray, int dbnum, int flags, void(callback)( dbarray[j].setexpire->clear(); } } - if (g_pserver->cluster_enabled) { - if (async) { - slotToKeyFlushAsync(); - } else { - slotToKeyFlush(); + + /* Post-flush actions */ + if (!backup) { + if (g_pserver->cluster_enabled) { + if (async) { + slotToKeyFlushAsync(); + } else { + slotToKeyFlush(); + } } - } - if (dbnum == -1) flushSlaveKeysWithExpireList(); + if (dbnum == -1) flushSlaveKeysWithExpireList(); - /* Also fire the end event. Note that this event will fire almost - * immediately after the start event if the flush is asynchronous. */ - moduleFireServerEvent(REDISMODULE_EVENT_FLUSHDB, - REDISMODULE_SUBEVENT_FLUSHDB_END, - &fi); + /* Also fire the end event. Note that this event will fire almost + * immediately after the start event if the flush is asynchronous. */ + moduleFireServerEvent(REDISMODULE_EVENT_FLUSHDB, + REDISMODULE_SUBEVENT_FLUSHDB_END, + &fi); + } return removed; } diff --git a/src/replication.cpp b/src/replication.cpp index aa220c95a..11ac8eb4e 100644 --- a/src/replication.cpp +++ b/src/replication.cpp @@ -1663,8 +1663,8 @@ void disklessLoadRestoreBackups(redisDb *backup, int restore, int empty_db_flags g_pserver->db[i] = backup[i]; } } else { - /* Delete. */ - emptyDbGeneric(backup,-1,empty_db_flags,replicationEmptyDbCallback); + /* Delete (Pass EMPTYDB_BACKUP in order to avoid firing module events) . */ + emptyDbGeneric(backup,-1,empty_db_flags|EMPTYDB_BACKUP,replicationEmptyDbCallback); for (int i=0; iaof_state != AOF_OFF) stopAppendOnly(); - signalFlushedDb(-1); /* When diskless RDB loading is used by replicas, it may be configured * in order to save the current DB instead of throwing it away, @@ -1863,10 +1862,15 @@ void readSyncBulkPayload(connection *conn) { if (use_diskless_load && g_pserver->repl_diskless_load == REPL_DISKLESS_LOAD_SWAPDB) { + /* Create a backup of server.db[] and initialize to empty + * dictionaries */ diskless_load_backup = disklessLoadMakeBackups(); - } else { - emptyDb(-1,empty_db_flags,replicationEmptyDbCallback); } + /* We call to emptyDb even in case of REPL_DISKLESS_LOAD_SWAPDB + * (Where disklessLoadMakeBackups left server.db empty) because we + * want to execute all the auxiliary logic of emptyDb (Namely, + * fire module events) */ + emptyDb(-1,empty_db_flags,replicationEmptyDbCallback); /* Before loading the DB into memory we need to delete the readable * handler, otherwise it will get called recursively since diff --git a/src/server.h b/src/server.h index 957067bfa..b8a11e888 100644 --- a/src/server.h +++ b/src/server.h @@ -2627,6 +2627,7 @@ robj *dbUnshareStringValue(redisDb *db, robj *key, robj *o); #define EMPTYDB_NO_FLAGS 0 /* No flags. */ #define EMPTYDB_ASYNC (1<<0) /* Reclaim memory in another thread. */ +#define EMPTYDB_BACKUP (1<<2) /* DB array is a backup for REPL_DISKLESS_LOAD_SWAPDB. */ long long emptyDb(int dbnum, int flags, void(callback)(void*)); long long emptyDbGeneric(redisDb *dbarray, int dbnum, int flags, void(callback)(void*)); void flushAllDataAndResetRDB(int flags);