syncer(dm): prevent checkpoint flush after BeginTx error#12644
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Code Review
This pull request centralizes downstream execution error handling by introducing the isDownstreamExecutionError helper function, which now includes terror.ErrDBExecuteFailedBegin in its checks. This ensures that checkpoint flushes are correctly skipped when a downstream transaction fails to begin, preventing potential data inconsistency. The changes also include a new unit test to verify this behavior and a safety check in dml_worker.go to ensure key-not-found logic only executes when no other errors are present. I have no feedback to provide as there were no review comments to assess.
|
@GMHDBJD: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
What problem does this PR solve?
Issue Number: close #12626
What is changed and how it works?
This PR fixes a DM correctness issue where checkpoint flush could still advance after a downstream transaction
BeginTxfailure.Changes:
ErrDBExecuteFailedBegintogether with existingErrDBExecuteFailedandErrDBUnExpect.ExecuteSQLalready returned an execution error.ErrDBExecuteFailedBegin(sql.ErrConnDone)to ensure checkpoint flush is skipped.Check List
Tests
Also ran:
Questions
Will it cause performance regression or break compatibility?
No. The change only broadens existing checkpoint-blocking error classification to include downstream transaction begin failures and suppresses a misleading diagnostic after failed execution.
Do you need to update user documentation, design documentation or monitoring documentation?
No.
Release note