[Tarantool-patches] [PATCH] vinyl: unthrottle scheduler on checkpoint

[Nikita Pettik <korablev@tarantool.org>]

Before this patch box.snapshot() bails out immediately if it sees that
the scheduler is throttled due to errors. For instance:

box.error.injection.set('ERRINJ_VY_RUN_WRITE', true)
snapshot() -- fails due to ERRINJ_VY_RUN_WRITE
box.error.injection.set('ERRINJ_VY_RUN_WRITE', false)
snapshot() -- still fails despite the fact that injected error is unset

As a result, one has to wait up to a minute to make a snapshot. The
reason why throttling was introduced was to avoid flooding the log
in case of repeating disk errors.
On the other hand, checkpoint process is either called manually or on
schedule. What is more, to deal with schedule throttling in tests, we
had to introduce a new error injection (ERRINJ_VY_SCHED_TIMEOUT).
It reduces time duration during which the scheduler remains throttled,
which is ugly and race prone.
So, let's unthrottle scheduler when checkpoint process is launched.

Closes #3519
---
Note that VY_SCHED_TIMEOUT error injection is not completely removed
from tests, since at least one of them fails (instance crashes) without
it (vinyl/errinj_stat.test.lua). It's not a problem of this patch -
reproducer is extracted and filed in gh-4821.

Branch: https://github.com/tarantool/tarantool/tree/np/gh-3519-unthrottle-sched
Issue: https://github.com/tarantool/tarantool/issues/3519

 src/box/vy_scheduler.c           | 14 +++++---------
 test/box/errinj.result           |  8 --------
 test/box/errinj.test.lua         |  2 --
 test/vinyl/errinj.result         | 12 ++----------
 test/vinyl/errinj.test.lua       |  5 +----
 test/vinyl/errinj_vylog.result   | 14 --------------
 test/vinyl/errinj_vylog.test.lua |  4 ----
 7 files changed, 8 insertions(+), 51 deletions(-)

diff --git a/src/box/vy_scheduler.c b/src/box/vy_scheduler.c
index 9dba93d34..f57f10119 100644
--- a/src/box/vy_scheduler.c
+++ b/src/box/vy_scheduler.c
@@ -687,17 +687,13 @@ vy_scheduler_begin_checkpoint(struct vy_scheduler *scheduler)
 	assert(!scheduler->checkpoint_in_progress);
 
 	/*
-	 * If the scheduler is throttled due to errors, do not wait
-	 * until it wakes up as it may take quite a while. Instead
-	 * fail checkpoint immediately with the last error seen by
-	 * the scheduler.
+	 * It makes no sense throttling checkpoint process since
+	 * it can be either ran manually or due to timeout. At this
+	 * point let's ignore it.
 	 */
 	if (scheduler->is_throttled) {
-		struct error *e = diag_last_error(&scheduler->diag);
-		diag_add_error(diag_get(), e);
-		say_error("cannot checkpoint vinyl, "
-			  "scheduler is throttled with: %s", e->errmsg);
-		return -1;
+		say_info("scheduler is unthrottled due to checkpoint process");
+		scheduler->is_throttled = false;
 	}
 
 	if (!vy_scheduler_dump_in_progress(scheduler)) {
diff --git a/test/box/errinj.result b/test/box/errinj.result
index 8090deedc..34715b9ab 100644
--- a/test/box/errinj.result
+++ b/test/box/errinj.result
@@ -1561,10 +1561,6 @@ errinj.set('ERRINJ_VY_GC', true)
 ---
 - ok
 ...
-errinj.set('ERRINJ_VY_SCHED_TIMEOUT', 0.001)
----
-- ok
-...
 errinj.set('ERRINJ_VY_RUN_FILE_RENAME', true)
 ---
 - ok
@@ -1601,10 +1597,6 @@ errinj.set('ERRINJ_VY_INDEX_FILE_RENAME', false)
 ---
 - ok
 ...
-errinj.set('ERRINJ_VY_SCHED_TIMEOUT', 0)
----
-- ok
-...
 errinj.set('ERRINJ_VY_GC', false)
 ---
 - ok
diff --git a/test/box/errinj.test.lua b/test/box/errinj.test.lua
index d6186726f..718cf516f 100644
--- a/test/box/errinj.test.lua
+++ b/test/box/errinj.test.lua
@@ -568,7 +568,6 @@ box.snapshot()
 errinj.set('ERRINJ_VY_LOG_FILE_RENAME', false)
 
 errinj.set('ERRINJ_VY_GC', true)
-errinj.set('ERRINJ_VY_SCHED_TIMEOUT', 0.001)
 
 errinj.set('ERRINJ_VY_RUN_FILE_RENAME', true)
 box.space.test:insert{1}
@@ -583,7 +582,6 @@ box.space.test:insert{2}
 box.snapshot() -- error
 errinj.set('ERRINJ_VY_INDEX_FILE_RENAME', false)
 
-errinj.set('ERRINJ_VY_SCHED_TIMEOUT', 0)
 errinj.set('ERRINJ_VY_GC', false)
 
 #fio.glob(fio.pathjoin(box.cfg.vinyl_dir, '*.vylog.inprogress')) > 0
diff --git a/test/vinyl/errinj.result b/test/vinyl/errinj.result
index 47c10493b..e33edc286 100644
--- a/test/vinyl/errinj.result
+++ b/test/vinyl/errinj.result
@@ -78,10 +78,10 @@ errinj.set("ERRINJ_VY_RUN_WRITE", false);
 ---
 - ok
 ...
--- fails due to scheduler timeout
+-- does not fail once error is unset
 box.snapshot();
 ---
-- error: Error injection 'vinyl dump'
+- ok
 ...
 fiber.sleep(0.06);
 ---
@@ -366,10 +366,6 @@ box.snapshot();
 s:drop()
 ---
 ...
-errinj.set("ERRINJ_VY_SCHED_TIMEOUT", 0)
----
-- ok
-...
 --
 -- Check that upsert squash fiber does not crash if index or
 -- in-memory tree is gone.
@@ -1056,10 +1052,6 @@ s.index.pk:stat().disk.compaction.count -- 1
 ---
 - 1
 ...
-errinj.set("ERRINJ_VY_SCHED_TIMEOUT", 0)
----
-- ok
-...
 box.snapshot() -- ok
 ---
 - ok
diff --git a/test/vinyl/errinj.test.lua b/test/vinyl/errinj.test.lua
index 7b6118a8a..1e47c2607 100644
--- a/test/vinyl/errinj.test.lua
+++ b/test/vinyl/errinj.test.lua
@@ -38,7 +38,7 @@ num_rows = num_rows + range();
 -- fails due to error injection
 box.snapshot();
 errinj.set("ERRINJ_VY_RUN_WRITE", false);
--- fails due to scheduler timeout
+-- does not fail once error is unset
 box.snapshot();
 fiber.sleep(0.06);
 num_rows = num_rows + range();
@@ -128,8 +128,6 @@ box.snapshot();
 #s:select({1})
 s:drop()
 
-errinj.set("ERRINJ_VY_SCHED_TIMEOUT", 0)
-
 --
 -- Check that upsert squash fiber does not crash if index or
 -- in-memory tree is gone.
@@ -384,7 +382,6 @@ s.index.pk:stat().disk.compaction.count -- 0
 errinj.set("ERRINJ_WAL_IO", false)
 while s.index.pk:stat().disk.compaction.count == 0 do fiber.sleep(0.001) end
 s.index.pk:stat().disk.compaction.count -- 1
-errinj.set("ERRINJ_VY_SCHED_TIMEOUT", 0)
 
 box.snapshot() -- ok
 s:drop()
diff --git a/test/vinyl/errinj_vylog.result b/test/vinyl/errinj_vylog.result
index 06cc68188..a642f2b16 100644
--- a/test/vinyl/errinj_vylog.result
+++ b/test/vinyl/errinj_vylog.result
@@ -52,13 +52,6 @@ _ = s:create_index('pk')
 _ = s:insert{1, 'x'}
 ---
 ...
-SCHED_TIMEOUT = 0.05
----
-...
-box.error.injection.set('ERRINJ_VY_SCHED_TIMEOUT', SCHED_TIMEOUT)
----
-- ok
-...
 box.error.injection.set('ERRINJ_VY_LOG_FLUSH', true);
 ---
 - ok
@@ -71,13 +64,6 @@ box.error.injection.set('ERRINJ_VY_LOG_FLUSH', false);
 ---
 - ok
 ...
-fiber.sleep(2 * SCHED_TIMEOUT)
----
-...
-box.error.injection.set('ERRINJ_VY_SCHED_TIMEOUT', 0)
----
-- ok
-...
 _ = s:insert{2, 'y'}
 ---
 ...
diff --git a/test/vinyl/errinj_vylog.test.lua b/test/vinyl/errinj_vylog.test.lua
index 2936f879c..2ab200423 100644
--- a/test/vinyl/errinj_vylog.test.lua
+++ b/test/vinyl/errinj_vylog.test.lua
@@ -27,15 +27,11 @@ s = box.schema.space.create('test', {engine = 'vinyl'})
 _ = s:create_index('pk')
 _ = s:insert{1, 'x'}
 
-SCHED_TIMEOUT = 0.05
-box.error.injection.set('ERRINJ_VY_SCHED_TIMEOUT', SCHED_TIMEOUT)
 box.error.injection.set('ERRINJ_VY_LOG_FLUSH', true);
 
 box.snapshot()
 
 box.error.injection.set('ERRINJ_VY_LOG_FLUSH', false);
-fiber.sleep(2 * SCHED_TIMEOUT)
-box.error.injection.set('ERRINJ_VY_SCHED_TIMEOUT', 0)
 
 _ = s:insert{2, 'y'}
 
-- 
2.17.1