Re: [Tarantool-patches] [PATCH v2 1/3] gc/xlog: delay xlog cleanup until relays are subscribed

[Cyrill Gorcunov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

On Tue, Mar 23, 2021 at 02:25:26PM +0300, Cyrill Gorcunov wrote:
> On Tue, Mar 23, 2021 at 10:28:46AM +0300, Cyrill Gorcunov wrote:
> > 
> > > 
> > > Also you should use 'replication_anon' global variable instead
> > > of the config, which might be not installed at this moment yet.
> > 
> > What would happen if one setup both 'wal_cleanup_delay' and
> > 'replication_anon' in config at once. Which C's replication_anon
> > value I will be reading? The C's replication_anon is set after
> > the cfg procedure complete, so since I operate on values obrained
> > from Lua level I need to use cfg_geti("replication_anon") because
> > at this moment only Lua level is consistent and replication_anon
> > may have a value from previous box.cfg call.

Here is a diff, take a look please.
---
diff --git a/src/box/box.cc b/src/box/box.cc
index b9fd7af00..81e001680 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -774,23 +774,13 @@ box_check_wal_queue_max_size(void)
 static double
 box_check_wal_cleanup_delay(void)
 {
-	double value = cfg_geti("wal_cleanup_delay");
+	double value = cfg_getd("wal_cleanup_delay");
 	if (value < 0) {
 		diag_set(ClientError, ER_CFG, "wal_cleanup_delay",
 			 "the value must be >= 0");
 		return -1;
 	}
 
-	/*
-	 * Anonymous replicas do not require
-	 * delay the cleanup procedure since they
-	 * are read only.
-	 */
-	if (cfg_geti("replication_anon") != 0) {
-		if (value != 0)
-			value = 0;
-	}
-
 	return value;
 }
 
@@ -1496,6 +1486,13 @@ box_set_wal_cleanup_delay(void)
 	double delay = box_check_wal_cleanup_delay();
 	if (delay < 0)
 		return -1;
+	/*
+	 * Anonymous replicas do not require
+	 * delay the cleanup procedure since they
+	 * are read only.
+	 */
+	if (replication_anon)
+		delay = 0;
 	gc_set_wal_cleanup_delay(delay);
 	return 0;
 }
diff --git a/src/box/gc.c b/src/box/gc.c
index 5418fd31d..e1d7a1187 100644
--- a/src/box/gc.c
+++ b/src/box/gc.c
@@ -46,7 +46,6 @@
 #include <small/rlist.h>
 #include <tarantool_ev.h>
 
-#include "cfg.h"
 #include "diag.h"
 #include "errcode.h"
 #include "fiber.h"
@@ -253,10 +252,13 @@ gc_cleanup_fiber_f(va_list ap)
 	 * separate cycle to minimize branching on stage 2.
 	 */
 	if (gc.is_paused) {
-		ev_tstamp timeout = gc.wal_cleanup_delay;
+		double start_time = fiber_clock();
 		while (!fiber_is_cancelled()) {
-			ev_tstamp clock_start = fiber_clock();
-			if (fiber_yield_timeout(timeout)) {
+			double deadline = start_time + gc.wal_cleanup_delay;
+			double timeout = gc.wal_cleanup_delay;
+
+			if (fiber_clock() >= deadline ||
+			    fiber_yield_timeout(timeout)) {
 				say_info("wal/engine cleanup is resumed "
 					 "due to timeout expiration");
 				gc.is_paused = false;
@@ -270,17 +272,6 @@ gc_cleanup_fiber_f(va_list ap)
 			 */
 			if (!gc.is_paused)
 				break;
-
-			ev_tstamp elapsed = fiber_clock() - clock_start;
-			timeout = gc.wal_cleanup_delay - elapsed;
-			if (timeout <= 0) {
-				say_info("wal/engine cleanup is resumed "
-					 "due to reconfigured timeout "
-					 "expiration");
-				gc.is_paused = false;
-				gc.delay_ref = 0;
-				break;
-			}
 		}
 	}
 
diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
index 5ed8c12f0..44bb95ed1 100644
--- a/src/box/lua/load_cfg.lua
+++ b/src/box/lua/load_cfg.lua
@@ -352,6 +352,8 @@ local dynamic_cfg_order = {
     -- the new one. This should be fixed when box.cfg is able to
     -- apply some parameters together and atomically.
     replication_anon        = 250,
+    -- Cleanup delay should be ignored if replication_anon is set.
+    wal_cleanup_delay       = 260,
     election_mode           = 300,
     election_timeout        = 320,
 }
@@ -384,7 +386,6 @@ local dynamic_cfg_skip_at_load = {
     replication_skip_conflict = true,
     replication_anon        = true,
     wal_dir_rescan_delay    = true,
-    wal_cleanup_delay       = true,
     custom_proc_title       = true,
     force_recovery          = true,
     instance_uuid           = true,