From 04347ee7567402c4460468c7f80540ff532c2e45 Mon Sep 17 00:00:00 2001
From: Vladimir Davydov <vdavydov@tarantool.org>
Date: Thu, 11 Jul 2024 19:14:56 +0300
Subject: [PATCH] vinyl: use broadcast instead of signal to notify about dump
completion
There may be more than one fiber waiting on `vy_scheduler::dump_cond`:
```
box.snapshot
vinyl_engine_wait_checkpoint
vy_scheduler_wait_checkpoint
space.create_index
vinyl_space_build_index
vy_scheduler_dump
```
To avoid hang, we should use `fiber_cond_broadcast`.
Closes #10233
NO_DOC=bug fix
(cherry picked from commit 30547157db74a2869b7da4247e8c97c9ec39a6a9)
---
.../gh-10233-vy-snapshot-hang-fix.md | 4 ++
src/box/vy_scheduler.c | 4 +-
.../gh_10233_index_build_vs_snapshot_test.lua | 42 +++++++++++++++++++
3 files changed, 48 insertions(+), 2 deletions(-)
create mode 100644 changelogs/unreleased/gh-10233-vy-snapshot-hang-fix.md
create mode 100644 test/vinyl-luatest/gh_10233_index_build_vs_snapshot_test.lua
diff --git a/changelogs/unreleased/gh-10233-vy-snapshot-hang-fix.md b/changelogs/unreleased/gh-10233-vy-snapshot-hang-fix.md
new file mode 100644
index 0000000000..bf58a263f3
--- /dev/null
+++ b/changelogs/unreleased/gh-10233-vy-snapshot-hang-fix.md
@@ -0,0 +1,4 @@
+## bugfix/vinyl
+
+* Fixed a bug when `box.snapshot` hanged if executed concurrently with creation
+ of a new index (gh-10233).
diff --git a/src/box/vy_scheduler.c b/src/box/vy_scheduler.c
index ea9b438bdb..29e9c19245 100644
--- a/src/box/vy_scheduler.c
+++ b/src/box/vy_scheduler.c
@@ -670,7 +670,7 @@ vy_scheduler_complete_dump(struct vy_scheduler *scheduler)
scheduler->stat.dump_count++;
scheduler->dump_complete_cb(scheduler,
min_generation - 1, dump_duration);
- fiber_cond_signal(&scheduler->dump_cond);
+ fiber_cond_broadcast(&scheduler->dump_cond);
}
int
@@ -2068,7 +2068,7 @@ vy_scheduler_f(va_list va)
continue;
error:
/* Abort pending checkpoint. */
- fiber_cond_signal(&scheduler->dump_cond);
+ fiber_cond_broadcast(&scheduler->dump_cond);
/*
* A task can fail either due to lack of memory or IO
* error. In either case it is pointless to schedule
diff --git a/test/vinyl-luatest/gh_10233_index_build_vs_snapshot_test.lua b/test/vinyl-luatest/gh_10233_index_build_vs_snapshot_test.lua
new file mode 100644
index 0000000000..3a9ddec743
--- /dev/null
+++ b/test/vinyl-luatest/gh_10233_index_build_vs_snapshot_test.lua
@@ -0,0 +1,42 @@
+local server = require('luatest.server')
+local t = require('luatest')
+
+local g = t.group()
+
+g.before_all(function(cg)
+ cg.server = server:new()
+ cg.server:start()
+end)
+
+g.after_all(function(cg)
+ cg.server:drop()
+end)
+
+g.after_each(function(cg)
+ cg.server:exec(function()
+ if box.space.test ~= nil then
+ box.space.test:drop()
+ end
+ end)
+end)
+
+g.test_index_build_vs_snapshot = function(cg)
+ cg.server:exec(function()
+ local fiber = require('fiber')
+ local s = box.schema.space.create('test', {engine = 'vinyl'})
+ s:create_index('pk')
+ s:insert({1, 1})
+ local f1 = fiber.new(function()
+ s:create_index('sk', {parts = {2, 'unsigned'}})
+ end)
+ f1:set_joinable(true)
+ local f2 = fiber.new(function()
+ box.snapshot()
+ end)
+ f2:set_joinable(true)
+ fiber.sleep(0.1)
+ local timeout = 5
+ t.assert_equals({f1:join(timeout)}, {true})
+ t.assert_equals({f2:join(timeout)}, {true})
+ end)
+end
--
GitLab