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
+## 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
* 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
/* .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
* can be put back online properly.
*/
memtx_space->replace = memtx_space_replace_no_keys;
- memtx_space->bsize = 0;
}
static void
}
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 */
/* .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
/* .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
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
*/
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);
{
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
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
/* .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
/* .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
+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