raftstore-v2: clean up import sst file only if flushed epoch is stale. #15064

bufferflies · 2023-07-04T06:01:27Z

What is changed and how it works?

Issue Number: Close #15018

What's Changed:

1.  delete sst by using flushed epoch  not raft epoch to avoid replay this log（flushed epoch is the epoch in raft enginem, which is related with flushed admin index）
2. delete sst divide into two place：
   a. cf flush, the cf flushed index is bigger than the sst applied index 
   b. CleanUpSstTick, the sst epoch is stable than the flushed epoch.

Related changes

PR to update pingcap/docs/pingcap/docs-cn:
Need to cherry-pick to the release branch

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)

Side effects

Release note

raftstore-v2: clean up import sst file only if flushed epoch is stale.

Signed-off-by: bufferflies <1045931706@qq.com>

ti-chi-bot · 2023-07-04T06:01:29Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

SpadeA-Tang
overvenus

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.

Signed-off-by: bufferflies <1045931706@qq.com>

components/engine_traits/src/flush.rs

components/raftstore-v2/src/operation/ready/apply_trace.rs

components/engine_traits/src/flush.rs

components/raftstore-v2/src/raft/storage.rs

components/raftstore-v2/src/operation/ready/snapshot.rs

components/raftstore-v2/src/operation/ready/apply_trace.rs

Signed-off-by: bufferflies <1045931706@qq.com>

components/raftstore-v2/src/operation/ready/apply_trace.rs

Signed-off-by: bufferflies <1045931706@qq.com>

3pointer · 2023-07-06T10:59:48Z

components/raftstore-v2/src/raft/storage.rs

@@ -183,6 +199,7 @@ impl<EK: KvEngine, ER: RaftEngine> Storage<EK, ER> {
            gen_snap_task: RefCell::new(Box::new(None)),
            split_init: None,
            apply_trace,
+            flushed_epoch: epoch,


It seems this epoch is the newly epoch and irrelevant with flush.
Will it delete the file before ingest replayed after restart?

Q1: epoch refresh only if trace.admin.index has writed in the disk.
Q2: sst will be delete if the sst's epoch is stale than the flush epoch

Signed-off-by: bufferflies <1045931706@qq.com>

bufferflies · 2023-07-10T02:21:18Z

/ping @overvenus @3pointer

components/raftstore-v2/src/operation/command/write/ingest.rs

components/raftstore-v2/src/operation/ready/apply_trace.rs

Signed-off-by: bufferflies <1045931706@qq.com>

pingyu · 2023-07-11T02:33:34Z

/merge

ti-chi-bot · 2023-07-11T02:33:36Z

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

/run-all-tests

You only need to trigger /merge once, and 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.

If you have any questions about the PR merge process, please refer to pr process.

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-07-11T02:33:38Z

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

Commit hash: 725351a

ti-chi-bot · 2023-07-11T02:33:51Z

@bufferflies: 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.

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.

bufferflies · 2023-07-11T03:46:11Z

/test

ti-chi-bot · 2023-07-11T03:55:22Z

In response to a cherrypick label: new pull request created to branch release-7.1: #15101.

close tikv#15018 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

delete sst

a6e8c36

Signed-off-by: bufferflies <1045931706@qq.com>

ti-chi-bot bot added release-note do-not-merge/work-in-progress needs-cherry-pick-release-7.1 size/XL labels Jul 4, 2023

bufferflies force-pushed the delete_sst branch from 5f8f377 to 6811a9d Compare July 4, 2023 06:02

check epoch epoch

c10fe42

Signed-off-by: bufferflies <1045931706@qq.com>

bufferflies force-pushed the delete_sst branch from 6811a9d to c10fe42 Compare July 4, 2023 06:03

Merge branch 'master' into delete_sst

eb39227

bufferflies marked this pull request as ready for review July 4, 2023 06:05

ti-chi-bot bot removed the do-not-merge/work-in-progress label Jul 4, 2023

bufferflies mentioned this pull request Jul 4, 2023

raftstore-v2: clean up import sst file only if the flushed index is greater than the applied index #15032

Closed

fix lint

9a77413

Signed-off-by: bufferflies <1045931706@qq.com>

SpadeA-Tang reviewed Jul 4, 2023

View reviewed changes

components/engine_traits/src/flush.rs Outdated Show resolved Hide resolved

SpadeA-Tang reviewed Jul 4, 2023

View reviewed changes

components/raftstore-v2/src/operation/ready/apply_trace.rs Outdated Show resolved Hide resolved

ti-chi-bot bot added the needs-rebase label Jul 5, 2023

overvenus reviewed Jul 5, 2023

View reviewed changes

flushed after persist

4abe57c

Signed-off-by: bufferflies <1045931706@qq.com>

ti-chi-bot bot removed the needs-rebase label Jul 5, 2023

SpadeA-Tang reviewed Jul 5, 2023

View reviewed changes

components/raftstore-v2/src/operation/ready/apply_trace.rs Outdated Show resolved Hide resolved

SpadeA-Tang reviewed Jul 5, 2023

View reviewed changes

components/raftstore-v2/src/operation/ready/apply_trace.rs Outdated Show resolved Hide resolved

bufferflies added 4 commits July 6, 2023 14:22

fix commend

6827dae

Signed-off-by: bufferflies <1045931706@qq.com>

Merge branch 'master' into delete_sst

37dc0fb

replace vec by map

e649feb

Signed-off-by: bufferflies <1045931706@qq.com>

Merge branch 'delete_sst' of github.com:bufferflies/tikv into delete_sst

ae35eff

3pointer reviewed Jul 6, 2023

View reviewed changes

bufferflies requested review from SpadeA-Tang and overvenus July 7, 2023 01:45

Merge branch 'master' into delete_sst

d80f59f

SpadeA-Tang approved these changes Jul 7, 2023

View reviewed changes

ti-chi-bot bot added the status/LGT1 Status: PR - There is already 1 approval label Jul 7, 2023

bufferflies added 2 commits July 7, 2023 17:44

use default

2e9901c

Signed-off-by: bufferflies <1045931706@qq.com>

Merge branch 'delete_sst' of github.com:bufferflies/tikv into delete_sst

9cf01f5

bufferflies requested a review from 3pointer July 10, 2023 02:20

Merge branch 'master' into delete_sst

9c6c74f

overvenus reviewed Jul 10, 2023

View reviewed changes

components/raftstore-v2/src/operation/command/write/ingest.rs Outdated Show resolved Hide resolved

components/raftstore-v2/src/operation/ready/apply_trace.rs Show resolved Hide resolved

bufferflies added 2 commits July 10, 2023 14:10

unify log

d65da38

Signed-off-by: bufferflies <1045931706@qq.com>

Merge branch 'delete_sst' of github.com:bufferflies/tikv into delete_sst

529fe6c

overvenus approved these changes Jul 10, 2023

View reviewed changes

ti-chi-bot bot added status/LGT2 Status: PR - There are already 2 approvals and removed status/LGT1 Status: PR - There is already 1 approval labels Jul 10, 2023

pass ut and add comment for flush epoch

725351a

Signed-off-by: bufferflies <1045931706@qq.com>

ti-chi-bot bot added the status/can-merge Status: Can merge to base branch label Jul 11, 2023

Merge branch 'master' into delete_sst

170ba4a

ti-chi-bot bot merged commit 1d60d57 into tikv:master Jul 11, 2023
7 checks passed

ti-chi-bot bot added this to the Pool milestone Jul 11, 2023

ti-chi-bot mentioned this pull request Jul 11, 2023

raftstore-v2: clean up import sst file only if flushed epoch is stale. (#15064) #15101

Closed

ti-chi-bot pushed a commit to ti-chi-bot/tikv that referenced this pull request Jul 11, 2023

This is an automated cherry-pick of tikv#15064

203e3e5

close tikv#15018 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

tonyxuqqi mentioned this pull request Jul 17, 2023

[Dynamic Regions] after br full restored to multirocksdb cluster failed and tikv restarted, tikv panic #14946

Closed

bufferflies deleted the delete_sst branch August 29, 2023 05:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

raftstore-v2: clean up import sst file only if flushed epoch is stale. #15064

raftstore-v2: clean up import sst file only if flushed epoch is stale. #15064

bufferflies commented Jul 4, 2023 •

edited

ti-chi-bot bot commented Jul 4, 2023 •

edited

3pointer Jul 6, 2023

bufferflies Jul 6, 2023

bufferflies commented Jul 10, 2023

pingyu commented Jul 11, 2023

ti-chi-bot bot commented Jul 11, 2023

ti-chi-bot bot commented Jul 11, 2023

ti-chi-bot bot commented Jul 11, 2023

bufferflies commented Jul 11, 2023

ti-chi-bot commented Jul 11, 2023

raftstore-v2: clean up import sst file only if flushed epoch is stale. #15064

raftstore-v2: clean up import sst file only if flushed epoch is stale. #15064

Conversation

bufferflies commented Jul 4, 2023 • edited

What is changed and how it works?

Related changes

Check List

Release note

ti-chi-bot bot commented Jul 4, 2023 • edited

3pointer Jul 6, 2023

Choose a reason for hiding this comment

bufferflies Jul 6, 2023

Choose a reason for hiding this comment

bufferflies commented Jul 10, 2023

pingyu commented Jul 11, 2023

ti-chi-bot bot commented Jul 11, 2023

ti-chi-bot bot commented Jul 11, 2023

ti-chi-bot bot commented Jul 11, 2023

bufferflies commented Jul 11, 2023

ti-chi-bot commented Jul 11, 2023

bufferflies commented Jul 4, 2023 •

edited

ti-chi-bot bot commented Jul 4, 2023 •

edited