stats: add defensive guard for interval calculation#65889
stats: add defensive guard for interval calculation#65889ti-chi-bot[bot] merged 1 commit intopingcap:masterfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #65889 +/- ##
================================================
+ Coverage 77.7820% 78.6447% +0.8626%
================================================
Files 2001 1932 -69
Lines 545618 537652 -7966
================================================
- Hits 424393 422835 -1558
+ Misses 119563 114372 -5191
+ Partials 1662 445 -1217
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
86fc05c to
2b18cf5
Compare
refactor: rename docs: add comments test: add more cases
5e8397b to
1365b14
Compare
There was a problem hiding this comment.
🔢 Self-check (PR reviewed by myself and ready for feedback)
-
Code compiles successfully
-
Unit tests added
-
All tests pass
-
Bazel files updated
-
Comments added where necessary
-
PR title and description updated
-
Documentation PR created (or confirmed not needed)
-
PR size is reasonable
There was a problem hiding this comment.
Pull request overview
This PR hardens the auto-analyze priority queue’s interval calculations against bad or skewed mysql.analyze_jobs duration records, preventing negative durations from corrupting retry/skip logic. It also clarifies the NoRecord sentinel semantics and adds regression tests around negative durations.
Changes:
- Update
GetAverageAnalysisDurationandGetLastFailedAnalysisDurationto treat negative durations as non-usable (eitherNoRecordor a bounded default wait time) instead of flowing them into scheduling logic. - Switch last-failed duration reading to a signed integer and document that both functions return
NoRecordwhen no usable history exists. - Add unit tests that insert synthetic negative-duration records to validate the new defensive behavior for both average and last-failed analysis durations.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
pkg/statistics/handle/autoanalyze/priorityqueue/interval.go |
Adds defensive guards for negative average/failed analysis durations, uses signed integer for TIMESTAMPDIFF results, and updates comments to state that NoRecord is returned when there is no usable history. |
pkg/statistics/handle/autoanalyze/priorityqueue/interval_test.go |
Extends tests to cover negative finished and failed job durations, ensuring they map to NoRecord or the default failed-analysis wait time instead of influencing skip logic. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: henrybw, terry1purcell The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
[LGTM Timeline notifier]Timeline:
|
|
/retest |
|
/test all |
1 similar comment
|
/test all |
|
In response to a cherrypick label: new pull request created to branch |
What problem does this PR solve?
Issue Number: close #65815
Problem Summary:
Negative durations from mysql.analyze_jobs (e.g. clock skew/bad records) can happen and should be handled defensively without skewing retry logic.
What changed and how does it work?
Based on #65821.
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.