From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp52.i.mail.ru (smtp52.i.mail.ru [94.100.177.112]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id E16BA42EF5C for ; Fri, 19 Jun 2020 20:45:45 +0300 (MSK) References: <7ddc4d3a6b45dba38e7bc184d3125c42936fc512.1591701695.git.sergepetrenko@tarantool.org> <969b1847-08cb-5370-ec5d-a07a52397b48@tarantool.org> From: Serge Petrenko Message-ID: Date: Fri, 19 Jun 2020 20:45:44 +0300 MIME-Version: 1.0 In-Reply-To: <969b1847-08cb-5370-ec5d-a07a52397b48@tarantool.org> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit Content-Language: en-GB Subject: Re: [Tarantool-patches] [PATCH 2/8] replication: introduce replication_sync_quorum cfg List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy , sergos@tarantool.org, gorcunov@gmail.com, Leonid Vasiliev Cc: tarantool-patches@dev.tarantool.org 16.06.2020 02:05, Vladislav Shpilevoy пишет: > I appended a new commit on top of this one on the > branch. In the quorum commit I renamed the > option to > > replication_synchro_quorum > > ==================== > commit fc19662ec528c5217c7b611ae16d417497d9fe35 > Author: Vladislav Shpilevoy > Date: Tue Jun 16 00:47:24 2020 +0200 > > replication: introduce replication_synchro_timeout cfg > > [TO BE SQUASHED INTO THE PREVIOUS COMMIT] > > Part of #4844 > Part of #5073 > > diff --git a/src/box/box.cc b/src/box/box.cc > index c7a5f2e3c..9db55e05a 100644 > --- a/src/box/box.cc > +++ b/src/box/box.cc > @@ -489,6 +489,18 @@ box_check_replication_synchro_quorum(void) > return quorum; > } > > +static double > +box_check_replication_synchro_timeout(void) > +{ > + double timeout = cfg_getd("replication_synchro_timeout"); > + if (timeout <= 0) { > + diag_set(ClientError, ER_CFG, "replication_synchro_timeout", > + "the value must be greater than zero"); > + return -1; > + } > + return timeout; > +} > + > static double > box_check_replication_sync_timeout(void) > { > @@ -673,6 +685,8 @@ box_check_config() > box_check_replication_sync_lag(); > if (box_check_replication_synchro_quorum() < 0) > diag_raise(); > + if (box_check_replication_synchro_timeout() < 0) > + diag_raise(); > box_check_replication_sync_timeout(); > box_check_readahead(cfg_geti("readahead")); > box_check_checkpoint_count(cfg_geti("checkpoint_count")); > @@ -802,6 +816,16 @@ box_set_replication_synchro_quorum(void) > return 0; > } > > +int > +box_set_replication_synchro_timeout(void) > +{ > + double value = box_check_replication_synchro_timeout(); > + if (value < 0) > + return -1; > + replication_synchro_timeout = value; > + return 0; > +} > + > void > box_set_replication_sync_timeout(void) > { > @@ -2444,6 +2468,8 @@ box_cfg_xc(void) > box_set_replication_sync_lag(); > if (box_set_replication_synchro_quorum() != 0) > diag_raise(); > + if (box_set_replication_synchro_timeout() != 0) > + diag_raise(); > box_set_replication_sync_timeout(); > box_set_replication_skip_conflict(); > box_set_replication_anon(); > diff --git a/src/box/box.h b/src/box/box.h > index 24802d0f1..f9789154e 100644 > --- a/src/box/box.h > +++ b/src/box/box.h > @@ -244,6 +244,7 @@ void box_set_replication_connect_timeout(void); > void box_set_replication_connect_quorum(void); > void box_set_replication_sync_lag(void); > int box_set_replication_synchro_quorum(void); > +int box_set_replication_synchro_timeout(void); > void box_set_replication_sync_timeout(void); > void box_set_replication_skip_conflict(void); > void box_set_replication_anon(void); > diff --git a/src/box/lua/cfg.cc b/src/box/lua/cfg.cc > index 01e8958cd..d481155cd 100644 > --- a/src/box/lua/cfg.cc > +++ b/src/box/lua/cfg.cc > @@ -321,6 +321,14 @@ lbox_cfg_set_replication_synchro_quorum(struct lua_State *L) > return 0; > } > > +static int > +lbox_cfg_set_replication_synchro_timeout(struct lua_State *L) > +{ > + if (box_set_replication_synchro_timeout() != 0) > + luaT_error(L); > + return 0; > +} > + > static int > lbox_cfg_set_replication_sync_timeout(struct lua_State *L) > { > @@ -379,6 +387,7 @@ box_lua_cfg_init(struct lua_State *L) > {"cfg_set_replication_connect_timeout", lbox_cfg_set_replication_connect_timeout}, > {"cfg_set_replication_sync_lag", lbox_cfg_set_replication_sync_lag}, > {"cfg_set_replication_synchro_quorum", lbox_cfg_set_replication_synchro_quorum}, > + {"cfg_set_replication_synchro_timeout", lbox_cfg_set_replication_synchro_timeout}, > {"cfg_set_replication_sync_timeout", lbox_cfg_set_replication_sync_timeout}, > {"cfg_set_replication_skip_conflict", lbox_cfg_set_replication_skip_conflict}, > {"cfg_set_replication_anon", lbox_cfg_set_replication_anon}, > diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua > index 991e919e4..1155248a5 100644 > --- a/src/box/lua/load_cfg.lua > +++ b/src/box/lua/load_cfg.lua > @@ -90,6 +90,7 @@ local default_cfg = { > replication_sync_lag = 10, > replication_sync_timeout = 300, > replication_synchro_quorum = 1, > + replication_synchro_timeout = 5, > replication_connect_timeout = 30, > replication_connect_quorum = nil, -- connect all > replication_skip_conflict = false, > @@ -166,6 +167,7 @@ local template_cfg = { > replication_sync_lag = 'number', > replication_sync_timeout = 'number', > replication_synchro_quorum = 'number', > + replication_synchro_timeout = 'number', > replication_connect_timeout = 'number', > replication_connect_quorum = 'number', > replication_skip_conflict = 'boolean', > @@ -278,6 +280,7 @@ local dynamic_cfg = { > replication_sync_lag = private.cfg_set_replication_sync_lag, > replication_sync_timeout = private.cfg_set_replication_sync_timeout, > replication_synchro_quorum = private.cfg_set_replication_synchro_quorum, > + replication_synchro_timeout = private.cfg_set_replication_synchro_timeout, > replication_skip_conflict = private.cfg_set_replication_skip_conflict, > replication_anon = private.cfg_set_replication_anon, > instance_uuid = check_instance_uuid, > @@ -312,6 +315,7 @@ local dynamic_cfg_order = { > replication_sync_lag = 150, > replication_sync_timeout = 150, > replication_synchro_quorum = 150, > + replication_synchro_timeout = 150, > replication_connect_timeout = 150, > replication_connect_quorum = 150, > replication = 200, > @@ -348,6 +352,7 @@ local dynamic_cfg_skip_at_load = { > replication_sync_lag = true, > replication_sync_timeout = true, > replication_synchro_quorum = true, > + replication_synchro_timeout = true, > replication_skip_conflict = true, > replication_anon = true, > wal_dir_rescan_delay = true, > diff --git a/src/box/replication.cc b/src/box/replication.cc > index 5b52f3864..01e9e876a 100644 > --- a/src/box/replication.cc > +++ b/src/box/replication.cc > @@ -52,6 +52,7 @@ double replication_connect_timeout = 30.0; /* seconds */ > int replication_connect_quorum = REPLICATION_CONNECT_QUORUM_ALL; > double replication_sync_lag = 10.0; /* seconds */ > int replication_synchro_quorum = 1; > +double replication_synchro_timeout = 5.0; /* seconds */ > double replication_sync_timeout = 300.0; /* seconds */ > bool replication_skip_conflict = false; > bool replication_anon = false; > diff --git a/src/box/replication.h b/src/box/replication.h > index 05e3eb943..a081870f9 100644 > --- a/src/box/replication.h > +++ b/src/box/replication.h > @@ -131,6 +131,12 @@ extern double replication_sync_lag; > */ > extern int replication_synchro_quorum; > > +/** > + * Time in seconds which the master node is able to wait for ACKs > + * for a synchronous transaction until it is rolled back. > + */ > +extern double replication_synchro_timeout; > + > /** > * Max time to wait for appliers to synchronize before entering > * the orphan mode. > diff --git a/test/app-tap/init_script.result b/test/app-tap/init_script.result > index 2987b60b9..857f0c95f 100644 > --- a/test/app-tap/init_script.result > +++ b/test/app-tap/init_script.result > @@ -31,6 +31,7 @@ replication_skip_conflict:false > replication_sync_lag:10 > replication_sync_timeout:300 > replication_synchro_quorum:1 > +replication_synchro_timeout:5 > replication_timeout:1 > slab_alloc_factor:1.05 > sql_cache_size:5242880 > diff --git a/test/box/admin.result b/test/box/admin.result > index 35ecc7617..ab3e80a97 100644 > --- a/test/box/admin.result > +++ b/test/box/admin.result > @@ -83,6 +83,8 @@ cfg_filter(box.cfg) > - 300 > - - replication_synchro_quorum > - 1 > + - - replication_synchro_timeout > + - 5 > - - replication_timeout > - 1 > - - slab_alloc_factor > diff --git a/test/box/cfg.result b/test/box/cfg.result > index cdc0773f2..bdd210b09 100644 > --- a/test/box/cfg.result > +++ b/test/box/cfg.result > @@ -71,6 +71,8 @@ cfg_filter(box.cfg) > | - 300 > | - - replication_synchro_quorum > | - 1 > + | - - replication_synchro_timeout > + | - 5 > | - - replication_timeout > | - 1 > | - - slab_alloc_factor > @@ -176,6 +178,8 @@ cfg_filter(box.cfg) > | - 300 > | - - replication_synchro_quorum > | - 1 > + | - - replication_synchro_timeout > + | - 5 > | - - replication_timeout > | - 1 > | - - slab_alloc_factor Thanks! Looks good. I squashed the patch into the previous commit. Here's the new commit message I came up with:     replication: introduce replication_synchro_quorum and replication_synchro_timeout cfg options     Synchronous transactions are supposed to be replicated on a     specified number of replicas before committed on master. The     number of replicas can be specified using     replication_synchro_quorum option. It is 1 by default, so sync     transactions work like asynchronous when not configured anyhow.     1 means successful WAL write on master is enough for commit.     When replication_synchro_quorum is greater than 1, an instance has to     wait for the specified number of replicas to  reply with success. If     enough replies aren't collected during replication_synchro_timeout,     the instance rolls back the tx in question.     Part of #4844     Part of #5073 -- Serge Petrenko