Dual channel replication should not update lastbgsave_status when tra…

…nsfer error (valkey-io#811) Currently lastbgsave_status is used in bgsave or disk-replication, and the target is the disk. In valkey-io#60, we update it when transfer error, i think it is mainly used in tests, so we can use log to replace it. It changes lastbgsave_status to err in this case, but it is strange that it does not set ok or err in the above if and the following else. Also noted this will affect stop-writes-on-bgsave-error. Signed-off-by: Binbin <[email protected]> Signed-off-by: mwish <[email protected]>
mapleFU · Aug 22, 2024 · 5eb40f8 · 5eb40f8
1 parent 91afb64
commit 5eb40f8
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 24 deletions.
diff --git a/src/rdb.c b/src/rdb.c
@@ -3450,10 +3450,9 @@ static void backgroundSaveDoneHandlerSocket(int exitcode, int bysignal) {
     if (!bysignal && exitcode == 0) {
         serverLog(LL_NOTICE, "Background RDB transfer terminated with success");
     } else if (!bysignal && exitcode != 0) {
-        serverLog(LL_WARNING, "Background transfer error");
-        server.lastbgsave_status = C_ERR;
+        serverLog(LL_WARNING, "Background RDB transfer error");
     } else {
-        serverLog(LL_WARNING, "Background transfer terminated by signal %d", bysignal);
+        serverLog(LL_WARNING, "Background RDB transfer terminated by signal %d", bysignal);
     }
     if (server.rdb_child_exit_pipe != -1) close(server.rdb_child_exit_pipe);
     if (server.rdb_pipe_read > 0) {

diff --git a/tests/integration/dual-channel-replication.tcl b/tests/integration/dual-channel-replication.tcl
@@ -890,18 +890,15 @@ start_server {tags {"dual-channel-replication external:skip"}} {
             $primary debug log "killing replica main connection"
             set replica_main_conn_id [get_client_id_by_last_cmd $primary "psync"]
             assert {$replica_main_conn_id != ""}
+            set loglines [count_log_lines -1]
             $primary client kill id $replica_main_conn_id
             # Wait for primary to abort the sync
             wait_for_condition 50 1000 {
                 [string match {*replicas_waiting_psync:0*} [$primary info replication]]
             } else {
                 fail "Primary did not free repl buf block after sync failure"
             }
-            wait_for_condition 1000 10 {
-                [s -1 rdb_last_bgsave_status] eq "err"
-            } else {
-                fail "bgsave did not stop in time"
-            }
+            wait_for_log_messages -1 {"*Background RDB transfer error*"} $loglines 1000 10
         }
 
         test "Test dual channel replication slave of no one after main conn kill" {
@@ -927,14 +924,10 @@ start_server {tags {"dual-channel-replication external:skip"}} {
             $primary debug log "killing replica rdb connection"
             set replica_rdb_channel_id [get_client_id_by_last_cmd $primary "sync"]
             assert {$replica_rdb_channel_id != ""}
+            set loglines [count_log_lines -1]
             $primary client kill id $replica_rdb_channel_id
             # Wait for primary to abort the sync
-            wait_for_condition 1000 10 {
-                [s -1 rdb_bgsave_in_progress] eq 0 &&
-                [s -1 rdb_last_bgsave_status] eq "err" 
-            } else {
-                fail "Primary should abort sync"
-            }
+            wait_for_log_messages -1 {"*Background RDB transfer error*"} $loglines 1000 10
         }
 
         test "Test dual channel replication slave of no one after rdb conn kill" {
@@ -1073,18 +1066,15 @@ start_server {tags {"dual-channel-replication external:skip"}} {
             $primary debug log "killing replica rdb connection"
             set replica_rdb_channel_id [get_client_id_by_last_cmd $primary "sync"]
             assert {$replica_rdb_channel_id != ""}
+            set loglines [count_log_lines -1]
             $primary client kill id $replica_rdb_channel_id
             # Wait for primary to abort the sync
             wait_for_condition 50 1000 {
                 [string match {*replicas_waiting_psync:0*} [$primary info replication]]
             } else {
                 fail "Primary did not free repl buf block after sync failure"
             }
-            wait_for_condition 1000 10 {
-                [s -1 rdb_last_bgsave_status] eq "err"
-            } else {
-                fail "bgsave did not stop in time"
-            }
+            wait_for_log_messages -1 {"*Background RDB transfer error*"} $loglines 1000 10
             # Replica should retry
             verify_replica_online $primary 0 500
             stop_write_load $load_handle
@@ -1111,18 +1101,15 @@ start_server {tags {"dual-channel-replication external:skip"}} {
             $primary debug log "killing replica main connection"
             set replica_main_conn_id [get_client_id_by_last_cmd $primary "sync"]
             assert {$replica_main_conn_id != ""}
+            set loglines [count_log_lines -1]
             $primary client kill id $replica_main_conn_id
             # Wait for primary to abort the sync
             wait_for_condition 50 1000 {
                 [string match {*replicas_waiting_psync:0*} [$primary info replication]]
             } else {
                 fail "Primary did not free repl buf block after sync failure"
             }
-            wait_for_condition 1000 10 {
-                [s -1 rdb_last_bgsave_status] eq "err"
-            } else {
-                fail "bgsave did not stop in time"
-            }
+            wait_for_log_messages -1 {"*Background RDB transfer error*"} $loglines 1000 10
             # Replica should retry
             verify_replica_online $primary 0 500
             stop_write_load $load_handle