From: Vladimir Davydov <vdavydov.dev@gmail.com> To: tarantool-patches@freelists.org Subject: [PATCH] vinyl: fix assertion failure in vy_tx_handle_deferred_delete Date: Mon, 17 Jun 2019 18:30:41 +0300 [thread overview] Message-ID: <a9d71e1b4d2a4613fe609f3fb4a0b6ba37f490fb.1560785297.git.vdavydov.dev@gmail.com> (raw) vy_tx_handle_deferred_delete() expects (righteously) that in case a deferred DELETE overwrites a statement in a secondary index write set and the overwritten statement wasn't skipped on commit (i.e. doesn't have txv->is_overwritten flag set), both the old and the new statement must be REPLACEs (see the comment to the corresponding assertions for more details). The problem is we don't set is_overwritten flag in case a statement doesn't have any effect (txv->is_nop is set), even if it was, in fact, overwritten in the primary index write set (see vy_tx_prepare). As a result, we get an assertion failure when we delete such statement in the same transaction, e.g. s = box.schema.space.create('test', {engine = 'vinyl'}) s:create_index('pk', {parts = {1, 'unsigned'}}) s:create_index('sk', {parts = {2, 'unsigned'}}) s:replace{1, 1, 1} box.begin() s:update(1, {{'+', 3, 1}}) -- adds no-op REPLACE to sk write set s:delete(1) -- produces deferred DELETE for sk box.commit() results in vy_tx_handle_deferred_delete: Assertion `vy_stmt_type(stmt) == IPROTO_REPLACE' failed. Fix this by making sure we set is_overwritten for all overwritten statements in a secondary index write set. Closes #4294 --- https://github.com/tarantool/tarantool/issues/4294 https://github.com/tarantool/tarantool/commits/dv/gh-4294-vy-assert-failure-fix src/box/vy_tx.c | 9 ++++----- test/vinyl/gh.result | 37 +++++++++++++++++++++++++++++++++++++ test/vinyl/gh.test.lua | 15 +++++++++++++++ 3 files changed, 56 insertions(+), 5 deletions(-) diff --git a/src/box/vy_tx.c b/src/box/vy_tx.c index 76d184fb..56b9f292 100644 --- a/src/box/vy_tx.c +++ b/src/box/vy_tx.c @@ -717,10 +717,6 @@ vy_tx_prepare(struct vy_tx *tx) } assert(lsm->space_id == current_space_id); - /* Do not save statements that was overwritten by the same tx */ - if (v->is_overwritten || v->is_nop) - continue; - if (lsm->index_id > 0 && repsert == NULL && delete == NULL) { /* * This statement is for a secondary index, @@ -735,9 +731,12 @@ vy_tx_prepare(struct vy_tx *tx) * skip it for secondary indexes as well. */ v->is_overwritten = true; - continue; } + /* Do not save statements that was overwritten by the same tx */ + if (v->is_overwritten || v->is_nop) + continue; + enum iproto_type type = vy_stmt_type(v->entry.stmt); /* Optimize out INSERT + DELETE for the same key. */ diff --git a/test/vinyl/gh.result b/test/vinyl/gh.result index 76beab09..78cb2a28 100644 --- a/test/vinyl/gh.result +++ b/test/vinyl/gh.result @@ -718,3 +718,40 @@ while finished ~= 2 do fiber.sleep(0.01) end s:drop() --- ... +-- +-- gh-4294: assertion failure when deleting a no-op statement from +-- a secondary index write set. +-- +s = box.schema.space.create('test', {engine = 'vinyl'}) +--- +... +_ = s:create_index('pk') +--- +... +_ = s:create_index('sk', {parts = {2, 'unsigned'}}) +--- +... +s:replace{1, 1, 1} +--- +- [1, 1, 1] +... +box.begin() +--- +... +s:update(1, {{'+', 3, 1}}) +--- +- [1, 1, 2] +... +s:delete(1) +--- +... +box.commit() +--- +... +s:select() +--- +- [] +... +s:drop() +--- +... diff --git a/test/vinyl/gh.test.lua b/test/vinyl/gh.test.lua index 2044f6cc..9ad85798 100644 --- a/test/vinyl/gh.test.lua +++ b/test/vinyl/gh.test.lua @@ -312,3 +312,18 @@ cont = false while finished ~= 2 do fiber.sleep(0.01) end s:drop() + +-- +-- gh-4294: assertion failure when deleting a no-op statement from +-- a secondary index write set. +-- +s = box.schema.space.create('test', {engine = 'vinyl'}) +_ = s:create_index('pk') +_ = s:create_index('sk', {parts = {2, 'unsigned'}}) +s:replace{1, 1, 1} +box.begin() +s:update(1, {{'+', 3, 1}}) +s:delete(1) +box.commit() +s:select() +s:drop() -- 2.11.0
next reply other threads:[~2019-06-17 15:30 UTC|newest] Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-06-17 15:30 Vladimir Davydov [this message] 2019-06-18 5:45 ` [tarantool-patches] " Konstantin Osipov 2019-06-18 13:45 ` Kirill Yukhin
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=a9d71e1b4d2a4613fe609f3fb4a0b6ba37f490fb.1560785297.git.vdavydov.dev@gmail.com \ --to=vdavydov.dev@gmail.com \ --cc=tarantool-patches@freelists.org \ --subject='Re: [PATCH] vinyl: fix assertion failure in vy_tx_handle_deferred_delete' \ /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