Fix race conditions when calculating previous_index#82
Merged
Conversation
IgRich
approved these changes
Mar 23, 2026
* Refactor logging with ActiveSupport::Instrumentation
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
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
When multiple model transactions run concurrently, they INSERT into
artery_messagesand receive sequential auto-increment IDs. However, transactions can COMMIT in arbitrary order. Sinceprevious_indexis computed inafter_commit(and only sees committed rows), a later-committed message with a lower ID can end up with the sameprevious_indexas an already-committed message with a higher ID. This causes the consumer to silently drop the out-of-order message, leading to data loss.Example: Two concurrent transactions produce messages with IDs 100 and 101. Transaction for ID 101 commits first. Both messages end up with
previous_index = 99. If the consumer processes 101 first, message 100 is skipped as "already handled".Approach
New
artery_model_infostable — storeslatest_indexper model, used both as a lock target (SELECT ... FOR UPDATE) and as a fast lookup forlatest_index/previous_index.Deferred message creation via
before_commit—after_create/after_update/after_destroycallbacks now queue pending notifications instead of creating artery messages immediately. Abefore_commitcallback flushes the queue, minimizing the lock window to just INSERT + COMMIT.around_create :lock_on_modelonArtery::ActiveRecord::Message— acquires aFOR UPDATElock on the model's row inartery_model_infos, cachesprevious_indexfromlatest_index, creates the message, then updateslatest_index. This serializes message creation per model, preventing the race.Configurable
model_info_class— follows the same pattern asmessage_classandsubscription_info_class.Deployment
artery_model_infosfrom existingartery_messagesdata)latest_indexself-corrects once all processes are on the new codeacquire_lock!(creates the lock row on first use)