storage: Add APIs for RawKV GC by AmoebaProtozoa · Pull Request #4937 · tikv/pd

AmoebaProtozoa · 2022-05-12T09:03:27Z

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

What problem does this PR solve?

Add APIs for RawKV GC (storage part of #4866).
Issue Number: ref #4865
RFC: tikv/rfcs#90

What is changed and how does it work?

Add APIs for RawKV GC.

Check List

Tests

Unit test

Code changes

Has configuration change
No
Has HTTP API interfaces changed (Don't forget to add the declarative for the new API)
No
Has persistent data change
No

Side effects
No

Release note

Add APIs for RawKV GC.

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

ti-chi-bot · 2022-05-12T09:03:28Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

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

AmoebaProtozoa · 2022-05-12T09:08:01Z

/cc @nolouch @AndreMouche
PTAL, thanks!

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

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

pingyu

LGTM~

ti-chi-bot · 2022-05-13T04:18:05Z

@pingyu: Thanks for your review. The bot only counts approvals from reviewers and higher roles in list, but you're still welcome to leave your comments.

Details

In response to this:

LGTM~

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.

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

AndreMouche

Rest LGTM

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

nolouch · 2022-05-16T12:11:01Z

+}
+
+// KeySpaceServiceSafePointPath returns the path of given service's service safe point.
+// Path: /key_space/gc_safepoint/{space_id}/service/{service_id}


Can we know the endpoint types from service_id? such as it's from tidb, raw_kv client, cdc or others?

I don't think we can tell by service_id alone,
for example, both TiDB and raw_kv's gc_worker may use gc_worker as their service_id

So... Is it different in space_id? like cdc_xxx, tidb_xxx, client_xxxx . I would like to know who use the path.

currently, we use spaceID to differentiate key spaces, things like
default_rawkv, default_txnkv, default_tidb
serviceID is used to identify the specific service under that KeySpace, like
cdc, br, etc.

tidb cdc and rawkv cdc may have the same serviceID but different spaceIDs
rawkv cdc and rawkv br have the same spaceID but different serviceIDs

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

AndreMouche

LGTM

codecov · 2022-05-17T06:27:35Z

Codecov Report

Merging #4937 (d8dd130) into master (4a9d7c0) will decrease coverage by 0.13%.
The diff coverage is 68.75%.

@@            Coverage Diff             @@
##           master    #4937      +/-   ##
==========================================
- Coverage   75.57%   75.43%   -0.14%     
==========================================
  Files         307      308       +1     
  Lines       30201    30281      +80     
==========================================
+ Hits        22823    22842      +19     
- Misses       5396     5453      +57     
- Partials     1982     1986       +4

Flag	Coverage Δ
unittests	`75.43% <68.75%> (-0.14%)`	⬇️

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

Impacted Files	Coverage Δ
server/storage/storage.go	`70.27% <ø> (+5.40%)`	⬆️
server/storage/endpoint/gc_key_space.go	`66.66% <66.66%> (ø)`
server/storage/endpoint/key_path.go	`100.00% <100.00%> (ø)`
pkg/errs/errs.go	`75.00% <0.00%> (-25.00%)`	⬇️
pkg/dashboard/adapter/manager.go	`74.71% <0.00%> (-11.50%)`	⬇️
server/tso/allocator_manager.go	`59.73% <0.00%> (-3.33%)`	⬇️
server/region_syncer/server.go	`83.15% <0.00%> (-3.27%)`	⬇️
server/election/lease.go	`84.05% <0.00%> (-2.90%)`	⬇️
server/schedule/operator_controller.go	`84.56% <0.00%> (-1.21%)`	⬇️
server/schedulers/evict_leader.go	`78.82% <0.00%> (-0.91%)`	⬇️
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4a9d7c0...d8dd130. Read the comment docs.

nolouch

LGTM. better merge it to master after release-6.1 checkout.

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

AndreMouche · 2022-05-19T06:22:23Z

Could we merge this PR?

nolouch · 2022-05-25T04:25:43Z

We can merge it now, cc @AndreMouche

AndreMouche

LGTM

nolouch · 2022-05-26T07:46:55Z

/merge

ti-chi-bot · 2022-05-26T07:46:56Z

@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 · 2022-05-26T07:46:58Z

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

Details

Commit hash: 769a40b

added storage methods for RawKV GC

32c44ee

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

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

ti-chi-bot requested review from disksing and rleungx May 12, 2022 09:03

ti-chi-bot requested review from AndreMouche and nolouch May 12, 2022 09:08

AmoebaProtozoa added 4 commits May 12, 2022 19:38

push back on updating kvproto in go.mod

b12eb9b

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

linting

d0b4f3b

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

changed storage path structure

a2ba0e7

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

update comments

b9bb3e4

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

pingyu reviewed May 12, 2022

View reviewed changes

Comment thread server/storage/endpoint/gc_safe_point.go Outdated

Comment thread server/storage/endpoint/key_path.go Outdated

Comment thread server/storage/endpoint/gc_key_space.go Outdated

added ByKeySpace suffix for disambiguity

a7e3ece

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

pingyu reviewed May 13, 2022

View reviewed changes

Comment thread server/storage/endpoint/gc_key_space.go Outdated

removed default key spaces

7ff125e

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

pingyu approved these changes May 13, 2022

View reviewed changes

Merge branch 'master' into RawKV_GC_API_storage

1bdb642

AmoebaProtozoa mentioned this pull request May 13, 2022

Add PD APIs for RawKV GC #4865

Closed

5 tasks

AndreMouche reviewed May 13, 2022

View reviewed changes

AmoebaProtozoa added 3 commits May 16, 2022 14:36

renaming, move delete expired safepoints to a goroutine

1ebb51d

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

update tests

312704a

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

lint

766b344

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

AndreMouche reviewed May 16, 2022

View reviewed changes

Comment thread server/storage/endpoint/gc_key_space.go Outdated

Comment thread server/storage/endpoint/gc_key_space.go Outdated

AmoebaProtozoa added 2 commits May 16, 2022 16:45

added back KeySpaceGCSafePoint

de77dfc

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

remove expired all at once

d6eda93

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

AndreMouche reviewed May 16, 2022

View reviewed changes

Comment thread server/storage/endpoint/gc_key_space.go Outdated

Comment thread server/storage/endpoint/gc_key_space.go

AmoebaProtozoa added 2 commits May 16, 2022 17:43

address comments

cbe1ed0

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

Merge branch 'master' into RawKV_GC_API_storage

9b27bcd

AndreMouche reviewed May 16, 2022

View reviewed changes

Comment thread server/storage/storage_gc_test.go Outdated

move sleep to failpoint

88610a0

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

nolouch reviewed May 16, 2022

View reviewed changes

AndreMouche reviewed May 17, 2022

View reviewed changes

Comment thread server/storage/endpoint/gc_key_space.go Outdated

Comment thread server/storage/endpoint/gc_key_space.go Outdated

Comment thread server/storage/storage_gc_test.go Outdated

Comment thread server/storage/storage_gc_test.go Outdated

AmoebaProtozoa added 2 commits May 17, 2022 11:13

modified failpoint to eliminate sleep

995033e

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

Merge branch 'master' into RawKV_GC_API_storage

3099939

AndreMouche reviewed May 17, 2022

View reviewed changes

rleungx reviewed May 17, 2022

View reviewed changes

Comment thread server/storage/endpoint/gc_key_space.go Outdated

nolouch approved these changes May 18, 2022

View reviewed changes

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

log error when failed to remove expired service safe point

a825d37

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

AmoebaProtozoa mentioned this pull request May 18, 2022

storage: Use etcd lease to manage safe point expiration #4984

Closed

AmoebaProtozoa closed this May 19, 2022

AmoebaProtozoa reopened this May 19, 2022

Merge branch 'master' into RawKV_GC_API_storage

c1cb1c0

AndreMouche approved these changes May 26, 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 May 26, 2022

Merge branch 'master' into RawKV_GC_API_storage

769a40b

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

Merge branch 'master' into RawKV_GC_API_storage

d8dd130

ti-chi-bot merged commit 86a6821 into tikv:master May 26, 2022

Uh oh!

Conversation

AmoebaProtozoa commented May 12, 2022

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

Uh oh!

ti-chi-bot commented May 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AmoebaProtozoa commented May 12, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pingyu left a comment

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot commented May 13, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AndreMouche left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nolouch May 16, 2022

Choose a reason for hiding this comment

Uh oh!

AmoebaProtozoa May 16, 2022

Choose a reason for hiding this comment

Uh oh!

nolouch May 17, 2022

Choose a reason for hiding this comment

Uh oh!

AmoebaProtozoa May 17, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AndreMouche left a comment

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented May 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

nolouch left a comment

Choose a reason for hiding this comment

Uh oh!

AndreMouche commented May 19, 2022

Uh oh!

nolouch commented May 25, 2022

Uh oh!

AndreMouche left a comment

Choose a reason for hiding this comment

Uh oh!

nolouch commented May 26, 2022

Uh oh!

ti-chi-bot commented May 26, 2022

Uh oh!

ti-chi-bot commented May 26, 2022

Uh oh!

Reviewers

Assignees

ti-chi-bot commented May 12, 2022 •

edited

Loading

codecov Bot commented May 17, 2022 •

edited

Loading