From 2e84764f1b392d9cc9a31ae474367dd21eb0e460 Mon Sep 17 00:00:00 2001
From: John Sully <john@csquare.ca>
Date: Wed, 29 Jan 2020 13:26:04 -0500
Subject: [PATCH] Fix active rep not merging on sync and add tests

Former-commit-id: fcb77d0431b195553eb1cd563286e5441f0c94cd
---
 src/replication.cpp                      | 54 ++++++++++++++----------
 tests/integration/replication-active.tcl | 28 ++++++++++++
 2 files changed, 59 insertions(+), 23 deletions(-)

diff --git a/src/replication.cpp b/src/replication.cpp
index 95bbadd9a..0201204b1 100644
--- a/src/replication.cpp
+++ b/src/replication.cpp
@@ -1872,17 +1872,21 @@ void readSyncBulkPayload(connection *conn) {
     /* We need to stop any AOF rewriting child before flusing and parsing
      * the RDB, otherwise we'll create a copy-on-write disaster. */
     if (g_pserver->aof_state != AOF_OFF) stopAppendOnly();
-    signalFlushedDb(-1);
 
-    /* When diskless RDB loading is used by replicas, it may be configured
-     * in order to save the current DB instead of throwing it away,
-     * so that we can restore it in case of failed transfer. */
-    if (use_diskless_load &&
-        g_pserver->repl_diskless_load == REPL_DISKLESS_LOAD_SWAPDB)
+    if (!fUpdate)
     {
-        diskless_load_backup = disklessLoadMakeBackups();
-    } else {
-        emptyDb(-1,empty_db_flags,replicationEmptyDbCallback);
+        signalFlushedDb(-1);
+
+        /* When diskless RDB loading is used by replicas, it may be configured
+        * in order to save the current DB instead of throwing it away,
+        * so that we can restore it in case of failed transfer. */
+        if (use_diskless_load &&
+            g_pserver->repl_diskless_load == REPL_DISKLESS_LOAD_SWAPDB)
+        {
+            diskless_load_backup = disklessLoadMakeBackups();
+        } else {
+            emptyDb(-1,empty_db_flags,replicationEmptyDbCallback);
+        }
     }
 
     /* Before loading the DB into memory we need to delete the readable
@@ -1910,14 +1914,16 @@ void readSyncBulkPayload(connection *conn) {
                 "from socket");
             cancelReplicationHandshake(mi);
             rioFreeConn(&rdb, NULL);
-            if (g_pserver->repl_diskless_load == REPL_DISKLESS_LOAD_SWAPDB) {
-                /* Restore the backed up databases. */
-                disklessLoadRestoreBackups(diskless_load_backup,1,
-                                           empty_db_flags);
-            } else {
-                /* Remove the half-loaded data in case we started with
-                 * an empty replica. */
-                emptyDb(-1,empty_db_flags,replicationEmptyDbCallback);
+            if (!fUpdate) {
+                if (g_pserver->repl_diskless_load == REPL_DISKLESS_LOAD_SWAPDB) {
+                    /* Restore the backed up databases. */
+                    disklessLoadRestoreBackups(diskless_load_backup,1,
+                                            empty_db_flags);
+                } else {
+                    /* Remove the half-loaded data in case we started with
+                    * an empty replica. */
+                    emptyDb(-1,empty_db_flags,replicationEmptyDbCallback);
+                }
             }
 
             /* Note that there's no point in restarting the AOF on SYNC
@@ -1927,12 +1933,14 @@ void readSyncBulkPayload(connection *conn) {
         }
         stopLoading(1);
 
-        /* RDB loading succeeded if we reach this point. */
-        if (g_pserver->repl_diskless_load == REPL_DISKLESS_LOAD_SWAPDB) {
-            /* Delete the backup databases we created before starting to load
-             * the new RDB. Now the RDB was loaded with success so the old
-             * data is useless. */
-            disklessLoadRestoreBackups(diskless_load_backup,0,empty_db_flags);
+        if (!fUpdate) {
+            /* RDB loading succeeded if we reach this point. */
+            if (g_pserver->repl_diskless_load == REPL_DISKLESS_LOAD_SWAPDB) {
+                /* Delete the backup databases we created before starting to load
+                * the new RDB. Now the RDB was loaded with success so the old
+                * data is useless. */
+                disklessLoadRestoreBackups(diskless_load_backup,0,empty_db_flags);
+            }
         }
 
         /* Verify the end mark is correct. */
diff --git a/tests/integration/replication-active.tcl b/tests/integration/replication-active.tcl
index 4f37f2adf..fee3a724c 100644
--- a/tests/integration/replication-active.tcl
+++ b/tests/integration/replication-active.tcl
@@ -129,3 +129,31 @@ start_server {tags {"active-repl"} overrides {active-replica yes}} {
         }
     }
 }
+
+foreach mdl {no yes} {
+    foreach sdl {disabled swapdb} {
+        start_server {tags {"active-repl"} overrides {active-replica yes}} {
+            set master [srv 0 client]
+            $master config set repl-diskless-sync $mdl
+            set master_host [srv 0 host]
+            set master_port [srv 0 port]
+            r set masterkey foo
+            start_server {overrides {active-replica yes}} {
+                test "Active replication databases are merged, master diskless=$mdl, replica diskless=$sdl" {
+                    r config set repl-diskless-load $sdl
+                    r set slavekey bar
+                    r replicaof $master_host $master_port
+                    
+                    wait_for_condition 50 400 {
+                        [string match *state=online* [$master info]]
+                    } else {
+                        fail "Replica never came online"
+                    }
+                    
+                    assert_equal bar [r get slavekey]
+                    assert_equal foo [r get masterkey]
+                }
+            }
+        }
+    }
+}