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

changelogs/unreleased/fix-is-recovering-function

+## bugfix/core
+
+* Fix lbox_ctl_is_recovery_finished(): in some cases it might return true
+  even if recovery was still in the progress.

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;
 }
 

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)

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
 

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

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')