fix(canal): skip expired notification when patch is no longer pending - #383
Merged
Conversation
When a patch is accepted, the review flow calls the wiki API while holding a FOR UPDATE row lock on the patch. The resulting binlog event can be processed by the canal consumer before the local transaction commits, so canal sees the patch as still pending and judges it outdated. Its UPDATE then waits on the row lock and, after the accept commits, matches zero rows because state is no longer pending. Reject/Accept queries were :exec and ignored RowsAffected, so canal still sent an outdated notification even though the update was a no-op, producing both expired and accepted notifications for the same patch. Change Reject/Accept queries to :execrows and have canal check the affected row count, only notifying when the patch state was actually changed.
Contributor
|
Tick the box to add this pull request to the merge queue (same as
|
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
1 participant
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 a reviewer accepts a patch, the review flow calls the external wiki API while holding a
FOR UPDATErow lock on the patch (subject/character/person). The API write immediately produces a binlog event. The canal (Debezium CDC) consumer can process that event before the local transaction commits, so it sees the patch as stillpendingand judges it outdated.The patch row lock makes canal's
UPDATE ... WHERE state = 0block until the accept transaction commits; after commitstateis no longerpending, so the update affects zero rows. But the Reject/Accept queries were generated as:execand ignoredRowsAffected, so canal still sent an "expired" notification — the user received both an expired and an accepted notification for the same patch.Fix
RejectSubjectPatch/RejectCharacterPatch/RejectPersonPatchandAcceptSubjectPatch/AcceptCharacterPatch/AcceptPersonPatchfrom:execto:execrows(return affected row count).canal.go, check the affected row count and skip the expired notification when it is 0 (the patch state was changed by another flow, e.g. accepted).FOR UPDATE+ API call inside the transaction), so there is no intermediate "processing" state that could be left behind after a crash — row locks are released automatically on rollback.Verification
go build ./...,go vet ./...,golangci-lint-v2 run ./...,go test ./...all pass.dal/query.sql.gowas updated manually to match sqlc's:execrowsoutput (no sqlc/DB available in this environment); runtask genin an environment with a database to confirm there is no diff.