Skip to content

Commit

Permalink
box: allow to truncate temp and local spaces in ro mode
Browse files Browse the repository at this point in the history
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 tarantool#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#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
  • Loading branch information
locker committed Jul 14, 2023
1 parent 0e5a3cc commit 054526a
Show file tree
Hide file tree
Showing 7 changed files with 204 additions and 18 deletions.
4 changes: 4 additions & 0 deletions changelogs/unreleased/gh-5616-temp-space-truncate-ro.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
## feature/box

* Allowed truncation of temporary and local spaces in the read-only mode
(gh-5616).
27 changes: 17 additions & 10 deletions src/box/alter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2624,6 +2624,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)
Expand All @@ -2632,6 +2636,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
Expand All @@ -2640,14 +2656,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
Expand All @@ -2673,8 +2681,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
Expand Down
12 changes: 9 additions & 3 deletions src/box/box.cc
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ box_check_slice_slow(void)
return fiber_check_slice();
}

static int
int
box_check_writable(void)
{
if (!is_ro_summary)
Expand Down Expand Up @@ -3540,11 +3540,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;
Expand Down
7 changes: 7 additions & 0 deletions src/box/box.h
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,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);

Expand Down
167 changes: 167 additions & 0 deletions test/box-luatest/gh_5616_temp_space_truncate_ro_test.lua
Original file line number Diff line number Diff line change
@@ -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
4 changes: 0 additions & 4 deletions test/replication/anon.result
Original file line number Diff line number Diff line change
Expand Up @@ -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')
| ---
Expand Down
1 change: 0 additions & 1 deletion test/replication/anon.test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand Down

0 comments on commit 054526a

Please sign in to comment.