-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
VTOrc: Cleanup node registration and unused code #15617
Conversation
Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Manan Gupta <[email protected]>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
@@ -21,7 +21,6 @@ import ( | |||
) | |||
|
|||
// CandidatePromotionRule describe the promotion preference/rule for an instance. | |||
// It maps to promotion_rule column in candidate_database_instance |
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.
I see we removed that table. Where does this map to now? Is it helpful to document that?
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.
Nothing, we don't store promotion rules in the tables anymore. They're gotten from the interface implementation.
@@ -87,9 +85,6 @@ func GetReplicationAnalysis(keyspace string, shard string, hints *ReplicationAna | |||
primary_instance.read_only AS read_only, | |||
MIN(primary_instance.gtid_errant) AS gtid_errant, | |||
MIN(primary_instance.alias) IS NULL AS is_invalid, | |||
MIN(primary_instance.data_center) AS data_center, |
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.
Can you explain why we don't need to store this any more? It's hard to see when you are reviewing a diff where we store the cell info which essentially provides the same functionality.
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.
We still store it, we just don't need to read it at this point, because it was unused. ERS code will read it anyways when it gets all the tablets from the shard.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #15617 +/- ##
==========================================
+ Coverage 68.06% 68.13% +0.06%
==========================================
Files 1561 1556 -5
Lines 195288 194976 -312
==========================================
- Hits 132927 132845 -82
+ Misses 62361 62131 -230 ☔ View full report in Codecov by Sentry. |
Description
This PR does more cleanup in VTOrc, removing most of the code around node health and election. This code was useful for orchestrator wherein multiple nodes would use the same MySQL instance.
Other unused code and fields have been removed as well as part of this PR.
Related Issue(s)
Checklist
Deployment Notes