From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp40.i.mail.ru (smtp40.i.mail.ru [94.100.177.100]) (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 226814696C3 for ; Tue, 31 Mar 2020 17:42:38 +0300 (MSK) From: Nikita Pettik Date: Tue, 31 Mar 2020 17:42:32 +0300 Message-Id: Subject: [Tarantool-patches] [PATCH] vinyl: unthrottle scheduler on checkpoint List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org Cc: v.shpilevoy@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