[Tarantool-patches] [PATCH] memtx: fix tuples references on concurrent replaces

[Ilya Kosarev]

Since 527b02a2ee6a9a205d8e2c8f38bbb84edf0d6557 (memtx: add yields
during index build) memtx_build_on_replace was introduced to handle
concurrent updates. The problem here was that the tuples being handled
with this trigger did not get reference counter promotion, leading to a
number of wrong behavior cases. Now this problem is solved.
This problem was found through primary index altering with updates in
background fiber. Corresponding test is introduced.

Closes #4973
---
Branch: https://github.com/tarantool/tarantool/tree/i.kosarev/gh-4973-segfaults-on-alter
Issue: https://github.com/tarantool/tarantool/issues/4973

@ChangeLog:
 * Fix concurrent replaces on index building. Tuples are now referenced
 on all needed execution paths.

 src/box/memtx_space.c                         |  8 ++-
 test/engine/engine.cfg                        |  5 +-
 ...-4973-segfaults-on-concurrent-alter.result | 72 +++++++++++++++++++
 ...973-segfaults-on-concurrent-alter.test.lua | 30 ++++++++
 4 files changed, 113 insertions(+), 2 deletions(-)
 create mode 100644 test/engine/gh-4973-segfaults-on-concurrent-alter.result
 create mode 100644 test/engine/gh-4973-segfaults-on-concurrent-alter.test.lua

diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
index 09f0de4cec..a1ec9d03ee 100644
--- a/src/box/memtx_space.c
+++ b/src/box/memtx_space.c
@@ -975,10 +975,16 @@ memtx_build_on_replace(struct trigger *trigger, void *event)
 						    DUP_REPLACE_OR_INSERT;
 	state->rc = index_replace(state->index, stmt->old_tuple,
 				  stmt->new_tuple, mode, &delete);
-
 	if (state->rc != 0) {
 		diag_move(diag_get(), &state->diag);
+		return 0;
 	}
+	/*
+	 * All tuples stored in a memtx space must be
+	 * referenced by the primary index.
+	 */
+	if (state->index->def->iid == 0)
+		tuple_ref(stmt->new_tuple);
 	return 0;
 }
 
diff --git a/test/engine/engine.cfg b/test/engine/engine.cfg
index f1e7de2745..0b09f7c1d2 100644
--- a/test/engine/engine.cfg
+++ b/test/engine/engine.cfg
@@ -5,7 +5,10 @@
     },
     "func_index.test.lua": {
         "memtx": {"engine": "memtx"}
-     }
+    },
+    "gh-4973-segfaults-on-concurrent-alter.test.lua": {
+        "memtx": {"engine": "memtx"}
+    }
 }
 
 
diff --git a/test/engine/gh-4973-segfaults-on-concurrent-alter.result b/test/engine/gh-4973-segfaults-on-concurrent-alter.result
new file mode 100644
index 0000000000..e0f7eec718
--- /dev/null
+++ b/test/engine/gh-4973-segfaults-on-concurrent-alter.result
@@ -0,0 +1,72 @@
+-- test-run result file version 2
+test_run = require('test_run').new()
+ | ---
+ | ...
+errinj = box.error.injection
+ | ---
+ | ...
+
+space = box.schema.space.create('gh-4973-alter', {engine = 'memtx'})
+ | ---
+ | ...
+space:format({ {'key', 'unsigned'}, {'value', 'string'}, {'key_new', 'unsigned'} })
+ | ---
+ | ...
+index = space:create_index('primary', {parts = {'key'}})
+ | ---
+ | ...
+
+N = 10000
+ | ---
+ | ...
+value = string.rep('a', 10)
+ | ---
+ | ...
+box.atomic(function() for i = 1, N do space:insert({i, value, i}) end end)
+ | ---
+ | ...
+
+test_run:cmd("setopt delimiter ';'")
+ | ---
+ | - true
+ | ...
+function random_update()
+    local x = space.index.primary:random(math.random(N))
+    space:update({x[1]}, {{'=', 2, string.rep('b', 10)}})
+end;
+ | ---
+ | ...
+
+finished_updates = false;
+ | ---
+ | ...
+fiber = require('fiber');
+ | ---
+ | ...
+updater = fiber.create(function()
+    for _ = 1, N / 10 do random_update() end
+    finished_updates = true
+end)
+test_run:cmd("setopt delimiter ''");
+ | ---
+ | ...
+
+space.index.primary:alter({parts = {'key_new'}})
+ | ---
+ | ...
+errinj.set('ERRINJ_BUILD_INDEX_DELAY', true)
+ | ---
+ | - ok
+ | ...
+test_run:wait_cond(function() return finished_updates end, 2)
+ | ---
+ | - true
+ | ...
+errinj.set('ERRINJ_BUILD_INDEX_DELAY', false)
+ | ---
+ | - ok
+ | ...
+box.snapshot()
+ | ---
+ | - ok
+ | ...
diff --git a/test/engine/gh-4973-segfaults-on-concurrent-alter.test.lua b/test/engine/gh-4973-segfaults-on-concurrent-alter.test.lua
new file mode 100644
index 0000000000..ca26f540d3
--- /dev/null
+++ b/test/engine/gh-4973-segfaults-on-concurrent-alter.test.lua
@@ -0,0 +1,30 @@
+test_run = require('test_run').new()
+errinj = box.error.injection
+
+space = box.schema.space.create('gh-4973-alter', {engine = 'memtx'})
+space:format({ {'key', 'unsigned'}, {'value', 'string'}, {'key_new', 'unsigned'} })
+index = space:create_index('primary', {parts = {'key'}})
+
+N = 10000
+value = string.rep('a', 10)
+box.atomic(function() for i = 1, N do space:insert({i, value, i}) end end)
+
+test_run:cmd("setopt delimiter ';'")
+function random_update()
+    local x = space.index.primary:random(math.random(N))
+    space:update({x[1]}, {{'=', 2, string.rep('b', 10)}})
+end;
+
+finished_updates = false;
+fiber = require('fiber');
+updater = fiber.create(function()
+    for _ = 1, N / 10 do random_update() end
+    finished_updates = true
+end)
+test_run:cmd("setopt delimiter ''");
+
+space.index.primary:alter({parts = {'key_new'}})
+errinj.set('ERRINJ_BUILD_INDEX_DELAY', true)
+test_run:wait_cond(function() return finished_updates end, 2)
+errinj.set('ERRINJ_BUILD_INDEX_DELAY', false)
+box.snapshot()
-- 
2.17.1