From 8e949116f78b03a7c68f262af2fb4b56b427e35b Mon Sep 17 00:00:00 2001
From: Hui Xiao <huixiao@fb.com>
Date: Wed, 4 Oct 2023 14:42:35 -0700
Subject: [PATCH] Fix comments about
 creation_time/oldest_ancester_time/oldest_key_time (#11921)

Summary:
Code reference for the comments change:

https://github.com/facebook/rocksdb/blob/40b618f2349b509eabdd175f75faf7ce84cf0696/table/block_based/block_based_table_builder.cc?fbclid=IwAR0JlfnG8wysclFP5wv0fSngFbi_j32BUCKbFayeGdr10tzDhyyk5QqpclA#L2093

https://github.com/facebook/rocksdb/blob/40b618f2349b509eabdd175f75faf7ce84cf0696/db/flush_job.cc?fbclid=IwAR1ri6eTX3wyD_2fAEBRzFSwZItcbmDS8LaB11k1letDMQmB2L8nF6TfXDs#L945-L949

https://github.com/facebook/rocksdb/blob/40b618f2349b509eabdd175f75faf7ce84cf0696/db/compaction/compaction_job.cc#L1882-L1904

Pull Request resolved: https://github.com/facebook/rocksdb/pull/11921

Reviewed By: cbi42

Differential Revision: D49921304

Pulled By: hx235

fbshipit-source-id: 2ae17e43c0fd52044404d7b63fea254d2d1f3595
---
 db/compaction/compaction_job.cc    |  1 +
 db/version_edit.h                  | 14 ++++++++++----
 include/rocksdb/table_properties.h | 17 ++++++++++++++---
 3 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/db/compaction/compaction_job.cc b/db/compaction/compaction_job.cc
index bf8ce25a67..257848e46e 100644
--- a/db/compaction/compaction_job.cc
+++ b/db/compaction/compaction_job.cc
@@ -1944,6 +1944,7 @@ Status CompactionJob::OpenCompactionOutputFile(SubcompactionState* sub_compact,
       db_options_.stats, listeners, db_options_.file_checksum_gen_factory.get(),
       tmp_set.Contains(FileType::kTableFile), false));
 
+  // TODO(hx235): pass in the correct `oldest_key_time` instead of `0`
   TableBuilderOptions tboptions(
       *cfd->ioptions(), *(sub_compact->compaction->mutable_cf_options()),
       cfd->internal_comparator(), cfd->int_tbl_prop_collector_factories(),
diff --git a/db/version_edit.h b/db/version_edit.h
index e6d54d31d1..5d7687204f 100644
--- a/db/version_edit.h
+++ b/db/version_edit.h
@@ -219,10 +219,16 @@ struct FileMetaData {
   // refers to. 0 is an invalid value; BlobDB numbers the files starting from 1.
   uint64_t oldest_blob_file_number = kInvalidBlobFileNumber;
 
-  // The file could be the compaction output from other SST files, which could
-  // in turn be outputs for compact older SST files. We track the memtable
-  // flush timestamp for the oldest SST file that eventually contribute data
-  // to this file. 0 means the information is not available.
+  // For flush output file, oldest ancestor time is the oldest key time in the
+  // file.  If the oldest key time is not available, flush time is used.
+  //
+  // For compaction output file, oldest ancestor time is the oldest
+  // among all the oldest key time of its input files, since the file could be
+  // the compaction output from other SST files, which could in turn be outputs
+  // for compact older SST files. If that's not available, creation time of this
+  // compaction output file is used.
+  //
+  // 0 means the information is not available.
   uint64_t oldest_ancester_time = kUnknownOldestAncesterTime;
 
   // Unix time when the SST file is created.
diff --git a/include/rocksdb/table_properties.h b/include/rocksdb/table_properties.h
index ebde339ddd..0256fbddd2 100644
--- a/include/rocksdb/table_properties.h
+++ b/include/rocksdb/table_properties.h
@@ -219,9 +219,20 @@ struct TableProperties {
   // by column_family_name.
   uint64_t column_family_id = ROCKSDB_NAMESPACE::
       TablePropertiesCollectorFactory::Context::kUnknownColumnFamily;
-  // Timestamp of the latest key. 0 means unknown.
-  // TODO(sagar0): Should be changed to latest_key_time ... but don't know the
-  // full implications of backward compatibility. Hence retaining for now.
+
+  // Oldest ancester time. 0 means unknown.
+  //
+  // For flush output file, oldest ancestor time is the oldest key time in the
+  // file.  If the oldest key time is not available, flush time is used.
+  //
+  // For compaction output file, oldest ancestor time is the oldest
+  // among all the oldest key time of its input files, since the file could be
+  // the compaction output from other SST files, which could in turn be outputs
+  // for compact older SST files. If that's not available, creation time of this
+  // compaction output file is used.
+  //
+  // TODO(sagar0): Should be changed to oldest_ancester_time ... but don't know
+  // the full implications of backward compatibility. Hence retaining for now.
   uint64_t creation_time = 0;
 
   // Timestamp of the earliest key. 0 means unknown.