keyspace: Refactor keyspace manager using RunInTxn #5843

AmoebaProtozoa · 2023-01-09T12:16:05Z

What problem does this PR solve?

Issue Number: Ref #5293

Previous PR #5347 added ability to batch multiple kv operations into a single RunInTxn style execution block, this PR uses that to refactor Keyspace manager.

In addition, a new keyspace state tombstone is added to denote that user data related to keyspace has been completely removed from storage.

It also updates kvproto to pingcap/kvproto#1033

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>

# Conflicts: # pkg/storage/kv/etcd_kv.go # pkg/storage/kv/levedb_kv.go

Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>

ti-chi-bot · 2023-01-09T12:16:06Z

[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.

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

ti-chi-bot · 2023-01-09T12:16:14Z

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.

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.

…anager

Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>

codecov · 2023-01-09T12:49:34Z

Codecov Report

Base: 74.85% // Head: 75.48% // Increases project coverage by +0.62% 🎉

Coverage data is based on head (c9b07ba) compared to base (59a6203).
Patch coverage: 68.80% of modified lines in pull request are covered.

❗ Current head c9b07ba differs from pull request most recent head 07f70cd. Consider uploading reports for the commit 07f70cd to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5843      +/-   ##
==========================================
+ Coverage   74.85%   75.48%   +0.62%     
==========================================
  Files         337      338       +1     
  Lines       33941    33983      +42     
==========================================
+ Hits        25406    25651     +245     
+ Misses       6351     6126     -225     
- Partials     2184     2206      +22

Flag	Coverage Δ
unittests	`75.48% <68.80%> (+0.62%)`	⬆️

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

Impacted Files	Coverage Δ
client/metrics.go	`100.00% <ø> (ø)`
server/server.go	`75.12% <33.33%> (+0.41%)`	⬆️
client/keyspace_client.go	`63.63% <66.66%> (+1.41%)`	⬆️
server/keyspace/keyspace.go	`70.38% <67.27%> (+5.58%)`	⬆️
server/keyspace_service.go	`61.19% <71.42%> (+5.63%)`	⬆️
pkg/storage/endpoint/keyspace.go	`74.35% <76.92%> (-5.65%)`	⬇️
server/keyspace/util.go	`85.71% <100.00%> (ø)`
pkg/dashboard/adapter/manager.go	`81.60% <0.00%> (-6.90%)`	⬇️
server/tso/tso.go	`70.22% <0.00%> (-1.69%)`	⬇️
pkg/utils/etcdutil/etcdutil.go	`84.88% <0.00%> (-1.17%)`	⬇️
... and 32 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: David <8039876+AmoebaProtozoa@users.noreply.github.com>

server/server.go

Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>

client/keyspace_client.go

Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>

nolouch

LGTM

nolouch · 2023-01-11T08:39:21Z

client/keyspace_client.go

+//
+//	ENABLED -> {ENABLED, DISABLED}
+//	DISABLED -> {ENABLED, DISABLED, ARCHIVED}
+//	ARCHIVED -> {ARCHIVED, TOMBSTONE}


What the different between the {ARCHIVED, TOMBSTONE}?

Archive is used to denote that keyspace's corresponding data in store nodes are safe to remove/ currently being removed (or being archived into cold storage somewhere else), a point of no return for the keyspace (as oppose to disabled which means data are still intact). This tells gc worker to keep cleaning up data belong to this keyspace.

Tombstone is used to denote that keyspace's data in storage has been completely removed, gc worker can safely skip gcing this keyspace

disksing · 2023-01-11T09:08:25Z

/merge

ti-chi-bot · 2023-01-11T09:08:27Z

@disksing: 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 · 2023-01-11T09:08:29Z

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

Commit hash: c9b07ba

disksing · 2023-01-11T09:28:20Z

/test build

…anager

Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>

disksing · 2023-01-11T09:44:45Z

/merge

ti-chi-bot · 2023-01-11T09:44:47Z

@disksing: 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 · 2023-01-11T09:44:48Z

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

Commit hash: 07f70cd

disksing · 2023-01-11T09:44:56Z

/test run

ti-chi-bot · 2023-01-11T09:44:58Z

@disksing: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test build

Use /test all to run all jobs.

In response to this:

/test run

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.

disksing · 2023-01-11T09:45:11Z

/test build

AmoebaProtozoa added 10 commits July 22, 2022 14:13

added RunInTxn style interface to base kv

dec67b5

Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>

added unit tests

898b2d1

Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>

update comments

8c61ff2

Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>

Merge branch 'master' into RunInTxn

cf54caf

Merge remote-tracking branch 'upstream/master' into RunInTxn

0ad3539

# Conflicts: # pkg/storage/kv/etcd_kv.go # pkg/storage/kv/levedb_kv.go

sort import

be81039

Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>

Merge branch 'master' into RunInTxn

d98d3d2

pin create revision when key not exist

149ce0c

Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>

init commit

d5f462e

Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>

fix test

dfaa8cd

Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>

ti-chi-bot requested review from disksing and HunDunDM January 9, 2023 12:16

AmoebaProtozoa added 2 commits January 9, 2023 20:28

Merge remote-tracking branch 'upstream/master' into RefactorKeyspaceM…

3106c83

…anager

fix test

b65de48

Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>

AmoebaProtozoa mentioned this pull request Jan 10, 2023

implement keyspace management #5293

Closed

11 tasks

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 added 2 commits January 10, 2023 15:20

update kvproto

000e450

Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>

Merge branch 'master' into RefactorKeyspaceManager

3fbf9d5

AmoebaProtozoa changed the title ~~[DNM] keyspace: Refactor keyspace manager using RunInTxn~~ keyspace: Refactor keyspace manager using RunInTxn Jan 10, 2023

update pd-tso-bench go sum

fed5880

Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>

AmoebaProtozoa marked this pull request as ready for review January 10, 2023 08:46

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

Merge branch 'master' into RefactorKeyspaceManager

e474b76

disksing reviewed Jan 11, 2023

View reviewed changes

server/server.go Outdated Show resolved Hide resolved

log error when fail to bootstrap keyspace manager

74e1d66

Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>

ystaticy reviewed Jan 11, 2023

View reviewed changes

client/keyspace_client.go Show resolved Hide resolved

disksing approved these changes Jan 11, 2023

View reviewed changes

added comments

c9b07ba

Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>

nolouch approved these changes Jan 11, 2023

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Jan 11, 2023

disksing approved these changes Jan 11, 2023

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 Jan 11, 2023

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jan 11, 2023

Merge branch 'master' into RefactorKeyspaceManager

371c6c1

Merge remote-tracking branch 'upstream/master' into RefactorKeyspaceM…

0305e2e

…anager

ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Jan 11, 2023

go mod tidy msc

07f70cd

Signed-off-by: David <8039876+AmoebaProtozoa@users.noreply.github.com>

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jan 11, 2023

ti-chi-bot merged commit bac5bf4 into tikv:master Jan 11, 2023

AmoebaProtozoa deleted the RefactorKeyspaceManager branch January 11, 2023 09:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

keyspace: Refactor keyspace manager using RunInTxn #5843

keyspace: Refactor keyspace manager using RunInTxn #5843

AmoebaProtozoa commented Jan 9, 2023 •

edited

Loading

ti-chi-bot commented Jan 9, 2023 •

edited

Loading

ti-chi-bot commented Jan 9, 2023

codecov bot commented Jan 9, 2023 •

edited

Loading

nolouch left a comment

nolouch Jan 11, 2023

AmoebaProtozoa Jan 11, 2023

disksing commented Jan 11, 2023

ti-chi-bot commented Jan 11, 2023

ti-chi-bot commented Jan 11, 2023

disksing commented Jan 11, 2023

disksing commented Jan 11, 2023

ti-chi-bot commented Jan 11, 2023

ti-chi-bot commented Jan 11, 2023

disksing commented Jan 11, 2023

ti-chi-bot commented Jan 11, 2023

disksing commented Jan 11, 2023

keyspace: Refactor keyspace manager using RunInTxn #5843

keyspace: Refactor keyspace manager using RunInTxn #5843

Conversation

AmoebaProtozoa commented Jan 9, 2023 • edited Loading

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

ti-chi-bot commented Jan 9, 2023 • edited Loading

ti-chi-bot commented Jan 9, 2023

codecov bot commented Jan 9, 2023 • edited Loading

Codecov Report

nolouch left a comment

Choose a reason for hiding this comment

nolouch Jan 11, 2023

Choose a reason for hiding this comment

AmoebaProtozoa Jan 11, 2023

Choose a reason for hiding this comment

disksing commented Jan 11, 2023

ti-chi-bot commented Jan 11, 2023

ti-chi-bot commented Jan 11, 2023

disksing commented Jan 11, 2023

disksing commented Jan 11, 2023

ti-chi-bot commented Jan 11, 2023

ti-chi-bot commented Jan 11, 2023

disksing commented Jan 11, 2023

ti-chi-bot commented Jan 11, 2023

disksing commented Jan 11, 2023

AmoebaProtozoa commented Jan 9, 2023 •

edited

Loading

ti-chi-bot commented Jan 9, 2023 •

edited

Loading

codecov bot commented Jan 9, 2023 •

edited

Loading