diff --git a/test/storage-luatest/garbage_collector_test.lua b/test/storage-luatest/garbage_collector_test.lua index c69dd593..1fa949d3 100644 --- a/test/storage-luatest/garbage_collector_test.lua +++ b/test/storage-luatest/garbage_collector_test.lua @@ -1074,3 +1074,159 @@ test_group.test_route_map_is_cleared = function(g) vtest.cluster_wait_vclock_all(g) g.replica_1_b:exec(bucket_set_protection, {true}) end + +-- +-- gh-661: bucket GC resumes after the last tuple deleted by the preceding +-- transaction on Tarantool versions with index pagination. Older versions and +-- indexes that do not support positions keep restarting from the bucket prefix. +-- +local function test_resumable_cursor_template(g, expected_resumes) + g.replica_1_a:exec(function(expected_resumes) + _G.bucket_gc_pause() + local s = box.space.test + local bucket_index = s.index.bucket_id + local bid = _G.get_first_bucket() + local old_chunk_size = ivconst.BUCKET_CHUNK_SIZE + local old_pairs = bucket_index.pairs + local chunk_size = 10 + local tuple_count = chunk_size * 2 + 5 + + -- Two full chunks and a non-empty tail make GC open three iterators. + -- Record whether each one resumed from a position (1) or restarted at + -- the bucket prefix (0). The first iterator always starts at the + -- prefix. The next two resume only on cores with index pagination. + -- 'after' is opaque, so its presence is the observable signal. + local resumes = {} + bucket_index.pairs = function(self, key, opts) + local after = opts and opts.after + table.insert(resumes, after ~= nil and 1 or 0) + if opts == nil then + return old_pairs(self, key) + end + return old_pairs(self, key, opts) + end + + local ok, err = pcall(function() + ivconst.BUCKET_CHUNK_SIZE = chunk_size + box.begin() + for i = 1, tuple_count do + s:replace{100000 + i, bid} + end + box.commit() + ivshard.storage.bucket_delete_garbage(bid, {force = true}) + ilt.assert_equals(bucket_index:count({bid}), 0) + ilt.assert_equals(resumes, expected_resumes) + end) + + bucket_index.pairs = old_pairs + ivconst.BUCKET_CHUNK_SIZE = old_chunk_size + _G.bucket_gc_continue() + ilt.assert(ok, err) + end, {expected_resumes}) + vtest.cluster_wait_vclock_all(g) +end + +-- +-- gh-661: pagination-capable cores resume the second and third iterators after +-- the preceding chunk instead of restarting at the bucket prefix. +-- +test_group.test_resumable_cursor_supported = function(g) + t.run_only_if(vutil.feature.index_pagination) + test_resumable_cursor_template(g, {0, 1, 1}) +end + +-- +-- gh-661: without index pagination (Tarantool < 2.11) every iterator restarts +-- at the bucket prefix. +-- +test_group.test_resumable_cursor_unsupported = function(g) + t.run_only_if(not vutil.feature.index_pagination) + test_resumable_cursor_template(g, {0, 0, 0}) +end + +-- +-- gh-661: functional and multikey sharding indexes report type == 'TREE' and +-- pass find_sharded_spaces(), but reject positions with "does not support +-- position by tuple"; hash indexes are not TREE at all. GC must fall back to +-- restarting from the bucket prefix for them and still delete the bucket. +-- +test_group.test_resumable_cursor_unsupported_index = function(g) + t.run_only_if(vutil.feature.index_pagination) + g.replica_1_a:exec(function() + _G.bucket_gc_pause() + local bid = _G.get_first_bucket() + local spaces = {} + local func_name = 'test_extract_bucket' + local old_chunk_size = ivconst.BUCKET_CHUNK_SIZE + + -- Each case fills exactly one chunk, so GC has to open a second + -- iterator after committing the delete. Cleanup succeeds only if an + -- index which cannot derive a position from a tuple falls back to + -- restarting at the bucket prefix. + local function run(name, space_opts, index_opts, tuple) + local space = box.schema.create_space(name, space_opts) + table.insert(spaces, space) + space:create_index('pk') + space:create_index('bucket_id', index_opts) + space:replace(tuple) + ivshard.storage.bucket_delete_garbage(bid, {force = true}) + ilt.assert_equals(space:count(), 0) + space:drop() + spaces[#spaces] = nil + end + + local ok, err = pcall(function() + ivconst.BUCKET_CHUNK_SIZE = 1 + + run('test_hash', { + engine = 'memtx', + format = {{'id', 'unsigned'}, {'bid', 'unsigned'}}, + }, { + type = 'hash', unique = true, parts = {2}, + }, {300000, bid}) + + run('test_multikey', { + engine = 'vinyl', + format = { + {'id', 'unsigned'}, + {'bid', 'unsigned'}, + {'tags', 'array'}, + }, + }, { + unique = false, + parts = { + {field = 2, type = 'unsigned'}, + {field = 3, type = 'string', path = '[*]'}, + }, + }, {300001, bid, {'tag'}}) + + -- Memtx MVCC does not support functional indexes. + if not box.cfg.memtx_use_mvcc_engine then + box.schema.func.create(func_name, { + body = [[function(tuple) return {tuple[2]} end]], + is_deterministic = true, + is_sandboxed = true, + }) + run('test_functional', { + engine = 'memtx', + format = {{'id', 'unsigned'}, {'bid', 'unsigned'}}, + }, { + unique = false, + func = func_name, + parts = {{field = 1, type = 'unsigned'}}, + }, {300002, bid}) + end + end) + + ivconst.BUCKET_CHUNK_SIZE = old_chunk_size + for _, space in ipairs(spaces) do + space:drop() + end + if box.func[func_name] ~= nil then + box.schema.func.drop(func_name) + end + _G.bucket_gc_continue() + ilt.assert(ok, err) + end) + vtest.cluster_wait_vclock_all(g) +end diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua index 3d619677..5ac6a660 100644 --- a/vshard/storage/init.lua +++ b/vshard/storage/init.lua @@ -2351,20 +2351,36 @@ end -- local function gc_bucket_in_space_xc(space, bucket_id, status) local bucket_index = space.index[lschema.shard_index] - if #bucket_index:select({bucket_id}, {limit = 1}) == 0 then + local first = bucket_index:select({bucket_id}, {limit = 1})[1] + if first == nil then return end local bucket_generation = M.bucket_generation local pk_parts = space.index[0].parts + -- A new iterator started at the bucket prefix has to walk past the + -- tombstones left by all preceding batches, which is expensive on Vinyl. + -- Resume from the position of the last committed tuple instead. The tuple + -- is deleted, but its position stays valid once the tuple is gone. + -- Pagination is a TREE-index feature, but functional and multikey TREE + -- indexes still reject positions with "does not support position by tuple" + -- even though find_sharded_spaces() accepts them. Probe the index once and + -- fall back to restarting from the bucket prefix when it is unsupported. + local use_cursor = util.feature.index_pagination and + bucket_index.type == 'TREE' and + pcall(bucket_index.tuple_pos, bucket_index, first) + local after ::restart:: local limit = consts.BUCKET_CHUNK_SIZE box.begin() M._on_bucket_event:run(consts.BUCKET_EVENT.GC, bucket_id, {spaces = {space.name}}) - for _, tuple in bucket_index:pairs({bucket_id}) do + for _, tuple in bucket_index:pairs({bucket_id}, {after = after}) do space:delete(util.tuple_extract_key(tuple, pk_parts)) limit = limit - 1 if limit == 0 then + if use_cursor then + after = bucket_index:tuple_pos(tuple) + end box.commit() bucket_generation = bucket_guard_xc(bucket_generation, bucket_id, status) diff --git a/vshard/util.lua b/vshard/util.lua index 6e80a524..78da3c98 100644 --- a/vshard/util.lua +++ b/vshard/util.lua @@ -438,6 +438,7 @@ end local feature = { msgpack_object = lmsgpack.object ~= nil, netbox_return_raw = version_is_at_least(2, 10, 0, 'beta', 2, 86), + index_pagination = version_is_at_least(2, 11, 0, nil, 0, 0), multilisten = luri.parse_many ~= nil, -- It appeared earlier but at 2.9.0 there is a strange bug about an immortal -- tuple - after :delete(pk) it still stays in the space. That makes the