From 62153b3b2fdd96086c0eaa1735793b2ef8bb311c Mon Sep 17 00:00:00 2001
From: Yanqi Lv <lvyanqi.lyq@alibaba-inc.com>
Date: Thu, 1 Feb 2024 19:41:02 +0800
Subject: [PATCH] Refine the purpose of rdb saving with accurate flags (#12925)

In Redis, rdb is produced in three scenarios mainly.

- backup, such as `bgsave` and `save` command
- full sync in replication
- aof rewrite if `aof-use-rdb-preamble` is yes

We also have some RDB flags to identify the purpose of rdb saving.
```C
/* flags on the purpose of rdb save or load */
#define RDBFLAGS_NONE 0                 /* No special RDB loading. */
#define RDBFLAGS_AOF_PREAMBLE (1<<0)    /* Load/save the RDB as AOF preamble. */
#define RDBFLAGS_REPLICATION (1<<1)     /* Load/save for SYNC. */
```

But currently, it seems that these flags and purposes of rdb saving
don't exactly match. I find it in `rdbSaveRioWithEOFMark` which calls
`startSaving` with `RDBFLAGS_REPLICATION` but `rdbSaveRio` with
`RDBFLAGS_NONE`.
```C
int rdbSaveRioWithEOFMark(int req, rio *rdb, int *error, rdbSaveInfo *rsi) {
    char eofmark[RDB_EOF_MARK_SIZE];

    startSaving(RDBFLAGS_REPLICATION);
    getRandomHexChars(eofmark,RDB_EOF_MARK_SIZE);
    if (error) *error = 0;
    if (rioWrite(rdb,"$EOF:",5) == 0) goto werr;
    if (rioWrite(rdb,eofmark,RDB_EOF_MARK_SIZE) == 0) goto werr;
    if (rioWrite(rdb,"\r\n",2) == 0) goto werr;
    if (rdbSaveRio(req,rdb,error,RDBFLAGS_NONE,rsi) == C_ERR) goto werr;
    if (rioWrite(rdb,eofmark,RDB_EOF_MARK_SIZE) == 0) goto werr;
    stopSaving(1);
    return C_OK;

werr: /* Write error. */
    /* Set 'error' only if not already set by rdbSaveRio() call. */
    if (error && *error == 0) *error = errno;
    stopSaving(0);
    return C_ERR;
}
```

In this PR, I refine the purpose of rdb saving with accurate flags.
---
 src/rdb.c         | 7 ++++---
 src/rdb.h         | 2 +-
 src/replication.c | 2 +-
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/rdb.c b/src/rdb.c
index ac88c7be0..a8d58c08f 100644
--- a/src/rdb.c
+++ b/src/rdb.c
@@ -1424,7 +1424,8 @@ werr:
     return C_ERR;
 }
 
-/* This is just a wrapper to rdbSaveRio() that additionally adds a prefix
+/* This helper function is only used for diskless replication. 
+ * This is just a wrapper to rdbSaveRio() that additionally adds a prefix
  * and a suffix to the generated RDB dump. The prefix is:
  *
  * $EOF:<40 bytes unguessable hex string>\r\n
@@ -1441,7 +1442,7 @@ int rdbSaveRioWithEOFMark(int req, rio *rdb, int *error, rdbSaveInfo *rsi) {
     if (rioWrite(rdb,"$EOF:",5) == 0) goto werr;
     if (rioWrite(rdb,eofmark,RDB_EOF_MARK_SIZE) == 0) goto werr;
     if (rioWrite(rdb,"\r\n",2) == 0) goto werr;
-    if (rdbSaveRio(req,rdb,error,RDBFLAGS_NONE,rsi) == C_ERR) goto werr;
+    if (rdbSaveRio(req,rdb,error,RDBFLAGS_REPLICATION,rsi) == C_ERR) goto werr;
     if (rioWrite(rdb,eofmark,RDB_EOF_MARK_SIZE) == 0) goto werr;
     stopSaving(1);
     return C_OK;
@@ -1528,7 +1529,7 @@ int rdbSave(int req, char *filename, rdbSaveInfo *rsi, int rdbflags) {
     char tmpfile[256];
     char cwd[MAXPATHLEN]; /* Current working dir path for error messages. */
 
-    startSaving(RDBFLAGS_NONE);
+    startSaving(rdbflags);
     snprintf(tmpfile,256,"temp-%d.rdb", (int) getpid());
 
     if (rdbSaveInternal(req,tmpfile,rsi,rdbflags) != C_OK) {
diff --git a/src/rdb.h b/src/rdb.h
index f0cca977a..3b7d639bb 100644
--- a/src/rdb.h
+++ b/src/rdb.h
@@ -128,7 +128,7 @@
 #define RDB_LOAD_SDS    (1<<2)
 
 /* flags on the purpose of rdb save or load */
-#define RDBFLAGS_NONE 0                 /* No special RDB loading. */
+#define RDBFLAGS_NONE 0                 /* No special RDB loading or saving. */
 #define RDBFLAGS_AOF_PREAMBLE (1<<0)    /* Load/save the RDB as AOF preamble. */
 #define RDBFLAGS_REPLICATION (1<<1)     /* Load/save for SYNC. */
 #define RDBFLAGS_ALLOW_DUP (1<<2)       /* Allow duplicated keys when loading.*/
diff --git a/src/replication.c b/src/replication.c
index 07e88c151..28c4b1411 100644
--- a/src/replication.c
+++ b/src/replication.c
@@ -878,7 +878,7 @@ int startBgsaveForReplication(int mincapa, int req) {
             retval = rdbSaveToSlavesSockets(req,rsiptr);
         else {
             /* Keep the page cache since it'll get used soon */
-            retval = rdbSaveBackground(req,server.rdb_filename,rsiptr,RDBFLAGS_KEEP_CACHE);
+            retval = rdbSaveBackground(req, server.rdb_filename, rsiptr, RDBFLAGS_REPLICATION | RDBFLAGS_KEEP_CACHE);
         }
     } else {
         serverLog(LL_WARNING,"BGSAVE for replication: replication information not available, can't generate the RDB file right now. Try later.");