Tarantool development patches archive
 help / color / mirror / Atom feed
From: Olga Arkhangelskaia <krishtal.olja@gmail.com>
To: tarantool-patches@freelists.org
Cc: Olga Arkhangelskaia <krishtal.olja@gmail.com>
Subject: [tarantool-patches] [PATCH v5 3/3] box: adds replication sync after cfg. update
Date: Wed, 29 Aug 2018 21:56:42 +0300	[thread overview]
Message-ID: <20180829185642.49479-3-krishtal.olja@gmail.com> (raw)
In-Reply-To: <20180829185642.49479-1-krishtal.olja@gmail.com>

When replica reconnects to replica set not for the first time, we
suffer from absence of synchronization. Such behavior leads to giving
away outdated data.

@TarantoolBot document
Title: Orphan status after configuration update or initial bootstrap.
In case of initial bootstrap or after configuration update we can get
an orphan status in two cases. If we synced up with number of replicas
that is smaller than quorum or if we failed to sync up during the time
specified in replication_sync_lag_timeout.

Closes #3427
---
https://github.com/tarantool/tarantool/issues/3427
https://github.com/tarantool/tarantool/tree/OKriw/gh-3427-replication-no-sync-1.9

v1:
https://www.freelists.org/post/tarantool-patches/PATCH-replication-adds-replication-sync-after-cfg-update

v2:
https://www.freelists.org/post/tarantool-patches/PATCH-v2-replication-adds-replication-sync-after-cfg-update

v3:
https://www.freelists.org/post/tarantool-patches/PATCH-v3-box-adds-replication-sync-after-cfg-update

v4:
https://www.freelists.org/post/tarantool-patches/PATCH-v4-22-box-adds-replication-sync-after-cfg-update

Changes in v2:
- fixed test
- changed replicaset_sync

Changes in v3:
- now we raise the exception when sync is not successful.
- fixed test
- renamed test 

Changes in v4:
- fixed test
- replication_sync_lag is made dynamicall in separate patch
- removed unnecessary error type
- moved say_crit to another place
- in case of sync error we rollback to prev. config

Changes in v5:
- added test case
- now we don't roll back to prev. cfg

 src/box/box.cc                 |   7 ++-
 src/box/replication.cc         |  12 ++---
 src/box/replication.h          |   6 +--
 test/replication/sync.result   | 112 +++++++++++++++++++++++++++++++++++++++++
 test/replication/sync.test.lua |  57 +++++++++++++++++++++
 5 files changed, 184 insertions(+), 10 deletions(-)
 create mode 100644 test/replication/sync.result
 create mode 100644 test/replication/sync.test.lua

diff --git a/src/box/box.cc b/src/box/box.cc
index 0f8364ebc..6bae9ea78 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -646,6 +646,10 @@ box_set_replication(void)
 	box_sync_replication(true);
 	/* Follow replica */
 	replicaset_follow();
+	/* Sync replica up to quorum */
+	if (!replicaset_sync()) {
+		say_crit("entering orphan mode");
+	}
 }
 
 void
@@ -1967,7 +1971,8 @@ box_cfg_xc(void)
 	is_box_configured = true;
 
 	if (!is_bootstrap_leader)
-		replicaset_sync();
+		if (!replicaset_sync())
+			say_crit("entering orphan mode");
 
 	say_info("ready to accept requests");
 }
diff --git a/src/box/replication.cc b/src/box/replication.cc
index 731b05faf..86d6f454b 100644
--- a/src/box/replication.cc
+++ b/src/box/replication.cc
@@ -661,13 +661,13 @@ replicaset_follow(void)
 	}
 }
 
-void
+bool
 replicaset_sync(void)
 {
 	int quorum = replicaset_quorum();
 
 	if (quorum == 0)
-		return;
+		return true;
 
 	say_verbose("synchronizing with %d replicas", quorum);
 
@@ -680,8 +680,8 @@ replicaset_sync(void)
 	       replicaset.applier.loading >= quorum) {
 		if (fiber_cond_wait_timeout(&replicaset.applier.cond,
 				            replication_sync_lag_timeout) != 0) {
-			say_crit("replication_sync_lag_timeout fired, entering orphan mode");
-			return;
+			say_crit("replication_sync_lag_timeout fired");
+			return false;
 		}
 
 	}
@@ -692,12 +692,12 @@ replicaset_sync(void)
 		 * Do not stall configuration, leave the instance
 		 * in 'orphan' state.
 		 */
-		say_crit("entering orphan mode");
-		return;
+		return false;
 	}
 
 	say_crit("replica set sync complete, quorum of %d "
 		 "replicas formed", quorum);
+	return true;
 }
 
 void
diff --git a/src/box/replication.h b/src/box/replication.h
index 71c17dc8e..512a4085e 100644
--- a/src/box/replication.h
+++ b/src/box/replication.h
@@ -379,10 +379,10 @@ replicaset_follow(void);
 
 /**
  * Wait until a replication quorum is formed.
- * Return immediately if a quorum cannot be
- * formed because of errors.
+ * @return true in case of success.
+ * @return false if a quorum cannot be formed because of errors.
  */
-void
+bool
 replicaset_sync(void);
 
 /**
diff --git a/test/replication/sync.result b/test/replication/sync.result
new file mode 100644
index 000000000..875dfaa41
--- /dev/null
+++ b/test/replication/sync.result
@@ -0,0 +1,112 @@
+--
+-- gh-3427: no sync after configuration update
+--
+--
+-- successful sync
+--
+env = require('test_run')
+---
+...
+test_run = env.new()
+---
+...
+engine = test_run:get_cfg('engine')
+---
+...
+box.schema.user.grant('guest', 'replication')
+---
+...
+test_run:cmd("create server replica with rpl_master=default, script='replication/replica.lua'")
+---
+- true
+...
+test_run:cmd("start server replica")
+---
+- true
+...
+s = box.schema.space.create('test', {engine = engine})
+---
+...
+index = s:create_index('primary')
+---
+...
+-- change replica configuration
+test_run:cmd("switch replica")
+---
+- true
+...
+box.cfg{replication_sync_lag = 0.1}
+---
+...
+replication = box.cfg.replication
+---
+...
+box.cfg{replication={}}
+---
+...
+test_run:cmd("switch default")
+---
+- true
+...
+-- insert values on the master while replica is unconfigured
+a = 100 box.begin() while a > 0 do a = a-1 box.space.test:insert{a,a} end box.commit()
+---
+...
+test_run:cmd("switch replica")
+---
+- true
+...
+box.cfg{replication = replication}
+---
+...
+box.space.test:count() == 100
+---
+- true
+...
+--
+-- unsuccessful sync
+--
+box.cfg{replication={}}
+---
+...
+box.cfg{replication_sync_lag_timeout = 0.001}
+---
+...
+test_run:cmd("switch default")
+---
+- true
+...
+-- insert values on the master while replica is unconfigured
+a = 200 box.begin() while a > 100 do a = a-1 box.space.test:insert{a,a} end box.commit()
+---
+...
+test_run:cmd("switch replica")
+---
+- true
+...
+box.cfg{replication = replication}
+---
+...
+box.space.test:count() < 200
+---
+- true
+...
+test_run:cmd("switch default")
+---
+- true
+...
+-- cleanup
+test_run:cmd("stop server replica")
+---
+- true
+...
+test_run:cmd("cleanup server replica")
+---
+- true
+...
+box.space.test:drop()
+---
+...
+box.schema.user.revoke('guest', 'replication')
+---
+...
diff --git a/test/replication/sync.test.lua b/test/replication/sync.test.lua
new file mode 100644
index 000000000..e63edd0d3
--- /dev/null
+++ b/test/replication/sync.test.lua
@@ -0,0 +1,57 @@
+--
+-- gh-3427: no sync after configuration update
+--
+
+--
+-- successful sync
+--
+
+env = require('test_run')
+test_run = env.new()
+engine = test_run:get_cfg('engine')
+
+box.schema.user.grant('guest', 'replication')
+
+test_run:cmd("create server replica with rpl_master=default, script='replication/replica.lua'")
+test_run:cmd("start server replica")
+
+s = box.schema.space.create('test', {engine = engine})
+index = s:create_index('primary')
+
+-- change replica configuration
+test_run:cmd("switch replica")
+box.cfg{replication_sync_lag = 0.1}
+replication = box.cfg.replication
+box.cfg{replication={}}
+
+test_run:cmd("switch default")
+-- insert values on the master while replica is unconfigured
+a = 100 box.begin() while a > 0 do a = a-1 box.space.test:insert{a,a} end box.commit()
+
+test_run:cmd("switch replica")
+box.cfg{replication = replication}
+
+box.space.test:count() == 100
+
+
+--
+-- unsuccessful sync
+--
+box.cfg{replication={}}
+box.cfg{replication_sync_lag_timeout = 0.001}
+
+test_run:cmd("switch default")
+-- insert values on the master while replica is unconfigured
+a = 200 box.begin() while a > 100 do a = a-1 box.space.test:insert{a,a} end box.commit()
+
+test_run:cmd("switch replica")
+box.cfg{replication = replication}
+box.space.test:count() < 200
+
+test_run:cmd("switch default")
+
+-- cleanup
+test_run:cmd("stop server replica")
+test_run:cmd("cleanup server replica")
+box.space.test:drop()
+box.schema.user.revoke('guest', 'replication')
-- 
2.14.3 (Apple Git-98)

  parent reply	other threads:[~2018-08-29 18:57 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-29 18:56 [tarantool-patches] [PATCH v2 1/3] box: make replication_sync_lag option dynamic Olga Arkhangelskaia
2018-08-29 18:56 ` [tarantool-patches] [PATCH 2/3] box: add replication_sync_lag_timeout Olga Arkhangelskaia
2018-08-30 10:02   ` Vladimir Davydov
2018-08-29 18:56 ` Olga Arkhangelskaia [this message]
2018-08-30 10:11   ` [tarantool-patches] [PATCH v5 3/3] box: adds replication sync after cfg. update Vladimir Davydov
2018-08-30  9:48 ` [tarantool-patches] [PATCH v2 1/3] box: make replication_sync_lag option dynamic Vladimir Davydov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180829185642.49479-3-krishtal.olja@gmail.com \
    --to=krishtal.olja@gmail.com \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [tarantool-patches] [PATCH v5 3/3] box: adds replication sync after cfg. update' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox