Merged
Conversation
This should help to handle very-loaded models and reduce lock wait time
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
The
FOR UPDATElock onartery_model_infosis acquired inside the model'sbefore_commitcallback, meaning it is held frombefore_commituntil the model transaction commits. Under high concurrency (30-50 Sidekiq threads writing the same model), threads queue on this lock for 7-11 seconds, causing database timeouts.Solution
Add a per-model option
non_atomic_notificationthat moves artery message creation frombefore_commit(inside the model transaction) toafter_commit(in a separate short transaction). This reduces lock hold time from "remainder of the model transaction" to ~2-5ms (just the INSERT + UPDATE + COMMIT of the artery message).Trade-offs
non_atomic_notification: false): artery message creation is atomic with the model change. If message creation fails, the entire transaction rolls back. No risk of missing notifications.non_atomic_notification: true: artery message creation happens after the model transaction commits. If it fails, the model change is already persisted but the notification may be lost. Retry logic (up to 3 attempts) mitigates transient failures; persistent failures are reported viaArtery.handle_error.