[PATCH 2/3] vinyl: don't produce deferred DELETE on commit if key isn't updated
Vladimir Davydov
vdavydov.dev at gmail.com
Sat May 25 00:53:41 MSK 2019
Consider the following example:
s = box.schema.space.create('test', {engine = 'vinyl'})
s:create_index('primary')
s:create_index('secondary', {parts = {2, 'unsigned'}})
s:insert{1, 1, 1}
s:replace{1, 1, 2}
When REPLACE{1,1} is committed to the secondary index, the overwritten
tuple, i.e. INSERT{1,1}, is found in the primary index memory, and so
deferred DELETE{1,1} is generated right away and committed along with
REPLACE{1,1}. However, there's no need to commit anything to the
secondary index in this case, because its key isn't updated. Apart from
eating memory and loading disk, this also breaks index stats, as vy_tx
implementation doesn't expect two statements committed for the same key
in a single transaction.
Fix this by checking if there's a statement in the log for the deleted
key and if there's skipping them both as we do in the regular case, see
the comment in vy_tx_set.
Closes #3693
---
src/box/vy_tx.c | 18 +++++++++
test/vinyl/deferred_delete.result | 78 +++++++++++++++++++++++++++++++++++++
test/vinyl/deferred_delete.test.lua | 26 +++++++++++++
test/vinyl/quota.result | 2 +-
test/vinyl/stat.result | 6 +--
test/vinyl/stat.test.lua | 2 +-
test/vinyl/write_iterator.result | 2 +-
test/vinyl/write_iterator.test.lua | 2 +-
8 files changed, 129 insertions(+), 7 deletions(-)
diff --git a/src/box/vy_tx.c b/src/box/vy_tx.c
index aedd0610..119d975b 100644
--- a/src/box/vy_tx.c
+++ b/src/box/vy_tx.c
@@ -629,6 +629,23 @@ vy_tx_handle_deferred_delete(struct vy_tx *tx, struct txv *v)
struct vy_lsm *lsm = vy_lsm(space->index[i]);
struct vy_entry entry;
vy_stmt_foreach_entry(entry, delete_stmt, lsm->cmp_def) {
+ struct txv *other = write_set_search_key(&tx->write_set,
+ lsm, entry);
+ if (other != NULL && !other->is_overwritten) {
+ /*
+ * The write set contains a statement
+ * for the key to be deleted. This can
+ * only occur if it's a REPLACE that
+ * happens not to update the secondary
+ * index key parts. It's safe to skip it,
+ * see vy_tx_set_entry().
+ */
+ assert(vy_stmt_type(stmt) == IPROTO_REPLACE);
+ assert(vy_stmt_type(other->entry.stmt) ==
+ IPROTO_REPLACE);
+ other->is_nop = true;
+ continue;
+ }
struct txv *delete_txv = txv_new(tx, lsm, entry);
if (delete_txv == NULL) {
rc = -1;
@@ -717,6 +734,7 @@ vy_tx_prepare(struct vy_tx *tx)
* to the primary index LSM tree. So we must
* skip it for secondary indexes as well.
*/
+ v->is_overwritten = true;
continue;
}
diff --git a/test/vinyl/deferred_delete.result b/test/vinyl/deferred_delete.result
index 422f2fe0..3f187a5b 100644
--- a/test/vinyl/deferred_delete.result
+++ b/test/vinyl/deferred_delete.result
@@ -726,6 +726,84 @@ sk:select() -- ditto
s:drop()
---
...
+--
+-- gh-3693 If a REPLACE doesn't update secondary index key parts,
+-- we must not generate a deferred DELETE for it on commit.
+-- Moreover, we must skip it as it has no effect.
+--
+s = box.schema.space.create('test', {engine = 'vinyl'})
+---
+...
+pk = s:create_index('pk')
+---
+...
+sk = s:create_index('sk', {parts = {2, 'unsigned'}})
+---
+...
+s:insert{1, 1, 1}
+---
+- [1, 1, 1]
+...
+s:insert{2, 2, 2}
+---
+- [2, 2, 2]
+...
+s:replace{2, 2, 3}
+---
+- [2, 2, 3]
+...
+pk:stat().rows -- 3: INSERT{1,1,1} + INSERT{2,2,2} + REPLACE{2,2,3}
+---
+- 3
+...
+sk:stat().rows -- 2: INSERT{1,1} + INSERT{2,2}
+---
+- 2
+...
+box.begin()
+---
+...
+s:replace{1, 1, 2}
+---
+- [1, 1, 2]
+...
+s:delete{1}
+---
+...
+box.commit()
+---
+...
+pk:stat().rows -- 4: INSERT{1,1,1} + INSERT{2,2,2} + REPLACE{2,2,3} + DELETE{1}
+---
+- 4
+...
+sk:stat().rows -- 3: INSERT{1,1} + INSERT{2,2} + DELETE{1,1}
+---
+- 3
+...
+box.snapshot()
+---
+- ok
+...
+pk:stat().rows -- 1: INSERT{2,2,3}
+---
+- 1
+...
+sk:stat().rows -- 1: INSERT{2,2}
+---
+- 1
+...
+pk:select()
+---
+- - [2, 2, 3]
+...
+sk:select()
+---
+- - [2, 2, 3]
+...
+s:drop()
+---
+...
box.cfg{vinyl_cache = vinyl_cache}
---
...
diff --git a/test/vinyl/deferred_delete.test.lua b/test/vinyl/deferred_delete.test.lua
index 6dfcbde5..689c8f93 100644
--- a/test/vinyl/deferred_delete.test.lua
+++ b/test/vinyl/deferred_delete.test.lua
@@ -265,6 +265,32 @@ box.snapshot()
sk:select() -- ditto
s:drop()
+--
+-- gh-3693 If a REPLACE doesn't update secondary index key parts,
+-- we must not generate a deferred DELETE for it on commit.
+-- Moreover, we must skip it as it has no effect.
+--
+s = box.schema.space.create('test', {engine = 'vinyl'})
+pk = s:create_index('pk')
+sk = s:create_index('sk', {parts = {2, 'unsigned'}})
+s:insert{1, 1, 1}
+s:insert{2, 2, 2}
+s:replace{2, 2, 3}
+pk:stat().rows -- 3: INSERT{1,1,1} + INSERT{2,2,2} + REPLACE{2,2,3}
+sk:stat().rows -- 2: INSERT{1,1} + INSERT{2,2}
+box.begin()
+s:replace{1, 1, 2}
+s:delete{1}
+box.commit()
+pk:stat().rows -- 4: INSERT{1,1,1} + INSERT{2,2,2} + REPLACE{2,2,3} + DELETE{1}
+sk:stat().rows -- 3: INSERT{1,1} + INSERT{2,2} + DELETE{1,1}
+box.snapshot()
+pk:stat().rows -- 1: INSERT{2,2,3}
+sk:stat().rows -- 1: INSERT{2,2}
+pk:select()
+sk:select()
+s:drop()
+
box.cfg{vinyl_cache = vinyl_cache}
--
diff --git a/test/vinyl/quota.result b/test/vinyl/quota.result
index 1a0842f9..d1b28ee5 100644
--- a/test/vinyl/quota.result
+++ b/test/vinyl/quota.result
@@ -89,7 +89,7 @@ _ = space:replace{1, 1, string.rep('a', 1024 * 1024 * 5)}
...
box.stat.vinyl().memory.level0
---
-- 5341267
+- 5292076
...
space:drop()
---
diff --git a/test/vinyl/stat.result b/test/vinyl/stat.result
index aa47a7be..e2991d53 100644
--- a/test/vinyl/stat.result
+++ b/test/vinyl/stat.result
@@ -1158,7 +1158,7 @@ s = box.schema.space.create('test', {engine = 'vinyl'})
i1 = s:create_index('i1', {parts = {1, 'unsigned'}})
---
...
-i2 = s:create_index('i2', {parts = {2, 'unsigned'}})
+i2 = s:create_index('i2', {parts = {2, 'unsigned'}, unique = false})
---
...
for i = 1, 100 do s:replace{i, i, string.rep('x', 1000)} end
@@ -1191,7 +1191,7 @@ box.snapshot()
stat_diff(gstat(), st, 'scheduler')
---
- dump_input: 10420
- dump_output: 10371
+ dump_output: 10411
tasks_completed: 2
dump_count: 1
...
@@ -1206,7 +1206,7 @@ while i1:stat().disk.compaction.count == 0 do fiber.sleep(0.01) end
...
stat_diff(gstat(), st, 'scheduler')
---
-- compaction_input: 112188
+- compaction_input: 112228
tasks_completed: 1
compaction_output: 101984
...
diff --git a/test/vinyl/stat.test.lua b/test/vinyl/stat.test.lua
index 4a360f33..4c751aad 100644
--- a/test/vinyl/stat.test.lua
+++ b/test/vinyl/stat.test.lua
@@ -333,7 +333,7 @@ s:drop()
-- sched stats
s = box.schema.space.create('test', {engine = 'vinyl'})
i1 = s:create_index('i1', {parts = {1, 'unsigned'}})
-i2 = s:create_index('i2', {parts = {2, 'unsigned'}})
+i2 = s:create_index('i2', {parts = {2, 'unsigned'}, unique = false})
for i = 1, 100 do s:replace{i, i, string.rep('x', 1000)} end
st = gstat()
diff --git a/test/vinyl/write_iterator.result b/test/vinyl/write_iterator.result
index 5c5200b8..f0244113 100644
--- a/test/vinyl/write_iterator.result
+++ b/test/vinyl/write_iterator.result
@@ -931,7 +931,7 @@ box.snapshot()
- ok
...
-- Some padding to trigger minor compaction.
-for i = 1001, 1000 + PAD2 do s:replace{i, i} end
+for i = 1001, 1000 + PAD2 do s:delete{i} end
---
...
box.snapshot()
diff --git a/test/vinyl/write_iterator.test.lua b/test/vinyl/write_iterator.test.lua
index fa44961a..0de21f21 100644
--- a/test/vinyl/write_iterator.test.lua
+++ b/test/vinyl/write_iterator.test.lua
@@ -389,7 +389,7 @@ _ = s:insert{7, 7}
_ = s:insert{8, 8}
box.snapshot()
-- Some padding to trigger minor compaction.
-for i = 1001, 1000 + PAD2 do s:replace{i, i} end
+for i = 1001, 1000 + PAD2 do s:delete{i} end
box.snapshot()
-- Generate DELETE+INSERT statements and write them to disk.
s:delete{1} s:insert{1, 100}
--
2.11.0
More information about the Tarantool-patches
mailing list