[server] add support of rebuilding KV from local snapshot by zuston · Pull Request #3977 · apache/fluss

zuston · 2026-08-13T09:52:32Z

Purpose

This PR enables KV tablets to be rebuilt from valid local snapshots, speeding up recovery—especially when the tablet server restarts without a leader change.

Brief change log

reserve the local latest snapshot to the local KV tablet main dir
recover KV tablet from the latest snapshot if it's valid

Tests

API and Format

Documentation

fresh-borzoni

@zuston Thank you for the PR, left a couple of comments and questions, PTAL

fresh-borzoni · 2026-08-29T11:52:17Z

@@ -323,16 +327,9 @@ protected NativeRocksDBSnapshotResources(

        @Override
        public void release() {


release() is empty now and SnapshotRunner is its only caller. Remove it?

I have introduced the dedicated option for this feature, so let's reserve this

fresh-borzoni · 2026-08-29T11:52:42Z

-        // make as leader again, should restore from snapshot
+        // Recover as leader after the in-place restart, without an intervening role transition.
        makeKvReplicaAsLeader(kvReplica, 2);
+        assertThat(downloadedRemoteSnapshot).isFalse();


Can we get the other half: invalid checkpoint, assert downloadedRemoteSnapshot is true?

fresh-borzoni · 2026-08-29T11:54:34Z

+                }

                // as we have downloaded kv files into the tablet dir, now, we can load it
                kvTablet = kvManager.loadKv(tabletDir, schemaGetter, this::onKvFlushComplete);


When the local copy won't open, loadKv throws and we never reach downloadKvSnapshots, so the broken-snapshot handling from #1482 never runs, and the retries at 743 just repeat. Therefore replica ends up stuck with nothing clearing the directory.

Shall we drop the local checkpoint and fall through to the download instead? Not via handleSnapshotBroken though, the remote snapshot is fine here.

fresh-borzoni · 2026-08-29T11:55:37Z

        }
+        // The local checkpoint is useful after an in-place restart. Keep the committed snapshot and
+        // remove older or uncommitted checkpoints only after the remote commit succeeds.
+        LocalKvSnapshotUtils.retainOnly(instanceBasePath, completedSnapshotId);


Should this be behind a flag? Local recovery is normally opt-in, like Flink has state.backend.local-recovery and also #2179 shipped snapshot retention with a TTL and metrics.
This turns retention on for every KV bucket with no way off, so feels off.

fresh-borzoni · 2026-08-29T12:07:27Z

This deletes the whole tablet dir for standbys too, so the retained checkpoint only survives while the replica stays leader. A standby that gets promoted still downloads from remote.

Is keeping it for standbys in scope, or deliberately out? Looks like what the TODO in processRetryOfflineLeader is anticipating.

Good point. This PR only optimizes in-place restarts of the leader TabletServer. Standby promotion still recovers from remote storage; maintaining local snapshots on standbys requires a separate lifecycle and is out of scope for this PR. We can track it separately, that is another bigger feature to speed up the KV leader transition.

fresh-borzoni

@zuston Thank you, LGTM 👍 One non-blocking comment

cc @swuferhong

fresh-borzoni · 2026-08-31T09:51:33Z

                                    + "The default setting is 10 minutes.");

+    public static final ConfigOption<Boolean> KV_SNAPSHOT_LOCAL_RECOVERY_ENABLED =
+            key("kv.snapshot.local-recovery.enabled")


This one's static, while the other KV knobs can be changed at runtime (kv.snapshot.interval, kv.leader-replica.memory-reserved, the disk ratios). Since it costs disk, someone watching DISK_USAGE_RATIO go up can only switch it off by restarting.

Could we do what the interval does? PeriodicSnapshotManager takes a LongSupplier for it and the key is in ALLOWED_CONFIG_KEYS.

Though switching it off wouldn't free anything by itself. release() would stop keeping new checkpoints, but snap-N is still there until the next remote restore, so notifySnapshotComplete would need to clear the snap-* dirs when it's off.

This one's static, while the other KV knobs can be changed at runtime (kv.snapshot.interval, kv.leader-replica.memory-reserved, the disk ratios). Since it costs disk, someone watching DISK_USAGE_RATIO go up can only switch it off by restarting.

Yes, this could be updated dynamiclly to follow up in the next PR. One tip that this feature won't cost extra disk capacity because the snapshot is just a hard link referenced by the rocksdb files.

Not quite. snap-N hard-links the SSTs at snapshot time. Compaction later removes them from db/, but snap-N keeps them alive until N+1 completes.

Could we at least mention this in the option description?

zuston · 2026-09-15T12:00:19Z

rebased @fresh-borzoni

fresh-borzoni

@zuston Thank you, left some comments, PTAL

fresh-borzoni · 2026-09-24T14:53:41Z

     */
    public void startup() {
-        cleanupStaleKvDirectories();
+        if (!conf.get(ConfigOptions.KV_SNAPSHOT_LOCAL_RECOVERY_ENABLED)) {


With the flag on this skips the #4401 cleanup, so #4400 comes back.

Controlled shutdown keeps the old leader's KV dir, since CoordinatorRequestBatch's addNotifyLeaderRequestForTabletServers skips shutting-down servers. The server restarts as a follower, and Replica.dropKv() is a no-op because kvTablet == null. So db/ + snap-N stay on every bucket it now follows, and they survive a table drop.

Reproduced this behaviour in ReplicaTest: with the flag on, db/ + snap-0 survive restart -> follower -> Replica.delete(). With the flag off, the startup cleanup removes them.

Could we keep the dirs at startup, but delete a leftover dir when the replica becomes follower with no open tablet? And how would dirs of buckets that are never assigned back get reclaimed?

@swuferhong WDYT?

fresh-borzoni · 2026-09-24T14:54:28Z

                                    + "The default setting is 10 minutes.");

+    public static final ConfigOption<Boolean> KV_SNAPSHOT_LOCAL_RECOVERY_ENABLED =
+            key("kv.snapshot.local-recovery.enabled")


Not quite. snap-N hard-links the SSTs at snapshot time. Compaction later removes them from db/, but snap-N keeps them alive until N+1 completes.

Could we at least mention this in the option description?

fresh-borzoni · 2026-09-24T14:56:40Z

-
-                // as we have downloaded kv files into the tablet dir, now, we can load it
-                kvTablet = kvManager.loadKv(tabletDir, schemaGetter, this::onKvFlushComplete);
+                kvTablet = restoreKvTablet(completedSnapshot);


Not specifically coming from this PR, but noticed while reading and reproduced on main: if recoverKvTablet fails once after a snapshot restore, all 5 initKvTablet attempts fail. Attempt 1's RocksDB is still open, so each retry re-downloads the snapshot and fails with RocksDBException: lock hold by current process .../db/LOCK. The no-snapshot branch already reuses the open tablet. Shall we file an issue for this since we are here?

fresh-borzoni · 2026-09-24T14:57:20Z

+                        completedSnapshot.getSnapshotID(),
+                        System.currentTimeMillis() - start);
+                return restoredKvTablet;
+            } catch (KvBuildingException localRecoveryException) {


nit: a counter for local hits vs fallbacks would show whether this actually kicks in. #2179 shipped metrics with snapshot retention, is it applicable here, wdyt?

fresh-borzoni · 2026-09-24T14:57:51Z

+        Path restoreDirectory = kvTabletDir.toPath().resolve(RESTORE_DIRECTORY_PREFIX + snapshotId);
+        Path activeDbDirectory = RocksDBKvBuilder.getInstanceRocksDBPath(kvTabletDir).toPath();
+        try {
+            FileUtils.deleteDirectory(restoreDirectory.toFile());


nit: a leftover .db-restore- gets removed only by a later restore with the same id.
Could retainOnly also sweep .db-restore-*?

fresh-borzoni · 2026-09-24T14:59:12Z


+    private KvTablet restoreKvTablet(CompletedSnapshot completedSnapshot) throws Exception {
+        checkNotNull(kvManager);
+        long start = System.currentTimeMillis();


nit: clock.milliseconds() like initKvTablet above? Is there any specific reason to use different one?

fresh-borzoni · 2026-09-24T15:04:04Z

-
-                // as we have downloaded kv files into the tablet dir, now, we can load it
-                kvTablet = kvManager.loadKv(tabletDir, schemaGetter, this::onKvFlushComplete);
+                kvTablet = restoreKvTablet(completedSnapshot);


Which setup is this aimed at?

With RF≥2, leadership moves to another replica on controlled shutdown or crash, so this server restarts as a follower. The local copy helps only if leadership returns before the new leader's next snapshot. After that the snapshot id moves on and we download anyway.

So it mostly helps RF=1 or ISR={leader}. Shall we say that in the option description?

good point. the motivation of this pr is to speed up the local recovery for multi RFs. When I upgrade the whole fluss cluster to do some quick experiment or the new features validation, I will shutdown the coordinator directly firstly, and then restart the tablet servers to avoid leader changes to reduce the recovery time cost. (btw for the recommended recovery way, the recovery time is really too long especially when rolling upgrading for the large scale bucket numbers)

based on the above requirements, I have to optimize the local recovery speed without leader change.

zuston force-pushed the rebuildKV branch from a865709 to d358391 Compare August 29, 2026 02:33

fresh-borzoni reviewed Aug 29, 2026

View reviewed changes

zuston requested a review from fresh-borzoni August 29, 2026 14:27

fresh-borzoni approved these changes Aug 31, 2026

View reviewed changes

zuston requested a review from fresh-borzoni September 14, 2026 02:20

zuston force-pushed the rebuildKV branch from e67b154 to c9cf3cb Compare September 15, 2026 11:59

zhangjunfan and others added 5 commits September 24, 2026 17:11

[server] add support of rebuilding KV from local snapshot

1b457b5

remove test

50ef914

feat(server): add configurable local recovery for KV snapshots

a3063d2

add fallback logic

97b2796

add tests for corruption local files

291ddbc

zuston force-pushed the rebuildKV branch from c9cf3cb to 291ddbc Compare September 24, 2026 09:18

fresh-borzoni reviewed Sep 24, 2026

View reviewed changes

modification with anton comments

22dd124

		@@ -323,16 +327,9 @@ protected NativeRocksDBSnapshotResources(

		@Override
		public void release() {

Conversation

zuston commented Aug 13, 2026

Purpose

Brief change log

Tests

API and Format

Documentation

Uh oh!

fresh-borzoni left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fresh-borzoni left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zuston Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zuston commented Sep 15, 2026

Uh oh!

fresh-borzoni left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

zuston Aug 31, 2026 •

edited

Loading