From e9bff7978a4e4b5cc7ab958715d1a6214dc68919 Mon Sep 17 00:00:00 2001 From: chenyang8094 Date: Thu, 13 Jan 2022 14:49:26 +0800 Subject: [PATCH] Always create base AOF file when redis start from empty. (#10102) Force create a BASE file (use a foreground `rewriteAppendOnlyFile`) when redis starts from an empty data set and `appendonly` is yes. The reasoning is that normally, after redis is running for some time, and the AOF has gone though a few rewrites, there's always a base rdb file. and the scenario where the base file is missing, is kinda rare (happens only at empty startup), so this change normalizes it. But more importantly, there are or could be some complex modules that are started with some configuration, when they create persistence they write that configuration to RDB AUX fields, so that can can always know with which configuration the persistence file they're loading was created (could be critical). there is (was) one scenario in which they could load their persisted data, and that configuration was missing, and this change fixes it. Add a new module event: REDISMODULE_SUBEVENT_PERSISTENCE_SYNC_AOF_START, similar to REDISMODULE_SUBEVENT_PERSISTENCE_AOF_START which is async. Co-authored-by: Oran Agra --- src/aof.c | 23 ++++++++++--- src/module.c | 1 + src/rdb.c | 6 ++-- src/redismodule.h | 3 +- tests/integration/aof-multi-part.tcl | 50 +++++++++++++++++++++++++++- tests/modules/hooks.c | 6 +++- tests/unit/moduleapi/hooks.tcl | 3 ++ 7 files changed, 83 insertions(+), 9 deletions(-) diff --git a/src/aof.c b/src/aof.c index d360d59f2..20b7e57b8 100644 --- a/src/aof.c +++ b/src/aof.c @@ -46,6 +46,7 @@ off_t getAppendOnlyFileSize(sds filename); off_t getBaseAndIncrAppendOnlyFilesSize(aofManifest *am); int getBaseAndIncrAppendOnlyFilesNum(aofManifest *am); int aofFileExist(char *filename); +int rewriteAppendOnlyFile(char *filename); /* ---------------------------------------------------------------------------- * AOF Manifest file implementation. @@ -667,11 +668,12 @@ int aofDelHistoryFiles(void) { } /* Called after `loadDataFromDisk` when redis start. If `server.aof_state` is - * 'AOF_ON', It will do two things: - * 1. Open the last opened INCR type AOF for writing, If not, create a new one - * 2. Synchronously update the manifest file to the disk + * 'AOF_ON', It will do three things: + * 1. Force create a BASE file when redis starts with an empty dataset + * 2. Open the last opened INCR type AOF for writing, If not, create a new one + * 3. Synchronously update the manifest file to the disk * - * If any of the above two steps fails, the redis process will exit. + * If any of the above steps fails, the redis process will exit. */ void aofOpenIfNeededOnServerStart(void) { if (server.aof_state != AOF_ON) { @@ -687,6 +689,18 @@ void aofOpenIfNeededOnServerStart(void) { exit(1); } + /* If we start with an empty dataset, we will force create a BASE file. */ + if (!server.aof_manifest->base_aof_info && + !listLength(server.aof_manifest->incr_aof_list)) + { + sds base_name = getNewBaseFileNameAndMarkPreAsHistory(server.aof_manifest); + sds base_filepath = makePath(server.aof_dirname, base_name); + if (rewriteAppendOnlyFile(base_filepath) != C_OK) { + exit(1); + } + sdsfree(base_filepath); + } + /* Because we will 'exit(1)' if open AOF or persistent manifest fails, so * we don't need atomic modification here. */ sds aof_name = getLastIncrAofName(server.aof_manifest); @@ -701,6 +715,7 @@ void aofOpenIfNeededOnServerStart(void) { exit(1); } + /* Persist our changes. */ int ret = persistAofManifest(server.aof_manifest); if (ret != C_OK) { exit(1); diff --git a/src/module.c b/src/module.c index 36c4202c5..6cc9c2b0e 100644 --- a/src/module.c +++ b/src/module.c @@ -9046,6 +9046,7 @@ static uint64_t moduleEventVersions[] = { * * `REDISMODULE_SUBEVENT_PERSISTENCE_RDB_START` * * `REDISMODULE_SUBEVENT_PERSISTENCE_AOF_START` * * `REDISMODULE_SUBEVENT_PERSISTENCE_SYNC_RDB_START` + * * `REDISMODULE_SUBEVENT_PERSISTENCE_SYNC_AOF_START` * * `REDISMODULE_SUBEVENT_PERSISTENCE_ENDED` * * `REDISMODULE_SUBEVENT_PERSISTENCE_FAILED` * diff --git a/src/rdb.c b/src/rdb.c index 91645d981..b13240f5b 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -2702,14 +2702,16 @@ void stopLoading(int success) { success? REDISMODULE_SUBEVENT_LOADING_ENDED: REDISMODULE_SUBEVENT_LOADING_FAILED, - NULL); + NULL); } void startSaving(int rdbflags) { /* Fire the persistence modules end event. */ int subevent; - if (rdbflags & RDBFLAGS_AOF_PREAMBLE) + if (rdbflags & RDBFLAGS_AOF_PREAMBLE && getpid() != server.pid) subevent = REDISMODULE_SUBEVENT_PERSISTENCE_AOF_START; + else if (rdbflags & RDBFLAGS_AOF_PREAMBLE) + subevent = REDISMODULE_SUBEVENT_PERSISTENCE_SYNC_AOF_START; else if (getpid()!=server.pid) subevent = REDISMODULE_SUBEVENT_PERSISTENCE_RDB_START; else diff --git a/src/redismodule.h b/src/redismodule.h index 16c7950d4..0f8ab6fb2 100644 --- a/src/redismodule.h +++ b/src/redismodule.h @@ -383,7 +383,8 @@ static const RedisModuleEvent #define REDISMODULE_SUBEVENT_PERSISTENCE_SYNC_RDB_START 2 #define REDISMODULE_SUBEVENT_PERSISTENCE_ENDED 3 #define REDISMODULE_SUBEVENT_PERSISTENCE_FAILED 4 -#define _REDISMODULE_SUBEVENT_PERSISTENCE_NEXT 5 +#define REDISMODULE_SUBEVENT_PERSISTENCE_SYNC_AOF_START 5 +#define _REDISMODULE_SUBEVENT_PERSISTENCE_NEXT 6 #define REDISMODULE_SUBEVENT_LOADING_RDB_START 0 #define REDISMODULE_SUBEVENT_LOADING_AOF_START 1 diff --git a/tests/integration/aof-multi-part.tcl b/tests/integration/aof-multi-part.tcl index b5b2f3ff9..ac5ed4f33 100644 --- a/tests/integration/aof-multi-part.tcl +++ b/tests/integration/aof-multi-part.tcl @@ -687,7 +687,7 @@ tags {"external:skip"} { waitForBgrewriteaof $redis assert_aof_manifest_content $aof_manifest_name { - {file " file seq .aof .1.base.rdb" seq 1 type b} + {file " file seq .aof .2.base.rdb" seq 2 type b} {file " file seq .aof .2.incr.aof" seq 2 type i} } @@ -696,6 +696,54 @@ tags {"external:skip"} { set d2 [$redis debug digest] assert {$d1 eq $d2} } + + clean_aof_persistence $aof_dirpath + } + + test {Multi Part AOF can create BASE (RDB format) when redis starts from empty} { + start_server_aof [list dir $server_path] { + set client [redis [dict get $srv host] [dict get $srv port] 0 $::tls] + wait_done_loading $client + + assert_equal 1 [check_file_exist $aof_dirpath "${aof_basename}.1${::base_aof_sufix}${::rdb_format_suffix}"] + + assert_aof_manifest_content $aof_manifest_file { + {file appendonly.aof.1.base.rdb seq 1 type b} + {file appendonly.aof.1.incr.aof seq 1 type i} + } + + $client set foo behavior + + set d1 [$client debug digest] + $client debug loadaof + set d2 [$client debug digest] + assert {$d1 eq $d2} + } + + clean_aof_persistence $aof_dirpath + } + + test {Multi Part AOF can create BASE (AOF format) when redis starts from empty} { + start_server_aof [list dir $server_path aof-use-rdb-preamble no] { + set client [redis [dict get $srv host] [dict get $srv port] 0 $::tls] + wait_done_loading $client + + assert_equal 1 [check_file_exist $aof_dirpath "${aof_basename}.1${::base_aof_sufix}${::aof_format_suffix}"] + + assert_aof_manifest_content $aof_manifest_file { + {file appendonly.aof.1.base.aof seq 1 type b} + {file appendonly.aof.1.incr.aof seq 1 type i} + } + + $client set foo behavior + + set d1 [$client debug digest] + $client debug loadaof + set d2 [$client debug digest] + assert {$d1 eq $d2} + } + + clean_aof_persistence $aof_dirpath } # Test Part 2 diff --git a/tests/modules/hooks.c b/tests/modules/hooks.c index 6fb3d2325..af4681cf9 100644 --- a/tests/modules/hooks.c +++ b/tests/modules/hooks.c @@ -189,14 +189,18 @@ void persistenceCallback(RedisModuleCtx *ctx, RedisModuleEvent e, uint64_t sub, switch (sub) { case REDISMODULE_SUBEVENT_PERSISTENCE_RDB_START: keyname = "persistence-rdb-start"; break; case REDISMODULE_SUBEVENT_PERSISTENCE_AOF_START: keyname = "persistence-aof-start"; break; + case REDISMODULE_SUBEVENT_PERSISTENCE_SYNC_AOF_START: keyname = "persistence-syncaof-start"; break; case REDISMODULE_SUBEVENT_PERSISTENCE_SYNC_RDB_START: keyname = "persistence-syncrdb-start"; break; case REDISMODULE_SUBEVENT_PERSISTENCE_ENDED: keyname = "persistence-end"; break; case REDISMODULE_SUBEVENT_PERSISTENCE_FAILED: keyname = "persistence-failed"; break; } /* modifying the keyspace from the fork child is not an option, using log instead */ RedisModule_Log(ctx, "warning", "module-event-%s", keyname); - if (sub == REDISMODULE_SUBEVENT_PERSISTENCE_SYNC_RDB_START) + if (sub == REDISMODULE_SUBEVENT_PERSISTENCE_SYNC_RDB_START || + sub == REDISMODULE_SUBEVENT_PERSISTENCE_SYNC_AOF_START) + { LogNumericEvent(ctx, keyname, 0); + } } void loadingCallback(RedisModuleCtx *ctx, RedisModuleEvent e, uint64_t sub, void *data) diff --git a/tests/unit/moduleapi/hooks.tcl b/tests/unit/moduleapi/hooks.tcl index c4af59bd2..17570d00f 100644 --- a/tests/unit/moduleapi/hooks.tcl +++ b/tests/unit/moduleapi/hooks.tcl @@ -2,6 +2,9 @@ set testmodule [file normalize tests/modules/hooks.so] tags "modules" { start_server [list overrides [list loadmodule "$testmodule" appendonly yes]] { + test {Test module aof save on server start from empty} { + assert {[r hooks.event_count persistence-syncaof-start] == 1} + } test {Test clients connection / disconnection hooks} { for {set j 0} {$j < 2} {incr j} {