Make drop_table idempotent so concurrent retention runs don't crash (#3595) - #3616
Open
youdie006 wants to merge 1 commit into
Open
Make drop_table idempotent so concurrent retention runs don't crash (#3595)#3616youdie006 wants to merge 1 commit into
youdie006 wants to merge 1 commit into
Conversation
ProcessMessageRetentionScheduledTask's lock is stealable after a few minutes, so two workers can call Provisioner#drop_table for the same table. The bare DROP TABLE then raises Mysql2::Error (Unknown table) on the second worker and crashes the task (postalserver#3595). Use DROP TABLE IF EXISTS, mirroring the concurrent-run tolerance create_raw_table already has via its rescue. Adds a spec asserting drop_table does not raise when the table is gone.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
ProcessMessageRetentionScheduledTaskcrashes withMysql2::Error: Unknown tablewhen two workers run retention concurrently (#3595). The task's lock is stealable after a few minutes, so two workers can callProvisioner#drop_tablefor the sameraw-<date>table; the second one runs a bareDROP TABLEon an already-dropped table and raises.In
lib/postal/message_db/provisioner.rb,drop_tablewas the one un-guarded destructive query. Its siblingcreate_raw_tablealready tolerates the same concurrent-run race withrescue Mysql2::Error ... raise unless ... already exists.Fix
Use
DROP TABLE IF EXISTS, so dropping an already-removed table is a no-op instead of an error. This mirrors the concurrent-run tolerancecreate_raw_tablehas, but at the SQL level so it doesn't swallow any unrelatedMysql2::Error.Adds a spec asserting
drop_tabledoes not raise when the table no longer exists (it fails on the old bareDROP TABLEand passes withIF EXISTS).Scope
This is the safe, idempotent guard for the crash itself. @willpower232 noted on the issue that the underlying lock-renewal behaviour (two workers holding the retention lock at once) may also be worth revisiting — that's a separate change, and
IF EXISTSstops the crash regardless of whether the lock overlap is later tightened.AI disclosure: prepared with AI assistance (Claude Code); the fix was traced against the code and syntax-checked with
ruby -cbefore submitting. The message_db specs run against MySQL, so the added spec is exercised by CI.