commit de59504c2bdb0369cdd27af892301f8515293fe1 from: Vladimir Davydov via: Vladimir Davydov date: Mon Aug 26 11:18:23 2024 UTC vinyl: do not discard run on dump/compaction abort if index was dropped If an index is dropped while a dump or compaction task is in progress we must not write any information about it to the vylog when the task completes otherwise there's a risk of getting a vylog recovery failure in case the garbage collector manages to purge the index from the vylog. We disabled logging on successful completion of a dump task quite a while ago, in commit 29e2931c66c5 ("vinyl: fix race between compaction and gc of dropped LSM"), and for compaction only recently, in commit ae6a02ebab0b ("vinyl: do not log dump if index was dropped"), but the issue remains for a dump/compaction failure, when we log a discard record for a run file we failed to write. These results in errors like: ``` ER_INVALID_VYLOG_FILE: Invalid VYLOG file: Run 6 deleted twice ``` or ``` ER_INVALID_VYLOG_FILE: Invalid VYLOG file: Run 5768 deleted but not registered ``` Let's fix these issues in exactly the same way as we fixed them for successful dump/compaction completion - by skipping writing to vylog in case the index is marked as dropped. Closes #10452 NO_DOC=bug fix commit - 850673db5a69df2c7250d174ab15305624b2634a commit + de59504c2bdb0369cdd27af892301f8515293fe1 blob - /dev/null blob + f1671ea1dd48c0ff1e9ffaf5c25a546bb7209580 (mode 644) --- /dev/null +++ changelogs/unreleased/gh-10452-vy-log-discard-run-after-index-drop-fix.md @@ -0,0 +1,6 @@ +## bugfix/vinyl + +* Fixed a bug when recovery could fail with the error "Invalid VYLOG file: + Run XXXX deleted but not registered" or "Invalid VYLOG file: Run XXX deleted + twice" in case a dump or compaction completed with a disk write error after + the target index was dropped (gh-10452). blob - 38098599268d6532a2babb42830ef3a9bd5585ef blob + 6a36fb6a4f4caf29e1809a9a9dbbe77d52dc8cdb --- src/box/vy_scheduler.c +++ src/box/vy_scheduler.c @@ -1352,7 +1352,16 @@ vy_task_dump_abort(struct vy_task *task) error_log(e); say_error("%s: dump failed", vy_lsm_name(lsm)); - vy_run_discard(task->new_run); + /* + * The LSM tree could have been dropped while we were writing the new + * run. In this case we should discard the run without committing to + * vylog, because all the information about the LSM tree and its runs + * could have already been garbage collected from vylog. + */ + if (lsm->is_dropped) + vy_run_unref(task->new_run); + else + vy_run_discard(task->new_run); lsm->is_dumping = false; vy_scheduler_update_lsm(scheduler, lsm); @@ -1680,7 +1689,16 @@ vy_task_compaction_abort(struct vy_task *task) say_error("%s: failed to compact range %s", vy_lsm_name(lsm), vy_range_str(range)); - vy_run_discard(task->new_run); + /* + * The LSM tree could have been dropped while we were writing the new + * run. In this case we should discard the run without committing to + * vylog, because all the information about the LSM tree and its runs + * could have already been garbage collected from vylog. + */ + if (lsm->is_dropped) + vy_run_unref(task->new_run); + else + vy_run_discard(task->new_run); assert(heap_node_is_stray(&range->heap_node)); vy_range_heap_insert(&lsm->range_heap, range); blob - /dev/null blob + e39f0c0ce2dbdd518d64f0b8577e2ee8b735645c (mode 644) --- /dev/null +++ test/vinyl-luatest/gh_10452_discard_run_after_index_drop_test.lua @@ -0,0 +1,111 @@ +local server = require('luatest.server') +local t = require('luatest') + +local g = t.group() + +g.before_each(function(cg) + t.tarantool.skip_if_not_debug() + cg.server = server:new{ + box_cfg = { + checkpoint_count = 1, + }, + } + cg.server:start() +end) + +g.after_each(function(cg) + cg.server:drop() +end) + +g.test_dump = function(cg) + cg.server:exec(function() + local fiber = require('fiber') + + -- Pause garbage collection. + box.backup.start() + + -- Create a space. + local s = box.schema.space.create('test', {engine = 'vinyl'}) + s:create_index('pk') + + -- Create a checkpoint. + s:insert({1}) + box.snapshot() + + -- Start checkpointing in background. + box.error.injection.set('ERRINJ_VY_DUMP_DELAY', true) + s:insert({2}) + local f = fiber.new(box.snapshot) + f:set_joinable(true) + fiber.sleep(0.1) + t.assert_equals(box.info.gc().checkpoint_is_in_progress, true) + + -- Drop the space. + s:drop() + + -- Resume garbage collection and wait for it to complete. + box.backup.stop() + t.helpers.retrying({}, function() + t.assert_equals(#box.info.gc().checkpoints, 1) + end) + + -- Make background checkpointing fail. + box.error.injection.set('ERRINJ_VY_RUN_WRITE', true) + box.error.injection.set('ERRINJ_VY_DUMP_DELAY', false) + t.assert_error_msg_equals( + "Error injection 'vinyl dump'", + function() + local ok, ret = f:join() + if not ok then + error(ret) + end + return ret + end) + + -- Try again to create a checkpoint. + box.error.injection.set('ERRINJ_VY_RUN_WRITE', false) + box.snapshot() + end) +end + +g.test_compaction = function(cg) + cg.server:exec(function() + -- Pause garbage collection. + box.backup.start() + + -- Create a space. + local s = box.schema.space.create('test', {engine = 'vinyl'}) + s:create_index('pk') + + -- Start compaction in background. + box.error.injection.set('ERRINJ_VY_COMPACTION_DELAY', true) + s:insert({1}) + box.snapshot() + s:insert({2}) + box.snapshot() + s.index.pk:compact() + t.helpers.retrying({}, function() + t.assert_ge(box.stat.vinyl().scheduler.tasks_inprogress, 1) + end) + + -- Drop the space. + s:drop() + + -- Resume garbage collection and wait for it to complete. + box.backup.stop() + t.helpers.retrying({}, function() + t.assert_equals(#box.info.gc().checkpoints, 1) + end) + + -- Make background compaction fail. + box.error.injection.set('ERRINJ_VY_RUN_WRITE', true) + box.error.injection.set('ERRINJ_VY_COMPACTION_DELAY', false) + t.helpers.retrying({}, function() + t.assert_ge(box.stat.vinyl().scheduler.tasks_failed, 1) + end) + + -- Try to create a checkpoint. + box.error.injection.set('ERRINJ_VY_RUN_WRITE', false) + box.snapshot() + end) +end