From 4bf52367a745bd4d33d4727e2a8c574f2da9094d Mon Sep 17 00:00:00 2001
From: Ilya Verbin <iverbin@tarantool.org>
Date: Fri, 13 May 2022 17:57:53 +0300
Subject: [PATCH] wal: allow spurious wakeups in wal_write
It's possible to wakeup a fiber, which is waiting for WAL write
completion, using Tarantool C API. This results in an error like:
```
main/118/lua F> Journal result code -1 can't be converted to an error
```
This patch introduces a flag, which is set when WAL write is
finished, that allows fibers to yield until the flag is set.
Closes #6506
NO_DOC=bugfix
---
...506-allow-spurious-wakeups-in-wal_write.md | 4 ++
src/box/journal.h | 8 ++++
src/box/wal.c | 7 +--
test/CMakeLists.txt | 1 +
test/box-luatest/CMakeLists.txt | 1 +
.../gh_6506_wakeup_writing_to_wal_fiber.c | 17 ++++++++
..._6506_wakeup_writing_to_wal_fiber_test.lua | 43 +++++++++++++++++++
7 files changed, 78 insertions(+), 3 deletions(-)
create mode 100644 changelogs/unreleased/gh-6506-allow-spurious-wakeups-in-wal_write.md
create mode 100644 test/box-luatest/CMakeLists.txt
create mode 100644 test/box-luatest/gh_6506_wakeup_writing_to_wal_fiber.c
create mode 100644 test/box-luatest/gh_6506_wakeup_writing_to_wal_fiber_test.lua
diff --git a/changelogs/unreleased/gh-6506-allow-spurious-wakeups-in-wal_write.md b/changelogs/unreleased/gh-6506-allow-spurious-wakeups-in-wal_write.md
new file mode 100644
index 0000000000..012556cb58
--- /dev/null
+++ b/changelogs/unreleased/gh-6506-allow-spurious-wakeups-in-wal_write.md
@@ -0,0 +1,4 @@
+## bugfix/core
+
+* Allowed spurious wakeup of a fiber, which is waiting for WAL write completion
+ (gh-6506).
diff --git a/src/box/journal.h b/src/box/journal.h
index 8572457791..7d3e525f8f 100644
--- a/src/box/journal.h
+++ b/src/box/journal.h
@@ -101,6 +101,11 @@ struct journal_entry {
* Approximate size of this request when encoded.
*/
size_t approx_len;
+ /**
+ * Set to true when execution of a batch that contains this
+ * journal entry is completed.
+ */
+ bool is_complete;
/**
* The number of rows in the request.
*/
@@ -128,6 +133,7 @@ journal_entry_create(struct journal_entry *entry, size_t n_rows,
entry->n_rows = n_rows;
entry->res = JOURNAL_ENTRY_ERR_UNKNOWN;
entry->flags = 0;
+ entry->is_complete = false;
}
/**
@@ -244,6 +250,8 @@ journal_async_complete(struct journal_entry *entry)
{
assert(entry->write_async_cb != NULL);
+ entry->is_complete = true;
+
journal_queue_on_complete(entry);
entry->write_async_cb(entry);
diff --git a/src/box/wal.c b/src/box/wal.c
index 57742632e7..c50fc4f9b4 100644
--- a/src/box/wal.c
+++ b/src/box/wal.c
@@ -1309,9 +1309,10 @@ wal_write(struct journal *journal, struct journal_entry *entry)
if (wal_write_async(journal, entry) != 0)
return -1;
- bool cancellable = fiber_set_cancellable(false);
- fiber_yield();
- fiber_set_cancellable(cancellable);
+ assert(!entry->is_complete);
+ do {
+ fiber_yield();
+ } while (!entry->is_complete);
return 0;
}
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 6e206e065e..1f8632adf3 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -60,6 +60,7 @@ add_subdirectory(app)
add_subdirectory(app-tap)
add_subdirectory(box)
add_subdirectory(box-tap)
+add_subdirectory(box-luatest)
add_subdirectory(sql-tap)
add_subdirectory(sql-luatest)
if(ENABLE_FUZZER)
diff --git a/test/box-luatest/CMakeLists.txt b/test/box-luatest/CMakeLists.txt
new file mode 100644
index 0000000000..d654163f05
--- /dev/null
+++ b/test/box-luatest/CMakeLists.txt
@@ -0,0 +1 @@
+build_module(gh_6506_lib gh_6506_wakeup_writing_to_wal_fiber.c)
diff --git a/test/box-luatest/gh_6506_wakeup_writing_to_wal_fiber.c b/test/box-luatest/gh_6506_wakeup_writing_to_wal_fiber.c
new file mode 100644
index 0000000000..ef65040b86
--- /dev/null
+++ b/test/box-luatest/gh_6506_wakeup_writing_to_wal_fiber.c
@@ -0,0 +1,17 @@
+#include "module.h"
+
+static struct fiber *saved = NULL;
+
+int
+save_fiber(box_function_ctx_t *ctx, const char *args, const char *args_end)
+{
+ saved = fiber_self();
+ return 0;
+}
+
+int
+wakeup_saved(box_function_ctx_t *ctx, const char *args, const char *args_end)
+{
+ fiber_wakeup(saved);
+ return 0;
+}
diff --git a/test/box-luatest/gh_6506_wakeup_writing_to_wal_fiber_test.lua b/test/box-luatest/gh_6506_wakeup_writing_to_wal_fiber_test.lua
new file mode 100644
index 0000000000..23e505bb5e
--- /dev/null
+++ b/test/box-luatest/gh_6506_wakeup_writing_to_wal_fiber_test.lua
@@ -0,0 +1,43 @@
+local server = require('test.luatest_helpers.server')
+local t = require('luatest')
+local g = t.group()
+
+g.before_all = function()
+ g.server = server:new{alias = 'master'}
+ g.server:start()
+end
+
+g.after_all = function()
+ g.server:drop()
+end
+
+g.test_wakeup_writing_to_wal_fiber = function()
+ g.server:exec(function()
+ local t = require('luatest')
+ local fiber = require('fiber')
+
+ local build_path = os.getenv("BUILDDIR")
+ package.cpath = build_path..'/test/box-luatest/?.so;'..build_path..'/test/box-luatest/?.dylib;'..package.cpath
+ local lib = box.lib.load('gh_6506_lib')
+ local save_fiber = lib:load('save_fiber')
+ local wakeup_saved = lib:load('wakeup_saved')
+
+ local s = box.schema.create_space('test')
+ s:create_index('pk')
+
+ local f = fiber.new(function()
+ save_fiber()
+ s:replace{1}
+ fiber.yield()
+ end)
+ f:set_joinable(true)
+
+ -- Start fiber f
+ fiber.yield()
+ -- Wakeup f while it is writing to WAL
+ wakeup_saved()
+ -- Check that f did not crash
+ local st, _ = f:join()
+ t.assert(st)
+ end)
+end
--
GitLab