-
Notifications
You must be signed in to change notification settings - Fork 6.1k
sessionctx: mark tidb_enable_auto_analyze_priority_queue as deprecated #62659
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR marks the tidb_enable_auto_analyze_priority_queue system variable as deprecated, enforcing that TiDB will always use a priority queue for auto-analyze scheduling.
- Changes the validation logic to reject attempts to disable the setting
- Adds deprecation comments to the variable definition
- Improves code formatting in the system variable definition
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/sessionctx/variable/sysvar.go | Updates validation logic to enforce priority queue usage and improves formatting |
| pkg/sessionctx/vardef/tidb_vars.go | Adds deprecation comment to the variable definition |
Comments suppressed due to low confidence (1)
pkg/sessionctx/variable/sysvar.go:1117
- [nitpick] The error message should be more consistent with typical deprecation patterns. Consider using 'Setting X to OFF is no longer supported' format for clarity.
return "", errors.New("tidb_enable_auto_analyze_priority_queue has been deprecated and TiDB will always use priority queue to schedule auto analyze.")
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #62659 +/- ##
================================================
+ Coverage 72.8568% 74.3460% +1.4892%
================================================
Files 1776 1822 +46
Lines 486675 507489 +20814
================================================
+ Hits 354576 377298 +22722
+ Misses 110465 107798 -2667
- Partials 21634 22393 +759
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
/test all |
Signed-off-by: 0xPoe <[email protected]>
…PriorityQueue Updated the validation function to correctly check for the deprecated setting, ensuring it returns an error when the option is not enabled. Signed-off-by: 0xPoe <[email protected]>
…_queue setting Enhanced the test for enabling the auto analyze priority queue to check for errors when the setting is set to OFF, confirming the deprecation message is returned as expected. Signed-off-by: 0xPoe <[email protected]>
…_queue Removed the trailing period from the deprecation error message in both the validation function and the corresponding test case to ensure consistency in error reporting. Signed-off-by: 0xPoe <[email protected]>
|
@0xPoe: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions 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. |
0xPoe
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔢 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 (pingcap/docs#21526)
-
PR size is reasonable
terry1purcell
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[LGTM Timeline notifier]Timeline:
|
yudongusa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also open a document PR.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: terry1purcell, time-and-fate, yudongusa The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What problem does this PR solve?
Issue Number: close None
Problem Summary:
What changed and how does it work?
Mark TiDBEnableAutoAnalyzePriorityQueue as deprecated.
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.