Skip to content

keyspace: add region label when creating keyspace#5844

Merged
disksing merged 17 commits intotikv:masterfrom
AmoebaProtozoa:KeyspaceRegionLabel
Jan 12, 2023
Merged

keyspace: add region label when creating keyspace#5844
disksing merged 17 commits intotikv:masterfrom
AmoebaProtozoa:KeyspaceRegionLabel

Conversation

@AmoebaProtozoa
Copy link
Contributor

@AmoebaProtozoa AmoebaProtozoa commented Jan 9, 2023

What problem does this PR solve?

Issue Number: Ref #5293

When a new keyspace is being created, we also create a set of region labels to makes sure two keyspace's data will be located in different region.

What is changed and how does it work?

Check List

Tests

  • Unit test
  • Integration test

Release note

None.

Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>
Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>
Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>
# Conflicts:
#	pkg/storage/kv/etcd_kv.go
#	pkg/storage/kv/levedb_kv.go
Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>
Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>
Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>
Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>
Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>
Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Jan 9, 2023

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • disksing
  • nolouch

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.

Details

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

@ti-chi-bot ti-chi-bot added do-not-merge/needs-linked-issue do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. needs-ok-to-test Indicates a PR created by contributors and need ORG member send '/ok-to-test' to start testing. labels Jan 9, 2023
@ti-chi-bot
Copy link
Member

Hi @AmoebaProtozoa. Thanks for your PR.

I'm waiting for a tikv member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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/test-infra repository.

@ti-chi-bot ti-chi-bot requested review from disksing and nolouch January 9, 2023 16:36
@codecov
Copy link

codecov bot commented Jan 9, 2023

Codecov Report

Base: 74.96% // Head: 74.94% // Decreases project coverage by -0.01% ⚠️

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

❗ Current head b43b58b differs from pull request most recent head 55a94eb. Consider uploading reports for the commit 55a94eb to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5844      +/-   ##
==========================================
- Coverage   74.96%   74.94%   -0.02%     
==========================================
  Files         337      337              
  Lines       34065    34116      +51     
==========================================
+ Hits        25536    25569      +33     
- Misses       6341     6353      +12     
- Partials     2188     2194       +6     
Flag Coverage Δ
unittests 74.94% <80.35%> (-0.02%) ⬇️

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

Impacted Files Coverage Δ
server/keyspace/keyspace.go 69.02% <55.00%> (-1.37%) ⬇️
server/server.go 75.48% <60.00%> (+1.02%) ⬆️
server/keyspace/util.go 95.23% <100.00%> (+9.52%) ⬆️
tests/cluster.go 79.20% <100.00%> (+0.20%) ⬆️
pkg/dashboard/adapter/manager.go 74.71% <0.00%> (-6.90%) ⬇️
server/tso/tso.go 68.53% <0.00%> (-5.06%) ⬇️
server/region_syncer/server.go 81.86% <0.00%> (-4.40%) ⬇️
pkg/utils/etcdutil/etcdutil.go 82.55% <0.00%> (-3.49%) ⬇️
pkg/storage/kv/etcd_kv.go 72.22% <0.00%> (-3.18%) ⬇️
pkg/member/member.go 62.10% <0.00%> (-1.58%) ⬇️
... and 22 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.

@AmoebaProtozoa AmoebaProtozoa mentioned this pull request Jan 10, 2023
11 tasks
@ti-chi-bot ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/needs-linked-issue do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jan 10, 2023
@AmoebaProtozoa AmoebaProtozoa changed the title [DNM] Keyspace region label keyspace: add region label when creating keyspace Jan 10, 2023
Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>
@AmoebaProtozoa AmoebaProtozoa marked this pull request as ready for review January 10, 2023 08:46
@ti-chi-bot ti-chi-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 10, 2023
Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>
@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Jan 12, 2023
@ti-chi-bot 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 Jan 12, 2023
@nolouch
Copy link
Contributor

nolouch commented Jan 12, 2023

/merge

@ti-chi-bot
Copy link
Member

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

/run-all-tests

Details

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
Copy link
Member

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

DetailsCommit hash: b43b58b

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jan 12, 2023
@ti-chi-bot
Copy link
Member

@AmoebaProtozoa: Your PR was out of date, I have automatically updated it for you.

If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

Details

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.

@disksing
Copy link
Contributor

/test build

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ok-to-test Indicates a PR created by contributors and need ORG member send '/ok-to-test' to start testing. release-note-none Denotes a PR that doesn't merit a release note. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants