Merge branch '1.9' into 1.10

src/box/vy_tx.c

@@ -846,6 +846,34 @@ vy_tx_set(struct vy_tx *tx, struct vy_lsm *lsm, struct tuple *stmt)
 						vy_stmt_column_mask(old->stmt));
 	}
 
+	if (lsm->index_id > 0 && vy_stmt_type(stmt) == IPROTO_REPLACE &&
+	    old != NULL && vy_stmt_type(old->stmt) == IPROTO_DELETE) {
+		/*
+		 * The column mask of an update operation may have a bit
+		 * set even if the corresponding field doesn't actually
+		 * get updated, because a column mask is generated
+		 * only from update operations, before applying them.
+		 * E.g. update(1, {{'+', 2, 0}}) doesn't modify the
+		 * second field, but the column mask will say it does.
+		 *
+		 * To discard DELETE statements in the write iterator
+		 * (see optimization #6), we turn a REPLACE into an
+		 * INSERT in case the REPLACE was generated by an
+		 * update that changed secondary key fields. So we
+		 * can't tolerate inaccuracy in a column mask.
+		 *
+		 * So if the update didn't actually modify secondary
+		 * key fields, i.e. DELETE and REPLACE generated by the
+		 * update have the same key fields, we forcefully clear
+		 * key bits in the column mask to ensure that no REPLACE
+		 * statement will be written for this secondary key.
+		 */
+		uint64_t column_mask = vy_stmt_column_mask(stmt);
+		if (column_mask != UINT64_MAX)
+			vy_stmt_set_column_mask(stmt, column_mask &
+						~lsm->cmp_def->column_mask);
+	}
+
 	v->overwritten = old;
 	write_set_insert(&tx->write_set, v);
 	tx->write_set_version++;

test/box/errinj.result

@@ -1488,8 +1488,9 @@ errinj.set('ERRINJ_VY_LOG_FILE_RENAME', false)
 ---
 - ok
 ...
-for i = 1, 10 do box.space.test:insert{i} end
+errinj.set('ERRINJ_VY_GC', true)
 ---
+- ok
 ...
 errinj.set('ERRINJ_VY_SCHED_TIMEOUT', 0.001)
 ---
@@ -1499,20 +1500,29 @@ errinj.set('ERRINJ_VY_RUN_FILE_RENAME', true)
 ---
 - ok
 ...
+box.space.test:insert{1}
+---
+- [1]
+...
 box.snapshot() -- error
 ---
 - error: Error injection 'vinyl run file rename'
 ...
-errinj.set('ERRINJ_VY_INDEX_FILE_RENAME', true)
+errinj.set('ERRINJ_VY_RUN_FILE_RENAME', false)
 ---
 - ok
 ...
-errinj.set('ERRINJ_VY_RUN_FILE_RENAME', false)
+-- Wait for the scheduler to unthrottle.
+repeat fiber.sleep(0.001) until pcall(box.snapshot)
+---
+...
+errinj.set('ERRINJ_VY_INDEX_FILE_RENAME', true)
 ---
 - ok
 ...
-fiber.sleep(0.01)
+box.space.test:insert{2}
 ---
+- [2]
 ...
 box.snapshot() -- error
 ---
@@ -1526,6 +1536,10 @@ errinj.set('ERRINJ_VY_SCHED_TIMEOUT', 0)
 ---
 - ok
 ...
+errinj.set('ERRINJ_VY_GC', false)
+---
+- ok
+...
 #fio.glob(fio.pathjoin(box.cfg.vinyl_dir, '*.vylog.inprogress')) > 0
 ---
 - true

test/box/errinj.test.lua

@@ -526,17 +526,24 @@ errinj.set('ERRINJ_VY_LOG_FILE_RENAME', true)
 box.snapshot()
 errinj.set('ERRINJ_VY_LOG_FILE_RENAME', false)
 
-for i = 1, 10 do box.space.test:insert{i} end
-
+errinj.set('ERRINJ_VY_GC', true)
 errinj.set('ERRINJ_VY_SCHED_TIMEOUT', 0.001)
+
 errinj.set('ERRINJ_VY_RUN_FILE_RENAME', true)
+box.space.test:insert{1}
 box.snapshot() -- error
-errinj.set('ERRINJ_VY_INDEX_FILE_RENAME', true)
 errinj.set('ERRINJ_VY_RUN_FILE_RENAME', false)
-fiber.sleep(0.01)
+
+-- Wait for the scheduler to unthrottle.
+repeat fiber.sleep(0.001) until pcall(box.snapshot)
+
+errinj.set('ERRINJ_VY_INDEX_FILE_RENAME', true)
+box.space.test:insert{2}
 box.snapshot() -- error
 errinj.set('ERRINJ_VY_INDEX_FILE_RENAME', false)
+
 errinj.set('ERRINJ_VY_SCHED_TIMEOUT', 0)
+errinj.set('ERRINJ_VY_GC', false)
 
 #fio.glob(fio.pathjoin(box.cfg.vinyl_dir, '*.vylog.inprogress')) > 0
 #fio.glob(fio.pathjoin(box.cfg.vinyl_dir, box.space.test.id, 0, '*.run.inprogress')) > 0

test/box/lua/bitset.lua

@@ -36,7 +36,7 @@ function drop_space()
 end
 
 function dump(...)
-	return iterate('tweedledum', 'bitset', 1, 2, ...)
+	return utils.iterate('tweedledum', 'bitset', 1, 2, ...)
 end
 
 function test_insert_delete(n)

test/vinyl/update_optimize.result

@@ -711,3 +711,50 @@ lookups()
 space:drop()
 ---
 ...
+--
+-- gh-3607: phantom tuples in secondary index if UPDATE does not
+-- change key fields.
+--
+s = box.schema.space.create('test', {engine = 'vinyl'})
+---
+...
+_ = s:create_index('pk')
+---
+...
+_ = s:create_index('sk', {parts = {2, 'unsigned'}, run_count_per_level = 10})
+---
+...
+s:insert{1, 10}
+---
+- [1, 10]
+...
+box.snapshot()
+---
+- ok
+...
+s:update(1, {{'=', 2, 10}})
+---
+- [1, 10]
+...
+s:delete(1)
+---
+...
+box.snapshot()
+---
+- ok
+...
+s.index.sk:stat().rows -- INSERT in the first run + DELETE the second run
+---
+- 2
+...
+s:insert{1, 20}
+---
+- [1, 20]
+...
+s.index.sk:select()
+---
+- - [1, 20]
+...
+s:drop()
+---
+...

test/vinyl/update_optimize.test.lua

@@ -234,3 +234,25 @@ space:update(1, {{'+', 5, 1}})
 lookups()
 
 space:drop()
+
+--
+-- gh-3607: phantom tuples in secondary index if UPDATE does not
+-- change key fields.
+--
+s = box.schema.space.create('test', {engine = 'vinyl'})
+_ = s:create_index('pk')
+_ = s:create_index('sk', {parts = {2, 'unsigned'}, run_count_per_level = 10})
+
+s:insert{1, 10}
+box.snapshot()
+
+s:update(1, {{'=', 2, 10}})
+s:delete(1)
+box.snapshot()
+
+s.index.sk:stat().rows -- INSERT in the first run + DELETE the second run
+
+s:insert{1, 20}
+s.index.sk:select()
+
+s:drop()