From 037bd58cdb4b2aaebeacffddd49e731d311d6638 Mon Sep 17 00:00:00 2001
From: Serge Petrenko <sergepetrenko@tarantool.org>
Date: Mon, 2 Sep 2019 11:30:02 +0300
Subject: [PATCH] replication: disallow bootstrap of read-only masters

In a configuration with several read-only and read-write instances, if
replication_connect_quorum is not greater than the amount of read-only
instances and replication_connect_timeout happens to be small enough
for some read-only instances to form a quorum and exceed the timeout
before any of the read-write instaces start, all these read-only
instances will choose themselves a read-only bootstrap leader.
This 'leader' will successfully bootstrap itself, but will fail to
register any of the other instances in _cluster table, since it isn't
writeable. As a result, some of the read-only instances will just die
unable to bootstrap from a read-only bootstrap leader, and when the
read-write instances are finally up, they'll see a single read-only
instance which managed to bootstrap itself and now gets a
REPLICASET_UUID_MISMATCH error, since no read-write instance will
choose it as bootstrap leader, and will rather bootstrap from one of
its read-write mates.

The described situation is clearly not what user has hoped for, so
throw an error, when a read-only instance tries to initiate the
bootstrap. The error will give the user a cue that he should increase
replication_connect_timeout.

Closes #4321

@TarantoolBot document
Title: replication: forbid to bootstrap read-only masters.

It is no longer possible to bootstrap a read-only instance in an emply
data directory as a master. You will see the following error trying to
do so:
```
ER_BOOTSTRAP_READONLY: Trying to bootstrap a local read-only instance as master
```
Now if you have a fresh instance, which has
`read_only=true` in an initial `box.cfg` call, you need to set up
replication from an instance which is either read-write, or has your
local instance's uuid in its `_cluster` table.

In case you have multiple read-only and read-write instances with
replication set up, and you still see the aforementioned error message,
this means that none of your read-write instances managed to start
listening on their port before read_only instances have exceeded the
`replication_connect_timeout`. In this case you should raise
`replication_connect_timeout` to a greater value.
---
 src/box/box.cc            |  4 ++++
 src/box/errcode.h         |  1 +
 test/app-tap/cfg.test.lua | 18 ++++++++++--------
 test/box-tap/cfg.test.lua | 11 ++++++++++-
 test/box/misc.result      |  1 +
 5 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/src/box/box.cc b/src/box/box.cc
index ac10c21ad5..cf6d96e58d 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -1734,6 +1734,10 @@ engine_init()
 static void
 bootstrap_master(const struct tt_uuid *replicaset_uuid)
 {
+	/* Do not allow to bootstrap a readonly instance as master. */
+	if (cfg_geti("read_only") == 1) {
+		tnt_raise(ClientError, ER_BOOTSTRAP_READONLY);
+	}
 	engine_bootstrap_xc();
 
 	uint32_t replica_id = 1;
diff --git a/src/box/errcode.h b/src/box/errcode.h
index 0c1310bdf5..dcb90f6789 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -255,6 +255,7 @@ struct errcode_record {
 	/*200 */_(ER_FUNC_INDEX_PARTS,		"Wrong functional index definition: %s") \
 	/*201 */_(ER_NO_SUCH_FIELD_NAME,	"Field '%s' was not found in the tuple") \
 	/*202 */_(ER_FUNC_WRONG_ARG_COUNT,	"Wrong number of arguments is passed to %s(): expected %s, got %d") \
+	/*203 */_(ER_BOOTSTRAP_READONLY,	"Trying to bootstrap a local read-only instance as master") \
 
 /*
  * !IMPORTANT! Please follow instructions at start of the file
diff --git a/test/app-tap/cfg.test.lua b/test/app-tap/cfg.test.lua
index 7d8e9a05e3..a2cbed8dde 100755
--- a/test/app-tap/cfg.test.lua
+++ b/test/app-tap/cfg.test.lua
@@ -3,7 +3,7 @@ local fiber = require('fiber')
 local tap = require('tap')
 local test = tap.test("cfg")
 
-test:plan(8)
+test:plan(9)
 
 test:is(type(box.ctl), "table", "box.ctl is available before box.cfg")
 test:is(type(box.ctl.wait_ro), "function", "box.ctl.wait_ro is available")
@@ -15,17 +15,19 @@ local f_rw = fiber.create(function() box.ctl.wait_rw() end)
 test:is(f_ro:status(), "suspended", "initially the server is neither read only nor read-write")
 test:is(f_rw:status(), "suspended", "initially the server is neither read only nor read-write")
 
-box.cfg{read_only=true}
+box.cfg{}
 
 while f_ro:status() ~= "dead" do fiber.sleep(0.01) end
-test:is(f_ro:status(), "dead", "entered read-only mode")
-
-test:is(f_rw:status(), "suspended", "the read-write waiter is still blocked")
+test:is(f_ro:status(), "dead", "initialized read-write mode. read-only waiter dies.")
+while f_rw:status() ~= "dead" do fiber.sleep(0.01) end
+test:is(f_rw:status(), "dead", "initialized read-write mode. read-write waiter dies.")
+f_ro = fiber.create(function() box.ctl.wait_ro() end)
+test:is(f_ro:status(), "suspended", "new read-only waiter is blocked")
 
-box.cfg{read_only=false}
+box.cfg{read_only=true}
 
-while f_rw:status() ~= "dead" do fiber.sleep(0.01) end
-test:is(f_rw:status(), "dead", "initialized read-write mode")
+while f_ro:status() ~= "dead" do fiber.sleep(0.01) end
+test:is(f_ro:status(), "dead", "entered read-only mode")
 
 test:check()
 os.exit(0)
diff --git a/test/box-tap/cfg.test.lua b/test/box-tap/cfg.test.lua
index 55de5e41c0..8faedaec60 100755
--- a/test/box-tap/cfg.test.lua
+++ b/test/box-tap/cfg.test.lua
@@ -6,7 +6,7 @@ local socket = require('socket')
 local fio = require('fio')
 local uuid = require('uuid')
 local msgpack = require('msgpack')
-test:plan(104)
+test:plan(105)
 
 --------------------------------------------------------------------------------
 -- Invalid values
@@ -585,5 +585,14 @@ test:is(run_script(code1), 0, "create huge tuple")
 test:is(run_script(code2), PANIC, "panic on huge tuple recovery")
 fio.rmtree(dir)
 
+--
+-- gh-4321 don't bootstrap a readonly instance as master
+--
+code=[[
+box.cfg{read_only=true}
+]]
+test:is(run_script(code), PANIC, "panic on bootstrapping a read-only instance as master")
+
+
 test:check()
 os.exit(0)
diff --git a/test/box/misc.result b/test/box/misc.result
index c46c5a9d6a..c0e031642d 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -532,6 +532,7 @@ t;
   200: box.error.FUNC_INDEX_PARTS
   201: box.error.NO_SUCH_FIELD_NAME
   202: box.error.FUNC_WRONG_ARG_COUNT
+  203: box.error.BOOTSTRAP_READONLY
 ...
 test_run:cmd("setopt delimiter ''");
 ---
-- 
GitLab