From 2fc54ba103869db7e16aab80dbae46768095af99 Mon Sep 17 00:00:00 2001
From: Vladimir Davydov <vdavydov@tarantool.org>
Date: Fri, 14 Jul 2023 11:25:49 +0300
Subject: [PATCH] box: allow to truncate temp and local spaces in ro mode
To achieve that, we bypass the read-only check for the _truncate system
space in box_process1() and perform it in the on_replace system trigger
instead, when we know which space is truncated.
Note, we have to move the check for insertion of a new record into the
_truncate system space before the read-only check in the on_replace
trigger callback; this is needed for initial recovery with a non-empty
_truncate space to work. While we are at it, let's use recovery_state to
make the check explicit.
Closes #5616
@TarantoolBot document
Title: Mention that temp and local spaces can be truncated in ro mode
DML operations on temporary and local spaces can be performed even if
the instance is in the read-only mode, but DDL operations (such as
`alter`) are forbidden in this case[^1]. Technically, `truncate` is
a DDL operation so initially it was forbidden as well. However, it
should be safe to perform this operation on a temporary or local space
because logically it only modifies the data stored in the space (like
DML) and it isn't replicated (see tarantool/tarantool#4263). So starting
from Tarantool 2.11.1 we allow users to truncate temporary spaces in the
read-only mode.
[^1]: https://www.tarantool.io/en/doc/latest/concepts/replication/repl_architecture/#replication-local
(cherry picked from commit 054526ac8472d005ae7468774f06290597278a1d)
---
.../gh-5616-temp-space-truncate-ro.md | 4 +
src/box/alter.cc | 27 +--
src/box/box.cc | 12 +-
src/box/box.h | 7 +
.../gh_5616_temp_space_truncate_ro_test.lua | 167 ++++++++++++++++++
test/replication/anon.result | 4 -
test/replication/anon.test.lua | 1 -
7 files changed, 204 insertions(+), 18 deletions(-)
create mode 100644 changelogs/unreleased/gh-5616-temp-space-truncate-ro.md
create mode 100644 test/box-luatest/gh_5616_temp_space_truncate_ro_test.lua
diff --git a/changelogs/unreleased/gh-5616-temp-space-truncate-ro.md b/changelogs/unreleased/gh-5616-temp-space-truncate-ro.md
new file mode 100644
index 0000000000..127aac42ce
--- /dev/null
+++ b/changelogs/unreleased/gh-5616-temp-space-truncate-ro.md
@@ -0,0 +1,4 @@
+## feature/box
+
+* Allowed truncation of temporary and local spaces in the read-only mode
+ (gh-5616).
diff --git a/src/box/alter.cc b/src/box/alter.cc
index c7c6c62884..25934eb4f9 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -2584,6 +2584,10 @@ on_replace_dd_truncate(struct trigger * /* trigger */, void *event)
/* Space drop - nothing to do. */
return 0;
}
+ if (recovery_state == INITIAL_RECOVERY) {
+ /* Space creation during initial recovery - nothing to do. */
+ return 0;
+ }
uint32_t space_id;
if (tuple_field_u32(new_tuple, BOX_TRUNCATE_FIELD_SPACE_ID, &space_id) != 0)
@@ -2592,6 +2596,18 @@ on_replace_dd_truncate(struct trigger * /* trigger */, void *event)
if (old_space == NULL)
return -1;
+ /*
+ * box_process1() bypasses the read-only check for the _truncate system
+ * space because there the space that is going to be truncated isn't yet
+ * known. Perform the check here if this statement was issued by this
+ * replica and the space isn't temporary or local.
+ */
+ bool is_temp = space_is_temporary(old_space) ||
+ space_is_local(old_space);
+ if (!is_temp && stmt->row->replica_id == 0 &&
+ box_check_writable() != 0)
+ return -1;
+
/*
* Check if a write privilege was given, return an error if not.
* The check should precede initial recovery check to correctly
@@ -2600,14 +2616,6 @@ on_replace_dd_truncate(struct trigger * /* trigger */, void *event)
if (access_check_space(old_space, PRIV_W) != 0)
return -1;
- if (stmt->row->type == IPROTO_INSERT) {
- /*
- * Space creation during initial recovery -
- * nothing to do.
- */
- return 0;
- }
-
/*
* System spaces use triggers to keep records in sync
* with internal objects. Since space truncation doesn't
@@ -2633,8 +2641,7 @@ on_replace_dd_truncate(struct trigger * /* trigger */, void *event)
* replication of local & temporary
* spaces truncation.
*/
- if (space_is_temporary(old_space) ||
- space_is_local(old_space)) {
+ if (is_temp) {
stmt->row->group_id = GROUP_LOCAL;
/*
* The trigger is invoked after txn->n_local_rows
diff --git a/src/box/box.cc b/src/box/box.cc
index 8817c39f3f..ad280609be 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -350,7 +350,7 @@ box_check_slice_slow(void)
return fiber_check_slice();
}
-static int
+int
box_check_writable(void)
{
if (!is_ro_summary)
@@ -3243,11 +3243,17 @@ box_process1(struct request *request, box_tuple_t **result)
{
if (box_check_slice() != 0)
return -1;
- /* Allow to write to temporary spaces in read-only mode. */
struct space *space = space_cache_find(request->space_id);
if (space == NULL)
return -1;
- if (!space_is_temporary(space) &&
+ /*
+ * Allow to write to temporary and local spaces in the read-only mode.
+ * To handle space truncation, we postpone the read-only check for the
+ * _truncate system space till the on_replace trigger is called, when
+ * we know which space is truncated.
+ */
+ if (space_id(space) != BOX_TRUNCATE_ID &&
+ !space_is_temporary(space) &&
!space_is_local(space) &&
box_check_writable() != 0)
return -1;
diff --git a/src/box/box.h b/src/box/box.h
index 720cd32315..f50e1de77b 100644
--- a/src/box/box.h
+++ b/src/box/box.h
@@ -138,6 +138,13 @@ box_check_slice(void)
}
}
+/**
+ * Check if a write operation can be performed on this instance.
+ * Returns 0 on success. On error, sets diag and returns -1.
+ */
+int
+box_check_writable(void);
+
void
box_set_ro(void);
diff --git a/test/box-luatest/gh_5616_temp_space_truncate_ro_test.lua b/test/box-luatest/gh_5616_temp_space_truncate_ro_test.lua
new file mode 100644
index 0000000000..0dd944760f
--- /dev/null
+++ b/test/box-luatest/gh_5616_temp_space_truncate_ro_test.lua
@@ -0,0 +1,167 @@
+local server = require('luatest.server')
+local t = require('luatest')
+
+local g_single = t.group('gh_5616_temp_space_truncate_ro.single')
+
+g_single.before_all(function(cg)
+ cg.server = server:new()
+ cg.server:start()
+end)
+
+g_single.after_all(function(cg)
+ cg.server:drop()
+end)
+
+g_single.after_each(function(cg)
+ cg.server:exec(function()
+ box.cfg({read_only = false})
+ if box.space.test ~= nil then
+ box.space.test:drop()
+ end
+ end)
+end)
+
+-- Checks that a temporary space can be truncated in the read-only mode.
+g_single.test_temp_space_truncate_ro = function(cg)
+ cg.server:exec(function()
+ local s = box.schema.create_space('test', {temporary = true})
+ s:create_index('primary')
+ s:insert({1})
+ box.cfg({read_only = true})
+ local ok, err = pcall(s.truncate, s)
+ t.assert(ok, err)
+ t.assert_equals(s:select(), {})
+ end)
+end
+
+-- Checks that a local space can be truncated in the read-only mode.
+g_single.test_local_space_truncate_ro = function(cg)
+ cg.server:exec(function()
+ local s = box.schema.create_space('test', {is_local = true})
+ s:create_index('primary')
+ s:insert({1})
+ box.cfg({read_only = true})
+ local ok, err = pcall(s.truncate, s)
+ t.assert(ok, err)
+ t.assert_equals(s:select(), {})
+ end)
+end
+
+-- Checks that a persistent space can't be truncated in the read-only mode.
+g_single.test_persistent_space_truncate_ro = function(cg)
+ cg.server:exec(function()
+ local s = box.schema.create_space('test')
+ s:create_index('primary')
+ s:insert({1})
+ box.cfg({read_only = true})
+ local ok, err = pcall(s.truncate, s)
+ t.assert_not(ok, err)
+ t.assert_equals(s:select(), {{1}})
+ end)
+end
+
+local g_replication = t.group('gh_5616_temp_space_truncate_ro.replication')
+
+g_replication.before_all(function(cg)
+ cg.master = server:new({alias = 'master'})
+ cg.master:start()
+ cg.replica = server:new({
+ alias = 'replica',
+ box_cfg = {
+ read_only = true,
+ replication = cg.master.net_box_uri,
+ },
+ })
+ cg.replica:start()
+end)
+
+g_replication.after_all(function(cg)
+ cg.replica:drop()
+ cg.master:drop()
+end)
+
+g_replication.after_each(function(cg)
+ cg.master:exec(function()
+ if box.space.test ~= nil then
+ box.space.test:drop()
+ end
+ end)
+ cg.replica:wait_for_vclock_of(cg.master)
+end)
+
+-- Checks that a truncate operation for a temporary space isn't replicated to
+-- a read-only replica.
+g_replication.test_temp_space_truncate_ro = function(cg)
+ cg.master:exec(function()
+ local s = box.schema.create_space('test', {temporary = true})
+ s:create_index('primary')
+ s:insert({1})
+ end)
+ cg.replica:wait_for_vclock_of(cg.master)
+ cg.replica:exec(function()
+ local s = box.space.test
+ t.assert_equals(s:select(), {})
+ s:insert({2})
+ end)
+ cg.master:exec(function()
+ local s = box.space.test
+ s:truncate()
+ t.assert_equals(s:select(), {})
+ end)
+ cg.replica:wait_for_vclock_of(cg.master)
+ cg.replica:exec(function()
+ local s = box.space.test
+ t.assert_equals(s:select(), {{2}})
+ end)
+end
+
+-- Checks that a truncate operation for a local space isn't replicated to
+-- a read-only replica.
+g_replication.test_local_space_truncate_ro = function(cg)
+ cg.master:exec(function()
+ local s = box.schema.create_space('test', {is_local = true})
+ s:create_index('primary')
+ s:insert({1})
+ end)
+ cg.replica:wait_for_vclock_of(cg.master)
+ cg.replica:exec(function()
+ local s = box.space.test
+ t.assert_equals(s:select(), {})
+ s:insert({2})
+ end)
+ cg.master:exec(function()
+ local s = box.space.test
+ s:truncate()
+ t.assert_equals(s:select(), {})
+ end)
+ cg.replica:wait_for_vclock_of(cg.master)
+ cg.replica:exec(function()
+ local s = box.space.test
+ t.assert_equals(s:select(), {{2}})
+ end)
+end
+
+-- Checks that a truncate operation for a persistent space is replicated to
+-- a read-only replica.
+g_replication.test_persistent_space_truncate_ro = function(cg)
+ cg.master:exec(function()
+ local s = box.schema.create_space('test')
+ s:create_index('primary')
+ s:insert({1})
+ end)
+ cg.replica:wait_for_vclock_of(cg.master)
+ cg.replica:exec(function()
+ local s = box.space.test
+ t.assert_equals(s:select(), {{1}})
+ end)
+ cg.master:exec(function()
+ local s = box.space.test
+ s:truncate()
+ t.assert_equals(s:select(), {})
+ end)
+ cg.replica:wait_for_vclock_of(cg.master)
+ cg.replica:exec(function()
+ local s = box.space.test
+ t.assert_equals(s:select(), {})
+ end)
+end
diff --git a/test/replication/anon.result b/test/replication/anon.result
index 4dfadd4005..68e629f61b 100644
--- a/test/replication/anon.result
+++ b/test/replication/anon.result
@@ -143,10 +143,6 @@ box.space.loc:drop()
| ---
| - error: Can't modify data on a read-only instance - box.cfg.read_only is true
| ...
-box.space.loc:truncate()
- | ---
- | - error: Can't modify data on a read-only instance - box.cfg.read_only is true
- | ...
test_run:cmd('switch default')
| ---
diff --git a/test/replication/anon.test.lua b/test/replication/anon.test.lua
index c0b988eef9..97b2e7d67f 100644
--- a/test/replication/anon.test.lua
+++ b/test/replication/anon.test.lua
@@ -47,7 +47,6 @@ box.cfg{read_only=false}
box.space.test:insert{2}
box.space.loc:drop()
-box.space.loc:truncate()
test_run:cmd('switch default')
--
GitLab