[platinumhamburg]

Purpose

Linked issue: close #3806 3806

Brief change log

Tests

API and Format

Documentation

[Copilot]

Pull request overview

This PR introduces a KV close mode to distinguish between “preserve local state” closes and shutdown/drop closes where local unflushed RocksDB state is not relied upon for recovery. It wires Tablet Server shutdown and KV drop paths to use a discard mode that enables RocksDB’s avoid_flush_during_shutdown option immediately before closing, reducing shutdown-time disk I/O.

Changes:

• Add KvCloseMode and thread it through TabletServer → KvManager → KvTablet → RocksDBKv close/shutdown paths.
• Use DISCARD_UNPERSISTED_STATE for Tablet Server shutdown and tablet drop, while keeping default no-arg close/shutdown behavior preserving local state.
• Extend unit tests to validate close-mode propagation and the expected persistence behavior in preserve vs discard modes.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
fluss-server/src/main/java/org/apache/fluss/server/tablet/TabletServer.java	Uses discard close mode when shutting down KV manager during Tablet Server shutdown.
fluss-server/src/main/java/org/apache/fluss/server/kv/KvManager.java	Adds shutdown(KvCloseMode) and closes tablets with the provided mode.
fluss-server/src/main/java/org/apache/fluss/server/kv/KvTablet.java	Adds close(KvCloseMode) and uses discard mode for drop().
fluss-server/src/main/java/org/apache/fluss/server/kv/rocksdb/RocksDBKv.java	Adds close(KvCloseMode) and sets avoid_flush_during_shutdown in discard mode before closing.
fluss-server/src/main/java/org/apache/fluss/server/kv/KvCloseMode.java	New enum defining preserve vs discard close behaviors.
fluss-server/src/test/java/org/apache/fluss/server/tablet/TabletServerShutdownTest.java	Verifies shutdown uses DISCARD_UNPERSISTED_STATE.
fluss-server/src/test/java/org/apache/fluss/server/kv/KvManagerTest.java	Adds coverage for discard shutdown/drop not persisting unflushed state and null close-mode validation.
fluss-server/src/test/java/org/apache/fluss/server/kv/rocksdb/RocksDBKvTest.java	Adds tests for preserve vs discard close semantics and resource closing on option-set failure.
fluss-server/src/test/java/org/apache/fluss/server/kv/rocksdb/RocksDBResourceContainerTest.java	Asserts default DB options keep avoidFlushDuringShutdown disabled globally.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[luoyuxia]

@platinumhamburg Thanks for the comment. PTAL

[luoyuxia]

Could we still delete the KV directory when setting avoid_flush_during_shutdown fails?

RocksDBKv.close(DISCARD_UNPERSISTED_STATE) closes the DB, column-family handles, and options in its finally, but then rethrows the captured RocksDBException. That exception exits this lambda before FileUtils.deleteDirectory(...) runs. KvManager.dropKv() has already removed the tablet from currentKvs, so a retry through the manager cannot clean up the untracked directory.

Please either treat failure to apply this shutdown optimization as a warning once the fallback close has completed, or ensure the directory is deleted after confirming the native DB is closed while preserving the original exception. A regression test covering KvTablet.drop() with an injected option-setting failure would protect this path.

[luoyuxia]

Do we really need two close modes here?

From the current production call sites, both TabletServer shutdown and tablet drop can discard unflushed memtable state: recovery rebuilds the KV from snapshots/changelog, or the local directory is deleted. PRESERVE_LOCAL_STATE seems to exist only to retain the behavior of directly reopening the same local RocksDB directory after KvManager.shutdown().

Is that direct local-reopen behavior still a contract we need? If not, could we simplify this change by always setting avoid_flush_during_shutdown=true before closing RocksDB and remove KvCloseMode and its propagation? Since RocksDB WAL is disabled, this would intentionally make close terminal: callers must recover through Fluss instead of relying on unflushed local RocksDB state.

[platinumhamburg]

Yes, we do need the two close modes here. In the future, we plan to support dynamically altering options for primary key tables, which requires KvTablet to support dynamic reload — and in that mode, we expect to retain the flush capability.

[platinumhamburg]

@luoyuxia Thank you for your comments; I have addressed them. Please take a look when you have time.

[luoyuxia]

+1