diff --git a/db/db_etc2_test.cc b/db/db_etc2_test.cc index 14127b4225..ea3f5592cb 100644 --- a/db/db_etc2_test.cc +++ b/db/db_etc2_test.cc @@ -28,6 +28,7 @@ #include "rocksdb/wal_filter.h" #include "test_util/testutil.h" #include "util/defer.h" +#include "util/mutexlock.h" #include "util/random.h" #include "utilities/fault_injection_env.h" @@ -7902,7 +7903,10 @@ class FastOpenTestFS : public FileSystemWrapper { IODebugContext* dbg) override { if (file_opts.file_metadata != nullptr) { metadata_passed_on_open_.fetch_add(1, std::memory_order_relaxed); - last_metadata_received_ = *file_opts.file_metadata; + { + MutexLock l(&last_metadata_mu_); + last_metadata_received_ = *file_opts.file_metadata; + } } IOStatus s = target()->NewRandomAccessFile(fname, file_opts, result, dbg); if (s.ok()) { @@ -7921,18 +7925,21 @@ class FastOpenTestFS : public FileSystemWrapper { } std::string GetLastMetadataReceived() const { + MutexLock l(&last_metadata_mu_); return last_metadata_received_; } void ResetCounters() { metadata_retrieved_count_.store(0, std::memory_order_relaxed); metadata_passed_on_open_.store(0, std::memory_order_relaxed); + MutexLock l(&last_metadata_mu_); last_metadata_received_.clear(); } private: std::atomic metadata_retrieved_count_{0}; std::atomic metadata_passed_on_open_{0}; + mutable port::Mutex last_metadata_mu_; std::string last_metadata_received_; }; @@ -8274,6 +8281,122 @@ TEST_F(DBTest2, FastSstOpenDisableAfterMetadataPersisted) { ASSERT_OK(DestroyDB(test_dbname, options)); } +TEST_F(DBTest2, FastSstOpenMetadataPreservedAcrossManifestRotation) { + // Regression test: FileMetaData::file_open_metadata for unchanged live + // SSTs must survive a MANIFEST rotation. WriteCurrentStateToManifest + // snapshots live SSTs into a new MANIFEST; if it drops file_open_metadata + // for those unchanged SSTs, the fast_sst_open optimization is silently + // disabled for them until they are rewritten by flush/compaction/ingest. + auto test_fs = std::make_shared(env_->GetFileSystem()); + std::unique_ptr test_env(new CompositeEnvWrapper(env_, test_fs)); + + Options options; + options.env = test_env.get(); + options.fast_sst_open = true; + options.create_if_missing = true; + // Keep the pre-rotation SST from being rewritten by compaction. + options.disable_auto_compactions = true; + + std::string test_dbname = test::PerThreadDBPath("fast_open_manifest_rotate"); + ASSERT_OK(DestroyDB(test_dbname, options)); + + std::unique_ptr db; + ASSERT_OK(DB::Open(options, test_dbname, &db)); + + // Flush an SST; metadata is retrieved and persisted in the MANIFEST. + ASSERT_OK(db->Put(WriteOptions(), "key1", "value1")); + ASSERT_OK(db->Flush(FlushOptions())); + ASSERT_EQ(1, test_fs->GetMetadataRetrievedCount()); + + // Shrink max_manifest_file_size so the next MANIFEST write rotates and + // exercises WriteCurrentStateToManifest. + ASSERT_OK(db->SetDBOptions( + {{"max_manifest_file_size", "1"}, {"max_manifest_space_amp_pct", "0"}})); + + uint64_t manifest_before = + static_cast(db.get())->TEST_Current_Manifest_FileNo(); + + // Flush a second SST to trigger the rotation. SST #1 is unchanged, so + // the rotated MANIFEST snapshot must carry SST #1's file_open_metadata. + ASSERT_OK(db->Put(WriteOptions(), "key2", "value2")); + ASSERT_OK(db->Flush(FlushOptions())); + + uint64_t manifest_after = + static_cast(db.get())->TEST_Current_Manifest_FileNo(); + ASSERT_GT(manifest_after, manifest_before); + + db.reset(); + test_fs->ResetCounters(); + + // Reopen. Every live SST should be opened with file_open_metadata. + // Pre-fix: only SST #2 (added by the rotating edit) has metadata; SST #1 + // was stripped by the rotation snapshot, so the count would be 1. + ASSERT_OK(DB::Open(options, test_dbname, &db)); + + std::string value; + ASSERT_OK(db->Get(ReadOptions(), "key1", &value)); + ASSERT_EQ("value1", value); + ASSERT_OK(db->Get(ReadOptions(), "key2", &value)); + ASSERT_EQ("value2", value); + + ASSERT_EQ(2, test_fs->GetMetadataPassedOnOpenCount()); + + db.reset(); + ASSERT_OK(DestroyDB(test_dbname, options)); +} + +TEST_F(DBTest2, FastSstOpenMetadataPreservedAcrossTrivialMove) { + // Regression test: FileMetaData::file_open_metadata must survive a trivial + // move. Trivial move re-records an SST at a new level via DeleteFile + + // AddFile without physically rewriting the file. If AddFile drops + // file_open_metadata, the file loses its fast_sst_open advantage until + // it is next rewritten by real compaction/flush/ingest. + auto test_fs = std::make_shared(env_->GetFileSystem()); + std::unique_ptr test_env(new CompositeEnvWrapper(env_, test_fs)); + + Options options; + options.env = test_env.get(); + options.fast_sst_open = true; + options.create_if_missing = true; + options.disable_auto_compactions = true; + + std::string test_dbname = test::PerThreadDBPath("fast_open_trivial_move"); + ASSERT_OK(DestroyDB(test_dbname, options)); + + std::unique_ptr db; + ASSERT_OK(DB::Open(options, test_dbname, &db)); + + // Flush a single SST at L0; metadata is retrieved once and persisted. + ASSERT_OK(db->Put(WriteOptions(), "key1", "value1")); + ASSERT_OK(db->Flush(FlushOptions())); + ASSERT_EQ(1, test_fs->GetMetadataRetrievedCount()); + + // Manual compaction. With a single L0 file and empty lower levels, the + // compaction picker converts this into a trivial move (no physical + // rewrite), exercising PrepareTrivialMoveEdit. + ASSERT_OK(db->CompactRange(CompactRangeOptions(), nullptr, nullptr)); + // If the SST had been rewritten, metadata retrieval would have re-run on + // the new file. Unchanged count confirms this was a trivial move. + ASSERT_EQ(1, test_fs->GetMetadataRetrievedCount()); + + db.reset(); + test_fs->ResetCounters(); + + // Reopen. The trivially-moved SST should still carry its + // file_open_metadata and be opened via the fast_sst_open fast path. + // Pre-fix, the trivial move stripped it and this count would be 0. + ASSERT_OK(DB::Open(options, test_dbname, &db)); + + std::string value; + ASSERT_OK(db->Get(ReadOptions(), "key1", &value)); + ASSERT_EQ("value1", value); + + ASSERT_EQ(1, test_fs->GetMetadataPassedOnOpenCount()); + + db.reset(); + ASSERT_OK(DestroyDB(test_dbname, options)); +} + } // namespace ROCKSDB_NAMESPACE int main(int argc, char** argv) { diff --git a/db/db_impl/db_impl_compaction_flush.cc b/db/db_impl/db_impl_compaction_flush.cc index f49dd1835e..d383924cd6 100644 --- a/db/db_impl/db_impl_compaction_flush.cc +++ b/db/db_impl/db_impl_compaction_flush.cc @@ -1623,16 +1623,16 @@ void DBImpl::PrepareTrivialMoveEdit(Compaction& c, LogBuffer* log_buffer, for (size_t i = 0; i < c.num_input_files(l); i++) { FileMetaData* f = c.input(l, i); c.edit()->DeleteFile(c.level(l), f->fd.GetNumber()); - c.edit()->AddFile(c.output_level(), f->fd.GetNumber(), f->fd.GetPathId(), - f->fd.GetFileSize(), f->smallest, f->largest, - f->fd.smallest_seqno, f->fd.largest_seqno, - f->marked_for_compaction, f->temperature, - f->oldest_blob_file_number, f->oldest_ancester_time, - f->file_creation_time, f->epoch_number, - f->file_checksum, f->file_checksum_func_name, - f->unique_id, f->compensated_range_deletion_size, - f->tail_size, f->user_defined_timestamps_persisted, - f->min_timestamp, f->max_timestamp); + c.edit()->AddFile( + c.output_level(), f->fd.GetNumber(), f->fd.GetPathId(), + f->fd.GetFileSize(), f->smallest, f->largest, f->fd.smallest_seqno, + f->fd.largest_seqno, f->marked_for_compaction, f->temperature, + f->oldest_blob_file_number, f->oldest_ancester_time, + f->file_creation_time, f->epoch_number, f->file_checksum, + f->file_checksum_func_name, f->unique_id, + f->compensated_range_deletion_size, f->tail_size, + f->user_defined_timestamps_persisted, f->min_timestamp, + f->max_timestamp, f->file_open_metadata); moved_bytes += static_cast(c.input(l, i)->fd.GetFileSize()); ROCKS_LOG_BUFFER( log_buffer, "[%s] Moved #%" PRIu64 " to level-%d %" PRIu64 " bytes\n", @@ -2209,7 +2209,7 @@ Status DBImpl::ReFitLevel(ColumnFamilyData* cfd, int level, int target_level) { f->file_checksum, f->file_checksum_func_name, f->unique_id, f->compensated_range_deletion_size, f->tail_size, f->user_defined_timestamps_persisted, f->min_timestamp, - f->max_timestamp); + f->max_timestamp, f->file_open_metadata); } ROCKS_LOG_DEBUG(immutable_db_options_.info_log, "[%s] Apply version edit:\n%s", cfd->GetName().c_str(), diff --git a/db/db_impl/db_impl_experimental.cc b/db/db_impl/db_impl_experimental.cc index bb6a9a2e40..1829a53c6f 100644 --- a/db/db_impl/db_impl_experimental.cc +++ b/db/db_impl/db_impl_experimental.cc @@ -145,7 +145,7 @@ Status DBImpl::PromoteL0(ColumnFamilyHandle* column_family, int target_level) { f->file_checksum_func_name, f->unique_id, f->compensated_range_deletion_size, f->tail_size, f->user_defined_timestamps_persisted, f->min_timestamp, - f->max_timestamp); + f->max_timestamp, f->file_open_metadata); } status = versions_->LogAndApply(cfd, read_options, write_options, &edit, diff --git a/db/db_impl/db_impl_open.cc b/db/db_impl/db_impl_open.cc index b0c6e7ccd1..63d60da800 100644 --- a/db/db_impl/db_impl_open.cc +++ b/db/db_impl/db_impl_open.cc @@ -683,7 +683,8 @@ Status DBImpl::Recover( f->file_checksum, f->file_checksum_func_name, f->unique_id, f->compensated_range_deletion_size, f->tail_size, f->user_defined_timestamps_persisted, - f->min_timestamp, f->max_timestamp); + f->min_timestamp, f->max_timestamp, + f->file_open_metadata); ROCKS_LOG_WARN(immutable_db_options_.info_log, "[%s] Moving #%" PRIu64 " from from_level-%d to from_level-%d %" PRIu64 diff --git a/db/experimental.cc b/db/experimental.cc index b6efc1a475..0eeee1e0fe 100644 --- a/db/experimental.cc +++ b/db/experimental.cc @@ -158,17 +158,17 @@ Status UpdateManifestForFilesState( // Current state inconsistent with manifest ++files_updated; edit.DeleteFile(level, number); - edit.AddFile(level, lf->fd.GetNumber(), lf->fd.GetPathId(), - lf->fd.GetFileSize(), lf->smallest, lf->largest, - lf->fd.smallest_seqno, lf->fd.largest_seqno, - lf->marked_for_compaction, temp, - lf->oldest_blob_file_number, - lf->oldest_ancester_time, lf->file_creation_time, - lf->epoch_number, lf->file_checksum, - lf->file_checksum_func_name, lf->unique_id, - lf->compensated_range_deletion_size, lf->tail_size, - lf->user_defined_timestamps_persisted, - lf->min_timestamp, lf->max_timestamp); + edit.AddFile( + level, lf->fd.GetNumber(), lf->fd.GetPathId(), + lf->fd.GetFileSize(), lf->smallest, lf->largest, + lf->fd.smallest_seqno, lf->fd.largest_seqno, + lf->marked_for_compaction, temp, lf->oldest_blob_file_number, + lf->oldest_ancester_time, lf->file_creation_time, + lf->epoch_number, lf->file_checksum, + lf->file_checksum_func_name, lf->unique_id, + lf->compensated_range_deletion_size, lf->tail_size, + lf->user_defined_timestamps_persisted, lf->min_timestamp, + lf->max_timestamp, lf->file_open_metadata); } } } else { diff --git a/db/version_edit.h b/db/version_edit.h index 4dad44f8a6..3edc9b790a 100644 --- a/db/version_edit.h +++ b/db/version_edit.h @@ -811,7 +811,8 @@ class VersionEdit { const uint64_t compensated_range_deletion_size, uint64_t tail_size, bool user_defined_timestamps_persisted, const std::string& min_timestamp = "", - const std::string& max_timestamp = "") { + const std::string& max_timestamp = "", + const std::string& file_open_metadata = "") { assert(smallest_seqno <= largest_seqno); new_files_.emplace_back( level, @@ -822,6 +823,9 @@ class VersionEdit { epoch_number, file_checksum, file_checksum_func_name, unique_id, compensated_range_deletion_size, tail_size, user_defined_timestamps_persisted, min_timestamp, max_timestamp)); + if (!file_open_metadata.empty()) { + new_files_.back().second.file_open_metadata = file_open_metadata; + } files_to_quarantine_.push_back(file); if (!HasLastSequence() || largest_seqno > GetLastSequence()) { SetLastSequence(largest_seqno); diff --git a/db/version_set.cc b/db/version_set.cc index acbbf5c0aa..1a2b697313 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -7677,6 +7677,11 @@ Status VersionSet::WriteCurrentStateToManifest( for (const auto& f : level_files) { assert(f); + // Preserve file_open_metadata so live SSTs still benefit from + // fast_sst_open after a MANIFEST rotation. Without this, the + // rotated MANIFEST snapshot drops kFileOpenMetadata for unchanged + // files and their fast-open advantage is lost until they are + // rewritten. edit.AddFile(level, f->fd.GetNumber(), f->fd.GetPathId(), f->fd.GetFileSize(), f->smallest, f->largest, f->fd.smallest_seqno, f->fd.largest_seqno, @@ -7686,7 +7691,7 @@ Status VersionSet::WriteCurrentStateToManifest( f->file_checksum_func_name, f->unique_id, f->compensated_range_deletion_size, f->tail_size, f->user_defined_timestamps_persisted, f->min_timestamp, - f->max_timestamp); + f->max_timestamp, f->file_open_metadata); } }