Commit Diff


commit - 5e147360b6570eaad2a81221bfaa4191cf23075d
commit + 1babcf1ea49d9c8823a706a36d1a12ef43733d28
blob - /dev/null
blob + e11362fca6e2c9854e898458ef32123d6ec26dd6 (mode 644)
--- /dev/null
+++ changelogs/unreleased/gh-9247-fix-space-bsize-handling-on-space-alter.md
@@ -0,0 +1,4 @@
+## bugfix/core
+
+* Fixed a bug that could result in the incorrect `space:bsize()` when altering
+  a primary index concurrently with DML operations (gh-9247).
blob - 51abc28fe75cf7ecb7e398ffa27205b67bc7fc1e
blob + 8d8d40a2b839310d213a0de3e827a7158bdc5884
--- src/box/alter.cc
+++ src/box/alter.cc
@@ -1017,6 +1017,7 @@ alter_space_do(struct txn_stmt *stmt, struct alter_spa
 	 * The new space is ready. Time to update the space
 	 * cache with it.
 	 */
+	space_finish_alter(alter->old_space, alter->new_space);
 	space_cache_replace(alter->old_space, alter->new_space);
 	space_detach_constraints(alter->old_space);
 	space_unpin_collations(alter->old_space);
blob - 63b102aae370a9d68790ef38e289cc4b57704065
blob + 83e1d8bb96d6106f2d86e436a651823dea629f9a
--- src/box/blackhole.c
+++ src/box/blackhole.c
@@ -126,6 +126,7 @@ static const struct space_vtab blackhole_space_vtab = 
 	/* .build_index = */ generic_space_build_index,
 	/* .swap_index = */ generic_space_swap_index,
 	/* .prepare_alter = */ generic_space_prepare_alter,
+	/* .finish_alter = */ generic_space_finish_alter,
 	/* .prepare_upgrade = */ generic_space_prepare_upgrade,
 	/* .invalidate = */ generic_space_invalidate,
 };
blob - e36d5bf6e6cfaf2c0281b58f94909480b886b600
blob + 3f6594bd65809af3e3481ee1c5f3daebdd790d39
--- src/box/memtx_space.c
+++ src/box/memtx_space.c
@@ -1052,7 +1052,6 @@ memtx_space_drop_primary_key(struct space *space)
 	 *   can be put back online properly.
 	 */
 	memtx_space->replace = memtx_space_replace_no_keys;
-	memtx_space->bsize = 0;
 }
 
 static void
@@ -1369,8 +1368,24 @@ memtx_space_prepare_alter(struct space *old_space, str
 	}
 
 	new_memtx_space->replace = old_memtx_space->replace;
-	new_memtx_space->bsize = old_memtx_space->bsize;
 	return 0;
+}
+
+/**
+ * Copy bsize to the newly altered space from the old space.
+ * In case of DropIndex or TruncateIndex alter operations, the new space will be
+ * empty, and bsize must not be copied.
+ */
+static void
+memtx_space_finish_alter(struct space *old_space, struct space *new_space)
+{
+	struct memtx_space *old_memtx_space = (struct memtx_space *)old_space;
+	struct memtx_space *new_memtx_space = (struct memtx_space *)new_space;
+
+	bool is_empty = new_space->index_count == 0 ||
+			index_size(new_space->index[0]) == 0;
+	if (!is_empty)
+		new_memtx_space->bsize = old_memtx_space->bsize;
 }
 
 /* }}} DDL */
@@ -1395,6 +1410,7 @@ static const struct space_vtab memtx_space_vtab = {
 	/* .build_index = */ memtx_space_build_index,
 	/* .swap_index = */ generic_space_swap_index,
 	/* .prepare_alter = */ memtx_space_prepare_alter,
+	/* .finish_alter = */ memtx_space_finish_alter,
 	/* .prepare_upgrade = */ memtx_space_prepare_upgrade,
 	/* .invalidate = */ generic_space_invalidate,
 };
blob - 66fa9fc8307e103c6cd5abbdfeb9c3cafeb3da57
blob + 9c38cd3f96707affa936afd3a096558ec70100b1
--- src/box/session_settings.c
+++ src/box/session_settings.c
@@ -437,6 +437,7 @@ const struct space_vtab session_settings_space_vtab = 
 	/* .build_index = */ generic_space_build_index,
 	/* .swap_index = */ generic_space_swap_index,
 	/* .prepare_alter = */ generic_space_prepare_alter,
+	/* .finish_alter = */ generic_space_finish_alter,
 	/* .prepare_upgrade = */ generic_space_prepare_upgrade,
 	/* .invalidate = */ generic_space_invalidate,
 };
blob - b7fbf6fdbc5bf10c0248031218e2e03e20ba9bc4
blob + 4a2a51b7ddef415fd5b70d235453cbffe176d044
--- src/box/space.c
+++ src/box/space.c
@@ -971,6 +971,13 @@ generic_space_prepare_alter(struct space *old_space, s
 	return 0;
 }
 
+void
+generic_space_finish_alter(struct space *old_space, struct space *new_space)
+{
+	(void)old_space;
+	(void)new_space;
+}
+
 int
 generic_space_prepare_upgrade(struct space *old_space, struct space *new_space)
 {
blob - 9a101caf0ec7937cb7f53f2653b4fe0d114d3db2
blob + 930184cf329d1320f698f88a0a6a3c3e4542fb9b
--- src/box/space.h
+++ src/box/space.h
@@ -144,6 +144,12 @@ struct space_vtab {
 	 */
 	int (*prepare_alter)(struct space *old_space,
 			     struct space *new_space);
+	/**
+	 * Notify the engine after altering a space and before replacing
+	 * old_space with new_space in the space cache.
+	 */
+	void (*finish_alter)(struct space *old_space,
+			     struct space *new_space);
 	/** Prepares a space for online upgrade on alter. */
 	int (*prepare_upgrade)(struct space *old_space,
 			       struct space *new_space);
@@ -561,6 +567,13 @@ space_prepare_alter(struct space *old_space, struct sp
 {
 	assert(old_space->vtab == new_space->vtab);
 	return new_space->vtab->prepare_alter(old_space, new_space);
+}
+
+static inline void
+space_finish_alter(struct space *old_space, struct space *new_space)
+{
+	assert(old_space->vtab == new_space->vtab);
+	new_space->vtab->finish_alter(old_space, new_space);
 }
 
 static inline int
@@ -668,6 +681,7 @@ int generic_space_check_format(struct space *, struct 
 int generic_space_build_index(struct space *, struct index *,
 			      struct tuple_format *, bool);
 int generic_space_prepare_alter(struct space *, struct space *);
+void generic_space_finish_alter(struct space *, struct space *);
 int generic_space_prepare_upgrade(struct space *old_space,
 				  struct space *new_space);
 void generic_space_invalidate(struct space *);
blob - 6b3dfd790ddccc36be26d4badafce661e0dd4a5a
blob + 8b655b291df2fe1e940da25acf08e1e3fccab3fd
--- src/box/sysview.c
+++ src/box/sysview.c
@@ -505,6 +505,7 @@ static const struct space_vtab sysview_space_vtab = {
 	/* .build_index = */ generic_space_build_index,
 	/* .swap_index = */ generic_space_swap_index,
 	/* .prepare_alter = */ generic_space_prepare_alter,
+	/* .finish_alter = */ generic_space_finish_alter,
 	/* .prepare_upgrade = */ generic_space_prepare_upgrade,
 	/* .invalidate = */ generic_space_invalidate,
 };
blob - 2cb41a222d8d5e87b8bc93e393f57ea9e6253e46
blob + 259c1b9f682b8490eb5281e8f70cee39eb38f189
--- src/box/vinyl.c
+++ src/box/vinyl.c
@@ -4641,6 +4641,7 @@ static const struct space_vtab vinyl_space_vtab = {
 	/* .build_index = */ vinyl_space_build_index,
 	/* .swap_index = */ vinyl_space_swap_index,
 	/* .prepare_alter = */ vinyl_space_prepare_alter,
+	/* .finish_alter = */ generic_space_finish_alter,
 	/* .prepare_upgrade = */ generic_space_prepare_upgrade,
 	/* .invalidate = */ vinyl_space_invalidate,
 };
blob - /dev/null
blob + 5fe9a9bdf6f2e12959616387a5981d1c3e870748 (mode 644)
--- /dev/null
+++ test/box-luatest/gh_9247_bsize_on_concurrent_dml_and_alter_test.lua
@@ -0,0 +1,118 @@
+local server = require('luatest.server')
+local t = require('luatest')
+local g = t.group('gh-9247')
+
+g.before_all(function(cg)
+    cg.server = server:new()
+    cg.server:start()
+end)
+
+g.after_all(function(cg)
+    cg.server:drop()
+end)
+
+g.after_each(function(cg)
+    cg.server:exec(function()
+        box.space.test:drop()
+    end)
+end)
+
+-- Based on `test/box/alter-primary-index-tuple-leak-long.test.lua'.
+local function concurrent_insert_and_alter_helper(cg, rollback)
+    cg.server:exec(function(rollback)
+        local fiber = require('fiber')
+        local s = box.schema.space.create('test')
+        local idx = s:create_index('pk')
+        local alter_started = false
+        local data = string.rep('a', 66)
+
+        -- Create a table with enough tuples to make primary index alter in
+        -- background.
+        for i = 0, 7000, 100 do
+            box.begin()
+            for j = i, i + 99 do
+                s:insert{j, j, data}
+            end
+            box.commit()
+        end
+
+        -- Wait for fiber yield during altering and insert some tuples.
+        local fib_insert = fiber.new(function()
+            while not alter_started do
+                fiber.sleep(0)
+            end
+            box.begin()
+            for i = 8000, 9000 do
+                s:insert{i, i, data}
+            end
+            box.commit()
+        end)
+
+        -- Alter primary index.
+        -- If the index will not be altered in background, the test will fail
+        -- after timeout (fib_insert:join() will wait forever).
+        local fib_alter = fiber.new(function()
+            alter_started = true
+            box.begin()
+            idx:alter{parts = {{field = 2}}}
+            if rollback then
+                box.rollback()
+            else
+                box.commit()
+            end
+            alter_started = false
+        end)
+
+        fib_insert:set_joinable(true)
+        fib_alter:set_joinable(true)
+        fib_insert:join()
+        fib_alter:join()
+
+        local bsize = 0
+        for _, tuple in s:pairs() do
+            bsize = bsize + tuple:bsize()
+        end
+        t.assert_equals(s:bsize(), bsize)
+    end, {rollback})
+end
+
+-- Check that space:bsize() is correct for a built-in-background primary index
+-- with concurrent inserts.
+g.test_concurrent_insert_and_alter_commit = function(cg)
+    concurrent_insert_and_alter_helper(cg, false)
+end
+
+-- Check that space:bsize() is correct for a built-in-background primary index
+-- with concurrent inserts.
+g.test_concurrent_insert_and_alter_rollback = function(cg)
+    concurrent_insert_and_alter_helper(cg, true)
+end
+
+local function truncate_helper(cg, rollback)
+    cg.server:exec(function(rollback)
+        local s = box.schema.space.create('test')
+        s:create_index('pk')
+        s:insert{1, 2, 3}
+        local bsize_before_truncate = s:bsize()
+
+        box.begin()
+        s:truncate()
+        if rollback then
+            box.rollback()
+            t.assert_equals(s:bsize(), bsize_before_truncate)
+        else
+            box.commit()
+            t.assert_equals(s:bsize(), 0)
+        end
+    end, {rollback})
+end
+
+-- Check that space:bsize() is correct after successful space:truncate().
+g.test_truncate_commit = function(cg)
+    truncate_helper(cg, false)
+end
+
+-- Check that space:bsize() is correct after rolled back space:truncate().
+g.test_truncate_rollback = function(cg)
+    truncate_helper(cg, true)
+end