keypath: fix incorrect key #9133
Conversation
Signed-off-by: okJiang <[email protected]>
|
Skipping CI for Draft Pull Request. |
ti-chi-bot
bot
added
dco-signoff: yes
ti-chi-bot
bot
added
size/M
| // NOTE: schedulerConfigPathPrefix is "/pd/{cluster_id}/scheduler_config" before. | ||
| // Now it is "/pd/{cluster_id}/scheduler_config/". I think This has no impact. | ||
| // If it needs to be fixed, I can update it. |
There was a problem hiding this comment.
These comment will be removed after review
There was a problem hiding this comment.
I think it's best to remain consistent with what it was before.
There was a problem hiding this comment.
The previous commit had issues, so I submitted a new one. 6b83724
|
/retest |
| // NOTE: schedulerConfigPathPrefix is "/pd/{cluster_id}/scheduler_config" before. | ||
| // Now it is "/pd/{cluster_id}/scheduler_config/". I think This has no impact. | ||
| // If it needs to be fixed, I can update it. |
There was a problem hiding this comment.
I think it's best to remain consistent with what it was before.
| @@ -238,6 +238,9 @@ func (rw *Watcher) initializeRegionLabelWatcher() error { | |||
| rw.labelWatcher = etcdutil.NewLoopWatcher( | |||
| rw.ctx, &rw.wg, | |||
| rw.etcdClient, | |||
| // NOTE: regionLabelPathPrefix is "/pd/{cluster_id}/region_label" before. | |||
| // Now it is "/pd/{cluster_id}/region_label/". I think This has no impact. | |||
| // If it needs to be fixed, I can update it. | |||
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #9133 +/- ##
==========================================
+ Coverage 76.31% 76.33% +0.02%
==========================================
Files 473 473
Lines 71975 71981 +6
==========================================
+ Hits 54926 54946 +20
+ Misses 13612 13591 -21
- Partials 3437 3444 +7
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
| @@ -109,15 +109,15 @@ func (suite *keyspaceGroupManagerTestSuite) TestDeletedGroupCleanup() { | |||
| suite.applyEtcdEvents(re, []*etcdEvent{generateKeyspaceGroupPutEvent(1, []uint32{1}, []string{svcAddr})}) | |||
| // Check if the TSO key is created. | |||
| testutil.Eventually(re, func() bool { | |||
| ts, err := mgr.storage.LoadTimestamp(keypath.Prefix(keypath.TimestampPath(1))) | |||
| ts, err := mgr.storage.LoadTimestamp(keypath.TimestampPath(1)) | |||
There was a problem hiding this comment.
Do we need to add some other test to avoid the similiar problems?
There was a problem hiding this comment.
I think #9126 will be a stronger test to avoid this.
There was a problem hiding this comment.
BTW, the reason that we do not detect the problem of LoadTimestamp is the non-timestamp key has been skipped in this. So it will not cause any actual damage. 😂
pd/pkg/storage/endpoint/tso.go
Lines 56 to 60 in e245c1f
ti-chi-bot
bot
added
needs-1-more-lgtm
Signed-off-by: okJiang <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JmPotato, lhy1024 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 |
|
In response to a cherrypick label: new pull request created to branch |
close #9132 Signed-off-by: okJiang <[email protected]> Co-authored-by: okJiang <[email protected]>
What problem does this PR solve?
Issue Number: Close #9132
What is changed and how does it work?
Check List
Tests
ref #9131
Release note