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 |
📝 WalkthroughWalkthroughThis change documents ChangesDML timeout documentation
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Other Suggested reviewers: Merge Risk: 🟡 Moderate · up to Readers could retry non-idempotent work after an ambiguous commit and apply it twice. Correct the retry guidance before merging. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 502932fd-e248-4809-b823-3b1fad8ca0cd
📒 Files selected for processing (1)
system-variables.md
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.
| > | ||
| > - The statement might finish later than the configured timeout. | ||
| > - For `COMMIT` and autocommit DML statements, if the statement times out and TiDB cannot determine whether the transaction has committed, it closes the client connection. The client might receive a connection error instead of a statement timeout error. A connection loss does not mean that the transaction has been rolled back; the transaction might have committed. | ||
| > - When enabling this variable, it is recommended to set a relatively long timeout and allow sufficient headroom for normal DML execution and transaction commits. After TiDB interrupts a statement, requests already sent to TiKV might continue running or remain queued. An excessively short timeout combined with frequent application retries might cause retries to overlap with unfinished requests, increasing TiKV load and worsening request buildup during a failure. If your application retries an operation, use exponential backoff with jitter to reduce the additional load on TiKV. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Warn against blind retries after an ambiguous transaction outcome.
The preceding bullet states that a timed-out COMMIT or autocommit DML statement may have committed before the client connection closes. Retrying non-idempotent work can therefore apply its effects twice. Add outcome verification or an idempotency requirement while retaining exponential backoff for load control.
Suggested replacement
| > - When enabling this variable, it is recommended to set a relatively long timeout and allow sufficient headroom for normal DML execution and transaction commits. After TiDB interrupts a statement, requests already sent to TiKV might continue running or remain queued. An excessively short timeout combined with frequent application retries might cause retries to overlap with unfinished requests, increasing TiKV load and worsening request buildup during a failure. If your application retries an operation, use exponential backoff with jitter to reduce the additional load on TiKV. | |
| > - When enabling this variable, it is recommended to set a relatively long timeout and allow sufficient headroom for normal DML execution and transaction commits. After TiDB interrupts a statement, requests already sent to TiKV might continue running or remain queued. An excessively short timeout combined with frequent application retries might cause retries to overlap with unfinished requests, increasing TiKV load and worsening request buildup during a failure. If a timed-out `COMMIT` or autocommit DML statement causes TiDB to close the client connection before it confirms whether the transaction committed, do not blindly retry a non-idempotent operation. Verify the outcome or use an idempotency mechanism before retrying. If your application retries an operation, use exponential backoff with jitter to reduce the additional load on TiKV. |
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| > - When enabling this variable, it is recommended to set a relatively long timeout and allow sufficient headroom for normal DML execution and transaction commits. After TiDB interrupts a statement, requests already sent to TiKV might continue running or remain queued. An excessively short timeout combined with frequent application retries might cause retries to overlap with unfinished requests, increasing TiKV load and worsening request buildup during a failure. If your application retries an operation, use exponential backoff with jitter to reduce the additional load on TiKV. | |
| > - When enabling this variable, it is recommended to set a relatively long timeout and allow sufficient headroom for normal DML execution and transaction commits. After TiDB interrupts a statement, requests already sent to TiKV might continue running or remain queued. An excessively short timeout combined with frequent application retries might cause retries to overlap with unfinished requests, increasing TiKV load and worsening request buildup during a failure. When a connection closes before TiDB confirms the transaction outcome, do not retry non-idempotent operations blindly. Verify the outcome or use an idempotency mechanism before retrying. If your application retries an operation, use exponential backoff with jitter to reduce the additional load on TiKV. |
First-time contributors' checklist
What is changed, added, or deleted? (Required)
Add tidb_dml_max_execution_time system variable.
Which TiDB version(s) do your changes apply to? (Required)
Tips for choosing the affected version(s):
By default, CHOOSE MASTER ONLY so your changes will be applied to the next TiDB major or minor releases. If your PR involves a product feature behavior change or a compatibility change, CHOOSE THE AFFECTED RELEASE BRANCH(ES) AND MASTER.
For details, see tips for choosing the affected versions.
What is the related PR or file link(s)?
AI agent involvement
Do your changes match any of the following descriptions?
Summary by CodeRabbit
tidb_dml_max_execution_timesystem variable.SET_VARsupport.