Tarantool development patches archive
 help / color / mirror / Atom feed
From: Nikita Pettik <korablev@tarantool.org>
To: tarantool-patches@dev.tarantool.org
Cc: v.shpilevoy@tarantool.org
Subject: [Tarantool-patches] [PATCH] vinyl: fix assert in vy_tx_write()
Date: Thu, 21 May 2020 04:15:31 +0300	[thread overview]
Message-ID: <ca9d9c60b28e35db146541bf5e50c7c04c2ba3ad.1590022209.git.korablev@tarantool.org> (raw)

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

             reply	other threads:[~2020-05-21  1:15 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-21  1:15 Nikita Pettik [this message]
2020-05-21 14:12 ` Vladislav Shpilevoy
2020-05-22  3:04   ` Nikita Pettik
2020-05-26 21:44 ` Vladislav Shpilevoy
2020-05-27 12:51   ` Nikita Pettik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ca9d9c60b28e35db146541bf5e50c7c04c2ba3ad.1590022209.git.korablev@tarantool.org \
    --to=korablev@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH] vinyl: fix assert in vy_tx_write()' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox