[Tarantool-patches] [PATCH] vinyl: fix assert in vy_tx_write()

Nikita Pettik korablev at tarantool.org
Thu May 21 04:15:31 MSK 2020


Assert in vy_tx_write() validates that upsert applied on the top of
cached statement is always replace. In fact, it is not always so. If
vy_apply_upsert() fails due to the fact that PK is modified, it returns
old statement (i.e. statement which upsert is applied on). In this
regard, if tuple cache contains insert and invalid upsert is executed,
vy_apply_upsert() will return insert. As a result, assert will fire.
Let's fix it and take into account that vy_apply_upsert() is able to
return inserts as well.

Closes #5005
---
Branch: https://github.com/tarantool/tarantool/tree/np/gh-5005-upsert-changing-pk-crash
Issue: https://github.com/tarantool/tarantool/issues/5005

@ChangeLog:
 * Fix crash in debug build due to invalid upsert applied on
cached INSERT statement.

 src/box/vy_tx.c                               |  7 +++--
 test/vinyl/gh-5005-upsert-affecting-pk.result | 30 +++++++++++++++++++
 .../gh-5005-upsert-affecting-pk.test.lua      | 12 ++++++++
 3 files changed, 47 insertions(+), 2 deletions(-)
 create mode 100644 test/vinyl/gh-5005-upsert-affecting-pk.result
 create mode 100644 test/vinyl/gh-5005-upsert-affecting-pk.test.lua

diff --git a/src/box/vy_tx.c b/src/box/vy_tx.c
index d092e0cdb..846c63294 100644
--- a/src/box/vy_tx.c
+++ b/src/box/vy_tx.c
@@ -506,8 +506,11 @@ vy_tx_write(struct vy_lsm *lsm, struct vy_mem *mem,
 							mem->cmp_def, false);
 			tuple_unref(deleted.stmt);
 			if (applied.stmt != NULL) {
-				assert(vy_stmt_type(applied.stmt) ==
-							IPROTO_REPLACE);
+				enum iproto_type applied_type =
+					vy_stmt_type(applied.stmt);
+				assert(applied_type == IPROTO_REPLACE ||
+				       applied_type == IPROTO_INSERT);
+				(void) applied_type;
 				int rc = vy_lsm_set(lsm, mem, applied,
 						    region_stmt);
 				tuple_unref(applied.stmt);
diff --git a/test/vinyl/gh-5005-upsert-affecting-pk.result b/test/vinyl/gh-5005-upsert-affecting-pk.result
new file mode 100644
index 000000000..e2fea3501
--- /dev/null
+++ b/test/vinyl/gh-5005-upsert-affecting-pk.result
@@ -0,0 +1,30 @@
+-- test-run result file version 2
+-- Make sure that applying invalid upsert (which modifies PK)
+-- after read (so that key populates tuple cache) does not result
+-- in crash.
+--
+s = box.schema.create_space('test', {engine = 'vinyl'})
+ | ---
+ | ...
+pk = s:create_index('pk')
+ | ---
+ | ...
+s:insert{1, 1}
+ | ---
+ | - [1, 1]
+ | ...
+s:select()
+ | ---
+ | - - [1, 1]
+ | ...
+s:upsert({1}, {{'+', 1, 1}})
+ | ---
+ | ...
+s:select()
+ | ---
+ | - - [1, 1]
+ | ...
+
+s:drop()
+ | ---
+ | ...
diff --git a/test/vinyl/gh-5005-upsert-affecting-pk.test.lua b/test/vinyl/gh-5005-upsert-affecting-pk.test.lua
new file mode 100644
index 000000000..23dc46ecd
--- /dev/null
+++ b/test/vinyl/gh-5005-upsert-affecting-pk.test.lua
@@ -0,0 +1,12 @@
+-- Make sure that applying invalid upsert (which modifies PK)
+-- after read (so that key populates tuple cache) does not result
+-- in crash.
+--
+s = box.schema.create_space('test', {engine = 'vinyl'})
+pk = s:create_index('pk')
+s:insert{1, 1}
+s:select()
+s:upsert({1}, {{'+', 1, 1}})
+s:select()
+
+s:drop()
-- 
2.17.1



More information about the Tarantool-patches mailing list