Support on_apply_snapshot_committed to delete useless FAP snapshot #428

CalvinNeo · 2025-03-17T05:11:37Z

What is changed and how it works?

Issue Number: Close #xxx

What's Changed:

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)
No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Release note

Signed-off-by: Calvin Neo <[email protected]>

JaySon-Huang · 2025-03-31T05:02:51Z

Does the test need to be updated? Or something wrong with the logic?

https://prow.tidb.net/view/gs/prow-tidb-logs/pr-logs/pull/pingcap_tidb-engine-ext/428/pull-unit-test/1901501665530351616#1:build-log.txt%3A12376

test_fall_back_to_slow_path 2025/03/17 13:45:36.728 lib.rs:480: [CRIT] assertion `left == right` failed
  left: Persisted
 right: NotFound, backtrace:    0: tikv_util::set_panic_hook::{{closure}}
             at /home/prow/go/src/github.com/pingcap/tidb-engine-ext/components/tikv_util/src/lib.rs:479:18
...
             at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1/library/core/src/panicking.rs:297:5
   8: proxy::shared::fast_add_peer::fp::test_fall_back_to_slow_path::{{closure}}
             at proxy/shared/fast_add_peer/fp.rs:875:9
   9: mock_engine_store::mock_cluster::cluster_ext::ClusterExt::iter_ffi_helpers
             at /home/prow/go/src/github.com/pingcap/tidb-engine-ext/proxy_components/mock-engine-store/src/mock_cluster/cluster_ext.rs:243:21
  10: mock_engine_store::mock_cluster::v1::<impl mock_engine_store::mock_cluster::mixed_cluster::MixedCluster for mock_engine_store::mock_cluster::v1::cluster::Cluster<T>>::iter_ffi_helpers
             at /home/prow/go/src/github.com/pingcap/tidb-engine-ext/proxy_components/mock-engine-store/src/mock_cluster/v1/mod.rs:110:28
  11: mock_engine_store::mock_cluster::test_utils::iter_ffi_helpers
             at /home/prow/go/src/github.com/pingcap/tidb-engine-ext/proxy_components/mock-engine-store/src/mock_cluster/test_utils.rs:51:5
  12: proxy::shared::fast_add_peer::fp::test_fall_back_to_slow_path
             at proxy/shared/fast_add_peer/fp.rs:874:5
  13: proxy::shared::fast_add_peer::fp::test_fall_back_to_slow_path::{{closure}}
             at proxy/shared/fast_add_peer/fp.rs:842:33
  14: core::ops::function::FnOnce::call_once
             at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1/library/core/src/ops/function.rs:250:5
  15: test_util::runner::run_test_with_hook::{{closure}}::{{closure}}
             at /home/prow/go/src/github.com/pingcap/tidb-engine-ext/components/test_util/src/runner.rs:64:21
...
             at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1/library/core/src/ops/function.rs:250:5
  21: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1/library/alloc/src/boxed.rs:2015:9
      <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1/library/alloc/src/boxed.rs:2015:9
      std::sys::unix::thread::Thread::new::thread_start
             at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1/library/std/src/sys/unix/thread.rs:108:17
  22: start_thread
  23: clone
, location: proxy_tests/proxy/shared/fast_add_peer/fp.rs:875, thread_name: shared::fast_add_peer::fp::test_fall_back_to_slow_path, thread_id: 8

Signed-off-by: Calvin Neo <[email protected]>

CalvinNeo · 2025-03-31T05:53:22Z

Does the test need to be updated? Or something wrong with the logic?

https://prow.tidb.net/view/gs/prow-tidb-logs/pr-logs/pull/pingcap_tidb-engine-ext/428/pull-unit-test/1901501665530351616#1:build-log.txt%3A12376

test_fall_back_to_slow_path 2025/03/17 13:45:36.728 lib.rs:480: [CRIT] assertion `left == right` failed
  left: Persisted
 right: NotFound, backtrace:    0: tikv_util::set_panic_hook::{{closure}}
             at /home/prow/go/src/github.com/pingcap/tidb-engine-ext/components/tikv_util/src/lib.rs:479:18
...
             at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1/library/core/src/panicking.rs:297:5
   8: proxy::shared::fast_add_peer::fp::test_fall_back_to_slow_path::{{closure}}
             at proxy/shared/fast_add_peer/fp.rs:875:9
   9: mock_engine_store::mock_cluster::cluster_ext::ClusterExt::iter_ffi_helpers
             at /home/prow/go/src/github.com/pingcap/tidb-engine-ext/proxy_components/mock-engine-store/src/mock_cluster/cluster_ext.rs:243:21
  10: mock_engine_store::mock_cluster::v1::<impl mock_engine_store::mock_cluster::mixed_cluster::MixedCluster for mock_engine_store::mock_cluster::v1::cluster::Cluster<T>>::iter_ffi_helpers
             at /home/prow/go/src/github.com/pingcap/tidb-engine-ext/proxy_components/mock-engine-store/src/mock_cluster/v1/mod.rs:110:28
  11: mock_engine_store::mock_cluster::test_utils::iter_ffi_helpers
             at /home/prow/go/src/github.com/pingcap/tidb-engine-ext/proxy_components/mock-engine-store/src/mock_cluster/test_utils.rs:51:5
  12: proxy::shared::fast_add_peer::fp::test_fall_back_to_slow_path
             at proxy/shared/fast_add_peer/fp.rs:874:5
  13: proxy::shared::fast_add_peer::fp::test_fall_back_to_slow_path::{{closure}}
             at proxy/shared/fast_add_peer/fp.rs:842:33
  14: core::ops::function::FnOnce::call_once
             at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1/library/core/src/ops/function.rs:250:5
  15: test_util::runner::run_test_with_hook::{{closure}}::{{closure}}
             at /home/prow/go/src/github.com/pingcap/tidb-engine-ext/components/test_util/src/runner.rs:64:21
...
             at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1/library/core/src/ops/function.rs:250:5
  21: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1/library/alloc/src/boxed.rs:2015:9
      <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1/library/alloc/src/boxed.rs:2015:9
      std::sys::unix::thread::Thread::new::thread_start
             at /rustc/89e2160c4ca5808657ed55392620ed1dbbce78d1/library/std/src/sys/unix/thread.rs:108:17
  22: start_thread
  23: clone
, location: proxy_tests/proxy/shared/fast_add_peer/fp.rs:875, thread_name: shared::fast_add_peer::fp::test_fall_back_to_slow_path, thread_id: 8

They should be adapted, by enabling on_apply_snapshot_committed_allow_no_unips failpoint.

Signed-off-by: Calvin Neo <[email protected]>

CalvinNeo · 2025-03-31T07:54:55Z

/retest

…napshot_committed

CalvinNeo · 2025-03-31T08:03:10Z

/hold Do not merge this PR, because we should have merged the observer into tikv master first

Signed-off-by: Calvin Neo <[email protected]>

…napshot_committed

Signed-off-by: Calvin Neo <[email protected]>

proxy_components/mock-engine-store/src/mock_store/mock_snapshot_impls.rs

…t_impls.rs Co-authored-by: JaySon <[email protected]>

Signed-off-by: Calvin Neo <[email protected]>

…db-engine-ext into on_apply_snapshot_committed

Signed-off-by: Calvin Neo <[email protected]>

CalvinNeo · 2025-05-22T07:17:45Z

JaySon-Huang

lgtm

ti-chi-bot · 2025-05-22T07:22:35Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JaySon-Huang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~HACKED_OWNERS~~ [JaySon-Huang]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot · 2025-05-22T07:22:35Z

[LGTM Timeline notifier]

Timeline:

2025-05-22 07:22:35.54421271 +0000 UTC m=+77059.961272258: ☑️ agreed by JaySon-Huang.

CalvinNeo · 2025-05-22T07:46:45Z

/unhold

CalvinNeo added 4 commits March 14, 2025 14:54

not with tikv

0501621

Signed-off-by: Calvin Neo <[email protected]>

with tikv

c02ee27

Signed-off-by: Calvin Neo <[email protected]>

add tests

67665ce

Signed-off-by: Calvin Neo <[email protected]>

add tests

1514529

Signed-off-by: Calvin Neo <[email protected]>

ti-chi-bot bot added the size/XXL label Mar 17, 2025

CalvinNeo changed the title ~~On apply snapshot committed~~ [DNM] On apply snapshot committed Mar 31, 2025

remove some files

adf3315

Signed-off-by: Calvin Neo <[email protected]>

ti-chi-bot bot added size/XL and removed size/XXL labels Mar 31, 2025

fix flaky test

46ddf51

Signed-off-by: Calvin Neo <[email protected]>

Merge remote-tracking branch 'origin/raftstore-proxy' into on_apply_s…

7992dd6

…napshot_committed

ti-chi-bot bot added the do-not-merge/hold label Mar 31, 2025

CalvinNeo added 5 commits April 10, 2025 10:32

remove some comments

db49f7e

Signed-off-by: Calvin Neo <[email protected]>

remove some comments 2

dc7c2bc

Signed-off-by: Calvin Neo <[email protected]>

f3

c2e4d3f

Signed-off-by: Calvin Neo <[email protected]>

f4

6a52e1f

Signed-off-by: Calvin Neo <[email protected]>

Merge remote-tracking branch 'origin/raftstore-proxy' into on_apply_s…

c2ff5ca

…napshot_committed

CalvinNeo changed the title ~~[DNM] On apply snapshot committed~~ On apply snapshot committed May 21, 2025

CalvinNeo changed the title ~~On apply snapshot committed~~ Support on_apply_snapshot_committed to delete obselote FAP snapshot May 21, 2025

CalvinNeo changed the title ~~Support on_apply_snapshot_committed to delete obselote FAP snapshot~~ Support on_apply_snapshot_committed to delete obsolete FAP snapshot May 21, 2025

CalvinNeo changed the title ~~Support on_apply_snapshot_committed to delete obsolete FAP snapshot~~ Support on_apply_snapshot_committed to delete useless FAP snapshot May 21, 2025

CalvinNeo added 2 commits May 21, 2025 10:21

fix

ef453d0

Signed-off-by: Calvin Neo <[email protected]>

fix tyest

f608837

Signed-off-by: Calvin Neo <[email protected]>

JaySon-Huang reviewed May 21, 2025

View reviewed changes

proxy_components/mock-engine-store/src/mock_store/mock_snapshot_impls.rs Outdated Show resolved Hide resolved

CalvinNeo and others added 2 commits May 21, 2025 13:18

Update proxy_components/mock-engine-store/src/mock_store/mock_snapsho…

907a9ee

…t_impls.rs Co-authored-by: JaySon <[email protected]>

fix

6d860e9

Signed-off-by: Calvin Neo <[email protected]>

CalvinNeo added 2 commits May 21, 2025 14:53

Merge branch 'on_apply_snapshot_committed' of github.com:CalvinNeo/ti…

bbd5696

…db-engine-ext into on_apply_snapshot_committed

fmt

7d3b127

Signed-off-by: Calvin Neo <[email protected]>

CalvinNeo requested a review from JaySon-Huang May 21, 2025 09:22

JaySon-Huang approved these changes May 22, 2025

View reviewed changes

ti-chi-bot bot added the lgtm label May 22, 2025

ti-chi-bot bot added the approved label May 22, 2025

ti-chi-bot bot removed the do-not-merge/hold label May 22, 2025

ti-chi-bot bot merged commit 3e325b2 into pingcap:raftstore-proxy May 22, 2025
3 of 4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Support on_apply_snapshot_committed to delete useless FAP snapshot #428

Support on_apply_snapshot_committed to delete useless FAP snapshot #428

Uh oh!

CalvinNeo commented Mar 17, 2025

Uh oh!

JaySon-Huang commented Mar 31, 2025

Uh oh!

CalvinNeo commented Mar 31, 2025

Uh oh!

CalvinNeo commented Mar 31, 2025

Uh oh!

CalvinNeo commented Mar 31, 2025

Uh oh!

Uh oh!

CalvinNeo commented May 22, 2025

Uh oh!

JaySon-Huang left a comment

Uh oh!

ti-chi-bot bot commented May 22, 2025

Uh oh!

ti-chi-bot bot commented May 22, 2025

Uh oh!

CalvinNeo commented May 22, 2025

Uh oh!

Uh oh!

Uh oh!

Support on_apply_snapshot_committed to delete useless FAP snapshot #428

Support on_apply_snapshot_committed to delete useless FAP snapshot #428

Uh oh!

Conversation

CalvinNeo commented Mar 17, 2025

What is changed and how it works?

Related changes

Check List

Release note

Uh oh!

JaySon-Huang commented Mar 31, 2025

Uh oh!

CalvinNeo commented Mar 31, 2025

Uh oh!

CalvinNeo commented Mar 31, 2025

Uh oh!

CalvinNeo commented Mar 31, 2025

Uh oh!

Uh oh!

CalvinNeo commented May 22, 2025

Uh oh!

JaySon-Huang left a comment

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot bot commented May 22, 2025

Uh oh!

ti-chi-bot bot commented May 22, 2025

[LGTM Timeline notifier]

Uh oh!

CalvinNeo commented May 22, 2025

Uh oh!

Uh oh!

Uh oh!