raftstore: allow exec observers delay deletion of applied ssts #13061
Conversation
Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>
Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>
Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>
Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>
|
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
ti-chi-bot
added
do-not-merge/needs-linked-issue
release-note-none
contribution
|
/cc @BusyJay @tonyxuqqi |
|
/run-build-arm64 comment=true |
CalvinNeo
changed the title
…ngine-ext into merge-ingest-sst-delete
| _: &RegionState, | ||
| _: &mut ApplyCtxInfo<'_>, | ||
| ) -> bool { | ||
| self.called.fetch_add(18, Ordering::SeqCst); |
There was a problem hiding this comment.
what's the meaning of 18 and all other numbers below
There was a problem hiding this comment.
In this test, we assign unique index for each observer functions so that we can test all of the observer functions we want to be called are called. All numbers below 18 are used for other functions, so we use 18 for post_exec.
| pub struct ApplyCtxInfo<'a> { | ||
| pub pending_handle_ssts: &'a mut Option<Vec<SstMetaInfo>>, | ||
| pub delete_ssts: &'a mut Vec<SstMetaInfo>, | ||
| pub pending_delete_ssts: &'a mut Vec<SstMetaInfo>, |
There was a problem hiding this comment.
where's the code that consumes pending_delete_ssts and delete the SST files?
How can we ensure that these SST files are finally deleted?
There was a problem hiding this comment.
In TiKV's view:
post_exec will consumes pending_delete_ssts, and it will return true to suggest a immediate write to db.
In TiFlash's view:
When all data referred in the ssts is write into TiFlash's storage, TiFlash will consume pending_delete_ssts and suggest a write.
If TiFlash&Proxy restart before persistence, the recovered applied_index is before IngestSst command. So we can replay this command, and thus ensuring we will not lost track of sst files.
There was a problem hiding this comment.
In TiKV's view:
post_execwill consumespending_delete_ssts, and it will return true to suggest a immediate write to db. In TiFlash's view: When all data referred in the ssts is write into TiFlash's storage, TiFlash will consumepending_delete_sstsand suggest a write. If TiFlash&Proxy restart before persistence, the recovered applied_index is before IngestSst command. So we can replay this command, and thus ensuring we will not lost track of sst files.
IC. Then is there any metrics for pending_delete_ssts? We need to have a way to figure this out.
There was a problem hiding this comment.
We have a tikv_raft_applying_sst added
Signed-off-by: CalvinNeo <calvinneo1995@gmail.com>
ti-chi-bot
added
status/LGT2
CalvinNeo
force-pushed
the
merge-ingest-sst-delete
branch
from
eb58511
to
0f81b23
Compare
CalvinNeo
force-pushed
the
merge-ingest-sst-delete
branch
from
0f81b23
to
fdea78c
Compare
|
/merge |
|
@breezewish: It seems you want to merge this PR, I will help you trigger all the tests: /run-all-tests You only need to trigger 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. |
|
This pull request has been accepted and is ready to merge. Commit hash: 08ec612
|
|
@CalvinNeo: Your PR was out of date, I have automatically updated it for you. At the same time I will also trigger all tests for you: /run-all-tests 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. |
…13061) ref tikv#12849 allow exec observers delay deletion of applied ssts Signed-off-by: CalvinNeo <calvinneo1995@gmail.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>
…13061) ref tikv#12849 allow exec observers delay deletion of applied ssts Signed-off-by: CalvinNeo <calvinneo1995@gmail.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io> Signed-off-by: fengou1 <feng.ou@pingcap.com>
What is changed and how it works?
Issue Number: ref #12849
What's Changed:
Some KvEngine can't actually ingest sst, since they do not use RocksDB as a backend. So they may decode the sst, and store them in memory. If we delete the sst file immediately, we may lost data if we have not persist our data in KvEngine.
However, we can't do a persistence for every ingest sst request, this just cost too much. So we introduce this PR to allow a delay deletion of sst files, controlled by post_exec.
Related changes
pingcap/docs/pingcap/docs-cn:Check List
Tests
Side effects
Release note