From 8ac478986883b2a72093fca42533a10427543966 Mon Sep 17 00:00:00 2001
From: mechanik20051988 <mechanik20051988@tarantool.org>
Date: Fri, 15 Jan 2021 19:07:56 +0300
Subject: [PATCH] memtx: fix test for gh5304 issue and
memtx_space_is_recovering function
In previous version of patch we compared memtx state with
MEMTX_FINAL_RECOVERY to check that memtx recovery completed.
This is not quite true, memtx_state == MEMTX_FINAL_RECOVERY
means that the recovery from snapshot is finished, but recovery
from wals not. We need to compare memtx_state with MEMTX_OK
to check that recovery totally finished.
In previous test version on_replace trigger (created on
_user space) is never called. It's because is_recovery_finished()
always returns false: on_schema_init is invoked BEFORE
user's data recovery process (so trigger is not created at all
at this moment).
In new test version you can see correct user case:
we create on_replace trigger on _index system space,
which replaces/inserts/updates tuples in temp and loc spaces.
So each time user creates new space and index for it,
trigger replaces/inserts/updates tuples in temp and loc spaces.
Because trigger replaces/inserts/updates tuple with same
primary key, we get error when insert trigger called.
Follow-up #5304
---
.../unreleased/fix-is-recovering-function | 4 +
src/box/lua/ctl.c | 2 +-
src/box/memtx_space.h | 2 +-
test/box/gh-5304-insert_during_recovery.lua | 10 +-
.../box/gh-5304-insert_during_recovery.result | 108 ++++++++++++++++++
.../gh-5304-insert_during_recovery.test.lua | 35 ++++++
6 files changed, 155 insertions(+), 6 deletions(-)
create mode 100644 changelogs/unreleased/fix-is-recovering-function
diff --git a/changelogs/unreleased/fix-is-recovering-function b/changelogs/unreleased/fix-is-recovering-function
new file mode 100644
index 0000000000..ff2117d588
--- /dev/null
+++ b/changelogs/unreleased/fix-is-recovering-function
@@ -0,0 +1,4 @@
+## bugfix/core
+
+* Fix lbox_ctl_is_recovery_finished(): in some cases it might return true
+ even if recovery was still in the progress.
diff --git a/src/box/lua/ctl.c b/src/box/lua/ctl.c
index b44474b124..b81199662b 100644
--- a/src/box/lua/ctl.c
+++ b/src/box/lua/ctl.c
@@ -94,7 +94,7 @@ lbox_ctl_is_recovery_finished(struct lua_State *L)
struct memtx_engine *memtx;
memtx = (struct memtx_engine *)engine_by_name("memtx");
lua_pushboolean(L, (memtx ?
- (memtx->state < MEMTX_FINAL_RECOVERY ? 0 : 1) : 0));
+ (memtx->state < MEMTX_OK ? 0 : 1) : 0));
return 1;
}
diff --git a/src/box/memtx_space.h b/src/box/memtx_space.h
index 68e6264adb..a13dae7145 100644
--- a/src/box/memtx_space.h
+++ b/src/box/memtx_space.h
@@ -93,7 +93,7 @@ memtx_space_is_recovering(struct space *space)
{
assert(space_is_memtx(space));
struct memtx_engine *memtx = (struct memtx_engine *)space->engine;
- return memtx->state < MEMTX_FINAL_RECOVERY;
+ return memtx->state < MEMTX_OK;
}
#if defined(__cplusplus)
diff --git a/test/box/gh-5304-insert_during_recovery.lua b/test/box/gh-5304-insert_during_recovery.lua
index c8b6c5cfb7..2d84a95e30 100644
--- a/test/box/gh-5304-insert_during_recovery.lua
+++ b/test/box/gh-5304-insert_during_recovery.lua
@@ -32,13 +32,15 @@ end
if arg[2] == 'is_recovery_finished' then
box.ctl.on_schema_init(function()
- if box.ctl.is_recovery_finished() then
- box.space._user:on_replace(trigger)
- end
+ box.space._index:on_replace(function(old_space, new_space)
+ if box.ctl.is_recovery_finished() then
+ trigger(old_space, new_space)
+ end
+ end)
end)
else
box.ctl.on_schema_init(function()
- box.space._user:on_replace(trigger)
+ box.space._index:on_replace(trigger)
end)
end
diff --git a/test/box/gh-5304-insert_during_recovery.result b/test/box/gh-5304-insert_during_recovery.result
index 19823c71fd..deec6ee04d 100644
--- a/test/box/gh-5304-insert_during_recovery.result
+++ b/test/box/gh-5304-insert_during_recovery.result
@@ -61,6 +61,42 @@ test_run:cmd('start server test with args="replace is_recovery_finished"')
| ---
| - true
| ...
+test_run:cmd('switch test')
+ | ---
+ | - true
+ | ...
+box.space.temp:select()
+ | ---
+ | - []
+ | ...
+box.space.loc:select()
+ | ---
+ | - []
+ | ...
+-- Creating a new space and index invokes on_replace trigger in _index space,
+-- which inserts tuples in temp and loc spaces (see gh-5304-insert_during_recovery.lua)!
+s0 = box.schema.space.create('test')
+ | ---
+ | ...
+i0 = s0:create_index('test')
+ | ---
+ | ...
+box.space.temp:select()
+ | ---
+ | - - [1]
+ | ...
+box.space.loc:select()
+ | ---
+ | - - [1]
+ | ...
+s0:drop()
+ | ---
+ | ...
+
+test_run:cmd('switch default')
+ | ---
+ | - true
+ | ...
test_run:cmd('stop server test')
| ---
| - true
@@ -69,6 +105,44 @@ test_run:cmd('start server test with args="insert is_recovery_finished"')
| ---
| - true
| ...
+test_run:cmd('switch test')
+ | ---
+ | - true
+ | ...
+box.space.temp:select()
+ | ---
+ | - []
+ | ...
+box.space.loc:select()
+ | ---
+ | - - [1]
+ | ...
+-- Creating a new space and index invoke on_replace trigger in _index space.
+-- Here we get error during insert tuple in loc space, because the tuple
+-- with the same primary key is already in the loc space
+s0 = box.schema.space.create('test')
+ | ---
+ | ...
+i0 = s0:create_index('test')
+ | ---
+ | - error: Duplicate key exists in unique index 'test' in space 'loc'
+ | ...
+box.space.temp:select()
+ | ---
+ | - []
+ | ...
+box.space.loc:select()
+ | ---
+ | - - [1]
+ | ...
+s0:drop()
+ | ---
+ | ...
+
+test_run:cmd('switch default')
+ | ---
+ | - true
+ | ...
test_run:cmd('stop server test')
| ---
| - true
@@ -77,6 +151,40 @@ test_run:cmd('start server test with args="upsert is_recovery_finished"')
| ---
| - true
| ...
+test_run:cmd('switch test')
+ | ---
+ | - true
+ | ...
+box.space.temp:select()
+ | ---
+ | - []
+ | ...
+box.space.loc:select()
+ | ---
+ | - - [1]
+ | ...
+s0 = box.schema.space.create('test')
+ | ---
+ | ...
+i0 = s0:create_index('test')
+ | ---
+ | ...
+box.space.temp:select()
+ | ---
+ | - - [1]
+ | ...
+box.space.loc:select()
+ | ---
+ | - - [1]
+ | ...
+s0:drop()
+ | ---
+ | ...
+
+test_run:cmd('switch default')
+ | ---
+ | - true
+ | ...
test_run:cmd('stop server test')
| ---
| - true
diff --git a/test/box/gh-5304-insert_during_recovery.test.lua b/test/box/gh-5304-insert_during_recovery.test.lua
index 97171e9641..2846499c7f 100644
--- a/test/box/gh-5304-insert_during_recovery.test.lua
+++ b/test/box/gh-5304-insert_during_recovery.test.lua
@@ -18,10 +18,45 @@ test_run:cmd('start server test with args="replace" with crash_expected=True')
test_run:cmd('start server test with args="insert" with crash_expected=True')
test_run:cmd('start server test with args="upsert" with crash_expected=True')
test_run:cmd('start server test with args="replace is_recovery_finished"')
+test_run:cmd('switch test')
+box.space.temp:select()
+box.space.loc:select()
+-- Creating a new space and index invokes on_replace trigger in _index space,
+-- which inserts tuples in temp and loc spaces (see gh-5304-insert_during_recovery.lua)!
+s0 = box.schema.space.create('test')
+i0 = s0:create_index('test')
+box.space.temp:select()
+box.space.loc:select()
+s0:drop()
+
+test_run:cmd('switch default')
test_run:cmd('stop server test')
test_run:cmd('start server test with args="insert is_recovery_finished"')
+test_run:cmd('switch test')
+box.space.temp:select()
+box.space.loc:select()
+-- Creating a new space and index invoke on_replace trigger in _index space.
+-- Here we get error during insert tuple in loc space, because the tuple
+-- with the same primary key is already in the loc space
+s0 = box.schema.space.create('test')
+i0 = s0:create_index('test')
+box.space.temp:select()
+box.space.loc:select()
+s0:drop()
+
+test_run:cmd('switch default')
test_run:cmd('stop server test')
test_run:cmd('start server test with args="upsert is_recovery_finished"')
+test_run:cmd('switch test')
+box.space.temp:select()
+box.space.loc:select()
+s0 = box.schema.space.create('test')
+i0 = s0:create_index('test')
+box.space.temp:select()
+box.space.loc:select()
+s0:drop()
+
+test_run:cmd('switch default')
test_run:cmd('stop server test')
test_run:cmd('cleanup server test')
test_run:cmd('delete server test')
--
GitLab