*: supports delete store label #5510

nolouch · 2022-09-15T10:17:28Z

Signed-off-by: nolouch nolouch@gmail.com

What problem does this PR solve?

Issue Number: Close #5509

What is changed and how does it work?

*: supports store command to delete the specified label

Check List

Tests

Unit test

Release note

store command of pdctl supports to delete the specified label for a store

ti-chi-bot · 2022-09-15T10:17:29Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

CabinfeverB
rleungx

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

codecov · 2022-09-15T10:29:39Z

Codecov Report

Base: 75.58% // Head: 75.48% // Decreases project coverage by -0.10% ⚠️

Coverage data is based on head (fa66b85) compared to base (c1f8731).
Patch coverage: 62.92% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5510      +/-   ##
==========================================
- Coverage   75.58%   75.48%   -0.11%     
==========================================
  Files         320      320              
  Lines       31769    31847      +78     
==========================================
+ Hits        24014    24041      +27     
- Misses       5691     5719      +28     
- Partials     2064     2087      +23

Flag	Coverage Δ
unittests	`75.48% <62.92%> (-0.11%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
server/api/store.go	`66.31% <31.25%> (-1.57%)`	⬇️
server/cluster/cluster.go	`82.82% <66.66%> (-0.35%)`	⬇️
tools/pd-ctl/pdctl/command/store_command.go	`64.90% <69.23%> (+0.36%)`	⬆️
server/api/router.go	`97.93% <100.00%> (+<0.01%)`	⬆️
server/config/util.go	`86.66% <100.00%> (+0.45%)`	⬆️
server/core/store.go	`82.43% <100.00%> (ø)`
pkg/tempurl/tempurl.go	`45.00% <0.00%> (-15.00%)`	⬇️
...erver/config/service_middleware_persist_options.go	`91.66% <0.00%> (-8.34%)`	⬇️
server/id/id.go	`83.05% <0.00%> (-6.78%)`	⬇️
server/storage/kv/etcd_kv.go	`79.71% <0.00%> (-5.80%)`	⬇️
... and 20 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Signed-off-by: nolouch <nolouch@gmail.com>

server/cluster/cluster.go

tools/pd-ctl/pdctl/command/store_command.go

Signed-off-by: nolouch <nolouch@gmail.com>

disksing · 2022-09-16T02:42:14Z

server/cluster/cluster.go

+		}
+		labels = append(labels, label)
+	}
+	if !hit {


I think we don't need hit, can check if len(labels) == len(newStore.Labels) instead.

server/cluster/cluster.go

nolouch · 2022-09-16T04:01:45Z

PTAL @rleungx @CabinfeverB

Signed-off-by: nolouch <nolouch@gmail.com>

nolouch · 2022-09-16T04:30:24Z

ptal @disksing

nolouch · 2022-09-16T06:15:30Z

/merge

ti-chi-bot · 2022-09-16T06:15:30Z

@nolouch: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

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 ti-community-infra/tichi repository.

ti-chi-bot · 2022-09-16T06:15:32Z

This pull request has been accepted and is ready to merge.

Commit hash: fa66b85

ref #5510, close #6467 #5510 introduced a bug that would cause the store labels to be overwritten wrongly after the store reboot. This PR fixed this issue. Signed-off-by: JmPotato <ghzpotato@gmail.com>

ref tikv#5510, close tikv#6467 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

…6470) ref #5510, close #6467, ref #6468 #5510 introduced a bug that would cause the store labels to be overwritten wrongly after the store reboot. This PR fixed this issue. Signed-off-by: JmPotato <ghzpotato@gmail.com> Co-authored-by: JmPotato <ghzpotato@gmail.com>

…6469) ref #5510, close #6467, ref #6468 #5510 introduced a bug that would cause the store labels to be overwritten wrongly after the store reboot. This PR fixed this issue. Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io> Signed-off-by: JmPotato <ghzpotato@gmail.com> Co-authored-by: JmPotato <ghzpotato@gmail.com>

ti-chi-bot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Sep 15, 2022

ti-chi-bot requested review from disksing and rleungx September 15, 2022 10:17

nolouch force-pushed the label-delete branch from f8405f7 to 7e52960 Compare September 15, 2022 10:19

nolouch force-pushed the label-delete branch from 7e52960 to 161ee36 Compare September 15, 2022 10:30

*: supports delete store label

23d9cc2

Signed-off-by: nolouch <nolouch@gmail.com>

nolouch force-pushed the label-delete branch from e92f535 to 23d9cc2 Compare September 15, 2022 10:41

CabinfeverB reviewed Sep 15, 2022

View reviewed changes

server/cluster/cluster.go Outdated Show resolved Hide resolved

rleungx reviewed Sep 15, 2022

View reviewed changes

nolouch added 2 commits September 15, 2022 19:32

address comments

82b7455

Signed-off-by: nolouch <nolouch@gmail.com>

Merge branch 'master' into label-delete

bdf3030

disksing reviewed Sep 16, 2022

View reviewed changes

nolouch added 3 commits September 16, 2022 12:23

Merge branch 'master' into label-delete

99238b7

address comments

6458efd

Signed-off-by: nolouch <nolouch@gmail.com>

address comments

fa66b85

Signed-off-by: nolouch <nolouch@gmail.com>

rleungx approved these changes Sep 16, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 16, 2022

CabinfeverB approved these changes Sep 16, 2022

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Sep 16, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Sep 16, 2022

nolouch mentioned this pull request Sep 16, 2022

pdctl: add store label and restore store command documents pingcap/docs-cn#11318

Merged

13 tasks

ti-chi-bot merged commit eeb5d3f into tikv:master Sep 16, 2022

ti-chi-bot pushed a commit to ti-chi-bot/pd that referenced this pull request May 15, 2023

This is an automated cherry-pick of tikv#6468

234c928

ref tikv#5510, close tikv#6467 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: supports delete store label #5510

*: supports delete store label #5510

nolouch commented Sep 15, 2022 •

edited

Loading

ti-chi-bot commented Sep 15, 2022 •

edited

Loading

codecov bot commented Sep 15, 2022 •

edited

Loading

disksing Sep 16, 2022

nolouch commented Sep 16, 2022

nolouch commented Sep 16, 2022

nolouch commented Sep 16, 2022

ti-chi-bot commented Sep 16, 2022

ti-chi-bot commented Sep 16, 2022

*: supports delete store label #5510

*: supports delete store label #5510

Conversation

nolouch commented Sep 15, 2022 • edited Loading

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

ti-chi-bot commented Sep 15, 2022 • edited Loading

codecov bot commented Sep 15, 2022 • edited Loading

Codecov Report

disksing Sep 16, 2022

Choose a reason for hiding this comment

nolouch commented Sep 16, 2022

nolouch commented Sep 16, 2022

nolouch commented Sep 16, 2022

ti-chi-bot commented Sep 16, 2022

ti-chi-bot commented Sep 16, 2022

nolouch commented Sep 15, 2022 •

edited

Loading

ti-chi-bot commented Sep 15, 2022 •

edited

Loading

codecov bot commented Sep 15, 2022 •

edited

Loading