From cd8de095c4dc6687dabaa194a0fff4473c52299f Mon Sep 17 00:00:00 2001
From: kronwerk <kronwerk@users.noreply.github.com>
Date: Wed, 9 Oct 2024 14:11:53 +0300
Subject: [PATCH] Add flush-before-load option for repl-diskless-load (#909)

A new option for diskless replication on the replica side.

After a network failure, the replica may need to perform a full sync.
The other option for diskless full sync is `swapdb`, but it uses twice
as much memory, temporarily. In situations where this is not acceptable,
and where losing data is acceptable, the `flush-before-load` can be
useful. If the full sync fails, the old data is lost though. Therefore,
the new option is marked as "dangerous".

---------

Signed-off-by: kronwerk <ca11e5e22g@gmail.com>
Signed-off-by: kronwerk <kronwerk@users.noreply.github.com>
Co-authored-by: kronwerk <ca11e5e22g@gmail.com>
---
 src/config.c                      |  1 +
 src/replication.c                 |  1 +
 src/server.h                      |  1 +
 tests/integration/replication.tcl | 57 +++++++++++++++++++++++++++++++
 valkey.conf                       | 25 ++++++++------
 5 files changed, 74 insertions(+), 11 deletions(-)

diff --git a/src/config.c b/src/config.c
index 6c03cbb47..62ed5dd91 100644
--- a/src/config.c
+++ b/src/config.c
@@ -107,6 +107,7 @@ configEnum repl_diskless_load_enum[] = {
     {"disabled", REPL_DISKLESS_LOAD_DISABLED},
     {"on-empty-db", REPL_DISKLESS_LOAD_WHEN_DB_EMPTY},
     {"swapdb", REPL_DISKLESS_LOAD_SWAPDB},
+    {"flush-before-load", REPL_DISKLESS_LOAD_FLUSH_BEFORE_LOAD},
     {NULL, 0}};
 
 configEnum tls_auth_clients_enum[] = {
diff --git a/src/replication.c b/src/replication.c
index 64df85f19..e2941c6d9 100644
--- a/src/replication.c
+++ b/src/replication.c
@@ -1943,6 +1943,7 @@ void restartAOFAfterSYNC(void) {
 static int useDisklessLoad(void) {
     /* compute boolean decision to use diskless load */
     int enabled = server.repl_diskless_load == REPL_DISKLESS_LOAD_SWAPDB ||
+                  server.repl_diskless_load == REPL_DISKLESS_LOAD_FLUSH_BEFORE_LOAD ||
                   (server.repl_diskless_load == REPL_DISKLESS_LOAD_WHEN_DB_EMPTY && dbTotalServerKeyCount() == 0);
 
     if (enabled) {
diff --git a/src/server.h b/src/server.h
index 84a282b6f..84ce96a49 100644
--- a/src/server.h
+++ b/src/server.h
@@ -490,6 +490,7 @@ typedef enum {
 #define REPL_DISKLESS_LOAD_DISABLED 0
 #define REPL_DISKLESS_LOAD_WHEN_DB_EMPTY 1
 #define REPL_DISKLESS_LOAD_SWAPDB 2
+#define REPL_DISKLESS_LOAD_FLUSH_BEFORE_LOAD 3
 
 /* TLS Client Authentication */
 #define TLS_CLIENT_AUTH_NO 0
diff --git a/tests/integration/replication.tcl b/tests/integration/replication.tcl
index 203574e39..1b5b0c030 100644
--- a/tests/integration/replication.tcl
+++ b/tests/integration/replication.tcl
@@ -1478,6 +1478,63 @@ start_server {tags {"repl external:skip"}} {
     }
 }
 
+foreach dualchannel {yes no} {
+    test "replica actually flushes db if use diskless load with flush-before-load dual-channel-replication-enabled=$dualchannel" {
+        start_server {tags {"repl"}} {
+            set replica [srv 0 client]
+            set replica_log [srv 0 stdout]
+            start_server {} {
+                set master [srv 0 client]
+                set master_host [srv 0 host]
+                set master_port [srv 0 port]
+
+                # Fill both replica and master with data
+                $master debug populate 100 master 100000
+                $replica debug populate 201 replica 100000
+                assert_equal [$replica dbsize] 201
+                # Set up master
+                $master config set save ""
+                $master config set rdbcompression no
+                $master config set repl-diskless-sync yes
+                $master config set repl-diskless-sync-delay 0
+                $master config set dual-channel-replication-enabled $dualchannel
+                # Set master with a slow rdb generation, so that we can easily intercept loading
+                # 10ms per key, with 1000 keys is 10 seconds
+                $master config set rdb-key-save-delay 10000
+                # Set up replica
+                $replica config set save ""
+                $replica config set repl-diskless-load flush-before-load
+                $replica config set dual-channel-replication-enabled $dualchannel
+                # Start the replication process...
+                $replica replicaof $master_host $master_port
+
+                wait_for_condition 100 100 {
+                    [s -1 loading] eq 1
+                } else {
+                    fail "Replica didn't start loading"
+                }
+
+                # Make sure that next sync will not start immediately so that we can catch the replica in between syncs
+                $master config set repl-diskless-sync-delay 5
+
+                # Kill the replica connection on the master
+                set killed [$master client kill type replica]
+
+                wait_for_condition 100 100 {
+                    [s -1 loading] eq 0
+                } else {
+                    fail "Replica didn't disconnect"
+                }
+
+                assert_equal [$replica dbsize] 0
+
+                # Speed up shutdown
+                $master config set rdb-key-save-delay 0
+            }
+        }
+    } {} {external:skip}
+}
+
 start_server {tags {"repl external:skip"}} {
     set replica [srv 0 client]
     $replica set replica_key replica_value
diff --git a/valkey.conf b/valkey.conf
index 56110a52b..9793d7825 100644
--- a/valkey.conf
+++ b/valkey.conf
@@ -667,17 +667,20 @@ repl-diskless-sync-max-replicas 0
 # fully loaded in memory, resulting in higher memory usage.
 # For this reason we have the following options:
 #
-# "disabled"    - Don't use diskless load (store the rdb file to the disk first)
-# "swapdb"      - Keep current db contents in RAM while parsing the data directly
-#                 from the socket. Replicas in this mode can keep serving current
-#                 dataset while replication is in progress, except for cases where
-#                 they can't recognize primary as having a data set from same
-#                 replication history.
-#                 Note that this requires sufficient memory, if you don't have it,
-#                 you risk an OOM kill.
-# "on-empty-db" - Use diskless load only when current dataset is empty. This is 
-#                 safer and avoid having old and new dataset loaded side by side
-#                 during replication.
+# "disabled"          - Don't use diskless load (store the rdb file to the disk first)
+# "swapdb"            - Keep current db contents in RAM while parsing the data directly
+#                       from the socket. Replicas in this mode can keep serving current
+#                       dataset while replication is in progress, except for cases where
+#                       they can't recognize primary as having a data set from same
+#                       replication history.
+#                       Note that this requires sufficient memory, if you don't have it,
+#                       you risk an OOM kill.
+# "on-empty-db"       - Use diskless load only when current dataset is empty. This is 
+#                       safer and avoid having old and new dataset loaded side by side
+#                       during replication.
+# "flush-before-load" - [dangerous] Flush all data before parsing. Note that if
+#                       there's a problem before the replication succeeded you may
+#                       lose all your data.
 repl-diskless-load disabled
 
 # This dual channel replication sync feature optimizes the full synchronization process