From a3b0f187ff4b501c7fe4a67dbcc4ad6d433a454c Mon Sep 17 00:00:00 2001 From: Anand Ananthabhotla Date: Fri, 16 Oct 2026 12:15:57 -0700 Subject: [PATCH] Add FileSystem::IsPreallocationSupported() to skip WAL truncation on open (#15395) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/15395 **Why the change is required:** During `DB::Open`, RocksDB reopens the last WAL file and truncates it to its logical size. This is done even when every column family's log number has already advanced past that WAL, so the file is obsolete and will be deleted later in the same open. The only purpose of the truncation is to release space preallocated beyond the logical end of file (e.g. by `fallocate` with `FALLOC_FL_KEEP_SIZE`); `GetFileSize()` reports the apparent size, so truncating to it can only free preallocated blocks. On a FileSystem that never preallocates, this work is pure overhead: a `GetFileSize`, a `ReopenWritableFile`, a no-op `Truncate` (the default `FSWritableFile::Truncate` returns OK without doing anything), and a `Close` on every open. On remote or append-oriented FileSystems the reopen can be expensive. **Previous behavior:** - If recovery flushed all data, or the WALs contained no data (including when all WALs are older than the min log number to keep), `MaybeFlushFinalMemtableOrRestoreActiveLogFiles()` always called `GetLogSizeAndMaybeTruncate()` on the last WAL. In read-only mode it still issued a `GetFileSize()` whose result was discarded. - With `avoid_flush_during_recovery`, `RestoreAliveLogFiles()` always reopened and truncated the last live WAL. **New behavior:** A FileSystem can report that it never preallocates space by returning false from the new `FileSystem::IsPreallocationSupported()`. When it does, DB open skips reopening and truncating the WAL. In the flushed / no-data path it also skips the unused `GetFileSize()`, so no I/O is issued against the old WAL. The read-only no-data path no longer issues the discarded `GetFileSize()` either. **Implementation:** - `include/rocksdb/file_system.h`: new `virtual bool IsPreallocationSupported()`, defaulting to `true`. `FileSystemWrapper` forwards it to the target. - The policy lives in the two callers in `db_impl_open.cc`; `GetLogSizeAndMaybeTruncate()` is unchanged. `RestoreAliveLogFiles()` still calls `GetFileSize()` because the size is needed for `alive_wal_files_` and `wals_total_size_`. **Safety, compatibility, and limitations:** - The default is `true`, so every existing FileSystem, including ones that override `SupportedOps()` without calling the base class, keeps the current behavior. This is why it is a separate virtual function instead of an opt-in `FSSupportedOps` bit, which would silently disable truncation for those FileSystems. - No built-in FileSystem overrides the default: Posix, Windows, `MockFileSystem`, and `LegacyFileSystemWrapper` all report `true`, so their WAL truncation behavior is unchanged. Posix deliberately reports `true` even in builds without `ROCKSDB_FALLOCATE_PRESENT`: WALs may have been preallocated by a different build running on the same machine, and a build-dependent answer would leave that space unreclaimed. - The function describes what the FileSystem can do, not per-file options such as `allow_fallocate` or build configuration. A WAL may have been preallocated by an earlier process with different options, so gating on the current options could leak space. - A FileSystem that wrongly returns false can only leak preallocated space, in archived WALs (`WAL_ttl_seconds` / `WAL_size_limit_MB`) or across a crash loop before obsolete WALs are deleted. Data is unaffected. Reviewed By: joshkang97 Differential Revision: D125049741 fbshipit-source-id: ac9795f2045c46911cd86b8faacef5aa49043496 --- db/db_impl/db_impl_open.cc | 15 +-- db/db_wal_test.cc | 98 +++++++++++++++++++ env/env_test.cc | 6 ++ include/rocksdb/file_system.h | 14 +++ .../fs_is_preallocation_supported.md | 1 + 5 files changed, 128 insertions(+), 6 deletions(-) create mode 100644 unreleased_history/new_features/fs_is_preallocation_supported.md diff --git a/db/db_impl/db_impl_open.cc b/db/db_impl/db_impl_open.cc index 67b518cd3a..128b0c92cb 100644 --- a/db/db_impl/db_impl_open.cc +++ b/db/db_impl/db_impl_open.cc @@ -2024,12 +2024,12 @@ Status DBImpl::MaybeFlushFinalMemtableOrRestoreActiveLogFiles( if (status.ok()) { if (data_seen && !flushed) { status = RestoreAliveLogFiles(wal_numbers); - } else if (!wal_numbers.empty()) { // If there's no data in the WAL, or we - // flushed all the data, still + } else if (!wal_numbers.empty() && !read_only && + fs_->IsPreallocationSupported()) { + // If there's no data in the WAL, or we flushed all the data, still // truncate the log file. If the process goes into a crash loop before // the file is deleted, the preallocated space will never get freed. - const bool truncate = !read_only; - GetLogSizeAndMaybeTruncate(wal_numbers.back(), truncate, nullptr) + GetLogSizeAndMaybeTruncate(wal_numbers.back(), /*truncate=*/true, nullptr) .PermitUncheckedError(); } } @@ -2105,6 +2105,7 @@ Status DBImpl::RestoreAliveLogFiles(const std::vector& wal_numbers) { wal_empty_ = false; uint64_t min_wal_with_unflushed_data = versions_->MinLogNumberWithUnflushedData(); + const bool preallocation_supported = fs_->IsPreallocationSupported(); for (auto wal_number : wal_numbers) { if (!allow_2pc() && wal_number < min_wal_with_unflushed_data) { // In non-2pc mode, the WAL files not backing unflushed data are not @@ -2115,8 +2116,10 @@ Status DBImpl::RestoreAliveLogFiles(const std::vector& wal_numbers) { // preallocated space are not needed anymore. It is likely only the last // log has such preallocated space, so we only truncate for the last log. WalFileNumberSize log; - s = GetLogSizeAndMaybeTruncate( - wal_number, /*truncate=*/(wal_number == wal_numbers.back()), &log); + s = GetLogSizeAndMaybeTruncate(wal_number, + /*truncate=*/preallocation_supported && + (wal_number == wal_numbers.back()), + &log); if (!s.ok()) { break; } diff --git a/db/db_wal_test.cc b/db/db_wal_test.cc index 3e28f58443..4cfbe0562f 100644 --- a/db/db_wal_test.cc +++ b/db/db_wal_test.cc @@ -3090,6 +3090,104 @@ TEST_F(DBWALTest, ReadOnlyRecoveryNoTruncate) { #endif // ROCKSDB_FALLOCATE_PRESENT #endif // ROCKSDB_PLATFORM_POSIX +// Reports a configurable preallocation capability and counts +// ReopenWritableFile() calls on WAL files. +class PreallocationTestFs : public FileSystemWrapper { + public: + PreallocationTestFs(const std::shared_ptr& target, + bool preallocation_supported) + : FileSystemWrapper(target), + preallocation_supported_(preallocation_supported) {} + + const char* Name() const override { return "PreallocationTestFs"; } + + bool IsPreallocationSupported() override { return preallocation_supported_; } + + IOStatus ReopenWritableFile(const std::string& fname, + const FileOptions& file_opts, + std::unique_ptr* result, + IODebugContext* dbg) override { + uint64_t number; + FileType type; + if (ParseFileName(fname.substr(fname.find_last_of('/') + 1), &number, + &type) && + type == kWalFile) { + wal_reopen_count_.FetchAddRelaxed(1); + } + return target()->ReopenWritableFile(fname, file_opts, result, dbg); + } + + void SetPreallocationSupported(bool supported) { + preallocation_supported_ = supported; + } + + int GetWalReopenCount() const { return wal_reopen_count_.LoadRelaxed(); } + + void ResetWalReopenCount() { wal_reopen_count_.StoreRelaxed(0); } + + private: + bool preallocation_supported_; + RelaxedAtomic wal_reopen_count_{0}; +}; + +// Tests that the last WAL is reopened for truncation during recovery only if +// the FileSystem reports that it supports preallocation. +TEST_F(DBWALTest, SkipWalTruncateIfPreallocationNotSupported) { + // The Env must outlive the loop since DestroyAndReopen() destroys the + // previous DB using the previous iteration's options. + auto test_fs = std::make_shared( + env_->GetFileSystem(), /*preallocation_supported=*/true); + std::unique_ptr test_env(NewCompositeEnv(test_fs)); + for (bool preallocation_supported : {true, false}) { + // avoid_flush_during_recovery = true restores the WAL as alive, while + // false flushes the recovered data and leaves the WAL obsolete. + for (bool avoid_flush_during_recovery : {true, false}) { + SCOPED_TRACE( + "preallocation_supported=" + std::to_string(preallocation_supported) + + " avoid_flush_during_recovery=" + + std::to_string(avoid_flush_during_recovery)); + Options options = CurrentOptions(); + options.env = test_env.get(); + options.avoid_flush_during_recovery = avoid_flush_during_recovery; + options.avoid_flush_during_shutdown = true; + test_fs->SetPreallocationSupported(preallocation_supported); + DestroyAndReopen(options); + ASSERT_OK(Put("foo", "v1")); + Close(); + + test_fs->ResetWalReopenCount(); + Reopen(options); + ASSERT_EQ(preallocation_supported ? 1 : 0, test_fs->GetWalReopenCount()); + ASSERT_EQ("v1", Get("foo")); + Close(); + } + } +} + +// Tests that a read-only open never reopens a WAL for truncation, even if the +// FileSystem supports preallocation. +TEST_F(DBWALTest, ReadOnlyOpenSkipsWalTruncate) { + Options options = CurrentOptions(); + auto test_fs = std::make_shared( + env_->GetFileSystem(), /*preallocation_supported=*/true); + std::unique_ptr test_env(NewCompositeEnv(test_fs)); + options.env = test_env.get(); + options.avoid_flush_during_shutdown = true; + DestroyAndReopen(options); + ASSERT_OK(Put("foo", "v1")); + Close(); + + ASSERT_OK(ReadOnlyReopen(options)); + ASSERT_EQ("v1", Get("foo")); + Close(); + ASSERT_EQ(0, test_fs->GetWalReopenCount()); + + // The same WAL is truncated by a read-write open. + Reopen(options); + ASSERT_EQ(1, test_fs->GetWalReopenCount()); + Close(); +} + TEST_F(DBWALTest, WalInManifestButNotInSortedWals) { Options options = CurrentOptions(); options.track_and_verify_wals_in_manifest = true; diff --git a/env/env_test.cc b/env/env_test.cc index 42eed63cef..04f51e604a 100644 --- a/env/env_test.cc +++ b/env/env_test.cc @@ -1992,6 +1992,12 @@ TEST_F(EnvPosixTest, SupportedOpsNoAsyncIOOnIOUringInitFailure) { SyncPoint::GetInstance()->DisableProcessing(); SyncPoint::GetInstance()->ClearAllCallBacks(); } + +// The Posix FileSystem must report preallocation support regardless of build +// configuration, so that space preallocated by another build is still freed. +TEST_F(EnvPosixTest, PosixFileSystemReportsPreallocationSupported) { + ASSERT_TRUE(FileSystem::Default()->IsPreallocationSupported()); +} #ifdef OS_LINUX TEST_F(EnvPosixTest, IOUringAddressReuseNoTsanFalsePositive) { struct io_uring* probe = CreateIOUring(); diff --git a/include/rocksdb/file_system.h b/include/rocksdb/file_system.h index 092c91e024..b5de8cf810 100644 --- a/include/rocksdb/file_system.h +++ b/include/rocksdb/file_system.h @@ -864,6 +864,16 @@ class FileSystem : public Customizable { supported_ops |= (1 << FSSupportedOps::kFSPrefetch); } + // Returns false if this FileSystem never allocates space beyond a file's + // logical size (for example, via FSWritableFile::Allocate() or + // PrepareWrite()). RocksDB may then skip work whose only purpose is to + // release preallocated space, such as reopening and truncating the last + // WAL file during DB open. This should reflect the capability of the + // FileSystem rather than any per-file options or build configuration, + // since files may have been created by a prior process with different + // options or a different build. Defaults to true. + virtual bool IsPreallocationSupported() { return true; } + // If you're adding methods here, remember to add them to EnvWrapper too. private: @@ -1859,6 +1869,10 @@ class FileSystemWrapper : public FileSystem { return target_->SupportedOps(supported_ops); } + bool IsPreallocationSupported() override { + return target_->IsPreallocationSupported(); + } + protected: std::shared_ptr target_; }; diff --git a/unreleased_history/new_features/fs_is_preallocation_supported.md b/unreleased_history/new_features/fs_is_preallocation_supported.md new file mode 100644 index 0000000000..9d55b7e178 --- /dev/null +++ b/unreleased_history/new_features/fs_is_preallocation_supported.md @@ -0,0 +1 @@ +Added `FileSystem::IsPreallocationSupported()`. A `FileSystem` that never preallocates file space can return false so that DB open skips reopening and truncating the last WAL file.