metricsreader: move cluster-scoped metrics lifecycle into backend clusters#1105
metricsreader: move cluster-scoped metrics lifecycle into backend clusters#1105YangKeao wants to merge 1 commit intopingcap:mainfrom
Conversation
|
Skipping CI for Draft Pull Request. |
|
[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 |
7e1eaed to
5d2deeb
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1105 +/- ##
=======================================
Coverage ? 67.28%
=======================================
Files ? 145
Lines ? 15263
Branches ? 0
=======================================
Hits ? 10269
Misses ? 4297
Partials ? 697
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
5d2deeb to
1060582
Compare
|
/test all |
7bbf843 to
34cee09
Compare
|
/test all |
80b8d60 to
c81e4dc
Compare
c81e4dc to
89d650a
Compare
a8ab422 to
7add0f2
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 42467ff5a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| var vipEtcdCli *clientv3.Client | ||
| if cluster := srv.clusterManager.PrimaryCluster(); cluster != nil { | ||
| vipEtcdCli = cluster.EtcdClient() | ||
| } |
There was a problem hiding this comment.
Rebind VIP election client after cluster reconfiguration
The VIP manager is wired to a single etcd client captured only once at startup, but backend clusters are reloadable and syncClusters can replace and close the old cluster runtime (including its etcd client). In a single-cluster deployment with VIP enabled, updating backend-clusters (for example pd-addrs or ns-servers) can leave VIP election running on a closed client, after which the election loop exits and VIP failover no longer works until restart.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
For now, we don't support rebind VIP election client when the cluster comes down to 1.
There was a problem hiding this comment.
Also, for VIP deployment, we didn't expect to reload the clusters. However, it's a good suggestion to return error when the VIP has been set and the cluster is recreated!
1b299bc to
004d1fd
Compare
004d1fd to
26f0ec0
Compare
| func readerOwnerKeyPrefix(clusterName string) string { | ||
| clusterName = strings.TrimSpace(clusterName) | ||
| if clusterName == "" || clusterName == config.DefaultBackendClusterName { | ||
| return "/tiproxy/metric_reader" |
There was a problem hiding this comment.
Why remove the constant definitions?
What problem does this PR solve?
Issue Number: close #1099
What is changed and how it works:
Move backend metrics lifecycle to the cluster-scoped runtime introduced by the backend-cluster manager.
This PR changes metrics collection so that:
Check List
Tests
Notable changes
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.