Remove unused fields from FileMetaData (temporarily) (#10443)

Summary: FileMetaData::[min|max]_timestamp are not currently being used or tracked by RocksDB, even when user-defined timestamp is enabled. Each of them is a std::string which can occupy 32 bytes. Remove them for now. They may be added back when we have a pressing need for them. When we do add them back, consider store them in a more compact way, e.g. one boolean flag and a byte array of size 16. Per file min/max timestamp bounds are available as table properties. Pull Request resolved: #10443 Test Plan: make check Reviewed By: pdillinger Differential Revision: D38292275 Pulled By: riversand963 fbshipit-source-id: 841dc4e855ad8f8481c80cb020603de9607c9c94
facebook · Aug 2, 2022 · fbfcf5c · fbfcf5c
1 parent cc20998
commit fbfcf5c
Show file tree

Hide file tree

Showing 17 changed files with 79 additions and 176 deletions.
diff --git a/HISTORY.md b/HISTORY.md
@@ -22,6 +22,7 @@
 ### Behavior Change
 * Added checksum handshake during the copying of decompressed WAL fragment. This together with #9875, #10037, #10212, #10114 and #10319 provides end-to-end integrity protection for write batch during recovery.
 * PosixLogger is removed and by default EnvLogger will be used for info logging. The behavior of the two loggers should be very similar when using the default Posix Env.
+* Remove [min|max]_timestamp from VersionEdit for now since they are not tracked in MANIFEST anyway but consume two empty std::string (up to 64 bytes) for each file. Should they be added back in the future, we should store them more compactly.
 
 ## 7.5.0 (07/15/2022)
 ### New Features

diff --git a/db/compaction/compaction_job_test.cc b/db/compaction/compaction_job_test.cc
@@ -372,8 +372,7 @@ class CompactionJobTestBase : public testing::Test {
                  smallest_seqno, largest_seqno, false, Temperature::kUnknown,
                  oldest_blob_file_number, kUnknownOldestAncesterTime,
                  kUnknownFileCreationTime, kUnknownFileChecksum,
-                 kUnknownFileChecksumFuncName, kDisableUserTimestamp,
-                 kDisableUserTimestamp, kNullUniqueId64x2);
+                 kUnknownFileChecksumFuncName, kNullUniqueId64x2);
 
     mutex_.Lock();
     EXPECT_OK(

diff --git a/db/compaction/compaction_picker_test.cc b/db/compaction/compaction_picker_test.cc
@@ -116,8 +116,7 @@ class CompactionPickerTest : public testing::Test {
         InternalKey(largest, largest_seq, kTypeValue), smallest_seq,
         largest_seq, marked_for_compact, temperature, kInvalidBlobFileNumber,
         kUnknownOldestAncesterTime, kUnknownFileCreationTime,
-        kUnknownFileChecksum, kUnknownFileChecksumFuncName,
-        kDisableUserTimestamp, kDisableUserTimestamp, kNullUniqueId64x2);
+        kUnknownFileChecksum, kUnknownFileChecksumFuncName, kNullUniqueId64x2);
     f->compensated_file_size =
         (compensated_file_size != 0) ? compensated_file_size : file_size;
     f->oldest_ancester_time = oldest_ancestor_time;

diff --git a/db/db_impl/db_impl_compaction_flush.cc b/db/db_impl/db_impl_compaction_flush.cc
@@ -1686,8 +1686,7 @@ Status DBImpl::ReFitLevel(ColumnFamilyData* cfd, int level, int target_level) {
           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->file_checksum,
-          f->file_checksum_func_name, f->min_timestamp, f->max_timestamp,
-          f->unique_id);
+          f->file_checksum_func_name, f->unique_id);
     }
     ROCKS_LOG_DEBUG(immutable_db_options_.info_log,
                     "[%s] Apply version edit:\n%s", cfd->GetName().c_str(),
@@ -3313,7 +3312,7 @@ Status DBImpl::BackgroundCompaction(bool* made_progress,
             f->fd.largest_seqno, f->marked_for_compaction, f->temperature,
             f->oldest_blob_file_number, f->oldest_ancester_time,
             f->file_creation_time, f->file_checksum, f->file_checksum_func_name,
-            f->min_timestamp, f->max_timestamp, f->unique_id);
+            f->unique_id);
 
         ROCKS_LOG_BUFFER(
             log_buffer,

diff --git a/db/db_impl/db_impl_experimental.cc b/db/db_impl/db_impl_experimental.cc
@@ -131,13 +131,13 @@ Status DBImpl::PromoteL0(ColumnFamilyHandle* column_family, int target_level) {
     edit.SetColumnFamily(cfd->GetID());
     for (const auto& f : l0_files) {
       edit.DeleteFile(0, f->fd.GetNumber());
-      edit.AddFile(
-          target_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->file_checksum, f->file_checksum_func_name,
-          f->min_timestamp, f->max_timestamp, f->unique_id);
+      edit.AddFile(target_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->file_checksum,
+                   f->file_checksum_func_name, f->unique_id);
     }
 
     status = versions_->LogAndApply(cfd, *cfd->GetLatestMutableCFOptions(),

diff --git a/db/db_impl/db_impl_open.cc b/db/db_impl/db_impl_open.cc
@@ -1600,8 +1600,7 @@ Status DBImpl::WriteLevel0TableForRecovery(int job_id, ColumnFamilyData* cfd,
                   meta.marked_for_compaction, meta.temperature,
                   meta.oldest_blob_file_number, meta.oldest_ancester_time,
                   meta.file_creation_time, meta.file_checksum,
-                  meta.file_checksum_func_name, meta.min_timestamp,
-                  meta.max_timestamp, meta.unique_id);
+                  meta.file_checksum_func_name, meta.unique_id);
 
     for (const auto& blob : blob_file_additions) {
       edit->AddBlobFile(blob);

diff --git a/db/experimental.cc b/db/experimental.cc
@@ -105,14 +105,13 @@ Status UpdateManifestForFilesState(
               // Current state inconsistent with manifest
               ++files_updated;
               edit.DeleteFile(level, number);
-              edit.AddFile(level, number, 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->file_checksum, lf->file_checksum_func_name,
-                           lf->min_timestamp, lf->max_timestamp, lf->unique_id);
+              edit.AddFile(
+                  level, number, 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->file_checksum,
+                  lf->file_checksum_func_name, lf->unique_id);
             }
           }
         } else {

diff --git a/db/external_sst_file_ingestion_job.cc b/db/external_sst_file_ingestion_job.cc
@@ -450,8 +450,7 @@ Status ExternalSstFileIngestionJob::Run() {
         f.smallest_internal_key, f.largest_internal_key, f.assigned_seqno,
         f.assigned_seqno, false, f.file_temperature, kInvalidBlobFileNumber,
         oldest_ancester_time, current_time, f.file_checksum,
-        f.file_checksum_func_name, kDisableUserTimestamp, kDisableUserTimestamp,
-        f.unique_id);
+        f.file_checksum_func_name, f.unique_id);
     f_metadata.temperature = f.file_temperature;
     edit_.AddFile(f.picked_level, f_metadata);
   }

diff --git a/db/flush_job.cc b/db/flush_job.cc
@@ -1000,8 +1000,7 @@ Status FlushJob::WriteLevel0Table() {
                    meta_.marked_for_compaction, meta_.temperature,
                    meta_.oldest_blob_file_number, meta_.oldest_ancester_time,
                    meta_.file_creation_time, meta_.file_checksum,
-                   meta_.file_checksum_func_name, meta_.min_timestamp,
-                   meta_.max_timestamp, meta_.unique_id);
+                   meta_.file_checksum_func_name, meta_.unique_id);
 
     edit_->SetBlobFileAdditions(std::move(blob_file_additions));
   }

diff --git a/db/import_column_family_job.cc b/db/import_column_family_job.cc
@@ -160,7 +160,7 @@ Status ImportColumnFamilyJob::Run() {
                   file_metadata.largest_seqno, false, file_metadata.temperature,
                   kInvalidBlobFileNumber, oldest_ancester_time, current_time,
                   kUnknownFileChecksum, kUnknownFileChecksumFuncName,
-                  kDisableUserTimestamp, kDisableUserTimestamp, f.unique_id);
+                  f.unique_id);
 
     // If incoming sequence number is higher, update local sequence number.
     if (file_metadata.largest_seqno > versions_->LastSequence()) {

diff --git a/db/repair.cc b/db/repair.cc
@@ -651,7 +651,6 @@ class Repairer {
             table->meta.temperature, table->meta.oldest_blob_file_number,
             table->meta.oldest_ancester_time, table->meta.file_creation_time,
             table->meta.file_checksum, table->meta.file_checksum_func_name,
-            table->meta.min_timestamp, table->meta.max_timestamp,
             table->meta.unique_id);
       }
       assert(next_file_number_ > 0);