From b8d380907bb7376100238a9cc890caca7a842b0c Mon Sep 17 00:00:00 2001
From: Binbin <binloveplay1314@qq.com>
Date: Mon, 22 Jul 2024 22:02:12 +0800
Subject: [PATCH] Dual channel replication should not update lastbgsave_status
 when transfer error

Currently lastbgsave_status is used in bgsave or disk-replication,
and the target is the disk. In #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 <binloveplay1314@qq.com>
---
 src/rdb.c                                     |  5 ++--
 .../integration/dual-channel-replication.tcl  | 29 +++++--------------
 2 files changed, 10 insertions(+), 24 deletions(-)

diff --git a/src/rdb.c b/src/rdb.c
index 41d58e4e47..a344dd6966 100644
--- 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
index 7344d0d3b1..64d8702a12 100644
--- a/tests/integration/dual-channel-replication.tcl
+++ b/tests/integration/dual-channel-replication.tcl
@@ -870,6 +870,7 @@ 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 {
@@ -877,11 +878,7 @@ start_server {tags {"dual-channel-replication external:skip"}} {
             } 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" {
@@ -907,14 +904,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" {
@@ -1051,6 +1044,7 @@ 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 {
@@ -1058,11 +1052,7 @@ start_server {tags {"dual-channel-replication external:skip"}} {
             } 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
@@ -1089,6 +1079,7 @@ 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 {
@@ -1096,11 +1087,7 @@ start_server {tags {"dual-channel-replication external:skip"}} {
             } 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