TableProperty::oldest_key_time defaults to 0

Summary:
We don't propagate TableProperty::oldest_key_time on compaction and just write the default value to SST files. It is more natural to default the value to 0.

Also revert db_sst_test back to before #2842.
Closes https://github.com/facebook/rocksdb/pull/3079

Differential Revision: D6165702

Pulled By: yiwu-arbug

fbshipit-source-id: ca3ce5928d96ae79a5beb12bb7d8c640a71478a0

db/builder.h

@@ -6,7 +6,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file. See the AUTHORS file for names of contributors.
 #pragma once
-#include <limits>
 #include <string>
 #include <utility>
 #include <vector>
@@ -53,7 +52,7 @@ TableBuilder* NewTableBuilder(
     const CompressionOptions& compression_opts, int level,
     const std::string* compression_dict = nullptr,
     const bool skip_filters = false, const uint64_t creation_time = 0,
-    const uint64_t oldest_key_time = std::numeric_limits<uint64_t>::max());
+    const uint64_t oldest_key_time = 0);
 
 // Build a Table file from the contents of *iter.  The generated file
 // will be named according to number specified in meta. On success, the rest of
@@ -80,7 +79,6 @@ extern Status BuildTable(
     EventLogger* event_logger = nullptr, int job_id = 0,
     const Env::IOPriority io_priority = Env::IO_HIGH,
     TableProperties* table_properties = nullptr, int level = -1,
-    const uint64_t creation_time = 0,
-    const uint64_t oldest_key_time = std::numeric_limits<uint64_t>::max());
+    const uint64_t creation_time = 0, const uint64_t oldest_key_time = 0);
 
 }  // namespace rocksdb

db/db_sst_test.cc

@@ -647,9 +647,18 @@ TEST_F(DBSSTTest, OpenDBWithInfiniteMaxOpenFiles) {
 }
 
 TEST_F(DBSSTTest, GetTotalSstFilesSize) {
+  // We don't propagate oldest-key-time table property on compaction and
+  // just write 0 as default value. This affect the exact table size, since
+  // we encode table properties as varint64. Force time to be 0 to work around
+  // it. Should remove the workaround after we propagate the property on
+  // compaction.
+  std::unique_ptr<MockTimeEnv> mock_env(new MockTimeEnv(Env::Default()));
+  mock_env->set_current_time(0);
+
   Options options = CurrentOptions();
   options.disable_auto_compactions = true;
   options.compression = kNoCompression;
+  options.env = mock_env.get();
   DestroyAndReopen(options);
   // Generate 5 files in L0
   for (int i = 0; i < 5; i++) {
@@ -698,13 +707,9 @@ TEST_F(DBSSTTest, GetTotalSstFilesSize) {
   ASSERT_TRUE(dbfull()->GetIntProperty("rocksdb.total-sst-files-size",
                                        &total_sst_files_size));
   // Live SST files = 1 (compacted file)
-  // The 5 bytes difference comes from oldest-key-time table property isn't
-  // propagated on compaction. It is written with default value
-  // std::numeric_limits<uint64_t>::max as varint64.
-  ASSERT_EQ(live_sst_files_size, 1 * single_file_size + 5);
-
-  // Total SST files = 5 original files + compacted file
-  ASSERT_EQ(total_sst_files_size, 5 * single_file_size + live_sst_files_size);
+  // Total SST files = 6 (5 original files + compacted file)
+  ASSERT_EQ(live_sst_files_size, 1 * single_file_size);
+  ASSERT_EQ(total_sst_files_size, 6 * single_file_size);
 
   // hold current version
   std::unique_ptr<Iterator> iter2(dbfull()->NewIterator(ReadOptions()));
@@ -725,14 +730,14 @@ TEST_F(DBSSTTest, GetTotalSstFilesSize) {
                                        &total_sst_files_size));
   // Live SST files = 0
   // Total SST files = 6 (5 original files + compacted file)
-  ASSERT_EQ(total_sst_files_size, 5 * single_file_size + live_sst_files_size);
+  ASSERT_EQ(total_sst_files_size, 6 * single_file_size);
 
   iter1.reset();
   ASSERT_TRUE(dbfull()->GetIntProperty("rocksdb.total-sst-files-size",
                                        &total_sst_files_size));
   // Live SST files = 0
   // Total SST files = 1 (compacted file)
-  ASSERT_EQ(total_sst_files_size, live_sst_files_size);
+  ASSERT_EQ(total_sst_files_size, 1 * single_file_size);
 
   iter2.reset();
   ASSERT_TRUE(dbfull()->GetIntProperty("rocksdb.total-sst-files-size",
@@ -740,6 +745,9 @@ TEST_F(DBSSTTest, GetTotalSstFilesSize) {
   // Live SST files = 0
   // Total SST files = 0
   ASSERT_EQ(total_sst_files_size, 0);
+
+  // Close db before mock_env destruct.
+  Close();
 }
 
 TEST_F(DBSSTTest, GetTotalSstFilesSizeVersionsFilesShared) {

db/flush_job.cc

@@ -16,7 +16,6 @@
 #include <inttypes.h>
 
 #include <algorithm>
-#include <limits>
 #include <vector>
 
 #include "db/builder.h"

db/internal_stats.cc

@@ -802,10 +802,15 @@ bool InternalStats::HandleEstimateOldestKeyTime(uint64_t* value, DBImpl* /*db*/,
   *value = std::numeric_limits<uint64_t>::max();
   for (auto& p : collection) {
     *value = std::min(*value, p.second->oldest_key_time);
+    if (*value == 0) {
+      break;
+    }
+  }
+  if (*value > 0) {
+    *value = std::min({cfd_->mem()->ApproximateOldestKeyTime(),
+                       cfd_->imm()->ApproximateOldestKeyTime(), *value});
   }
-  *value = std::min({cfd_->mem()->ApproximateOldestKeyTime(),
-                     cfd_->imm()->ApproximateOldestKeyTime(), *value});
-  return *value < std::numeric_limits<uint64_t>::max();
+  return *value > 0 && *value < std::numeric_limits<uint64_t>::max();
 }
 
 void InternalStats::DumpDBStats(std::string* value) {

include/rocksdb/table_properties.h

@@ -4,7 +4,6 @@
 #pragma once
 
 #include <stdint.h>
-#include <limits>
 #include <map>
 #include <string>
 #include "rocksdb/status.h"
@@ -164,8 +163,8 @@ struct TableProperties {
   // The time when the SST file was created.
   // Since SST files are immutable, this is equivalent to last modified time.
   uint64_t creation_time = 0;
-  // Timestamp of the earliest key
-  uint64_t oldest_key_time = std::numeric_limits<uint64_t>::max();
+  // Timestamp of the earliest key. 0 means unknown.
+  uint64_t oldest_key_time = 0;
 
   // Name of the column family with which this SST file is associated.
   // If column family is unknown, `column_family_name` will be an empty string.

table/block_based_table_builder.cc

@@ -10,7 +10,6 @@
 #include "table/block_based_table_builder.h"
 
 #include <assert.h>
-#include <inttypes.h>
 #include <stdio.h>
 
 #include <list>
@@ -276,7 +275,7 @@ struct BlockBasedTableBuilder::Rep {
   uint32_t column_family_id;
   const std::string& column_family_name;
   uint64_t creation_time = 0;
-  uint64_t oldest_key_time = std::numeric_limits<uint64_t>::max();
+  uint64_t oldest_key_time = 0;
 
   std::vector<std::unique_ptr<IntTblPropCollector>> table_properties_collectors;
 

table/block_based_table_builder.h

@@ -48,7 +48,7 @@ class BlockBasedTableBuilder : public TableBuilder {
       const CompressionOptions& compression_opts,
       const std::string* compression_dict, const bool skip_filters,
       const std::string& column_family_name, const uint64_t creation_time = 0,
-      const uint64_t oldest_key_time = std::numeric_limits<uint64_t>::max());
+      const uint64_t oldest_key_time = 0);
 
   // REQUIRES: Either Finish() or Abandon() has been called.
   ~BlockBasedTableBuilder();

table/table_builder.h

@@ -10,7 +10,6 @@
 #pragma once
 
 #include <stdint.h>
-#include <limits>
 #include <string>
 #include <utility>
 #include <vector>
@@ -56,8 +55,7 @@ struct TableBuilderOptions {
       const CompressionOptions& _compression_opts,
       const std::string* _compression_dict, bool _skip_filters,
       const std::string& _column_family_name, int _level,
-      const uint64_t _creation_time = 0,
-      const int64_t _oldest_key_time = std::numeric_limits<uint64_t>::max())
+      const uint64_t _creation_time = 0, const int64_t _oldest_key_time = 0)
       : ioptions(_ioptions),
         internal_comparator(_internal_comparator),
         int_tbl_prop_collector_factories(_int_tbl_prop_collector_factories),