diff --git a/changelogs/unreleased/gh-9247-fix-space-bsize-handling-on-space-alter.md b/changelogs/unreleased/gh-9247-fix-space-bsize-handling-on-space-alter.md new file mode 100644 index 0000000000000000000000000000000000000000..e11362fca6e2c9854e898458ef32123d6ec26dd6 --- /dev/null +++ b/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). diff --git a/src/box/alter.cc b/src/box/alter.cc index 966d407220a9bbfef8b4a7e4589de4fdae8daf54..f8841bca222de416c6eb2da2e76f3b1ab5b2a220 100644 --- a/src/box/alter.cc +++ b/src/box/alter.cc @@ -1063,6 +1063,7 @@ alter_space_do(struct txn_stmt *stmt, struct alter_space *alter) * 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); diff --git a/src/box/blackhole.c b/src/box/blackhole.c index 63b102aae370a9d68790ef38e289cc4b57704065..83e1d8bb96d6106f2d86e436a651823dea629f9a 100644 --- a/src/box/blackhole.c +++ b/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, }; diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c index da71725ab0d2a903074684869e705d8930f45a1a..efd502c117be63e0f1af70098bc0021c9a3392a0 100644 --- a/src/box/memtx_space.c +++ b/src/box/memtx_space.c @@ -1058,7 +1058,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 @@ -1377,10 +1376,26 @@ memtx_space_prepare_alter(struct space *old_space, struct space *new_space) } 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 */ static const struct space_vtab memtx_space_vtab = { @@ -1403,6 +1418,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, }; diff --git a/src/box/session_settings.c b/src/box/session_settings.c index 66fa9fc8307e103c6cd5abbdfeb9c3cafeb3da57..9c38cd3f96707affa936afd3a096558ec70100b1 100644 --- a/src/box/session_settings.c +++ b/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, }; diff --git a/src/box/space.c b/src/box/space.c index 6e21f261005d7156603e2333066da97bc675a84d..ccb827e5923bc640838c2299c56c2f608267d4ad 100644 --- a/src/box/space.c +++ b/src/box/space.c @@ -1417,6 +1417,13 @@ generic_space_prepare_alter(struct space *old_space, struct space *new_space) 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) { diff --git a/src/box/space.h b/src/box/space.h index 4c6d039eace15e4081ab73ecf3faf1251ebddd05..604009ce1a242a662e32382b8c2f2b63be7e914a 100644 --- a/src/box/space.h +++ b/src/box/space.h @@ -145,6 +145,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); @@ -658,6 +664,13 @@ space_prepare_alter(struct space *old_space, struct space *new_space) 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 space_prepare_upgrade(struct space *old_space, struct space *new_space) { @@ -771,6 +784,7 @@ int generic_space_check_format(struct space *, struct tuple_format *); 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 *); diff --git a/src/box/sysview.c b/src/box/sysview.c index 77e21233c9216831b9e081e5678058adc694cbc0..2bfda2a743ee77c0fdc667b8022468ffcba0271d 100644 --- a/src/box/sysview.c +++ b/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, }; diff --git a/src/box/vinyl.c b/src/box/vinyl.c index 828e48bbe16d36a3264860362131d53456e5c31c..4e0669a26885c907e2fdb6ac640a5b916d08f6f9 100644 --- a/src/box/vinyl.c +++ b/src/box/vinyl.c @@ -4640,6 +4640,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, }; diff --git a/test/box-luatest/gh_9247_bsize_on_concurrent_dml_and_alter_test.lua b/test/box-luatest/gh_9247_bsize_on_concurrent_dml_and_alter_test.lua new file mode 100644 index 0000000000000000000000000000000000000000..5fe9a9bdf6f2e12959616387a5981d1c3e870748 --- /dev/null +++ b/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