Fix windows build errors (rdtsc and fnptr) (#12008)

Summary:
Combining best parts of https://github.com/facebook/rocksdb/issues/11794 and https://github.com/facebook/rocksdb/issues/11766, fixing the CircleCI config in the latter. I was going to amend the latter but wasn't granted access.

Ideally this would be ported back to 8.4 branch and crc32c part into 8.3 branch.

BACKPORT NOTE: removed CircleCI config changes from this diff as they
depend (at least superficially) on changes not in this branch.

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

Test Plan: CI

Reviewed By: hx235

Differential Revision: D50616172

Pulled By: pdillinger

fbshipit-source-id: fa7f778bc281e881a140522e774f480c6d1e5f48

CMakeLists.txt

@@ -1592,3 +1592,6 @@ option(WITH_BENCHMARK "build benchmark tests" OFF)
 if(WITH_BENCHMARK)
   add_subdirectory(${PROJECT_SOURCE_DIR}/microbench/)
 endif()
+
+target_include_directories(${PROJECT_NAME} PUBLIC
+  $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/include>)

HISTORY.md

@@ -1,6 +1,73 @@
 # Rocksdb Change Log
 > NOTE: Entries for next release do not go here. Follow instructions in `unreleased_history/README.txt`
 
+## 8.5.4 (09/26/2023)
+### Bug Fixes
+* Fixed a bug where compaction read under non direct IO still falls back to RocksDB internal prefetching after file system's prefetching returns non-OK status other than `Status::NotSupported()`
+
+### Behavior Changes
+* For non direct IO, eliminate the file system prefetching attempt for compaction read when `Options::compaction_readahead_size` is 0
+
+## 8.5.3 (09/01/2023)
+### Bug Fixes
+* Fixed a race condition in `GenericRateLimiter` that could cause it to stop granting requests
+
+## 8.5.2 (08/31/2023)
+### Bug fixes
+* Fix a bug where iterator may return incorrect result for DeleteRange() users if there was an error reading from a file.
+
+## 8.5.1 (08/31/2023)
+### Bug fixes
+* Fix a bug where if there is an error reading from offset 0 of a file from L1+ and that the file is not the first file in the sorted run, data can be lost in compaction and read/scan can return incorrect results.
+
+## 8.5.0 (07/21/2023)
+### Public API Changes
+* Removed recently added APIs `GeneralCache` and `MakeSharedGeneralCache()` as our plan changed to stop exposing a general-purpose cache interface. The old forms of these APIs, `Cache` and `NewLRUCache()`, are still available, although general-purpose caching support will be dropped eventually.
+
+### Behavior Changes
+* Option `periodic_compaction_seconds` no longer supports FIFO compaction: setting it has no effect on FIFO compactions. FIFO compaction users should only set option `ttl` instead.
+* Move prefetching responsibility to page cache for compaction read for non directIO use case
+
+### Performance Improvements
+* In case of direct_io, if buffer passed by callee is already aligned, RandomAccessFileRead::Read will avoid realloacting a new buffer, reducing memcpy and use already passed aligned buffer.
+* Small efficiency improvement to HyperClockCache by reducing chance of compiler-generated heap allocations
+
+### Bug Fixes
+* Fix use_after_free bug in async_io MultiReads when underlying FS enabled kFSBuffer. kFSBuffer is when underlying FS pass their own buffer instead of using RocksDB scratch in FSReadRequest. Right now it's an experimental feature.
+* Fix a bug in FileTTLBooster that can cause users with a large number of levels (more than 65) to see errors like "runtime error: shift exponent .. is too large.." (#11673).
+
+## 8.4.0 (06/26/2023)
+### New Features
+* Add FSReadRequest::fs_scratch which is a data buffer allocated and provided by underlying FileSystem to RocksDB during reads, when FS wants to provide its own buffer with data instead of using RocksDB provided FSReadRequest::scratch. This can help in cpu optimization by avoiding copy from file system's buffer to RocksDB buffer. More details on how to use/enable it in file_system.h. Right now its supported only for MultiReads(async + sync) with non direct io.
+* Start logging non-zero user-defined timestamp sizes in WAL to signal user key format in subsequent records and use it during recovery. This change will break recovery from WAL files written by early versions that contain user-defined timestamps. The workaround is to ensure there are no WAL files to recover (i.e. by flushing before close) before upgrade.
+* Added new property "rocksdb.obsolete-sst-files-size-property" that reports the size of SST files that have become obsolete but have not yet been deleted or scheduled for deletion
+* Start to record the value of the flag `AdvancedColumnFamilyOptions.persist_user_defined_timestamps` in the Manifest and table properties for a SST file when it is created. And use the recorded flag when creating a table reader for the SST file. This flag is only explicitly record if it's false.
+* Add a new option OptimisticTransactionDBOptions::shared_lock_buckets that enables sharing mutexes for validating transactions between DB instances, for better balancing memory efficiency and validation contention across DB instances. Different column families and DBs also now use different hash seeds in this validation, so that the same set of key names will not contend across DBs or column families.
+* Add a new ticker `rocksdb.files.marked.trash.deleted` to track the number of trash files deleted by background thread from the trash queue.
+* Add an API NewTieredVolatileCache() in include/rocksdb/cache.h to allocate an instance of a block cache with a primary block cache tier and a compressed secondary cache tier. A cache of this type distributes memory reservations against the block cache, such as WriteBufferManager, table reader memory etc., proportionally across both the primary and compressed secondary cache.
+* Add `WaitForCompact()` to wait for all flush and compactions jobs to finish. Jobs to wait include the unscheduled (queued, but not scheduled yet).
+* Add `WriteBatch::Release()` that releases the batch's serialized data to the caller.
+
+### Public API Changes
+* Add C API `rocksdb_options_add_compact_on_deletion_collector_factory_del_ratio`.
+* change the FileSystem::use_async_io() API to SupportedOps API in order to extend it to various operations supported by underlying FileSystem. Right now it contains FSSupportedOps::kAsyncIO and FSSupportedOps::kFSBuffer. More details about FSSupportedOps in filesystem.h
+* Add new tickers: `rocksdb.error.handler.bg.error.count`, `rocksdb.error.handler.bg.io.error.count`, `rocksdb.error.handler.bg.retryable.io.error.count` to replace the misspelled ones: `rocksdb.error.handler.bg.errro.count`, `rocksdb.error.handler.bg.io.errro.count`, `rocksdb.error.handler.bg.retryable.io.errro.count` ('error' instead of 'errro'). Users should switch to use the new tickers before 9.0 release as the misspelled old tickers will be completely removed then.
+* Overload the API CreateColumnFamilyWithImport() to support creating ColumnFamily by importing multiple ColumnFamilies It requires that CFs should not overlap in user key range.
+
+### Behavior Changes
+* Change the default value for option `level_compaction_dynamic_level_bytes` to true. This affects users who use leveled compaction and do not set this option explicitly. These users may see additional background compactions following DB open. These compactions help to shape the LSM according to `level_compaction_dynamic_level_bytes` such that the size of each level Ln is approximately size of Ln-1 * `max_bytes_for_level_multiplier`. Turning on this option has other benefits too: see more detail in wiki: https://github.com/facebook/rocksdb/wiki/Leveled-Compaction#option-level_compaction_dynamic_level_bytes-and-levels-target-size and in option comment in advanced_options.h (#11525).
+* For Leveled Compaction users, `CompactRange()` will now always try to compact to the last non-empty level. (#11468)
+For Leveled Compaction users, `CompactRange()` with `bottommost_level_compaction = BottommostLevelCompaction::kIfHaveCompactionFilter` will behave similar to `kForceOptimized` in that it will skip files created during this manual compaction when compacting files in the bottommost level. (#11468)
+* RocksDB will try to drop range tombstones during non-bottommost compaction when it is safe to do so. (#11459)
+* When a DB is openend with `allow_ingest_behind=true` (currently only Universal compaction is supported), files in the last level, i.e. the ingested files,  will not be included in any compaction. (#11489)
+* Statistics `rocksdb.sst.read.micros` scope is expanded to all SST reads except for file ingestion and column family import (some compaction reads were previously excluded).
+
+### Bug Fixes
+* Reduced cases of illegally using Env::Default() during static destruction by never destroying the internal PosixEnv itself (except for builds checking for memory leaks). (#11538)
+* Fix extra prefetching during seek in async_io when BlockBasedTableOptions.num_file_reads_for_auto_readahead is 1 leading to extra reads than required.
+* Fix a bug where compactions that are qualified to be run as 2 subcompactions were only run as one subcompaction.
+* Fix a use-after-move bug in block.cc.
+
 ## 8.3.0 (05/19/2023)
 ### New Features
 * Introduced a new option `block_protection_bytes_per_key`, which can be used to enable per key-value integrity protection for in-memory blocks in block cache (#11287).

cache/cache_bench_tool.cc

@@ -77,6 +77,10 @@ DEFINE_bool(lean, false,
             "If true, no additional computation is performed besides cache "
             "operations.");
 
+DEFINE_bool(early_exit, false,
+            "Exit before deallocating most memory. Good for malloc stats, e.g."
+            "MALLOC_CONF=\"stats_print:true\"");
+
 DEFINE_string(secondary_cache_uri, "",
               "Full URI for creating a custom secondary cache object");
 static class std::shared_ptr<ROCKSDB_NAMESPACE::SecondaryCache> secondary_cache;
@@ -593,6 +597,10 @@ class CacheBench {
       }
       thread->latency_ns_hist.Add(timer.ElapsedNanos());
     }
+    if (FLAGS_early_exit) {
+      MutexLock l(thread->shared->GetMutex());
+      exit(0);
+    }
     if (handle) {
       cache_->Release(handle);
       handle = nullptr;

cache/clock_cache.cc

@@ -9,6 +9,7 @@
 
 #include "cache/clock_cache.h"
 
+#include <cassert>
 #include <functional>
 #include <numeric>
 
@@ -118,74 +119,6 @@ inline bool ClockUpdate(ClockHandle& h) {
   }
 }
 
-}  // namespace
-
-void ClockHandleBasicData::FreeData(MemoryAllocator* allocator) const {
-  if (helper->del_cb) {
-    helper->del_cb(value, allocator);
-  }
-}
-
-HyperClockTable::HyperClockTable(
-    size_t capacity, bool /*strict_capacity_limit*/,
-    CacheMetadataChargePolicy metadata_charge_policy,
-    MemoryAllocator* allocator,
-    const Cache::EvictionCallback* eviction_callback, const uint32_t* hash_seed,
-    const Opts& opts)
-    : length_bits_(CalcHashBits(capacity, opts.estimated_value_size,
-                                metadata_charge_policy)),
-      length_bits_mask_((size_t{1} << length_bits_) - 1),
-      occupancy_limit_(static_cast<size_t>((uint64_t{1} << length_bits_) *
-                                           kStrictLoadFactor)),
-      array_(new HandleImpl[size_t{1} << length_bits_]),
-      allocator_(allocator),
-      eviction_callback_(*eviction_callback),
-      hash_seed_(*hash_seed) {
-  if (metadata_charge_policy ==
-      CacheMetadataChargePolicy::kFullChargeCacheMetadata) {
-    usage_ += size_t{GetTableSize()} * sizeof(HandleImpl);
-  }
-
-  static_assert(sizeof(HandleImpl) == 64U,
-                "Expecting size / alignment with common cache line size");
-}
-
-HyperClockTable::~HyperClockTable() {
-  // Assumes there are no references or active operations on any slot/element
-  // in the table.
-  for (size_t i = 0; i < GetTableSize(); i++) {
-    HandleImpl& h = array_[i];
-    switch (h.meta >> ClockHandle::kStateShift) {
-      case ClockHandle::kStateEmpty:
-        // noop
-        break;
-      case ClockHandle::kStateInvisible:  // rare but possible
-      case ClockHandle::kStateVisible:
-        assert(GetRefcount(h.meta) == 0);
-        h.FreeData(allocator_);
-#ifndef NDEBUG
-        Rollback(h.hashed_key, &h);
-        ReclaimEntryUsage(h.GetTotalCharge());
-#endif
-        break;
-      // otherwise
-      default:
-        assert(false);
-        break;
-    }
-  }
-
-#ifndef NDEBUG
-  for (size_t i = 0; i < GetTableSize(); i++) {
-    assert(array_[i].displacements.load() == 0);
-  }
-#endif
-
-  assert(usage_.load() == 0 ||
-         usage_.load() == size_t{GetTableSize()} * sizeof(HandleImpl));
-  assert(occupancy_ == 0);
-}
-
 // If an entry doesn't receive clock updates but is repeatedly referenced &
 // released, the acquire and release counters could overflow without some
 // intervention. This is that intervention, which should be inexpensive
@@ -259,8 +192,209 @@ inline void CorrectNearOverflow(uint64_t old_meta,
   }
 }
 
-inline Status HyperClockTable::ChargeUsageMaybeEvictStrict(
-    size_t total_charge, size_t capacity, bool need_evict_for_occupancy) {
+inline bool BeginSlotInsert(const ClockHandleBasicData& proto, ClockHandle& h,
+                            uint64_t initial_countdown, bool* already_matches) {
+  assert(*already_matches == false);
+  // Optimistically transition the slot from "empty" to
+  // "under construction" (no effect on other states)
+  uint64_t old_meta = h.meta.fetch_or(
+      uint64_t{ClockHandle::kStateOccupiedBit} << ClockHandle::kStateShift,
+      std::memory_order_acq_rel);
+  uint64_t old_state = old_meta >> ClockHandle::kStateShift;
+
+  if (old_state == ClockHandle::kStateEmpty) {
+    // We've started inserting into an available slot, and taken
+    // ownership.
+    return true;
+  } else if (old_state != ClockHandle::kStateVisible) {
+    // Slot not usable / touchable now
+    return false;
+  }
+  // Existing, visible entry, which might be a match.
+  // But first, we need to acquire a ref to read it. In fact, number of
+  // refs for initial countdown, so that we boost the clock state if
+  // this is a match.
+  old_meta =
+      h.meta.fetch_add(ClockHandle::kAcquireIncrement * initial_countdown,
+                       std::memory_order_acq_rel);
+  // Like Lookup
+  if ((old_meta >> ClockHandle::kStateShift) == ClockHandle::kStateVisible) {
+    // Acquired a read reference
+    if (h.hashed_key == proto.hashed_key) {
+      // Match. Release in a way that boosts the clock state
+      old_meta =
+          h.meta.fetch_add(ClockHandle::kReleaseIncrement * initial_countdown,
+                           std::memory_order_acq_rel);
+      // Correct for possible (but rare) overflow
+      CorrectNearOverflow(old_meta, h.meta);
+      // Insert detached instead (only if return handle needed)
+      *already_matches = true;
+      return false;
+    } else {
+      // Mismatch. Pretend we never took the reference
+      old_meta =
+          h.meta.fetch_sub(ClockHandle::kAcquireIncrement * initial_countdown,
+                           std::memory_order_acq_rel);
+    }
+  } else if (UNLIKELY((old_meta >> ClockHandle::kStateShift) ==
+                      ClockHandle::kStateInvisible)) {
+    // Pretend we never took the reference
+    // WART/FIXME?: there's a tiny chance we release last ref to invisible
+    // entry here. If that happens, we let eviction take care of it.
+    old_meta =
+        h.meta.fetch_sub(ClockHandle::kAcquireIncrement * initial_countdown,
+                         std::memory_order_acq_rel);
+  } else {
+    // For other states, incrementing the acquire counter has no effect
+    // so we don't need to undo it.
+    // Slot not usable / touchable now.
+  }
+  (void)old_meta;
+  return false;
+}
+
+inline void FinishSlotInsert(const ClockHandleBasicData& proto, ClockHandle& h,
+                             uint64_t initial_countdown, bool keep_ref) {
+  // Save data fields
+  ClockHandleBasicData* h_alias = &h;
+  *h_alias = proto;
+
+  // Transition from "under construction" state to "visible" state
+  uint64_t new_meta = uint64_t{ClockHandle::kStateVisible}
+                      << ClockHandle::kStateShift;
+
+  // Maybe with an outstanding reference
+  new_meta |= initial_countdown << ClockHandle::kAcquireCounterShift;
+  new_meta |= (initial_countdown - keep_ref)
+              << ClockHandle::kReleaseCounterShift;
+
+#ifndef NDEBUG
+  // Save the state transition, with assertion
+  uint64_t old_meta = h.meta.exchange(new_meta, std::memory_order_release);
+  assert(old_meta >> ClockHandle::kStateShift ==
+         ClockHandle::kStateConstruction);
+#else
+  // Save the state transition
+  h.meta.store(new_meta, std::memory_order_release);
+#endif
+}
+
+bool TryInsert(const ClockHandleBasicData& proto, ClockHandle& h,
+               uint64_t initial_countdown, bool keep_ref,
+               bool* already_matches) {
+  bool b = BeginSlotInsert(proto, h, initial_countdown, already_matches);
+  if (b) {
+    FinishSlotInsert(proto, h, initial_countdown, keep_ref);
+  }
+  return b;
+}
+
+template <class HandleImpl, class Func>
+void ConstApplyToEntriesRange(Func /*const HandleImpl& -> void*/ func,
+                              const HandleImpl* begin, const HandleImpl* end,
+                              bool apply_if_will_be_deleted) {
+  uint64_t check_state_mask = ClockHandle::kStateShareableBit;
+  if (!apply_if_will_be_deleted) {
+    check_state_mask |= ClockHandle::kStateVisibleBit;
+  }
+
+  for (const HandleImpl* h = begin; h < end; ++h) {
+    // Note: to avoid using compare_exchange, we have to be extra careful.
+    uint64_t old_meta = h->meta.load(std::memory_order_relaxed);
+    // Check if it's an entry visible to lookups
+    if ((old_meta >> ClockHandle::kStateShift) & check_state_mask) {
+      // Increment acquire counter. Note: it's possible that the entry has
+      // completely changed since we loaded old_meta, but incrementing acquire
+      // count is always safe. (Similar to optimistic Lookup here.)
+      old_meta = h->meta.fetch_add(ClockHandle::kAcquireIncrement,
+                                   std::memory_order_acquire);
+      // Check whether we actually acquired a reference.
+      if ((old_meta >> ClockHandle::kStateShift) &
+          ClockHandle::kStateShareableBit) {
+        // Apply func if appropriate
+        if ((old_meta >> ClockHandle::kStateShift) & check_state_mask) {
+          func(*h);
+        }
+        // Pretend we never took the reference
+        h->meta.fetch_sub(ClockHandle::kAcquireIncrement,
+                          std::memory_order_release);
+        // No net change, so don't need to check for overflow
+      } else {
+        // For other states, incrementing the acquire counter has no effect
+        // so we don't need to undo it. Furthermore, we cannot safely undo
+        // it because we did not acquire a read reference to lock the
+        // entry in a Shareable state.
+      }
+    }
+  }
+}
+
+}  // namespace
+
+void ClockHandleBasicData::FreeData(MemoryAllocator* allocator) const {
+  if (helper->del_cb) {
+    helper->del_cb(value, allocator);
+  }
+}
+
+template <class HandleImpl>
+HandleImpl* BaseClockTable::StandaloneInsert(
+    const ClockHandleBasicData& proto) {
+  // Heap allocated separate from table
+  HandleImpl* h = new HandleImpl();
+  ClockHandleBasicData* h_alias = h;
+  *h_alias = proto;
+  h->SetStandalone();
+  // Single reference (standalone entries only created if returning a refed
+  // Handle back to user)
+  uint64_t meta = uint64_t{ClockHandle::kStateInvisible}
+                  << ClockHandle::kStateShift;
+  meta |= uint64_t{1} << ClockHandle::kAcquireCounterShift;
+  h->meta.store(meta, std::memory_order_release);
+  // Keep track of how much of usage is standalone
+  standalone_usage_.fetch_add(proto.GetTotalCharge(),
+                              std::memory_order_relaxed);
+  return h;
+}
+
+template <class Table>
+typename Table::HandleImpl* BaseClockTable::CreateStandalone(
+    ClockHandleBasicData& proto, size_t capacity, bool strict_capacity_limit,
+    bool allow_uncharged) {
+  Table& derived = static_cast<Table&>(*this);
+  typename Table::InsertState state;
+  derived.StartInsert(state);
+
+  const size_t total_charge = proto.GetTotalCharge();
+  if (strict_capacity_limit) {
+    Status s = ChargeUsageMaybeEvictStrict<Table>(
+        total_charge, capacity,
+        /*need_evict_for_occupancy=*/false, state);
+    if (!s.ok()) {
+      if (allow_uncharged) {
+        proto.total_charge = 0;
+      } else {
+        return nullptr;
+      }
+    }
+  } else {
+    // Case strict_capacity_limit == false
+    bool success = ChargeUsageMaybeEvictNonStrict<Table>(
+        total_charge, capacity,
+        /*need_evict_for_occupancy=*/false, state);
+    if (!success) {
+      // Force the issue
+      usage_.fetch_add(total_charge, std::memory_order_relaxed);
+    }
+  }
+
+  return StandaloneInsert<typename Table::HandleImpl>(proto);
+}
+
+template <class Table>
+Status BaseClockTable::ChargeUsageMaybeEvictStrict(
+    size_t total_charge, size_t capacity, bool need_evict_for_occupancy,
+    typename Table::InsertState& state) {
   if (total_charge > capacity) {
     return Status::MemoryLimit(
         "Cache entry too large for a single cache shard: " +
@@ -287,7 +421,8 @@ inline Status HyperClockTable::ChargeUsageMaybeEvictStrict(
   if (request_evict_charge > 0) {
     size_t evicted_charge = 0;
     size_t evicted_count = 0;
-    Evict(request_evict_charge, &evicted_charge, &evicted_count);
+    static_cast<Table*>(this)->Evict(request_evict_charge, &evicted_charge,
+                                     &evicted_count, state);
     occupancy_.fetch_sub(evicted_count, std::memory_order_release);
     if (LIKELY(evicted_charge > need_evict_charge)) {
       assert(evicted_count > 0);
@@ -316,8 +451,10 @@ inline Status HyperClockTable::ChargeUsageMaybeEvictStrict(
   return Status::OK();
 }
 
-inline bool HyperClockTable::ChargeUsageMaybeEvictNonStrict(
-    size_t total_charge, size_t capacity, bool need_evict_for_occupancy) {
+template <class Table>
+inline bool BaseClockTable::ChargeUsageMaybeEvictNonStrict(
+    size_t total_charge, size_t capacity, bool need_evict_for_occupancy,
+    typename Table::InsertState& state) {
   // For simplicity, we consider that either the cache can accept the insert
   // with no evictions, or we must evict enough to make (at least) enough
   // space. It could lead to unnecessary failures or excessive evictions in
@@ -354,7 +491,8 @@ inline bool HyperClockTable::ChargeUsageMaybeEvictNonStrict(
   size_t evicted_charge = 0;
   size_t evicted_count = 0;
   if (need_evict_charge > 0) {
-    Evict(need_evict_charge, &evicted_charge, &evicted_count);
+    static_cast<Table*>(this)->Evict(need_evict_charge, &evicted_charge,
+                                     &evicted_count, state);
     // Deal with potential occupancy deficit
     if (UNLIKELY(need_evict_for_occupancy) && evicted_count == 0) {
       assert(evicted_charge == 0);
@@ -373,54 +511,43 @@ inline bool HyperClockTable::ChargeUsageMaybeEvictNonStrict(
   return true;
 }
 
-inline HyperClockTable::HandleImpl* HyperClockTable::StandaloneInsert(
-    const ClockHandleBasicData& proto) {
-  // Heap allocated separate from table
-  HandleImpl* h = new HandleImpl();
-  ClockHandleBasicData* h_alias = h;
-  *h_alias = proto;
-  h->SetStandalone();
-  // Single reference (standalone entries only created if returning a refed
-  // Handle back to user)
-  uint64_t meta = uint64_t{ClockHandle::kStateInvisible}
-                  << ClockHandle::kStateShift;
-  meta |= uint64_t{1} << ClockHandle::kAcquireCounterShift;
-  h->meta.store(meta, std::memory_order_release);
-  // Keep track of how much of usage is standalone
-  standalone_usage_.fetch_add(proto.GetTotalCharge(),
-                              std::memory_order_relaxed);
-  return h;
-}
+template <class Table>
+Status BaseClockTable::Insert(const ClockHandleBasicData& proto,
+                              typename Table::HandleImpl** handle,
+                              Cache::Priority priority, size_t capacity,
+                              bool strict_capacity_limit) {
+  using HandleImpl = typename Table::HandleImpl;
+  Table& derived = static_cast<Table&>(*this);
+
+  typename Table::InsertState state;
+  derived.StartInsert(state);
 
-Status HyperClockTable::Insert(const ClockHandleBasicData& proto,
-                               HandleImpl** handle, Cache::Priority priority,
-                               size_t capacity, bool strict_capacity_limit) {
   // Do we have the available occupancy? Optimistically assume we do
   // and deal with it if we don't.
   size_t old_occupancy = occupancy_.fetch_add(1, std::memory_order_acquire);
-  auto revert_occupancy_fn = [&]() {
-    occupancy_.fetch_sub(1, std::memory_order_relaxed);
-  };
   // Whether we over-committed and need an eviction to make up for it
-  bool need_evict_for_occupancy = old_occupancy >= occupancy_limit_;
+  bool need_evict_for_occupancy =
+      !derived.GrowIfNeeded(old_occupancy + 1, state);
 
   // Usage/capacity handling is somewhat different depending on
   // strict_capacity_limit, but mostly pessimistic.
   bool use_standalone_insert = false;
   const size_t total_charge = proto.GetTotalCharge();
   if (strict_capacity_limit) {
-    Status s = ChargeUsageMaybeEvictStrict(total_charge, capacity,
-                                           need_evict_for_occupancy);
+    Status s = ChargeUsageMaybeEvictStrict<Table>(
+        total_charge, capacity, need_evict_for_occupancy, state);
     if (!s.ok()) {
-      revert_occupancy_fn();
+      // Revert occupancy
+      occupancy_.fetch_sub(1, std::memory_order_relaxed);
       return s;
     }
   } else {
     // Case strict_capacity_limit == false
-    bool success = ChargeUsageMaybeEvictNonStrict(total_charge, capacity,
-                                                  need_evict_for_occupancy);
+    bool success = ChargeUsageMaybeEvictNonStrict<Table>(
+        total_charge, capacity, need_evict_for_occupancy, state);
     if (!success) {
-      revert_occupancy_fn();
+      // Revert occupancy
+      occupancy_.fetch_sub(1, std::memory_order_relaxed);
       if (handle == nullptr) {
         // Don't insert the entry but still return ok, as if the entry
         // inserted into cache and evicted immediately.
@@ -433,11 +560,6 @@ Status HyperClockTable::Insert(const ClockHandleBasicData& proto,
       }
     }
   }
-  auto revert_usage_fn = [&]() {
-    usage_.fetch_sub(total_charge, std::memory_order_relaxed);
-    // No underflow
-    assert(usage_.load(std::memory_order_relaxed) < SIZE_MAX / 2);
-  };
 
   if (!use_standalone_insert) {
     // Attempt a table insert, but abort if we find an existing entry for the
@@ -451,129 +573,37 @@ Status HyperClockTable::Insert(const ClockHandleBasicData& proto,
     uint64_t initial_countdown = GetInitialCountdown(priority);
     assert(initial_countdown > 0);
 
-    size_t probe = 0;
-    HandleImpl* e = FindSlot(
-        proto.hashed_key,
-        [&](HandleImpl* h) {
-          // Optimistically transition the slot from "empty" to
-          // "under construction" (no effect on other states)
-          uint64_t old_meta =
-              h->meta.fetch_or(uint64_t{ClockHandle::kStateOccupiedBit}
-                                   << ClockHandle::kStateShift,
-                               std::memory_order_acq_rel);
-          uint64_t old_state = old_meta >> ClockHandle::kStateShift;
+    HandleImpl* e =
+        derived.DoInsert(proto, initial_countdown, handle != nullptr, state);
 
-          if (old_state == ClockHandle::kStateEmpty) {
-            // We've started inserting into an available slot, and taken
-            // ownership Save data fields
-            ClockHandleBasicData* h_alias = h;
-            *h_alias = proto;
-
-            // Transition from "under construction" state to "visible" state
-            uint64_t new_meta = uint64_t{ClockHandle::kStateVisible}
-                                << ClockHandle::kStateShift;
-
-            // Maybe with an outstanding reference
-            new_meta |= initial_countdown << ClockHandle::kAcquireCounterShift;
-            new_meta |= (initial_countdown - (handle != nullptr))
-                        << ClockHandle::kReleaseCounterShift;
-
-#ifndef NDEBUG
-            // Save the state transition, with assertion
-            old_meta = h->meta.exchange(new_meta, std::memory_order_release);
-            assert(old_meta >> ClockHandle::kStateShift ==
-                   ClockHandle::kStateConstruction);
-#else
-            // Save the state transition
-            h->meta.store(new_meta, std::memory_order_release);
-#endif
-            return true;
-          } else if (old_state != ClockHandle::kStateVisible) {
-            // Slot not usable / touchable now
-            return false;
-          }
-          // Existing, visible entry, which might be a match.
-          // But first, we need to acquire a ref to read it. In fact, number of
-          // refs for initial countdown, so that we boost the clock state if
-          // this is a match.
-          old_meta = h->meta.fetch_add(
-              ClockHandle::kAcquireIncrement * initial_countdown,
-              std::memory_order_acq_rel);
-          // Like Lookup
-          if ((old_meta >> ClockHandle::kStateShift) ==
-              ClockHandle::kStateVisible) {
-            // Acquired a read reference
-            if (h->hashed_key == proto.hashed_key) {
-              // Match. Release in a way that boosts the clock state
-              old_meta = h->meta.fetch_add(
-                  ClockHandle::kReleaseIncrement * initial_countdown,
-                  std::memory_order_acq_rel);
-              // Correct for possible (but rare) overflow
-              CorrectNearOverflow(old_meta, h->meta);
-              // Insert standalone instead (only if return handle needed)
-              use_standalone_insert = true;
-              return true;
-            } else {
-              // Mismatch. Pretend we never took the reference
-              old_meta = h->meta.fetch_sub(
-                  ClockHandle::kAcquireIncrement * initial_countdown,
-                  std::memory_order_acq_rel);
-            }
-          } else if (UNLIKELY((old_meta >> ClockHandle::kStateShift) ==
-                              ClockHandle::kStateInvisible)) {
-            // Pretend we never took the reference
-            // WART: there's a tiny chance we release last ref to invisible
-            // entry here. If that happens, we let eviction take care of it.
-            old_meta = h->meta.fetch_sub(
-                ClockHandle::kAcquireIncrement * initial_countdown,
-                std::memory_order_acq_rel);
-          } else {
-            // For other states, incrementing the acquire counter has no effect
-            // so we don't need to undo it.
-            // Slot not usable / touchable now.
-          }
-          (void)old_meta;
-          return false;
-        },
-        [&](HandleImpl* /*h*/) { return false; },
-        [&](HandleImpl* h) {
-          h->displacements.fetch_add(1, std::memory_order_relaxed);
-        },
-        probe);
-    if (e == nullptr) {
-      // Occupancy check and never abort FindSlot above should generally
-      // prevent this, except it's theoretically possible for other threads
-      // to evict and replace entries in the right order to hit every slot
-      // when it is populated. Assuming random hashing, the chance of that
-      // should be no higher than pow(kStrictLoadFactor, n) for n slots.
-      // That should be infeasible for roughly n >= 256, so if this assertion
-      // fails, that suggests something is going wrong.
-      assert(GetTableSize() < 256);
-      use_standalone_insert = true;
-    }
-    if (!use_standalone_insert) {
+    if (e) {
       // Successfully inserted
       if (handle) {
         *handle = e;
       }
       return Status::OK();
     }
-    // Roll back table insertion
-    Rollback(proto.hashed_key, e);
-    revert_occupancy_fn();
+    // Not inserted
+    // Revert occupancy
+    occupancy_.fetch_sub(1, std::memory_order_relaxed);
     // Maybe fall back on standalone insert
     if (handle == nullptr) {
-      revert_usage_fn();
+      // Revert usage
+      usage_.fetch_sub(total_charge, std::memory_order_relaxed);
+      // No underflow
+      assert(usage_.load(std::memory_order_relaxed) < SIZE_MAX / 2);
       // As if unrefed entry immdiately evicted
       proto.FreeData(allocator_);
       return Status::OK();
     }
+
+    use_standalone_insert = true;
   }
 
   // Run standalone insert
   assert(use_standalone_insert);
 
-  *handle = StandaloneInsert(proto);
+  *handle = StandaloneInsert<HandleImpl>(proto);
 
   // The OkOverwritten status is used to count "redundant" insertions into
   // block cache. This implementation doesn't strictly check for redundant
@@ -583,37 +613,156 @@ Status HyperClockTable::Insert(const ClockHandleBasicData& proto,
   return Status::OkOverwritten();
 }
 
-HyperClockTable::HandleImpl* HyperClockTable::CreateStandalone(
-    ClockHandleBasicData& proto, size_t capacity, bool strict_capacity_limit,
-    bool allow_uncharged) {
-  const size_t total_charge = proto.GetTotalCharge();
-  if (strict_capacity_limit) {
-    Status s = ChargeUsageMaybeEvictStrict(total_charge, capacity,
-                                           /*need_evict_for_occupancy=*/false);
-    if (!s.ok()) {
-      if (allow_uncharged) {
-        proto.total_charge = 0;
-      } else {
-        return nullptr;
-      }
-    }
-  } else {
-    // Case strict_capacity_limit == false
-    bool success =
-        ChargeUsageMaybeEvictNonStrict(total_charge, capacity,
-                                       /*need_evict_for_occupancy=*/false);
-    if (!success) {
-      // Force the issue
-      usage_.fetch_add(total_charge, std::memory_order_relaxed);
+void BaseClockTable::Ref(ClockHandle& h) {
+  // Increment acquire counter
+  uint64_t old_meta = h.meta.fetch_add(ClockHandle::kAcquireIncrement,
+                                       std::memory_order_acquire);
+
+  assert((old_meta >> ClockHandle::kStateShift) &
+         ClockHandle::kStateShareableBit);
+  // Must have already had a reference
+  assert(GetRefcount(old_meta) > 0);
+  (void)old_meta;
+}
+
+#ifndef NDEBUG
+void BaseClockTable::TEST_RefN(ClockHandle& h, size_t n) {
+  // Increment acquire counter
+  uint64_t old_meta = h.meta.fetch_add(n * ClockHandle::kAcquireIncrement,
+                                       std::memory_order_acquire);
+
+  assert((old_meta >> ClockHandle::kStateShift) &
+         ClockHandle::kStateShareableBit);
+  (void)old_meta;
+}
+
+void BaseClockTable::TEST_ReleaseNMinus1(ClockHandle* h, size_t n) {
+  assert(n > 0);
+
+  // Like n-1 Releases, but assumes one more will happen in the caller to take
+  // care of anything like erasing an unreferenced, invisible entry.
+  uint64_t old_meta = h->meta.fetch_add(
+      (n - 1) * ClockHandle::kReleaseIncrement, std::memory_order_acquire);
+  assert((old_meta >> ClockHandle::kStateShift) &
+         ClockHandle::kStateShareableBit);
+  (void)old_meta;
+}
+#endif
+
+HyperClockTable::HyperClockTable(
+    size_t capacity, bool /*strict_capacity_limit*/,
+    CacheMetadataChargePolicy metadata_charge_policy,
+    MemoryAllocator* allocator,
+    const Cache::EvictionCallback* eviction_callback, const uint32_t* hash_seed,
+    const Opts& opts)
+    : BaseClockTable(metadata_charge_policy, allocator, eviction_callback,
+                     hash_seed),
+      length_bits_(CalcHashBits(capacity, opts.estimated_value_size,
+                                metadata_charge_policy)),
+      length_bits_mask_((size_t{1} << length_bits_) - 1),
+      occupancy_limit_(static_cast<size_t>((uint64_t{1} << length_bits_) *
+                                           kStrictLoadFactor)),
+      array_(new HandleImpl[size_t{1} << length_bits_]) {
+  if (metadata_charge_policy ==
+      CacheMetadataChargePolicy::kFullChargeCacheMetadata) {
+    usage_ += size_t{GetTableSize()} * sizeof(HandleImpl);
+  }
+
+  static_assert(sizeof(HandleImpl) == 64U,
+                "Expecting size / alignment with common cache line size");
+}
+
+HyperClockTable::~HyperClockTable() {
+  // Assumes there are no references or active operations on any slot/element
+  // in the table.
+  for (size_t i = 0; i < GetTableSize(); i++) {
+    HandleImpl& h = array_[i];
+    switch (h.meta >> ClockHandle::kStateShift) {
+      case ClockHandle::kStateEmpty:
+        // noop
+        break;
+      case ClockHandle::kStateInvisible:  // rare but possible
+      case ClockHandle::kStateVisible:
+        assert(GetRefcount(h.meta) == 0);
+        h.FreeData(allocator_);
+#ifndef NDEBUG
+        Rollback(h.hashed_key, &h);
+        ReclaimEntryUsage(h.GetTotalCharge());
+#endif
+        break;
+      // otherwise
+      default:
+        assert(false);
+        break;
     }
   }
 
-  return StandaloneInsert(proto);
+#ifndef NDEBUG
+  for (size_t i = 0; i < GetTableSize(); i++) {
+    assert(array_[i].displacements.load() == 0);
+  }
+#endif
+
+  assert(usage_.load() == 0 ||
+         usage_.load() == size_t{GetTableSize()} * sizeof(HandleImpl));
+  assert(occupancy_ == 0);
+}
+
+void HyperClockTable::StartInsert(InsertState&) {}
+
+bool HyperClockTable::GrowIfNeeded(size_t new_occupancy, InsertState&) {
+  return new_occupancy <= occupancy_limit_;
+}
+
+HyperClockTable::HandleImpl* HyperClockTable::DoInsert(
+    const ClockHandleBasicData& proto, uint64_t initial_countdown,
+    bool keep_ref, InsertState&) {
+  bool already_matches = false;
+  HandleImpl* e = FindSlot(
+      proto.hashed_key,
+      [&](HandleImpl* h) {
+        return TryInsert(proto, *h, initial_countdown, keep_ref,
+                         &already_matches);
+      },
+      [&](HandleImpl* h) {
+        if (already_matches) {
+          // Stop searching & roll back displacements
+          Rollback(proto.hashed_key, h);
+          return true;
+        } else {
+          // Keep going
+          return false;
+        }
+      },
+      [&](HandleImpl* h, bool is_last) {
+        if (is_last) {
+          // Search is ending. Roll back displacements
+          Rollback(proto.hashed_key, h);
+        } else {
+          h->displacements.fetch_add(1, std::memory_order_relaxed);
+        }
+      });
+  if (already_matches) {
+    // Insertion skipped
+    return nullptr;
+  }
+  if (e != nullptr) {
+    // Successfully inserted
+    return e;
+  }
+  // Else, no available slot found. Occupancy check should generally prevent
+  // this, except it's theoretically possible for other threads to evict and
+  // replace entries in the right order to hit every slot when it is populated.
+  // Assuming random hashing, the chance of that should be no higher than
+  // pow(kStrictLoadFactor, n) for n slots. That should be infeasible for
+  // roughly n >= 256, so if this assertion fails, that suggests something is
+  // going wrong.
+  assert(GetTableSize() < 256);
+  return nullptr;
 }
 
 HyperClockTable::HandleImpl* HyperClockTable::Lookup(
     const UniqueId64x2& hashed_key) {
-  size_t probe = 0;
   HandleImpl* e = FindSlot(
       hashed_key,
       [&](HandleImpl* h) {
@@ -673,7 +822,7 @@ HyperClockTable::HandleImpl* HyperClockTable::Lookup(
       [&](HandleImpl* h) {
         return h->displacements.load(std::memory_order_relaxed) == 0;
       },
-      [&](HandleImpl* /*h*/) {}, probe);
+      [&](HandleImpl* /*h*/, bool /*is_last*/) {});
 
   return e;
 }
@@ -753,43 +902,19 @@ bool HyperClockTable::Release(HandleImpl* h, bool useful,
   }
 }
 
-void HyperClockTable::Ref(HandleImpl& h) {
-  // Increment acquire counter
-  uint64_t old_meta = h.meta.fetch_add(ClockHandle::kAcquireIncrement,
-                                       std::memory_order_acquire);
-
-  assert((old_meta >> ClockHandle::kStateShift) &
-         ClockHandle::kStateShareableBit);
-  // Must have already had a reference
-  assert(GetRefcount(old_meta) > 0);
-  (void)old_meta;
-}
-
-void HyperClockTable::TEST_RefN(HandleImpl& h, size_t n) {
-  // Increment acquire counter
-  uint64_t old_meta = h.meta.fetch_add(n * ClockHandle::kAcquireIncrement,
-                                       std::memory_order_acquire);
-
-  assert((old_meta >> ClockHandle::kStateShift) &
-         ClockHandle::kStateShareableBit);
-  (void)old_meta;
-}
-
+#ifndef NDEBUG
 void HyperClockTable::TEST_ReleaseN(HandleImpl* h, size_t n) {
   if (n > 0) {
-    // Split into n - 1 and 1 steps.
-    uint64_t old_meta = h->meta.fetch_add(
-        (n - 1) * ClockHandle::kReleaseIncrement, std::memory_order_acquire);
-    assert((old_meta >> ClockHandle::kStateShift) &
-           ClockHandle::kStateShareableBit);
-    (void)old_meta;
+    // Do n-1 simple releases first
+    TEST_ReleaseNMinus1(h, n);
 
+    // Then the last release might be more involved
     Release(h, /*useful*/ true, /*erase_if_last_ref*/ false);
   }
 }
+#endif
 
 void HyperClockTable::Erase(const UniqueId64x2& hashed_key) {
-  size_t probe = 0;
   (void)FindSlot(
       hashed_key,
       [&](HandleImpl* h) {
@@ -856,48 +981,7 @@ void HyperClockTable::Erase(const UniqueId64x2& hashed_key) {
       [&](HandleImpl* h) {
         return h->displacements.load(std::memory_order_relaxed) == 0;
       },
-      [&](HandleImpl* /*h*/) {}, probe);
-}
-
-void HyperClockTable::ConstApplyToEntriesRange(
-    std::function<void(const HandleImpl&)> func, size_t index_begin,
-    size_t index_end, bool apply_if_will_be_deleted) const {
-  uint64_t check_state_mask = ClockHandle::kStateShareableBit;
-  if (!apply_if_will_be_deleted) {
-    check_state_mask |= ClockHandle::kStateVisibleBit;
-  }
-
-  for (size_t i = index_begin; i < index_end; i++) {
-    HandleImpl& h = array_[i];
-
-    // Note: to avoid using compare_exchange, we have to be extra careful.
-    uint64_t old_meta = h.meta.load(std::memory_order_relaxed);
-    // Check if it's an entry visible to lookups
-    if ((old_meta >> ClockHandle::kStateShift) & check_state_mask) {
-      // Increment acquire counter. Note: it's possible that the entry has
-      // completely changed since we loaded old_meta, but incrementing acquire
-      // count is always safe. (Similar to optimistic Lookup here.)
-      old_meta = h.meta.fetch_add(ClockHandle::kAcquireIncrement,
-                                  std::memory_order_acquire);
-      // Check whether we actually acquired a reference.
-      if ((old_meta >> ClockHandle::kStateShift) &
-          ClockHandle::kStateShareableBit) {
-        // Apply func if appropriate
-        if ((old_meta >> ClockHandle::kStateShift) & check_state_mask) {
-          func(h);
-        }
-        // Pretend we never took the reference
-        h.meta.fetch_sub(ClockHandle::kAcquireIncrement,
-                         std::memory_order_release);
-        // No net change, so don't need to check for overflow
-      } else {
-        // For other states, incrementing the acquire counter has no effect
-        // so we don't need to undo it. Furthermore, we cannot safely undo
-        // it because we did not acquire a read reference to lock the
-        // entry in a Shareable state.
-      }
-    }
-  }
+      [&](HandleImpl* /*h*/, bool /*is_last*/) {});
 }
 
 void HyperClockTable::EraseUnRefEntries() {
@@ -921,10 +1005,10 @@ void HyperClockTable::EraseUnRefEntries() {
   }
 }
 
+template <typename MatchFn, typename AbortFn, typename UpdateFn>
 inline HyperClockTable::HandleImpl* HyperClockTable::FindSlot(
-    const UniqueId64x2& hashed_key, std::function<bool(HandleImpl*)> match_fn,
-    std::function<bool(HandleImpl*)> abort_fn,
-    std::function<void(HandleImpl*)> update_fn, size_t& probe) {
+    const UniqueId64x2& hashed_key, MatchFn match_fn, AbortFn abort_fn,
+    UpdateFn update_fn) {
   // NOTE: upper 32 bits of hashed_key[0] is used for sharding
   //
   // We use double-hashing probing. Every probe in the sequence is a
@@ -938,20 +1022,21 @@ inline HyperClockTable::HandleImpl* HyperClockTable::FindSlot(
   // TODO: we could also reconsider linear probing, though locality benefits
   // are limited because each slot is a full cache line
   size_t increment = static_cast<size_t>(hashed_key[0]) | 1U;
-  size_t current = ModTableSize(base + probe * increment);
-  while (probe <= length_bits_mask_) {
+  size_t first = ModTableSize(base);
+  size_t current = first;
+  bool is_last;
+  do {
     HandleImpl* h = &array_[current];
     if (match_fn(h)) {
-      probe++;
       return h;
     }
     if (abort_fn(h)) {
       return nullptr;
     }
-    probe++;
-    update_fn(h);
     current = ModTableSize(current + increment);
-  }
+    is_last = current == first;
+    update_fn(h, is_last);
+  } while (!is_last);
   // We looped back.
   return nullptr;
 }
@@ -978,7 +1063,8 @@ inline void HyperClockTable::ReclaimEntryUsage(size_t total_charge) {
 }
 
 inline void HyperClockTable::Evict(size_t requested_charge,
-                                   size_t* freed_charge, size_t* freed_count) {
+                                   size_t* freed_charge, size_t* freed_count,
+                                   InsertState&) {
   // precondition
   assert(requested_charge > 0);
 
@@ -1063,35 +1149,32 @@ void ClockCacheShard<Table>::ApplyToSomeEntries(
                              size_t charge,
                              const Cache::CacheItemHelper* helper)>& callback,
     size_t average_entries_per_lock, size_t* state) {
-  // The state is essentially going to be the starting hash, which works
-  // nicely even if we resize between calls because we use upper-most
-  // hash bits for table indexes.
-  size_t length_bits = table_.GetLengthBits();
+  // The state will be a simple index into the table. Even with a dynamic
+  // hyper clock cache, entries will generally stay in their existing
+  // slots, so we don't need to be aware of the high-level organization
+  // that makes lookup efficient.
   size_t length = table_.GetTableSize();
 
   assert(average_entries_per_lock > 0);
-  // Assuming we are called with same average_entries_per_lock repeatedly,
-  // this simplifies some logic (index_end will not overflow).
-  assert(average_entries_per_lock < length || *state == 0);
 
-  size_t index_begin = *state >> (sizeof(size_t) * 8u - length_bits);
+  size_t index_begin = *state;
   size_t index_end = index_begin + average_entries_per_lock;
   if (index_end >= length) {
     // Going to end.
     index_end = length;
     *state = SIZE_MAX;
   } else {
-    *state = index_end << (sizeof(size_t) * 8u - length_bits);
+    *state = index_end;
   }
 
   auto hash_seed = table_.GetHashSeed();
-  table_.ConstApplyToEntriesRange(
+  ConstApplyToEntriesRange(
       [callback, hash_seed](const HandleImpl& h) {
         UniqueId64x2 unhashed;
         callback(ReverseHash(h.hashed_key, &unhashed, hash_seed), h.value,
                  h.GetTotalCharge(), h.helper);
       },
-      index_begin, index_end, false);
+      table_.HandlePtr(index_begin), table_.HandlePtr(index_end), false);
 }
 
 int HyperClockTable::CalcHashBits(
@@ -1146,18 +1229,15 @@ Status ClockCacheShard<Table>::Insert(const Slice& key,
   proto.value = value;
   proto.helper = helper;
   proto.total_charge = charge;
-  return table_.Insert(proto, handle, priority,
-                       capacity_.load(std::memory_order_relaxed),
-                       strict_capacity_limit_.load(std::memory_order_relaxed));
+  return table_.template Insert<Table>(
+      proto, handle, priority, capacity_.load(std::memory_order_relaxed),
+      strict_capacity_limit_.load(std::memory_order_relaxed));
 }
 
 template <class Table>
-typename ClockCacheShard<Table>::HandleImpl*
-ClockCacheShard<Table>::CreateStandalone(const Slice& key,
-                                         const UniqueId64x2& hashed_key,
-                                         Cache::ObjectPtr obj,
-                                         const Cache::CacheItemHelper* helper,
-                                         size_t charge, bool allow_uncharged) {
+typename Table::HandleImpl* ClockCacheShard<Table>::CreateStandalone(
+    const Slice& key, const UniqueId64x2& hashed_key, Cache::ObjectPtr obj,
+    const Cache::CacheItemHelper* helper, size_t charge, bool allow_uncharged) {
   if (UNLIKELY(key.size() != kCacheKeySize)) {
     return nullptr;
   }
@@ -1166,7 +1246,7 @@ ClockCacheShard<Table>::CreateStandalone(const Slice& key,
   proto.value = obj;
   proto.helper = helper;
   proto.total_charge = charge;
-  return table_.CreateStandalone(
+  return table_.template CreateStandalone<Table>(
       proto, capacity_.load(std::memory_order_relaxed),
       strict_capacity_limit_.load(std::memory_order_relaxed), allow_uncharged);
 }
@@ -1198,6 +1278,7 @@ bool ClockCacheShard<Table>::Release(HandleImpl* handle, bool useful,
   return table_.Release(handle, useful, erase_if_last_ref);
 }
 
+#ifndef NDEBUG
 template <class Table>
 void ClockCacheShard<Table>::TEST_RefN(HandleImpl* h, size_t n) {
   table_.TEST_RefN(*h, n);
@@ -1207,6 +1288,7 @@ template <class Table>
 void ClockCacheShard<Table>::TEST_ReleaseN(HandleImpl* h, size_t n) {
   table_.TEST_ReleaseN(h, n);
 }
+#endif
 
 template <class Table>
 bool ClockCacheShard<Table>::Release(HandleImpl* handle,
@@ -1249,7 +1331,7 @@ size_t ClockCacheShard<Table>::GetPinnedUsage() const {
   size_t table_pinned_usage = 0;
   const bool charge_metadata =
       metadata_charge_policy_ == kFullChargeCacheMetadata;
-  table_.ConstApplyToEntriesRange(
+  ConstApplyToEntriesRange(
       [&table_pinned_usage, charge_metadata](const HandleImpl& h) {
         uint64_t meta = h.meta.load(std::memory_order_relaxed);
         uint64_t refcount = GetRefcount(meta);
@@ -1262,7 +1344,7 @@ size_t ClockCacheShard<Table>::GetPinnedUsage() const {
           }
         }
       },
-      0, table_.GetTableSize(), true);
+      table_.HandlePtr(0), table_.HandlePtr(table_.GetTableSize()), true);
 
   return table_pinned_usage + table_.GetStandaloneUsage();
 }

cache/clock_cache.h

@@ -371,14 +371,127 @@ struct ClockHandle : public ClockHandleBasicData {
   static constexpr uint8_t kMaxCountdown = kHighCountdown;
   // TODO: make these coundown values tuning parameters for eviction?
 
-  // See above
-  std::atomic<uint64_t> meta{};
+  // See above. Mutable for read reference counting.
+  mutable std::atomic<uint64_t> meta{};
 
-  // Anticipating use for SecondaryCache support
-  void* reserved_for_future_use = nullptr;
+  // Whether this is a "deteched" handle that is independently allocated
+  // with `new` (so must be deleted with `delete`).
+  // TODO: ideally this would be packed into some other data field, such
+  // as upper bits of total_charge, but that incurs a measurable performance
+  // regression.
+  bool standalone = false;
+
+  inline bool IsStandalone() const { return standalone; }
+
+  inline void SetStandalone() { standalone = true; }
 };  // struct ClockHandle
 
-class HyperClockTable {
+class BaseClockTable {
+ public:
+  BaseClockTable(CacheMetadataChargePolicy metadata_charge_policy,
+                 MemoryAllocator* allocator,
+                 const Cache::EvictionCallback* eviction_callback,
+                 const uint32_t* hash_seed)
+      : metadata_charge_policy_(metadata_charge_policy),
+        allocator_(allocator),
+        eviction_callback_(*eviction_callback),
+        hash_seed_(*hash_seed) {}
+
+  template <class Table>
+  typename Table::HandleImpl* CreateStandalone(ClockHandleBasicData& proto,
+                                               size_t capacity,
+                                               bool strict_capacity_limit,
+                                               bool allow_uncharged);
+
+  template <class Table>
+  Status Insert(const ClockHandleBasicData& proto,
+                typename Table::HandleImpl** handle, Cache::Priority priority,
+                size_t capacity, bool strict_capacity_limit);
+
+  void Ref(ClockHandle& handle);
+
+  size_t GetOccupancy() const {
+    return occupancy_.load(std::memory_order_relaxed);
+  }
+
+  size_t GetUsage() const { return usage_.load(std::memory_order_relaxed); }
+
+  size_t GetStandaloneUsage() const {
+    return standalone_usage_.load(std::memory_order_relaxed);
+  }
+
+  uint32_t GetHashSeed() const { return hash_seed_; }
+
+#ifndef NDEBUG
+  // Acquire N references
+  void TEST_RefN(ClockHandle& handle, size_t n);
+  // Helper for TEST_ReleaseN
+  void TEST_ReleaseNMinus1(ClockHandle* handle, size_t n);
+#endif
+
+ private:  // fns
+  // Creates a "standalone" handle for returning from an Insert operation that
+  // cannot be completed by actually inserting into the table.
+  // Updates `standalone_usage_` but not `usage_` nor `occupancy_`.
+  template <class HandleImpl>
+  HandleImpl* StandaloneInsert(const ClockHandleBasicData& proto);
+
+  // Helper for updating `usage_` for new entry with given `total_charge`
+  // and evicting if needed under strict_capacity_limit=true rules. This
+  // means the operation might fail with Status::MemoryLimit. If
+  // `need_evict_for_occupancy`, then eviction of at least one entry is
+  // required, and the operation should fail if not possible.
+  // NOTE: Otherwise, occupancy_ is not managed in this function
+  template <class Table>
+  Status ChargeUsageMaybeEvictStrict(size_t total_charge, size_t capacity,
+                                     bool need_evict_for_occupancy,
+                                     typename Table::InsertState& state);
+
+  // Helper for updating `usage_` for new entry with given `total_charge`
+  // and evicting if needed under strict_capacity_limit=false rules. This
+  // means that updating `usage_` always succeeds even if forced to exceed
+  // capacity. If `need_evict_for_occupancy`, then eviction of at least one
+  // entry is required, and the operation should return false if such eviction
+  // is not possible. `usage_` is not updated in that case. Otherwise, returns
+  // true, indicating success.
+  // NOTE: occupancy_ is not managed in this function
+  template <class Table>
+  bool ChargeUsageMaybeEvictNonStrict(size_t total_charge, size_t capacity,
+                                      bool need_evict_for_occupancy,
+                                      typename Table::InsertState& state);
+
+ protected:  // data
+  // We partition the following members into different cache lines
+  // to avoid false sharing among Lookup, Release, Erase and Insert
+  // operations in ClockCacheShard.
+
+  // Clock algorithm sweep pointer.
+  std::atomic<uint64_t> clock_pointer_{};
+
+  ALIGN_AS(CACHE_LINE_SIZE)
+  // Number of elements in the table.
+  std::atomic<size_t> occupancy_{};
+
+  // Memory usage by entries tracked by the cache (including standalone)
+  std::atomic<size_t> usage_{};
+
+  // Part of usage by standalone entries (not in table)
+  std::atomic<size_t> standalone_usage_{};
+
+  ALIGN_AS(CACHE_LINE_SIZE)
+  const CacheMetadataChargePolicy metadata_charge_policy_;
+
+  // From Cache, for deleter
+  MemoryAllocator* const allocator_;
+
+  // A reference to Cache::eviction_callback_
+  const Cache::EvictionCallback& eviction_callback_;
+
+  // A reference to ShardedCacheBase::hash_seed_
+  const uint32_t& hash_seed_;
+};
+
+class HyperClockTable : public BaseClockTable {
  public:
   // Target size to be exactly a common cache line size (see static_assert in
   // clock_cache.cc)
@@ -387,16 +500,6 @@ class HyperClockTable {
     // up in this slot or a higher one.
     std::atomic<uint32_t> displacements{};
 
-    // Whether this is a "deteched" handle that is independently allocated
-    // with `new` (so must be deleted with `delete`).
-    // TODO: ideally this would be packed into some other data field, such
-    // as upper bits of total_charge, but that incurs a measurable performance
-    // regression.
-    bool standalone = false;
-
-    inline bool IsStandalone() const { return standalone; }
-
-    inline void SetStandalone() { standalone = true; }
   };  // struct HandleImpl
 
   struct Opts {
@@ -410,49 +513,46 @@ class HyperClockTable {
                   const uint32_t* hash_seed, const Opts& opts);
   ~HyperClockTable();
 
-  Status Insert(const ClockHandleBasicData& proto, HandleImpl** handle,
-                Cache::Priority priority, size_t capacity,
-                bool strict_capacity_limit);
+  // For BaseClockTable::Insert
+  struct InsertState {};
 
-  HandleImpl* CreateStandalone(ClockHandleBasicData& proto, size_t capacity,
-                               bool strict_capacity_limit,
-                               bool allow_uncharged);
+  void StartInsert(InsertState& state);
+
+  // Returns true iff there is room for the proposed number of entries.
+  bool GrowIfNeeded(size_t new_occupancy, InsertState& state);
+
+  HandleImpl* DoInsert(const ClockHandleBasicData& proto,
+                       uint64_t initial_countdown, bool take_ref,
+                       InsertState& state);
+
+  // Runs the clock eviction algorithm trying to reclaim at least
+  // requested_charge. Returns how much is evicted, which could be less
+  // if it appears impossible to evict the requested amount without blocking.
+  void Evict(size_t requested_charge, size_t* freed_charge, size_t* freed_count,
+             InsertState& state);
 
   HandleImpl* Lookup(const UniqueId64x2& hashed_key);
 
   bool Release(HandleImpl* handle, bool useful, bool erase_if_last_ref);
 
-  void Ref(HandleImpl& handle);
-
   void Erase(const UniqueId64x2& hashed_key);
 
-  void ConstApplyToEntriesRange(std::function<void(const HandleImpl&)> func,
-                                size_t index_begin, size_t index_end,
-                                bool apply_if_will_be_deleted) const;
-
   void EraseUnRefEntries();
 
   size_t GetTableSize() const { return size_t{1} << length_bits_; }
 
-  int GetLengthBits() const { return length_bits_; }
-
-  size_t GetOccupancy() const {
-    return occupancy_.load(std::memory_order_relaxed);
-  }
-
   size_t GetOccupancyLimit() const { return occupancy_limit_; }
 
-  size_t GetUsage() const { return usage_.load(std::memory_order_relaxed); }
+  const HandleImpl* HandlePtr(size_t idx) const { return &array_[idx]; }
 
-  size_t GetStandaloneUsage() const {
-    return standalone_usage_.load(std::memory_order_relaxed);
+#ifndef NDEBUG
+  size_t& TEST_MutableOccupancyLimit() const {
+    return const_cast<size_t&>(occupancy_limit_);
   }
 
-  uint32_t GetHashSeed() const { return hash_seed_; }
-
-  // Acquire/release N references
-  void TEST_RefN(HandleImpl& handle, size_t n);
+  // Release N references
   void TEST_ReleaseN(HandleImpl* handle, size_t n);
+#endif
 
  private:  // functions
   // Returns x mod 2^{length_bits_}.
@@ -460,28 +560,18 @@ class HyperClockTable {
     return static_cast<size_t>(x) & length_bits_mask_;
   }
 
-  // Runs the clock eviction algorithm trying to reclaim at least
-  // requested_charge. Returns how much is evicted, which could be less
-  // if it appears impossible to evict the requested amount without blocking.
-  inline void Evict(size_t requested_charge, size_t* freed_charge,
-                    size_t* freed_count);
-
-  // Returns the first slot in the probe sequence, starting from the given
-  // probe number, with a handle e such that match(e) is true. At every
-  // step, the function first tests whether match(e) holds. If this is false,
-  // it evaluates abort(e) to decide whether the search should be aborted,
-  // and in the affirmative returns -1. For every handle e probed except
-  // the last one, the function runs update(e).
-  // The probe parameter is modified as follows. We say a probe to a handle
-  // e is aborting if match(e) is false and abort(e) is true. Then the final
-  // value of probe is one more than the last non-aborting probe during the
-  // call. This is so that that the variable can be used to keep track of
-  // progress across consecutive calls to FindSlot.
-  inline HandleImpl* FindSlot(const UniqueId64x2& hashed_key,
-                              std::function<bool(HandleImpl*)> match,
-                              std::function<bool(HandleImpl*)> stop,
-                              std::function<void(HandleImpl*)> update,
-                              size_t& probe);
+  // Returns the first slot in the probe sequence with a handle e such that
+  // match_fn(e) is true. At every step, the function first tests whether
+  // match_fn(e) holds. If this is false, it evaluates abort_fn(e) to decide
+  // whether the search should be aborted, and if so, FindSlot immediately
+  // returns nullptr. For every handle e that is not a match and not aborted,
+  // FindSlot runs update_fn(e, is_last) where is_last is set to true iff that
+  // slot will be the last probed because the next would cycle back to the first
+  // slot probed. This function uses templates instead of std::function to
+  // minimize the risk of heap-allocated closures being created.
+  template <typename MatchFn, typename AbortFn, typename UpdateFn>
+  inline HandleImpl* FindSlot(const UniqueId64x2& hashed_key, MatchFn match_fn,
+                              AbortFn abort_fn, UpdateFn update_fn);
 
   // Re-decrement all displacements in probe path starting from beginning
   // until (not including) the given handle
@@ -494,33 +584,6 @@ class HyperClockTable {
   // before releasing it so that it can be provided to this function.
   inline void ReclaimEntryUsage(size_t total_charge);
 
-  // Helper for updating `usage_` for new entry with given `total_charge`
-  // and evicting if needed under strict_capacity_limit=true rules. This
-  // means the operation might fail with Status::MemoryLimit. If
-  // `need_evict_for_occupancy`, then eviction of at least one entry is
-  // required, and the operation should fail if not possible.
-  // NOTE: Otherwise, occupancy_ is not managed in this function
-  inline Status ChargeUsageMaybeEvictStrict(size_t total_charge,
-                                            size_t capacity,
-                                            bool need_evict_for_occupancy);
-
-  // Helper for updating `usage_` for new entry with given `total_charge`
-  // and evicting if needed under strict_capacity_limit=false rules. This
-  // means that updating `usage_` always succeeds even if forced to exceed
-  // capacity. If `need_evict_for_occupancy`, then eviction of at least one
-  // entry is required, and the operation should return false if such eviction
-  // is not possible. `usage_` is not updated in that case. Otherwise, returns
-  // true, indicating success.
-  // NOTE: occupancy_ is not managed in this function
-  inline bool ChargeUsageMaybeEvictNonStrict(size_t total_charge,
-                                             size_t capacity,
-                                             bool need_evict_for_occupancy);
-
-  // Creates a "standalone" handle for returning from an Insert operation that
-  // cannot be completed by actually inserting into the table.
-  // Updates `standalone_usage_` but not `usage_` nor `occupancy_`.
-  inline HandleImpl* StandaloneInsert(const ClockHandleBasicData& proto);
-
   MemoryAllocator* GetAllocator() const { return allocator_; }
 
   // Returns the number of bits used to hash an element in the hash
@@ -541,33 +604,6 @@ class HyperClockTable {
 
   // Array of slots comprising the hash table.
   const std::unique_ptr<HandleImpl[]> array_;
-
-  // From Cache, for deleter
-  MemoryAllocator* const allocator_;
-
-  // A reference to Cache::eviction_callback_
-  const Cache::EvictionCallback& eviction_callback_;
-
-  // A reference to ShardedCacheBase::hash_seed_
-  const uint32_t& hash_seed_;
-
-  // We partition the following members into different cache lines
-  // to avoid false sharing among Lookup, Release, Erase and Insert
-  // operations in ClockCacheShard.
-
-  ALIGN_AS(CACHE_LINE_SIZE)
-  // Clock algorithm sweep pointer.
-  std::atomic<uint64_t> clock_pointer_{};
-
-  ALIGN_AS(CACHE_LINE_SIZE)
-  // Number of elements in the table.
-  std::atomic<size_t> occupancy_{};
-
-  // Memory usage by entries tracked by the cache (including standalone)
-  std::atomic<size_t> usage_{};
-
-  // Part of usage by standalone entries (not in table)
-  std::atomic<size_t> standalone_usage_{};
 };  // class HyperClockTable
 
 // A single shard of sharded cache.
@@ -666,9 +702,14 @@ class ALIGN_AS(CACHE_LINE_SIZE) ClockCacheShard final : public CacheShardBase {
     return Lookup(key, hashed_key);
   }
 
+#ifndef NDEBUG
+  size_t& TEST_MutableOccupancyLimit() const {
+    return table_.TEST_MutableOccupancyLimit();
+  }
   // Acquire/release N references
   void TEST_RefN(HandleImpl* handle, size_t n);
   void TEST_ReleaseN(HandleImpl* handle, size_t n);
+#endif
 
  private:  // data
   Table table_;

cache/lru_cache.cc

@@ -712,12 +712,12 @@ std::shared_ptr<Cache> LRUCacheOptions::MakeSharedCache() const {
   return cache;
 }
 
-std::shared_ptr<GeneralCache> LRUCacheOptions::MakeSharedGeneralCache() const {
+std::shared_ptr<RowCache> LRUCacheOptions::MakeSharedRowCache() const {
   if (secondary_cache) {
-    // Not allowed for a GeneralCache
+    // Not allowed for a RowCache
     return nullptr;
   }
-  // Works while GeneralCache is an alias for Cache
+  // Works while RowCache is an alias for Cache
   return MakeSharedCache();
 }
 }  // namespace ROCKSDB_NAMESPACE

cache/lru_cache_test.cc

@@ -715,6 +715,62 @@ TEST_F(ClockCacheTest, ClockCounterOverflowTest) {
   ASSERT_EQ(val.deleted, 1);
 }
 
+TEST_F(ClockCacheTest, ClockTableFull) {
+  // Force clock cache table to fill up (not usually allowed) in order
+  // to test full probe sequence that is theoretically possible due to
+  // parallel operations
+  NewShard(6, /*strict_capacity_limit*/ false);
+  size_t size = shard_->GetTableAddressCount();
+  ASSERT_LE(size + 3, 256);  // for using char keys
+  // Modify occupancy and capacity limits to attempt insert on full
+  shard_->TEST_MutableOccupancyLimit() = size + 100;
+  shard_->SetCapacity(size + 100);
+
+  DeleteCounter val;
+  std::vector<HandleImpl*> handles;
+  // NOTE: the three extra insertions should create standalone entries
+  for (size_t i = 0; i < size + 3; ++i) {
+    UniqueId64x2 hkey = TestHashedKey(static_cast<char>(i));
+    ASSERT_OK(shard_->Insert(TestKey(hkey), hkey, &val, &kDeleteCounterHelper,
+                             1, &handles.emplace_back(),
+                             Cache::Priority::HIGH));
+  }
+
+  for (size_t i = 0; i < size + 3; ++i) {
+    UniqueId64x2 hkey = TestHashedKey(static_cast<char>(i));
+    HandleImpl* h = shard_->Lookup(TestKey(hkey), hkey);
+    if (i < size) {
+      ASSERT_NE(h, nullptr);
+      shard_->Release(h);
+    } else {
+      // Standalone entries not visible by lookup
+      ASSERT_EQ(h, nullptr);
+    }
+  }
+
+  for (size_t i = 0; i < size + 3; ++i) {
+    ASSERT_NE(handles[i], nullptr);
+    shard_->Release(handles[i]);
+    if (i < size) {
+      // Everything still in cache
+      ASSERT_EQ(val.deleted, 0);
+    } else {
+      // Standalone entries freed on release
+      ASSERT_EQ(val.deleted, i + 1 - size);
+    }
+  }
+
+  for (size_t i = size + 3; i > 0; --i) {
+    UniqueId64x2 hkey = TestHashedKey(static_cast<char>(i - 1));
+    shard_->Erase(TestKey(hkey), hkey);
+    if (i - 1 > size) {
+      ASSERT_EQ(val.deleted, 3);
+    } else {
+      ASSERT_EQ(val.deleted, 3 + size - (i - 1));
+    }
+  }
+}
+
 // This test is mostly to exercise some corner case logic, by forcing two
 // keys to have the same hash, and more
 TEST_F(ClockCacheTest, CollidingInsertEraseTest) {

cache/secondary_cache_adapter.cc

@@ -376,7 +376,7 @@ void CacheWithSecondaryAdapter::WaitAll(AsyncLookupHandle* async_handles,
     for (AsyncLookupHandle* cur : my_pending) {
       my_secondary_handles.push_back(cur->pending_handle);
     }
-    secondary_cache_->WaitAll(my_secondary_handles);
+    secondary_cache_->WaitAll(std::move(my_secondary_handles));
   }
 
   // Process results

db/builder.cc

@@ -15,6 +15,7 @@
 
 #include "db/blob/blob_file_builder.h"
 #include "db/compaction/compaction_iterator.h"
+#include "db/dbformat.h"
 #include "db/event_helpers.h"
 #include "db/internal_stats.h"
 #include "db/merge_helper.h"
@@ -205,21 +206,38 @@ Status BuildTable(
         /*compaction=*/nullptr, compaction_filter.get(),
         /*shutting_down=*/nullptr, db_options.info_log, full_history_ts_low);
 
+    const size_t ts_sz = ucmp->timestamp_size();
+    const bool strip_timestamp =
+        ts_sz > 0 && !ioptions.persist_user_defined_timestamps;
+
+    std::string key_after_flush_buf;
     c_iter.SeekToFirst();
     for (; c_iter.Valid(); c_iter.Next()) {
       const Slice& key = c_iter.key();
       const Slice& value = c_iter.value();
       const ParsedInternalKey& ikey = c_iter.ikey();
-      // Generate a rolling 64-bit hash of the key and values
-      // Note :
-      // Here "key" integrates 'sequence_number'+'kType'+'user key'.
-      s = output_validator.Add(key, value);
+      Slice key_after_flush = key;
+      // If user defined timestamps will be stripped from user key after flush,
+      // the in memory version of the key act logically the same as one with a
+      // minimum timestamp. We update the timestamp here so file boundary and
+      // output validator, block builder all see the effect of the stripping.
+      if (strip_timestamp) {
+        key_after_flush_buf.clear();
+        ReplaceInternalKeyWithMinTimestamp(&key_after_flush_buf, key, ts_sz);
+        key_after_flush = key_after_flush_buf;
+      }
+
+      //  Generate a rolling 64-bit hash of the key and values
+      //  Note :
+      //  Here "key" integrates 'sequence_number'+'kType'+'user key'.
+      s = output_validator.Add(key_after_flush, value);
       if (!s.ok()) {
         break;
       }
-      builder->Add(key, value);
+      builder->Add(key_after_flush, value);
 
-      s = meta->UpdateBoundaries(key, value, ikey.sequence, ikey.type);
+      s = meta->UpdateBoundaries(key_after_flush, value, ikey.sequence,
+                                 ikey.type);
       if (!s.ok()) {
         break;
       }
@@ -244,6 +262,7 @@ Status BuildTable(
            range_del_it->Next()) {
         auto tombstone = range_del_it->Tombstone();
         auto kv = tombstone.Serialize();
+        // TODO(yuzhangyu): handle range deletion for UDT in memtables only.
         builder->Add(kv.first.Encode(), kv.second);
         InternalKey tombstone_end = tombstone.SerializeEndKey();
         meta->UpdateBoundariesForRange(kv.first, tombstone_end, tombstone.seq_,
@@ -354,6 +373,8 @@ Status BuildTable(
       s = *io_status;
     }
 
+    // TODO(yuzhangyu): handle the key copy in the blob when ts should be
+    // stripped.
     if (blob_file_builder) {
       if (s.ok()) {
         s = blob_file_builder->Finish();

db/c.cc

@@ -1057,6 +1057,36 @@ rocksdb_column_family_handle_t* rocksdb_create_column_family(
   return handle;
 }
 
+rocksdb_column_family_handle_t** rocksdb_create_column_families(
+    rocksdb_t* db, const rocksdb_options_t* column_family_options,
+    int num_column_families, const char* const* column_family_names,
+    size_t* lencfs, char** errptr) {
+  std::vector<ColumnFamilyHandle*> handles;
+  std::vector<std::string> names;
+  for (int i = 0; i != num_column_families; ++i) {
+    names.push_back(std::string(column_family_names[i]));
+  }
+  SaveError(errptr, db->rep->CreateColumnFamilies(
+                        ColumnFamilyOptions(column_family_options->rep), names,
+                        &handles));
+
+  *lencfs = handles.size();
+  rocksdb_column_family_handle_t** c_handles =
+      static_cast<rocksdb_column_family_handle_t**>(
+          malloc(sizeof(rocksdb_column_family_handle_t*) * handles.size()));
+  for (size_t i = 0; i != handles.size(); ++i) {
+    c_handles[i] = new rocksdb_column_family_handle_t;
+    c_handles[i]->rep = handles[i];
+  }
+
+  return c_handles;
+}
+
+void rocksdb_create_column_families_destroy(
+    rocksdb_column_family_handle_t** list) {
+  free(list);
+}
+
 rocksdb_column_family_handle_t* rocksdb_create_column_family_with_ttl(
     rocksdb_t* db, const rocksdb_options_t* column_family_options,
     const char* column_family_name, int ttl, char** errptr) {
@@ -3740,16 +3770,21 @@ void rocksdb_options_set_hash_link_list_rep(rocksdb_options_t* opt,
       ROCKSDB_NAMESPACE::NewHashLinkListRepFactory(bucket_count));
 }
 
-void rocksdb_options_set_plain_table_factory(rocksdb_options_t* opt,
-                                             uint32_t user_key_len,
-                                             int bloom_bits_per_key,
-                                             double hash_table_ratio,
-                                             size_t index_sparseness) {
+void rocksdb_options_set_plain_table_factory(
+    rocksdb_options_t* opt, uint32_t user_key_len, int bloom_bits_per_key,
+    double hash_table_ratio, size_t index_sparseness, size_t huge_page_tlb_size,
+    char encoding_type, unsigned char full_scan_mode,
+    unsigned char store_index_in_file) {
   ROCKSDB_NAMESPACE::PlainTableOptions options;
   options.user_key_len = user_key_len;
   options.bloom_bits_per_key = bloom_bits_per_key;
   options.hash_table_ratio = hash_table_ratio;
   options.index_sparseness = index_sparseness;
+  options.huge_page_tlb_size = huge_page_tlb_size;
+  options.encoding_type =
+      static_cast<ROCKSDB_NAMESPACE::EncodingType>(encoding_type);
+  options.full_scan_mode = full_scan_mode;
+  options.store_index_in_file = store_index_in_file;
 
   ROCKSDB_NAMESPACE::TableFactory* factory =
       ROCKSDB_NAMESPACE::NewPlainTableFactory(options);
@@ -3881,6 +3916,14 @@ void rocksdb_options_set_row_cache(rocksdb_options_t* opt,
 }
 
 void rocksdb_options_add_compact_on_deletion_collector_factory(
+    rocksdb_options_t* opt, size_t window_size, size_t num_dels_trigger) {
+  std::shared_ptr<ROCKSDB_NAMESPACE::TablePropertiesCollectorFactory>
+      compact_on_del =
+          NewCompactOnDeletionCollectorFactory(window_size, num_dels_trigger);
+  opt->rep.table_properties_collector_factories.emplace_back(compact_on_del);
+}
+
+void rocksdb_options_add_compact_on_deletion_collector_factory_del_ratio(
     rocksdb_options_t* opt, size_t window_size, size_t num_dels_trigger,
     double deletion_ratio) {
   std::shared_ptr<ROCKSDB_NAMESPACE::TablePropertiesCollectorFactory>
@@ -5580,6 +5623,23 @@ int rocksdb_transactiondb_property_int(rocksdb_transactiondb_t* db,
   }
 }
 
+rocksdb_t* rocksdb_transactiondb_get_base_db(
+    rocksdb_transactiondb_t* txn_db) {
+  DB* base_db = txn_db->rep->GetBaseDB();
+
+  if (base_db != nullptr) {
+    rocksdb_t* result = new rocksdb_t;
+    result->rep = base_db;
+    return result;
+  }
+
+  return nullptr;
+}
+
+void rocksdb_transactiondb_close_base_db(rocksdb_t* base_db) {
+  delete base_db;
+}
+
 rocksdb_transaction_t* rocksdb_transaction_begin(
     rocksdb_transactiondb_t* txn_db,
     const rocksdb_writeoptions_t* write_options,

db/c_test.c

@@ -720,7 +720,9 @@ int main(int argc, char** argv) {
   rocksdb_compactoptions_set_exclusive_manual_compaction(coptions, 1);
 
   rocksdb_options_add_compact_on_deletion_collector_factory(options, 10000,
-                                                            10001, 0.0);
+                                                            10001);
+  rocksdb_options_add_compact_on_deletion_collector_factory_del_ratio(
+      options, 10000, 10001, 0.0);
 
   StartPhase("destroy");
   rocksdb_destroy_db(options, dbname, &err);
@@ -1672,7 +1674,8 @@ int main(int argc, char** argv) {
     rocksdb_options_set_prefix_extractor(
         options, rocksdb_slicetransform_create_fixed_prefix(3));
     rocksdb_options_set_hash_skip_list_rep(options, 5000, 4, 4);
-    rocksdb_options_set_plain_table_factory(options, 4, 10, 0.75, 16);
+    rocksdb_options_set_plain_table_factory(options, 4, 10, 0.75, 16, 0, 0, 0,
+                                            0);
     rocksdb_options_set_allow_concurrent_memtable_write(options, 0);
 
     db = rocksdb_open(options, dbname, &err);
@@ -3119,6 +3122,17 @@ int main(int argc, char** argv) {
     CheckTxnDBGetCF(txn_db, roptions, cfh, "cf_foo", NULL);
     CheckTxnDBPinGetCF(txn_db, roptions, cfh, "cf_foo", NULL);
 
+
+    //memory usage
+    rocksdb_t* base_db = rocksdb_transactiondb_get_base_db(txn_db);
+    rocksdb_memory_consumers_t* consumers = rocksdb_memory_consumers_create();
+    rocksdb_memory_consumers_add_db(consumers, base_db);
+    rocksdb_memory_usage_t* usage = rocksdb_approximate_memory_usage_create(consumers, &err);
+    CheckNoError(err);
+    rocksdb_approximate_memory_usage_destroy(usage);
+    rocksdb_memory_consumers_destroy(consumers);
+    rocksdb_transactiondb_close_base_db(base_db);
+
     // flush
     rocksdb_flushoptions_t* flush_options = rocksdb_flushoptions_create();
     rocksdb_flushoptions_set_wait(flush_options, 1);
@@ -3374,8 +3388,19 @@ int main(int argc, char** argv) {
     rocksdb_put(db, woptions, "key", 3, "value", 5, &err);
     CheckNoError(err);
     rocksdb_column_family_handle_t *cfh1, *cfh2;
-    cfh1 = rocksdb_create_column_family(db, db_options, "txn_db_cf1", &err);
-    cfh2 = rocksdb_create_column_family(db, db_options, "txn_db_cf2", &err);
+    char** list_const_cf_names = (char**)malloc(2 * sizeof(char*));
+    list_const_cf_names[0] = "txn_db_cf1";
+    list_const_cf_names[1] = "txn_db_cf2";
+    size_t cflen;
+    rocksdb_column_family_handle_t** list_cfh = rocksdb_create_column_families(
+        db, db_options, 2, (const char* const*)list_const_cf_names, &cflen,
+        &err);
+    free(list_const_cf_names);
+    CheckNoError(err);
+    assert(cflen == 2);
+    cfh1 = list_cfh[0];
+    cfh2 = list_cfh[1];
+    rocksdb_create_column_families_destroy(list_cfh);
     txn = rocksdb_optimistictransaction_begin(otxn_db, woptions, otxn_options,
                                               NULL);
     rocksdb_transaction_put_cf(txn, cfh1, "key_cf1", 7, "val_cf1", 7, &err);

db/column_family.cc

@@ -382,8 +382,8 @@ ColumnFamilyOptions SanitizeOptions(const ImmutableDBOptions& db_options,
 
   const uint64_t kAdjustedTtl = 30 * 24 * 60 * 60;
   if (result.ttl == kDefaultTtl) {
-    if (is_block_based_table &&
-        result.compaction_style != kCompactionStyleFIFO) {
+    if (is_block_based_table) {
+      // For FIFO, max_open_files is checked in ValidateOptions().
       result.ttl = kAdjustedTtl;
     } else {
       result.ttl = 0;
@@ -403,16 +403,12 @@ ColumnFamilyOptions SanitizeOptions(const ImmutableDBOptions& db_options,
       result.periodic_compaction_seconds = kAdjustedPeriodicCompSecs;
     }
   } else {
-    // result.compaction_style == kCompactionStyleFIFO
-    if (result.ttl == 0) {
-      if (is_block_based_table) {
-        if (result.periodic_compaction_seconds == kDefaultPeriodicCompSecs) {
-          result.periodic_compaction_seconds = kAdjustedPeriodicCompSecs;
-        }
-        result.ttl = result.periodic_compaction_seconds;
-      }
-    } else if (result.periodic_compaction_seconds != 0) {
-      result.ttl = std::min(result.ttl, result.periodic_compaction_seconds);
+    if (result.periodic_compaction_seconds != kDefaultPeriodicCompSecs &&
+        result.periodic_compaction_seconds > 0) {
+      ROCKS_LOG_WARN(
+          db_options.info_log.get(),
+          "periodic_compaction_seconds does not support FIFO compaction. You"
+          "may want to set option TTL instead.");
     }
   }
 

db/compaction/compaction.cc

@@ -23,14 +23,13 @@ namespace ROCKSDB_NAMESPACE {
 const uint64_t kRangeTombstoneSentinel =
     PackSequenceAndType(kMaxSequenceNumber, kTypeRangeDeletion);
 
-int sstableKeyCompare(const Comparator* user_cmp, const InternalKey& a,
-                      const InternalKey& b) {
-  auto c = user_cmp->CompareWithoutTimestamp(a.user_key(), b.user_key());
+int sstableKeyCompare(const Comparator* uc, const Slice& a, const Slice& b) {
+  auto c = uc->CompareWithoutTimestamp(ExtractUserKey(a), ExtractUserKey(b));
   if (c != 0) {
     return c;
   }
-  auto a_footer = ExtractInternalKeyFooter(a.Encode());
-  auto b_footer = ExtractInternalKeyFooter(b.Encode());
+  auto a_footer = ExtractInternalKeyFooter(a);
+  auto b_footer = ExtractInternalKeyFooter(b);
   if (a_footer == kRangeTombstoneSentinel) {
     if (b_footer != kRangeTombstoneSentinel) {
       return -1;

db/compaction/compaction.h

@@ -31,8 +31,19 @@ namespace ROCKSDB_NAMESPACE {
 // that key never appears in the database. We don't want adjacent sstables to
 // be considered overlapping if they are separated by the range tombstone
 // sentinel.
-int sstableKeyCompare(const Comparator* user_cmp, const InternalKey& a,
-                      const InternalKey& b);
+int sstableKeyCompare(const Comparator* user_cmp, const Slice&, const Slice&);
+inline int sstableKeyCompare(const Comparator* user_cmp, const Slice& a,
+                             const InternalKey& b) {
+  return sstableKeyCompare(user_cmp, a, b.Encode());
+}
+inline int sstableKeyCompare(const Comparator* user_cmp, const InternalKey& a,
+                             const Slice& b) {
+  return sstableKeyCompare(user_cmp, a.Encode(), b);
+}
+inline int sstableKeyCompare(const Comparator* user_cmp, const InternalKey& a,
+                             const InternalKey& b) {
+  return sstableKeyCompare(user_cmp, a.Encode(), b.Encode());
+}
 int sstableKeyCompare(const Comparator* user_cmp, const InternalKey* a,
                       const InternalKey& b);
 int sstableKeyCompare(const Comparator* user_cmp, const InternalKey& a,

db/compaction/compaction_job.cc

@@ -1096,17 +1096,17 @@ void CompactionJob::ProcessKeyValueCompaction(SubcompactionState* sub_compact) {
   // GenSubcompactionBoundaries doesn't strip away the timestamp.
   size_t ts_sz = cfd->user_comparator()->timestamp_size();
   if (start.has_value()) {
-    read_options.iterate_lower_bound = &start.value();
+    read_options.iterate_lower_bound = &(*start);
     if (ts_sz > 0) {
-      start_without_ts = StripTimestampFromUserKey(start.value(), ts_sz);
-      read_options.iterate_lower_bound = &start_without_ts.value();
+      start_without_ts = StripTimestampFromUserKey(*start, ts_sz);
+      read_options.iterate_lower_bound = &(*start_without_ts);
     }
   }
   if (end.has_value()) {
-    read_options.iterate_upper_bound = &end.value();
+    read_options.iterate_upper_bound = &(*end);
     if (ts_sz > 0) {
-      end_without_ts = StripTimestampFromUserKey(end.value(), ts_sz);
-      read_options.iterate_upper_bound = &end_without_ts.value();
+      end_without_ts = StripTimestampFromUserKey(*end, ts_sz);
+      read_options.iterate_upper_bound = &(*end_without_ts);
     }
   }
 
@@ -1138,8 +1138,7 @@ void CompactionJob::ProcessKeyValueCompaction(SubcompactionState* sub_compact) {
   }
 
   if (start.has_value()) {
-    start_ikey.SetInternalKey(start.value(), kMaxSequenceNumber,
-                              kValueTypeForSeek);
+    start_ikey.SetInternalKey(*start, kMaxSequenceNumber, kValueTypeForSeek);
     if (ts_sz > 0) {
       start_ikey.UpdateInternalKey(kMaxSequenceNumber, kValueTypeForSeek,
                                    &ts_slice);
@@ -1148,7 +1147,7 @@ void CompactionJob::ProcessKeyValueCompaction(SubcompactionState* sub_compact) {
     start_user_key = start_ikey.GetUserKey();
   }
   if (end.has_value()) {
-    end_ikey.SetInternalKey(end.value(), kMaxSequenceNumber, kValueTypeForSeek);
+    end_ikey.SetInternalKey(*end, kMaxSequenceNumber, kValueTypeForSeek);
     if (ts_sz > 0) {
       end_ikey.UpdateInternalKey(kMaxSequenceNumber, kValueTypeForSeek,
                                  &ts_slice);
@@ -1289,8 +1288,8 @@ void CompactionJob::ProcessKeyValueCompaction(SubcompactionState* sub_compact) {
   while (status.ok() && !cfd->IsDropped() && c_iter->Valid()) {
     // Invariant: c_iter.status() is guaranteed to be OK if c_iter->Valid()
     // returns true.
-    assert(!end.has_value() || cfd->user_comparator()->Compare(
-                                   c_iter->user_key(), end.value()) < 0);
+    assert(!end.has_value() ||
+           cfd->user_comparator()->Compare(c_iter->user_key(), *end) < 0);
 
     if (c_iter_stats.num_input_records % kRecordStatsEvery ==
         kRecordStatsEvery - 1) {
@@ -1818,10 +1817,10 @@ Status CompactionJob::OpenCompactionOutputFile(SubcompactionState* sub_compact,
   uint64_t current_time = static_cast<uint64_t>(temp_current_time);
   InternalKey tmp_start, tmp_end;
   if (sub_compact->start.has_value()) {
-    tmp_start.SetMinPossibleForUserKey(sub_compact->start.value());
+    tmp_start.SetMinPossibleForUserKey(*(sub_compact->start));
   }
   if (sub_compact->end.has_value()) {
-    tmp_end.SetMinPossibleForUserKey(sub_compact->end.value());
+    tmp_end.SetMinPossibleForUserKey(*(sub_compact->end));
   }
   uint64_t oldest_ancester_time =
       sub_compact->compaction->MinInputFileOldestAncesterTime(

db/compaction/compaction_outputs.cc

@@ -127,11 +127,6 @@ size_t CompactionOutputs::UpdateGrandparentBoundaryInfo(
   if (grandparents.empty()) {
     return curr_key_boundary_switched_num;
   }
-  assert(!internal_key.empty());
-  InternalKey ikey;
-  ikey.DecodeFrom(internal_key);
-  assert(ikey.Valid());
-
   const Comparator* ucmp = compaction_->column_family_data()->user_comparator();
 
   // Move the grandparent_index_ to the file containing the current user_key.
@@ -139,7 +134,7 @@ size_t CompactionOutputs::UpdateGrandparentBoundaryInfo(
   // index points to the last file containing the key.
   while (grandparent_index_ < grandparents.size()) {
     if (being_grandparent_gap_) {
-      if (sstableKeyCompare(ucmp, ikey,
+      if (sstableKeyCompare(ucmp, internal_key,
                             grandparents[grandparent_index_]->smallest) < 0) {
         break;
       }
@@ -152,13 +147,13 @@ size_t CompactionOutputs::UpdateGrandparentBoundaryInfo(
       being_grandparent_gap_ = false;
     } else {
       int cmp_result = sstableKeyCompare(
-          ucmp, ikey, grandparents[grandparent_index_]->largest);
+          ucmp, internal_key, grandparents[grandparent_index_]->largest);
       // If it's same key, make sure grandparent_index_ is pointing to the last
       // one.
       if (cmp_result < 0 ||
           (cmp_result == 0 &&
            (grandparent_index_ == grandparents.size() - 1 ||
-            sstableKeyCompare(ucmp, ikey,
+            sstableKeyCompare(ucmp, internal_key,
                               grandparents[grandparent_index_ + 1]->smallest) <
                 0))) {
         break;

db/compaction/compaction_outputs.h

@@ -206,10 +206,10 @@ class CompactionOutputs {
       // We may only split the output when the cursor is in the range. Split
       if ((!end.has_value() ||
            icmp->user_comparator()->Compare(
-               ExtractUserKey(output_split_key->Encode()), end.value()) < 0) &&
-          (!start.has_value() || icmp->user_comparator()->Compare(
-                                     ExtractUserKey(output_split_key->Encode()),
-                                     start.value()) > 0)) {
+               ExtractUserKey(output_split_key->Encode()), *end) < 0) &&
+          (!start.has_value() ||
+           icmp->user_comparator()->Compare(
+               ExtractUserKey(output_split_key->Encode()), *start) > 0)) {
         local_output_split_key_ = output_split_key;
       }
     }

db/compaction/compaction_picker_test.cc

@@ -1968,6 +1968,15 @@ TEST_F(CompactionPickerTest, OverlappingUserKeys11) {
   ASSERT_EQ(7U, compaction->input(1, 0)->fd.GetNumber());
 }
 
+TEST_F(CompactionPickerTest, FileTtlBoosterLargeNumLevels) {
+  const uint64_t kCurrentTime = 1000000;
+  FileTtlBooster booster(kCurrentTime, /*ttl=*/2048,
+                         /*num_non_empty_levels=*/100, /*level=*/1);
+  FileMetaData meta;
+  meta.oldest_ancester_time = kCurrentTime - 1023;
+  ASSERT_EQ(1, booster.GetBoostScore(&meta));
+}
+
 TEST_F(CompactionPickerTest, FileTtlBooster) {
   // Set TTL to 2048
   // TTL boosting for all levels starts at 1024,

db/compaction/file_pri.h

@@ -53,8 +53,10 @@ class FileTtlBooster {
       enabled_ = true;
       uint64_t all_boost_start_age = ttl / 2;
       uint64_t all_boost_age_range = (ttl / 32) * 31 - all_boost_start_age;
+      // TODO(cbi): more reasonable algorithm that gives different values
+      //   when num_non_empty_levels - level - 1 > 63.
       uint64_t boost_age_range =
-          all_boost_age_range >> (num_non_empty_levels - level - 1);
+          all_boost_age_range >> std::min(63, num_non_empty_levels - level - 1);
       boost_age_start_ = all_boost_start_age + boost_age_range;
       const uint64_t kBoostRatio = 16;
       // prevent 0 value to avoid divide 0 error.

db/db_compaction_test.cc

@@ -4600,6 +4600,7 @@ TEST_F(DBCompactionTest, LevelTtlCascadingCompactions) {
               PutVarint64(encoded_fieled, 0);
             }
           });
+      ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing();
 
       options.env = env_;
 
@@ -4616,7 +4617,6 @@ TEST_F(DBCompactionTest, LevelTtlCascadingCompactions) {
               ttl_compactions++;
             }
           });
-      ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing();
 
       // Add two L6 files with key ranges: [1 .. 100], [101 .. 200].
       Random rnd(301);
@@ -4748,6 +4748,7 @@ TEST_F(DBCompactionTest, LevelPeriodicCompaction) {
               PutVarint64(encoded_fieled, 0);
             }
           });
+      ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing();
 
       options.env = env_;
 
@@ -4764,7 +4765,6 @@ TEST_F(DBCompactionTest, LevelPeriodicCompaction) {
               periodic_compactions++;
             }
           });
-      ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing();
 
       Random rnd(301);
       for (int i = 0; i < kNumLevelFiles; ++i) {

db/db_impl/db_impl_compaction_flush.cc

@@ -852,8 +852,8 @@ void DBImpl::NotifyOnFlushBegin(ColumnFamilyData* cfd, FileMetaData* file_meta,
     }
   }
   mutex_.Lock();
-// no need to signal bg_cv_ as it will be signaled at the end of the
-// flush process.
+  // no need to signal bg_cv_ as it will be signaled at the end of the
+  // flush process.
 }
 
 void DBImpl::NotifyOnFlushCompleted(
@@ -2775,7 +2775,7 @@ ColumnFamilyData* DBImpl::PopFirstFromCompactionQueue() {
 
 DBImpl::FlushRequest DBImpl::PopFirstFromFlushQueue() {
   assert(!flush_queue_.empty());
-  FlushRequest flush_req = flush_queue_.front();
+  FlushRequest flush_req = std::move(flush_queue_.front());
   flush_queue_.pop_front();
   if (!immutable_db_options_.atomic_flush) {
     assert(flush_req.cfd_to_max_mem_id_to_persist.size() == 1);
@@ -2970,14 +2970,12 @@ Status DBImpl::BackgroundFlush(bool* made_progress, JobContext* job_context,
   autovector<ColumnFamilyData*> column_families_not_to_flush;
   while (!flush_queue_.empty()) {
     // This cfd is already referenced
-    const FlushRequest& flush_req = PopFirstFromFlushQueue();
-    FlushReason flush_reason = flush_req.flush_reason;
+    auto [flush_reason, cfd_to_max_mem_id_to_persist] =
+        PopFirstFromFlushQueue();
     superversion_contexts.clear();
-    superversion_contexts.reserve(
-        flush_req.cfd_to_max_mem_id_to_persist.size());
+    superversion_contexts.reserve(cfd_to_max_mem_id_to_persist.size());
 
-    for (const auto& iter : flush_req.cfd_to_max_mem_id_to_persist) {
-      ColumnFamilyData* cfd = iter.first;
+    for (const auto& [cfd, max_memtable_id] : cfd_to_max_mem_id_to_persist) {
       if (cfd->GetMempurgeUsed()) {
         // If imm() contains silent memtables (e.g.: because
         // MemPurge was activated), requesting a flush will
@@ -2991,7 +2989,7 @@ Status DBImpl::BackgroundFlush(bool* made_progress, JobContext* job_context,
         continue;
       }
       superversion_contexts.emplace_back(SuperVersionContext(true));
-      bg_flush_args.emplace_back(cfd, iter.second,
+      bg_flush_args.emplace_back(cfd, max_memtable_id,
                                  &(superversion_contexts.back()), flush_reason);
     }
     if (!bg_flush_args.empty()) {

db/db_impl/db_impl_open.cc

@@ -1655,6 +1655,7 @@ Status DBImpl::WriteLevel0TableForRecovery(int job_id, ColumnFamilyData* cfd,
       Version* version = cfd->current();
       version->Ref();
       const ReadOptions read_option(Env::IOActivity::kDBOpen);
+      uint64_t num_input_entries = 0;
       s = BuildTable(
           dbname_, versions_.get(), immutable_db_options_, tboptions,
           file_options_for_compaction_, read_option, cfd->table_cache(),
@@ -1664,7 +1665,8 @@ Status DBImpl::WriteLevel0TableForRecovery(int job_id, ColumnFamilyData* cfd,
           io_tracer_, BlobFileCreationReason::kRecovery,
           empty_seqno_time_mapping, &event_logger_, job_id, Env::IO_HIGH,
           nullptr /* table_properties */, write_hint,
-          nullptr /*full_history_ts_low*/, &blob_callback_, version);
+          nullptr /*full_history_ts_low*/, &blob_callback_, version,
+          &num_input_entries);
       version->Unref();
       LogFlush(immutable_db_options_.info_log);
       ROCKS_LOG_DEBUG(immutable_db_options_.info_log,
@@ -1678,6 +1680,19 @@ Status DBImpl::WriteLevel0TableForRecovery(int job_id, ColumnFamilyData* cfd,
       if (!io_s.ok() && s.ok()) {
         s = io_s;
       }
+
+      uint64_t total_num_entries = mem->num_entries();
+      if (s.ok() && total_num_entries != num_input_entries) {
+        std::string msg = "Expected " + std::to_string(total_num_entries) +
+                          " entries in memtable, but read " +
+                          std::to_string(num_input_entries);
+        ROCKS_LOG_WARN(immutable_db_options_.info_log,
+                       "[%s] [JOB %d] Level-0 flush during recover: %s",
+                       cfd->GetName().c_str(), job_id, msg.c_str());
+        if (immutable_db_options_.flush_verify_memtable_count) {
+          s = Status::Corruption(msg);
+        }
+      }
     }
   }
   ReleaseFileNumberFromPendingOutputs(pending_outputs_inserted_elem);

db/db_options_test.cc

@@ -880,9 +880,13 @@ TEST_F(DBOptionsTest, SanitizeFIFOPeriodicCompaction) {
   Options options;
   options.compaction_style = kCompactionStyleFIFO;
   options.env = CurrentOptions().env;
+  // Default value allows RocksDB to set ttl to 30 days.
+  ASSERT_EQ(30 * 24 * 60 * 60, dbfull()->GetOptions().ttl);
+
+  // Disable
   options.ttl = 0;
   Reopen(options);
-  ASSERT_EQ(30 * 24 * 60 * 60, dbfull()->GetOptions().ttl);
+  ASSERT_EQ(0, dbfull()->GetOptions().ttl);
 
   options.ttl = 100;
   Reopen(options);
@@ -892,15 +896,12 @@ TEST_F(DBOptionsTest, SanitizeFIFOPeriodicCompaction) {
   Reopen(options);
   ASSERT_EQ(100 * 24 * 60 * 60, dbfull()->GetOptions().ttl);
 
-  options.ttl = 200;
-  options.periodic_compaction_seconds = 300;
-  Reopen(options);
-  ASSERT_EQ(200, dbfull()->GetOptions().ttl);
-
+  // periodic_compaction_seconds should have no effect
+  // on FIFO compaction.
   options.ttl = 500;
   options.periodic_compaction_seconds = 300;
   Reopen(options);
-  ASSERT_EQ(300, dbfull()->GetOptions().ttl);
+  ASSERT_EQ(500, dbfull()->GetOptions().ttl);
 }
 
 TEST_F(DBOptionsTest, SetFIFOCompactionOptions) {

db/db_test.cc

@@ -6902,16 +6902,16 @@ TEST_F(DBTest, RowCache) {
   options.statistics = ROCKSDB_NAMESPACE::CreateDBStatistics();
   LRUCacheOptions cache_options;
   cache_options.capacity = 8192;
-  options.row_cache = cache_options.MakeSharedGeneralCache();
+  options.row_cache = cache_options.MakeSharedRowCache();
   // BEGIN check that Cache classes as aliases of each other.
-  // Currently, GeneralCache and BlockCache are aliases for Cache.
+  // Currently, RowCache and BlockCache are aliases for Cache.
   // This is expected to change (carefully, intentionally)
-  std::shared_ptr<GeneralCache> general_cache = options.row_cache;
-  std::shared_ptr<Cache> cache = general_cache;
-  std::shared_ptr<BlockCache> block_cache = general_cache;
-  general_cache = cache;
+  std::shared_ptr<RowCache> row_cache = options.row_cache;
+  std::shared_ptr<Cache> cache = row_cache;
+  std::shared_ptr<BlockCache> block_cache = row_cache;
+  row_cache = cache;
   block_cache = cache;
-  general_cache = block_cache;
+  row_cache = block_cache;
   cache = block_cache;
   // END check that Cache classes as aliases of each other.
   DestroyAndReopen(options);

db/db_wal_test.cc

@@ -311,21 +311,32 @@ TEST_F(DBWALTest, Recover) {
   } while (ChangeWalOptions());
 }
 
-class DBWALTestWithTimestamp : public DBBasicTestWithTimestampBase {
+class DBWALTestWithTimestamp
+    : public DBBasicTestWithTimestampBase,
+      public testing::WithParamInterface<test::UserDefinedTimestampTestMode> {
  public:
   DBWALTestWithTimestamp()
       : DBBasicTestWithTimestampBase("db_wal_test_with_timestamp") {}
 
+  void SetUp() override {
+    persist_udt_ = test::ShouldPersistUDT(GetParam());
+    DBBasicTestWithTimestampBase::SetUp();
+  }
+
   Status CreateAndReopenWithCFWithTs(const std::vector<std::string>& cfs,
-                                     const Options& options) {
+                                     const Options& options,
+                                     bool avoid_flush_during_recovery = false) {
     CreateColumnFamilies(cfs, options);
-    return ReopenColumnFamiliesWithTs(cfs, options);
+    return ReopenColumnFamiliesWithTs(cfs, options,
+                                      avoid_flush_during_recovery);
   }
 
   Status ReopenColumnFamiliesWithTs(const std::vector<std::string>& cfs,
-                                    Options ts_options) {
+                                    Options ts_options,
+                                    bool avoid_flush_during_recovery = false) {
     Options default_options = CurrentOptions();
     default_options.create_if_missing = false;
+    default_options.avoid_flush_during_recovery = avoid_flush_during_recovery;
     ts_options.create_if_missing = false;
 
     std::vector<Options> cf_options(cfs.size(), ts_options);
@@ -343,46 +354,88 @@ class DBWALTestWithTimestamp : public DBBasicTestWithTimestampBase {
   }
 
   void CheckGet(const ReadOptions& read_opts, uint32_t cf, const Slice& key,
-                const std::string& expected_value) {
+                const std::string& expected_value,
+                const std::string& expected_ts) {
     std::string actual_value;
-    ASSERT_OK(db_->Get(read_opts, handles_[cf], key, &actual_value));
+    std::string actual_ts;
+    ASSERT_OK(
+        db_->Get(read_opts, handles_[cf], key, &actual_value, &actual_ts));
     ASSERT_EQ(expected_value, actual_value);
+    ASSERT_EQ(expected_ts, actual_ts);
   }
+
+ protected:
+  bool persist_udt_;
 };
 
-TEST_F(DBWALTestWithTimestamp, Recover) {
+TEST_P(DBWALTestWithTimestamp, RecoverAndNoFlush) {
   // Set up the option that enables user defined timestmp size.
-  std::string ts = Timestamp(1, 0);
-  const size_t kTimestampSize = ts.size();
+  std::string ts1 = Timestamp(1, 0);
+  const size_t kTimestampSize = ts1.size();
   TestComparator test_cmp(kTimestampSize);
   Options ts_options;
   ts_options.create_if_missing = true;
   ts_options.comparator = &test_cmp;
+  // Test that user-defined timestamps are recovered from WAL regardless of
+  // the value of this flag because UDTs are saved in WAL nonetheless.
+  // We however need to explicitly disable flush during recovery by setting
+  // `avoid_flush_during_recovery=true` so that we can avoid timestamps getting
+  // stripped when the `persist_user_defined_timestamps` flag is false, so that
+  // all written timestamps are available for testing user-defined time travel
+  // read.
+  ts_options.persist_user_defined_timestamps = persist_udt_;
+  bool avoid_flush_during_recovery = true;
 
   ReadOptions read_opts;
-  Slice ts_slice = ts;
-  read_opts.timestamp = &ts_slice;
   do {
-    ASSERT_OK(CreateAndReopenWithCFWithTs({"pikachu"}, ts_options));
-    ASSERT_OK(Put(1, "foo", ts, "v1"));
-    ASSERT_OK(Put(1, "baz", ts, "v5"));
+    Slice ts_slice = ts1;
+    read_opts.timestamp = &ts_slice;
+    ASSERT_OK(CreateAndReopenWithCFWithTs({"pikachu"}, ts_options,
+                                          avoid_flush_during_recovery));
+    ASSERT_EQ(GetNumberOfSstFilesForColumnFamily(db_, "pikachu"), 0U);
+    ASSERT_OK(Put(1, "foo", ts1, "v1"));
+    ASSERT_OK(Put(1, "baz", ts1, "v5"));
 
-    ASSERT_OK(ReopenColumnFamiliesWithTs({"pikachu"}, ts_options));
-    CheckGet(read_opts, 1, "foo", "v1");
-    CheckGet(read_opts, 1, "baz", "v5");
-    ASSERT_OK(Put(1, "bar", ts, "v2"));
-    ASSERT_OK(Put(1, "foo", ts, "v3"));
+    ASSERT_OK(ReopenColumnFamiliesWithTs({"pikachu"}, ts_options,
+                                         avoid_flush_during_recovery));
+    ASSERT_EQ(GetNumberOfSstFilesForColumnFamily(db_, "pikachu"), 0U);
+    // Do a timestamped read with ts1 after second reopen.
+    CheckGet(read_opts, 1, "foo", "v1", ts1);
+    CheckGet(read_opts, 1, "baz", "v5", ts1);
 
-    ASSERT_OK(ReopenColumnFamiliesWithTs({"pikachu"}, ts_options));
-    CheckGet(read_opts, 1, "foo", "v3");
-    ASSERT_OK(Put(1, "foo", ts, "v4"));
-    CheckGet(read_opts, 1, "foo", "v4");
-    CheckGet(read_opts, 1, "bar", "v2");
-    CheckGet(read_opts, 1, "baz", "v5");
+    // Write more value versions for key "foo" and "bar" before and after second
+    // reopen.
+    std::string ts2 = Timestamp(2, 0);
+    ASSERT_OK(Put(1, "bar", ts2, "v2"));
+    ASSERT_OK(Put(1, "foo", ts2, "v3"));
+
+    ASSERT_OK(ReopenColumnFamiliesWithTs({"pikachu"}, ts_options,
+                                         avoid_flush_during_recovery));
+    ASSERT_EQ(GetNumberOfSstFilesForColumnFamily(db_, "pikachu"), 0U);
+    std::string ts3 = Timestamp(3, 0);
+    ASSERT_OK(Put(1, "foo", ts3, "v4"));
+
+    // Do a timestamped read with ts1 after third reopen.
+    CheckGet(read_opts, 1, "foo", "v1", ts1);
+    std::string value;
+    ASSERT_TRUE(db_->Get(read_opts, handles_[1], "bar", &value).IsNotFound());
+    CheckGet(read_opts, 1, "baz", "v5", ts1);
+
+    // Do a timestamped read with ts2 after third reopen.
+    ts_slice = ts2;
+    CheckGet(read_opts, 1, "foo", "v3", ts2);
+    CheckGet(read_opts, 1, "bar", "v2", ts2);
+    CheckGet(read_opts, 1, "baz", "v5", ts1);
+
+    // Do a timestamped read with ts3 after third reopen.
+    ts_slice = ts3;
+    CheckGet(read_opts, 1, "foo", "v4", ts3);
+    CheckGet(read_opts, 1, "bar", "v2", ts2);
+    CheckGet(read_opts, 1, "baz", "v5", ts1);
   } while (ChangeWalOptions());
 }
 
-TEST_F(DBWALTestWithTimestamp, RecoverInconsistentTimestamp) {
+TEST_P(DBWALTestWithTimestamp, RecoverInconsistentTimestamp) {
   // Set up the option that enables user defined timestmp size.
   std::string ts = Timestamp(1, 0);
   const size_t kTimestampSize = ts.size();
@@ -390,17 +443,72 @@ TEST_F(DBWALTestWithTimestamp, RecoverInconsistentTimestamp) {
   Options ts_options;
   ts_options.create_if_missing = true;
   ts_options.comparator = &test_cmp;
+  ts_options.persist_user_defined_timestamps = persist_udt_;
 
   ASSERT_OK(CreateAndReopenWithCFWithTs({"pikachu"}, ts_options));
   ASSERT_OK(Put(1, "foo", ts, "v1"));
   ASSERT_OK(Put(1, "baz", ts, "v5"));
 
+  // In real use cases, switching to a different user comparator is prohibited
+  // by a sanity check during DB open that does a user comparator name
+  // comparison. This test mocked and bypassed that sanity check because the
+  // before and after user comparator are both named "TestComparator". This is
+  // to test the user-defined timestamp recovery logic for WAL files have
+  // the intended consistency check.
+  // `HandleWriteBatchTimestampSizeDifference` in udt_util.h has more details.
   TestComparator diff_test_cmp(kTimestampSize + 1);
   ts_options.comparator = &diff_test_cmp;
   ASSERT_TRUE(
       ReopenColumnFamiliesWithTs({"pikachu"}, ts_options).IsInvalidArgument());
 }
 
+TEST_P(DBWALTestWithTimestamp, RecoverAndFlush) {
+  // Set up the option that enables user defined timestamp size.
+  std::string min_ts = Timestamp(0, 0);
+  std::string write_ts = Timestamp(1, 0);
+  const size_t kTimestampSize = write_ts.size();
+  TestComparator test_cmp(kTimestampSize);
+  Options ts_options;
+  ts_options.create_if_missing = true;
+  ts_options.comparator = &test_cmp;
+  ts_options.persist_user_defined_timestamps = persist_udt_;
+
+  std::string smallest_ukey_without_ts = "baz";
+  std::string largest_ukey_without_ts = "foo";
+
+  ASSERT_OK(CreateAndReopenWithCFWithTs({"pikachu"}, ts_options));
+  // No flush, no sst files, because of no data.
+  ASSERT_EQ(GetNumberOfSstFilesForColumnFamily(db_, "pikachu"), 0U);
+  ASSERT_OK(Put(1, largest_ukey_without_ts, write_ts, "v1"));
+  ASSERT_OK(Put(1, smallest_ukey_without_ts, write_ts, "v5"));
+
+  ASSERT_OK(ReopenColumnFamiliesWithTs({"pikachu"}, ts_options));
+  // Memtable recovered from WAL flushed because `avoid_flush_during_recovery`
+  // defaults to false, created one L0 file.
+  ASSERT_EQ(GetNumberOfSstFilesForColumnFamily(db_, "pikachu"), 1U);
+
+  std::vector<std::vector<FileMetaData>> level_to_files;
+  dbfull()->TEST_GetFilesMetaData(handles_[1], &level_to_files);
+  ASSERT_GT(level_to_files.size(), 1);
+  // L0 only has one SST file.
+  ASSERT_EQ(level_to_files[0].size(), 1);
+  auto meta = level_to_files[0][0];
+  if (persist_udt_) {
+    ASSERT_EQ(smallest_ukey_without_ts + write_ts, meta.smallest.user_key());
+    ASSERT_EQ(largest_ukey_without_ts + write_ts, meta.largest.user_key());
+  } else {
+    ASSERT_EQ(smallest_ukey_without_ts + min_ts, meta.smallest.user_key());
+    ASSERT_EQ(largest_ukey_without_ts + min_ts, meta.largest.user_key());
+  }
+}
+
+// Param 0: test mode for the user-defined timestamp feature
+INSTANTIATE_TEST_CASE_P(
+    DBWALTestWithTimestamp, DBWALTestWithTimestamp,
+    ::testing::Values(
+        test::UserDefinedTimestampTestMode::kStripUserDefinedTimestamp,
+        test::UserDefinedTimestampTestMode::kNormal));
+
 TEST_F(DBWALTest, RecoverWithTableHandle) {
   do {
     Options options = CurrentOptions();

db/db_with_timestamp_basic_test.cc

@@ -3273,6 +3273,76 @@ TEST_F(UpdateFullHistoryTsLowTest, ConcurrentUpdate) {
   Close();
 }
 
+// Tests the effect of flag `persist_user_defined_timestamps` on the file
+// boundaries contained in the Manifest, a.k.a FileMetaData.smallest,
+// FileMetaData.largest.
+class HandleFileBoundariesTest
+    : public DBBasicTestWithTimestampBase,
+      public testing::WithParamInterface<test::UserDefinedTimestampTestMode> {
+ public:
+  HandleFileBoundariesTest()
+      : DBBasicTestWithTimestampBase("/handle_file_boundaries") {}
+};
+
+TEST_P(HandleFileBoundariesTest, ConfigurePersistUdt) {
+  Options options = CurrentOptions();
+  options.env = env_;
+  // Write a timestamp that is not the min timestamp to help test the behavior
+  // of flag `persist_user_defined_timestamps`.
+  std::string write_ts = Timestamp(1, 0);
+  std::string min_ts = Timestamp(0, 0);
+  std::string smallest_ukey_without_ts = "bar";
+  std::string largest_ukey_without_ts = "foo";
+  const size_t kTimestampSize = write_ts.size();
+  TestComparator test_cmp(kTimestampSize);
+  options.comparator = &test_cmp;
+  bool persist_udt = test::ShouldPersistUDT(GetParam());
+  options.persist_user_defined_timestamps = persist_udt;
+  DestroyAndReopen(options);
+
+  ASSERT_OK(
+      db_->Put(WriteOptions(), smallest_ukey_without_ts, write_ts, "val1"));
+  ASSERT_OK(
+      db_->Put(WriteOptions(), largest_ukey_without_ts, write_ts, "val2"));
+
+  // Create a L0 SST file and its record is added to the Manfiest.
+  ASSERT_OK(Flush());
+  Close();
+
+  options.create_if_missing = false;
+  // Reopen the DB and process manifest file.
+  Reopen(options);
+
+  std::vector<std::vector<FileMetaData>> level_to_files;
+  dbfull()->TEST_GetFilesMetaData(dbfull()->DefaultColumnFamily(),
+                                  &level_to_files);
+  ASSERT_GT(level_to_files.size(), 1);
+  // L0 only has one SST file.
+  ASSERT_EQ(level_to_files[0].size(), 1);
+  auto file_meta = level_to_files[0][0];
+  if (persist_udt) {
+    ASSERT_EQ(smallest_ukey_without_ts + write_ts,
+              file_meta.smallest.user_key());
+    ASSERT_EQ(largest_ukey_without_ts + write_ts, file_meta.largest.user_key());
+  } else {
+    // If `persist_user_defined_timestamps` is false, the file boundaries should
+    // have the min timestamp. Behind the scenes, when file boundaries in
+    // FileMetaData is persisted to Manifest, the original user-defined
+    // timestamps in user key are stripped. When manifest is read and processed
+    // during DB open, a min timestamp is padded to the file boundaries. This
+    // test's writes contain non min timestamp to verify this logic end-to-end.
+    ASSERT_EQ(smallest_ukey_without_ts + min_ts, file_meta.smallest.user_key());
+    ASSERT_EQ(largest_ukey_without_ts + min_ts, file_meta.largest.user_key());
+  }
+  Close();
+}
+
+INSTANTIATE_TEST_CASE_P(
+    ConfigurePersistUdt, HandleFileBoundariesTest,
+    ::testing::Values(
+        test::UserDefinedTimestampTestMode::kStripUserDefinedTimestamp,
+        test::UserDefinedTimestampTestMode::kNormal));
+
 TEST_F(DBBasicTestWithTimestamp,
        GCPreserveRangeTombstoneWhenNoOrSmallFullHistoryLow) {
   Options options = CurrentOptions();

db/dbformat.cc

@@ -120,6 +120,16 @@ void StripTimestampFromInternalKey(std::string* result, const Slice& key,
                  kNumInternalBytes);
 }
 
+void ReplaceInternalKeyWithMinTimestamp(std::string* result, const Slice& key,
+                                        size_t ts_sz) {
+  const size_t key_sz = key.size();
+  assert(key_sz >= ts_sz + kNumInternalBytes);
+  result->reserve(key_sz);
+  result->append(key.data(), key_sz - kNumInternalBytes - ts_sz);
+  result->append(ts_sz, static_cast<unsigned char>(0));
+  result->append(key.data() + key_sz - kNumInternalBytes, kNumInternalBytes);
+}
+
 std::string ParsedInternalKey::DebugString(bool log_err_key, bool hex) const {
   std::string result = "'";
   if (log_err_key) {

db/dbformat.h

@@ -168,51 +168,57 @@ inline void UnPackSequenceAndType(uint64_t packed, uint64_t* seq,
 EntryType GetEntryType(ValueType value_type);
 
 // Append the serialization of "key" to *result.
-extern void AppendInternalKey(std::string* result,
-                              const ParsedInternalKey& key);
+void AppendInternalKey(std::string* result, const ParsedInternalKey& key);
 
 // Append the serialization of "key" to *result, replacing the original
 // timestamp with argument ts.
-extern void AppendInternalKeyWithDifferentTimestamp(
-    std::string* result, const ParsedInternalKey& key, const Slice& ts);
+void AppendInternalKeyWithDifferentTimestamp(std::string* result,
+                                             const ParsedInternalKey& key,
+                                             const Slice& ts);
 
 // Serialized internal key consists of user key followed by footer.
 // This function appends the footer to *result, assuming that *result already
 // contains the user key at the end.
-extern void AppendInternalKeyFooter(std::string* result, SequenceNumber s,
-                                    ValueType t);
+void AppendInternalKeyFooter(std::string* result, SequenceNumber s,
+                             ValueType t);
 
 // Append the key and a minimal timestamp to *result
-extern void AppendKeyWithMinTimestamp(std::string* result, const Slice& key,
-                                      size_t ts_sz);
+void AppendKeyWithMinTimestamp(std::string* result, const Slice& key,
+                               size_t ts_sz);
 
 // Append the key and a maximal timestamp to *result
-extern void AppendKeyWithMaxTimestamp(std::string* result, const Slice& key,
-                                      size_t ts_sz);
+void AppendKeyWithMaxTimestamp(std::string* result, const Slice& key,
+                               size_t ts_sz);
 
 // `key` is a user key with timestamp. Append the user key without timestamp
 // and the maximal timestamp to *result.
-extern void AppendUserKeyWithMaxTimestamp(std::string* result, const Slice& key,
-                                          size_t ts_sz);
+void AppendUserKeyWithMaxTimestamp(std::string* result, const Slice& key,
+                                   size_t ts_sz);
 
 // `key` is an internal key containing a user key without timestamp. Create a
 // new key in *result by padding a min timestamp of size `ts_sz` to the user key
 // and copying the remaining internal key bytes.
-extern void PadInternalKeyWithMinTimestamp(std::string* result,
-                                           const Slice& key, size_t ts_sz);
+void PadInternalKeyWithMinTimestamp(std::string* result, const Slice& key,
+                                    size_t ts_sz);
 
 // `key` is an internal key containing a user key with timestamp of size
 // `ts_sz`. Create a new internal key in *result by stripping the timestamp from
 // the user key and copying the remaining internal key bytes.
-extern void StripTimestampFromInternalKey(std::string* result, const Slice& key,
-                                          size_t ts_sz);
+void StripTimestampFromInternalKey(std::string* result, const Slice& key,
+                                   size_t ts_sz);
+
+// `key` is an internal key containing a user key with timestamp of size
+// `ts_sz`. Create a new internal key in *result while replace the original
+// timestamp with min timestamp.
+void ReplaceInternalKeyWithMinTimestamp(std::string* result, const Slice& key,
+                                        size_t ts_sz);
 
 // Attempt to parse an internal key from "internal_key".  On success,
 // stores the parsed data in "*result", and returns true.
 //
 // On error, returns false, leaves "*result" in an undefined state.
-extern Status ParseInternalKey(const Slice& internal_key,
-                               ParsedInternalKey* result, bool log_err_key);
+Status ParseInternalKey(const Slice& internal_key, ParsedInternalKey* result,
+                        bool log_err_key);
 
 // Returns the user key portion of an internal key.
 inline Slice ExtractUserKey(const Slice& internal_key) {
@@ -783,8 +789,7 @@ class InternalKeySliceTransform : public SliceTransform {
 // Read the key of a record from a write batch.
 // if this record represent the default column family then cf_record
 // must be passed as false, otherwise it must be passed as true.
-extern bool ReadKeyFromWriteBatchEntry(Slice* input, Slice* key,
-                                       bool cf_record);
+bool ReadKeyFromWriteBatchEntry(Slice* input, Slice* key, bool cf_record);
 
 // Read record from a write batch piece from input.
 // tag, column_family, key, value and blob are return values. Callers own the
@@ -793,9 +798,9 @@ extern bool ReadKeyFromWriteBatchEntry(Slice* input, Slice* key,
 // input will be advanced to after the record.
 // If user-defined timestamp is enabled for a column family, then the `key`
 // resulting from this call will include timestamp.
-extern Status ReadRecordFromWriteBatch(Slice* input, char* tag,
-                                       uint32_t* column_family, Slice* key,
-                                       Slice* value, Slice* blob, Slice* xid);
+Status ReadRecordFromWriteBatch(Slice* input, char* tag,
+                                uint32_t* column_family, Slice* key,
+                                Slice* value, Slice* blob, Slice* xid);
 
 // When user call DeleteRange() to delete a range of keys,
 // we will store a serialized RangeTombstone in MemTable and SST.

db/dbformat_test.cc

@@ -312,6 +312,27 @@ TEST_F(FormatTest, StripTimestampFromInternalKey) {
   ASSERT_EQ(kTypeValue, key_without_timestamp.type);
 }
 
+TEST_F(FormatTest, ReplaceInternalKeyWithMinTimestamp) {
+  std::string orig_user_key = "foo";
+  size_t ts_sz = 8;
+  orig_user_key.append(ts_sz, static_cast<unsigned char>(1));
+  std::string orig_internal_key = IKey(orig_user_key, 100, kTypeValue);
+
+  std::string key_buf;
+  ReplaceInternalKeyWithMinTimestamp(&key_buf, orig_internal_key, ts_sz);
+  ParsedInternalKey new_key;
+  Slice in(key_buf);
+  ASSERT_OK(ParseInternalKey(in, &new_key, true /*log_err_key*/));
+
+  std::string min_timestamp(ts_sz, static_cast<unsigned char>(0));
+  size_t ukey_diff_offset = new_key.user_key.difference_offset(orig_user_key);
+  ASSERT_EQ(min_timestamp,
+            Slice(new_key.user_key.data() + ukey_diff_offset, ts_sz));
+  ASSERT_EQ(orig_user_key.size(), new_key.user_key.size());
+  ASSERT_EQ(100, new_key.sequence);
+  ASSERT_EQ(kTypeValue, new_key.type);
+}
+
 }  // namespace ROCKSDB_NAMESPACE
 
 int main(int argc, char** argv) {

db/file_indexer.cc

@@ -56,17 +56,15 @@ void FileIndexer::GetNextLevelIndex(const size_t level, const size_t file_index,
   } else if (cmp_smallest == 0) {
     *left_bound = index.smallest_lb;
     *right_bound = index.smallest_rb;
-  } else if (cmp_smallest > 0 && cmp_largest < 0) {
+  } else if (cmp_largest < 0) {
     *left_bound = index.smallest_lb;
     *right_bound = index.largest_rb;
   } else if (cmp_largest == 0) {
     *left_bound = index.largest_lb;
     *right_bound = index.largest_rb;
-  } else if (cmp_largest > 0) {
+  } else {
     *left_bound = index.largest_lb;
     *right_bound = level_rb_[level + 1];
-  } else {
-    assert(false);
   }
 
   assert(*left_bound >= 0);

db/flush_job_test.cc

@@ -117,6 +117,8 @@ class FlushJobTestBase : public testing::Test {
     db_options_.statistics = CreateDBStatistics();
 
     cf_options_.comparator = ucmp_;
+    cf_options_.persist_user_defined_timestamps = persist_udt_;
+    cf_options_.paranoid_file_checks = paranoid_file_checks_;
 
     std::vector<ColumnFamilyDescriptor> column_families;
     cf_options_.table_factory = mock_table_factory_;
@@ -149,6 +151,9 @@ class FlushJobTestBase : public testing::Test {
   std::atomic<bool> shutting_down_;
   std::shared_ptr<mock::MockTableFactory> mock_table_factory_;
 
+  bool persist_udt_ = true;
+  bool paranoid_file_checks_ = false;
+
   SeqnoToTimeMapping empty_seqno_to_time_mapping_;
 };
 
@@ -600,7 +605,13 @@ TEST_F(FlushJobTest, GetRateLimiterPriorityForWrite) {
   }
 }
 
-class FlushJobTimestampTest : public FlushJobTestBase {
+// Test parameters:
+// param 0): paranoid file check
+// param 1): user-defined timestamp test mode
+class FlushJobTimestampTest
+    : public FlushJobTestBase,
+      public testing::WithParamInterface<
+          std::tuple<bool, test::UserDefinedTimestampTestMode>> {
  public:
   FlushJobTimestampTest()
       : FlushJobTestBase(test::PerThreadDBPath("flush_job_ts_gc_test"),
@@ -616,13 +627,36 @@ class FlushJobTimestampTest : public FlushJobTestBase {
   }
 
  protected:
+  void SetUp() override {
+    paranoid_file_checks_ = std::get<0>(GetParam());
+    auto udt_test_mode = std::get<1>(GetParam());
+    persist_udt_ = test::ShouldPersistUDT(udt_test_mode);
+    FlushJobTestBase::SetUp();
+  }
   static constexpr uint64_t kStartTs = 10;
   static constexpr SequenceNumber kStartSeq = 0;
   SequenceNumber curr_seq_{kStartSeq};
   std::atomic<uint64_t> curr_ts_{kStartTs};
+
+  void CheckFileMetaData(ColumnFamilyData* cfd,
+                         const InternalKey& expected_smallest,
+                         const InternalKey& expected_largest,
+                         const FileMetaData* meta_from_flush) const {
+    ASSERT_EQ(expected_smallest.Encode(), meta_from_flush->smallest.Encode());
+    ASSERT_EQ(expected_largest.Encode(), meta_from_flush->largest.Encode());
+
+    const VersionStorageInfo* storage_info = cfd->current()->storage_info();
+    const std::vector<FileMetaData*>& l0_files = storage_info->LevelFiles(0);
+
+    ASSERT_EQ(l0_files.size(), 1);
+    auto installed_file_meta = l0_files[0];
+    ASSERT_EQ(expected_smallest.Encode(),
+              installed_file_meta->smallest.Encode());
+    ASSERT_EQ(expected_largest.Encode(), installed_file_meta->largest.Encode());
+  }
 };
 
-TEST_F(FlushJobTimestampTest, AllKeysExpired) {
+TEST_P(FlushJobTimestampTest, AllKeysExpired) {
   ColumnFamilyData* cfd = versions_->GetColumnFamilySet()->GetDefault();
   autovector<MemTable*> to_delete;
 
@@ -669,17 +703,24 @@ TEST_F(FlushJobTimestampTest, AllKeysExpired) {
 
   {
     std::string key = test::EncodeInt(0);
-    key.append(test::EncodeInt(curr_ts_.load(std::memory_order_relaxed) - 1));
+    if (!persist_udt_) {
+      // When `AdvancedColumnFamilyOptions.persist_user_defined_timestamps` flag
+      // is set to false. The user-defined timestamp is stripped from user key
+      // during flush, making the user key logically containing the minimum
+      // timestamp.
+      key.append(test::EncodeInt(0));
+    } else {
+      key.append(test::EncodeInt(curr_ts_.load(std::memory_order_relaxed) - 1));
+    }
     InternalKey ikey(key, curr_seq_ - 1, ValueType::kTypeDeletionWithTimestamp);
-    ASSERT_EQ(ikey.Encode(), fmeta.smallest.Encode());
-    ASSERT_EQ(ikey.Encode(), fmeta.largest.Encode());
+    CheckFileMetaData(cfd, ikey, ikey, &fmeta);
   }
 
   job_context.Clean();
   ASSERT_TRUE(to_delete.empty());
 }
 
-TEST_F(FlushJobTimestampTest, NoKeyExpired) {
+TEST_P(FlushJobTimestampTest, NoKeyExpired) {
   ColumnFamilyData* cfd = versions_->GetColumnFamilySet()->GetDefault();
   autovector<MemTable*> to_delete;
 
@@ -722,18 +763,38 @@ TEST_F(FlushJobTimestampTest, NoKeyExpired) {
 
   {
     std::string ukey = test::EncodeInt(0);
-    std::string smallest_key =
-        ukey + test::EncodeInt(curr_ts_.load(std::memory_order_relaxed) - 1);
-    std::string largest_key = ukey + test::EncodeInt(kStartTs);
+    std::string smallest_key;
+    std::string largest_key;
+    if (!persist_udt_) {
+      // When `AdvancedColumnFamilyOptions.persist_user_defined_timestamps` flag
+      // is set to false. The user-defined timestamp is stripped from user key
+      // during flush, making the user key logically containing the minimum
+      // timestamp, which is hardcoded to be all zeros for now.
+      smallest_key = ukey + test::EncodeInt(0);
+      largest_key = ukey + test::EncodeInt(0);
+    } else {
+      smallest_key =
+          ukey + test::EncodeInt(curr_ts_.load(std::memory_order_relaxed) - 1);
+      largest_key = ukey + test::EncodeInt(kStartTs);
+    }
     InternalKey smallest(smallest_key, curr_seq_ - 1, ValueType::kTypeValue);
     InternalKey largest(largest_key, kStartSeq, ValueType::kTypeValue);
-    ASSERT_EQ(smallest.Encode(), fmeta.smallest.Encode());
-    ASSERT_EQ(largest.Encode(), fmeta.largest.Encode());
+    CheckFileMetaData(cfd, smallest, largest, &fmeta);
   }
   job_context.Clean();
   ASSERT_TRUE(to_delete.empty());
 }
 
+// Param 0: paranoid file check
+// Param 1: test mode for the user-defined timestamp feature
+INSTANTIATE_TEST_CASE_P(
+    FlushJobTimestampTest, FlushJobTimestampTest,
+    ::testing::Combine(
+        ::testing::Bool(),
+        ::testing::Values(
+            test::UserDefinedTimestampTestMode::kStripUserDefinedTimestamp,
+            test::UserDefinedTimestampTestMode::kNormal)));
+
 }  // namespace ROCKSDB_NAMESPACE
 
 int main(int argc, char** argv) {

db/memtable.cc

@@ -810,7 +810,7 @@ Status MemTable::Add(SequenceNumber s, ValueType type,
         earliest_seqno_.load(std::memory_order_relaxed);
     while (
         (cur_earliest_seqno == kMaxSequenceNumber || s < cur_earliest_seqno) &&
-        !first_seqno_.compare_exchange_weak(cur_earliest_seqno, s)) {
+        !earliest_seqno_.compare_exchange_weak(cur_earliest_seqno, s)) {
     }
   }
   if (type == kTypeRangeDeletion) {

db/repair_test.cc

@@ -315,7 +315,13 @@ TEST_F(RepairTest, UnflushedSst) {
   ASSERT_EQ(Get("key"), "val");
 }
 
-class RepairTestWithTimestamp : public DBBasicTestWithTimestampBase {
+// Test parameters:
+// param 0): paranoid file check
+// param 1): user-defined timestamp test mode
+class RepairTestWithTimestamp
+    : public DBBasicTestWithTimestampBase,
+      public testing::WithParamInterface<
+          std::tuple<bool, test::UserDefinedTimestampTestMode>> {
  public:
   RepairTestWithTimestamp()
       : DBBasicTestWithTimestampBase("repair_test_with_timestamp") {}
@@ -326,23 +332,46 @@ class RepairTestWithTimestamp : public DBBasicTestWithTimestampBase {
   }
 
   void CheckGet(const ReadOptions& read_opts, const Slice& key,
-                const std::string& expected_value) {
+                const std::string& expected_value,
+                const std::string& expected_ts) {
     std::string actual_value;
-    ASSERT_OK(db_->Get(read_opts, handles_[0], key, &actual_value));
+    std::string actual_ts;
+    ASSERT_OK(db_->Get(read_opts, handles_[0], key, &actual_value, &actual_ts));
     ASSERT_EQ(expected_value, actual_value);
+    ASSERT_EQ(expected_ts, actual_ts);
+  }
+
+  void CheckFileBoundaries(const Slice& smallest_user_key,
+                           const Slice& largest_user_key) {
+    std::vector<std::vector<FileMetaData>> level_to_files;
+    dbfull()->TEST_GetFilesMetaData(dbfull()->DefaultColumnFamily(),
+                                    &level_to_files);
+    ASSERT_GT(level_to_files.size(), 1);
+    // L0 only has one SST file.
+    ASSERT_EQ(level_to_files[0].size(), 1);
+    auto file_meta = level_to_files[0][0];
+    ASSERT_EQ(smallest_user_key, file_meta.smallest.user_key());
+    ASSERT_EQ(largest_user_key, file_meta.largest.user_key());
   }
 };
 
-TEST_F(RepairTestWithTimestamp, UnflushedSst) {
+TEST_P(RepairTestWithTimestamp, UnflushedSst) {
   Destroy(last_options_);
 
+  bool paranoid_file_checks = std::get<0>(GetParam());
+  bool persist_udt = test::ShouldPersistUDT(std::get<1>(GetParam()));
+  std::string smallest_ukey_without_ts = "bar";
+  std::string largest_ukey_without_ts = "foo";
   Options options = CurrentOptions();
   options.env = env_;
   options.create_if_missing = true;
-  std::string ts = Timestamp(0, 0);
-  const size_t kTimestampSize = ts.size();
+  std::string min_ts = Timestamp(0, 0);
+  std::string write_ts = Timestamp(1, 0);
+  const size_t kTimestampSize = write_ts.size();
   TestComparator test_cmp(kTimestampSize);
   options.comparator = &test_cmp;
+  options.persist_user_defined_timestamps = persist_udt;
+  options.paranoid_file_checks = paranoid_file_checks;
 
   ColumnFamilyOptions cf_options(options);
   std::vector<ColumnFamilyDescriptor> column_families;
@@ -351,7 +380,10 @@ TEST_F(RepairTestWithTimestamp, UnflushedSst) {
 
   ASSERT_OK(DB::Open(options, dbname_, column_families, &handles_, &db_));
 
-  ASSERT_OK(Put("key", ts, "val"));
+  ASSERT_OK(Put(smallest_ukey_without_ts, write_ts,
+                smallest_ukey_without_ts + ":val"));
+  ASSERT_OK(
+      Put(largest_ukey_without_ts, write_ts, largest_ukey_without_ts + ":val"));
   VectorLogPtr wal_files;
   ASSERT_OK(dbfull()->GetSortedWalFiles(wal_files));
   ASSERT_EQ(wal_files.size(), 1);
@@ -381,12 +413,46 @@ TEST_F(RepairTestWithTimestamp, UnflushedSst) {
     ASSERT_GT(total_ssts_size, 0);
   }
 
+  // Check file boundaries are correct for different
+  // `persist_user_defined_timestamps` option values.
+  if (persist_udt) {
+    CheckFileBoundaries(smallest_ukey_without_ts + write_ts,
+                        largest_ukey_without_ts + write_ts);
+  } else {
+    CheckFileBoundaries(smallest_ukey_without_ts + min_ts,
+                        largest_ukey_without_ts + min_ts);
+  }
+
   ReadOptions read_opts;
-  Slice read_ts_slice = ts;
+  Slice read_ts_slice = write_ts;
   read_opts.timestamp = &read_ts_slice;
-  CheckGet(read_opts, "key", "val");
+  if (persist_udt) {
+    CheckGet(read_opts, smallest_ukey_without_ts,
+             smallest_ukey_without_ts + ":val", write_ts);
+    CheckGet(read_opts, largest_ukey_without_ts,
+             largest_ukey_without_ts + ":val", write_ts);
+  } else {
+    // TODO(yuzhangyu): currently when `persist_user_defined_timestamps` is
+    //  false, ts is unconditionally stripped during flush.
+    //  When `full_history_ts_low` is set and respected during flush.
+    //  We should prohibit reading below `full_history_ts_low` all together.
+    CheckGet(read_opts, smallest_ukey_without_ts,
+             smallest_ukey_without_ts + ":val", min_ts);
+    CheckGet(read_opts, largest_ukey_without_ts,
+             largest_ukey_without_ts + ":val", min_ts);
+  }
 }
 
+// Param 0: paranoid file check
+// Param 1: test mode for the user-defined timestamp feature
+INSTANTIATE_TEST_CASE_P(
+    UnflushedSst, RepairTestWithTimestamp,
+    ::testing::Combine(
+        ::testing::Bool(),
+        ::testing::Values(
+            test::UserDefinedTimestampTestMode::kStripUserDefinedTimestamp,
+            test::UserDefinedTimestampTestMode::kNormal)));
+
 TEST_F(RepairTest, SeparateWalDir) {
   do {
     Options options = CurrentOptions();

db/table_cache.cc

@@ -111,13 +111,17 @@ Status TableCache::GetTableReader(
     RecordTick(ioptions_.stats, NO_FILE_OPENS);
   } else if (s.IsPathNotFound()) {
     fname = Rocks2LevelTableFileName(fname);
-    s = PrepareIOFromReadOptions(ro, ioptions_.clock, fopts.io_options);
-    if (s.ok()) {
-      s = ioptions_.fs->NewRandomAccessFile(fname, file_options, &file,
-                                            nullptr);
+    // If this file is also not found, we want to use the error message
+    // that contains the table file name which is less confusing.
+    Status temp_s =
+        PrepareIOFromReadOptions(ro, ioptions_.clock, fopts.io_options);
+    if (temp_s.ok()) {
+      temp_s = ioptions_.fs->NewRandomAccessFile(fname, file_options, &file,
+                                                 nullptr);
     }
-    if (s.ok()) {
+    if (temp_s.ok()) {
       RecordTick(ioptions_.stats, NO_FILE_OPENS);
+      s = temp_s;
     }
   }
 

db/version_edit.cc

@@ -93,7 +93,8 @@ void VersionEdit::Clear() {
   full_history_ts_low_.clear();
 }
 
-bool VersionEdit::EncodeTo(std::string* dst) const {
+bool VersionEdit::EncodeTo(std::string* dst,
+                           std::optional<size_t> ts_sz) const {
   if (has_db_id_) {
     PutVarint32(dst, kDbId);
     PutLengthPrefixedSlice(dst, db_id_);
@@ -133,6 +134,8 @@ bool VersionEdit::EncodeTo(std::string* dst) const {
   }
 
   bool min_log_num_written = false;
+
+  assert(new_files_.empty() || ts_sz.has_value());
   for (size_t i = 0; i < new_files_.size(); i++) {
     const FileMetaData& f = new_files_[i].second;
     if (!f.smallest.Valid() || !f.largest.Valid() ||
@@ -142,8 +145,7 @@ bool VersionEdit::EncodeTo(std::string* dst) const {
     PutVarint32(dst, kNewFile4);
     PutVarint32Varint64(dst, new_files_[i].first /* level */, f.fd.GetNumber());
     PutVarint64(dst, f.fd.GetFileSize());
-    PutLengthPrefixedSlice(dst, f.smallest.Encode());
-    PutLengthPrefixedSlice(dst, f.largest.Encode());
+    EncodeFileBoundaries(dst, f, ts_sz.value());
     PutVarint64Varint64(dst, f.fd.smallest_seqno, f.fd.largest_seqno);
     // Customized fields' format:
     // +-----------------------------+
@@ -458,6 +460,23 @@ const char* VersionEdit::DecodeNewFile4From(Slice* input) {
   return nullptr;
 }
 
+void VersionEdit::EncodeFileBoundaries(std::string* dst,
+                                       const FileMetaData& meta,
+                                       size_t ts_sz) const {
+  if (ts_sz == 0 || meta.user_defined_timestamps_persisted) {
+    PutLengthPrefixedSlice(dst, meta.smallest.Encode());
+    PutLengthPrefixedSlice(dst, meta.largest.Encode());
+    return;
+  }
+  std::string smallest_buf;
+  std::string largest_buf;
+  StripTimestampFromInternalKey(&smallest_buf, meta.smallest.Encode(), ts_sz);
+  StripTimestampFromInternalKey(&largest_buf, meta.largest.Encode(), ts_sz);
+  PutLengthPrefixedSlice(dst, smallest_buf);
+  PutLengthPrefixedSlice(dst, largest_buf);
+  return;
+};
+
 Status VersionEdit::DecodeFrom(const Slice& src) {
   Clear();
 #ifndef NDEBUG

db/version_edit.h

@@ -9,6 +9,7 @@
 
 #pragma once
 #include <algorithm>
+#include <optional>
 #include <set>
 #include <string>
 #include <utility>
@@ -489,6 +490,8 @@ class VersionEdit {
   using NewFiles = std::vector<std::pair<int, FileMetaData>>;
   const NewFiles& GetNewFiles() const { return new_files_; }
 
+  NewFiles& GetMutableNewFiles() { return new_files_; }
+
   // Retrieve all the compact cursors
   using CompactCursors = std::vector<std::pair<int, InternalKey>>;
   const CompactCursors& GetCompactCursors() const { return compact_cursors_; }
@@ -639,7 +642,17 @@ class VersionEdit {
   }
 
   // return true on success.
-  bool EncodeTo(std::string* dst) const;
+  // `ts_sz` is the size in bytes for the user-defined timestamp contained in
+  // a user key. This argument is optional because it's only required for
+  // encoding a `VersionEdit` with new SST files to add. It's used to handle the
+  // file boundaries: `smallest`, `largest` when
+  // `FileMetaData.user_defined_timestamps_persisted` is false. When reading
+  // the Manifest file, a mirroring change needed to handle
+  // file boundaries are not added to the `VersionEdit.DecodeFrom` function
+  // because timestamp size is not available at `VersionEdit` decoding time,
+  // it's instead added to `VersionEditHandler::OnNonCfOperation`.
+  bool EncodeTo(std::string* dst,
+                std::optional<size_t> ts_sz = std::nullopt) const;
   Status DecodeFrom(const Slice& src);
 
   std::string DebugString(bool hex_key = false) const;
@@ -660,6 +673,12 @@ class VersionEdit {
 
   const char* DecodeNewFile4From(Slice* input);
 
+  // Encode file boundaries `FileMetaData.smallest` and `FileMetaData.largest`.
+  // User-defined timestamps in the user key will be stripped if they shouldn't
+  // be persisted.
+  void EncodeFileBoundaries(std::string* dst, const FileMetaData& meta,
+                            size_t ts_sz) const;
+
   int max_level_ = 0;
   std::string db_id_;
   std::string comparator_;

db/version_edit_handler.cc

@@ -86,7 +86,8 @@ void VersionEditHandlerBase::Iterate(log::Reader& reader,
         message << ' ';
       }
       // append the filename to the corruption message
-      message << "in file " << reader.file()->file_name();
+      message << " The file " << reader.file()->file_name()
+              << " may be corrupted.";
       // overwrite the status with the extended status
       s = Status(s.code(), s.subcode(), s.severity(), message.str());
     }
@@ -308,6 +309,17 @@ Status VersionEditHandler::OnNonCfOperation(VersionEdit& edit,
       tmp_cfd = version_set_->GetColumnFamilySet()->GetColumnFamily(
           edit.column_family_);
       assert(tmp_cfd != nullptr);
+      // It's important to handle file boundaries before `MaybeCreateVersion`
+      // because `VersionEditHandlerPointInTime::MaybeCreateVersion` does
+      // `FileMetaData` verification that involves the file boundaries.
+      // All `VersionEditHandlerBase` subclasses that need to deal with
+      // `FileMetaData` for new files are also subclasses of
+      // `VersionEditHandler`, so it's sufficient to do the file boundaries
+      // handling in this method.
+      s = MaybeHandleFileBoundariesForNewFiles(edit, tmp_cfd);
+      if (!s.ok()) {
+        return s;
+      }
       s = MaybeCreateVersion(edit, tmp_cfd, /*force_create_version=*/false);
       if (s.ok()) {
         s = builder_iter->second->version_builder()->Apply(&edit);
@@ -647,6 +659,47 @@ Status VersionEditHandler::ExtractInfoFromVersionEdit(ColumnFamilyData* cfd,
   return s;
 }
 
+Status VersionEditHandler::MaybeHandleFileBoundariesForNewFiles(
+    VersionEdit& edit, const ColumnFamilyData* cfd) {
+  if (edit.GetNewFiles().empty()) {
+    return Status::OK();
+  }
+  auto ucmp = cfd->user_comparator();
+  assert(ucmp);
+  size_t ts_sz = ucmp->timestamp_size();
+  if (ts_sz == 0) {
+    return Status::OK();
+  }
+
+  VersionEdit::NewFiles& new_files = edit.GetMutableNewFiles();
+  assert(!new_files.empty());
+  bool file_boundaries_need_handling = false;
+  for (auto& new_file : new_files) {
+    FileMetaData& meta = new_file.second;
+    if (meta.user_defined_timestamps_persisted) {
+      // `FileMetaData.user_defined_timestamps_persisted` field is the value of
+      // the flag `AdvancedColumnFamilyOptions.persist_user_defined_timestamps`
+      // at the time when the SST file was created. As a result, all added SST
+      // files in one `VersionEdit` should have the same value for it.
+      if (file_boundaries_need_handling) {
+        return Status::Corruption(
+            "New files in one VersionEdit has different "
+            "user_defined_timestamps_persisted value.");
+      }
+      break;
+    }
+    file_boundaries_need_handling = true;
+    std::string smallest_buf;
+    std::string largest_buf;
+    PadInternalKeyWithMinTimestamp(&smallest_buf, meta.smallest.Encode(),
+                                   ts_sz);
+    PadInternalKeyWithMinTimestamp(&largest_buf, meta.largest.Encode(), ts_sz);
+    meta.smallest.DecodeFrom(smallest_buf);
+    meta.largest.DecodeFrom(largest_buf);
+  }
+  return Status::OK();
+}
+
 VersionEditHandlerPointInTime::VersionEditHandlerPointInTime(
     bool read_only, std::vector<ColumnFamilyDescriptor> column_families,
     VersionSet* version_set, const std::shared_ptr<IOTracer>& io_tracer,

db/version_edit_handler.h

@@ -206,6 +206,17 @@ class VersionEditHandler : public VersionEditHandlerBase {
  private:
   Status ExtractInfoFromVersionEdit(ColumnFamilyData* cfd,
                                     const VersionEdit& edit);
+
+  // When `FileMetaData.user_defined_timestamps_persisted` is false and
+  // user-defined timestamp size is non-zero. User-defined timestamps are
+  // stripped from file boundaries: `smallest`, `largest` in
+  // `VersionEdit.DecodeFrom` before they were written to Manifest.
+  // This is the mirroring change to handle file boundaries on the Manifest read
+  // path for this scenario: to pad a minimum timestamp to the user key in
+  // `smallest` and `largest` so their format are consistent with the running
+  // user comparator.
+  Status MaybeHandleFileBoundariesForNewFiles(VersionEdit& edit,
+                                              const ColumnFamilyData* cfd);
 };
 
 // A class similar to its base class, i.e. VersionEditHandler.

db/version_edit_test.cc

@@ -21,12 +21,20 @@
 namespace ROCKSDB_NAMESPACE {
 
 static void TestEncodeDecode(const VersionEdit& edit) {
+  // Encoding one `VersionEdit` and decoding it again should result in the
+  // exact same `VersionEdit`. However, a special handling is applied to file
+  // boundaries: `FileMetaData.smallest`, `FileMetaData.largest` when
+  // user-defined timestamps should not be persisted. In that scenario, this
+  // invariant does not hold. We disable this scenario in this util method to
+  // enable all other test cases continue to verify this invariant, while the
+  // special case is separately covered in test
+  // `EncodeDecodeNewFile4HandleFileBoundary`.
   std::string encoded, encoded2;
-  edit.EncodeTo(&encoded);
+  edit.EncodeTo(&encoded, 0 /* ts_sz */);
   VersionEdit parsed;
   Status s = parsed.DecodeFrom(encoded);
   ASSERT_TRUE(s.ok()) << s.ToString();
-  parsed.EncodeTo(&encoded2);
+  parsed.EncodeTo(&encoded2, 0 /* ts_sz */);
   ASSERT_EQ(encoded, encoded2);
 }
 
@@ -93,7 +101,7 @@ TEST_F(VersionEditTest, EncodeDecodeNewFile4) {
   TestEncodeDecode(edit);
 
   std::string encoded, encoded2;
-  edit.EncodeTo(&encoded);
+  edit.EncodeTo(&encoded, 0 /* ts_sz */);
   VersionEdit parsed;
   Status s = parsed.DecodeFrom(encoded);
   ASSERT_TRUE(s.ok()) << s.ToString();
@@ -119,6 +127,57 @@ TEST_F(VersionEditTest, EncodeDecodeNewFile4) {
   ASSERT_TRUE(new_files[3].second.user_defined_timestamps_persisted);
 }
 
+TEST_F(VersionEditTest, EncodeDecodeNewFile4HandleFileBoundary) {
+  static const uint64_t kBig = 1ull << 50;
+  size_t ts_sz = 16;
+  static std::string min_ts(ts_sz, static_cast<unsigned char>(0));
+  VersionEdit edit;
+  std::string smallest = "foo";
+  std::string largest = "zoo";
+  // In real manifest writing scenarios, one `VersionEdit` should not contain
+  // files with different `user_defined_timestamps_persisted` flag value.
+  // This is just for testing file boundaries handling w.r.t persisting user
+  // defined timestamps during `VersionEdit` encoding.
+  edit.AddFile(
+      3, 300, 3, 100, InternalKey(smallest + min_ts, kBig + 500, kTypeValue),
+      InternalKey(largest + min_ts, kBig + 600, kTypeDeletion), kBig + 500,
+      kBig + 600, true, Temperature::kUnknown, kInvalidBlobFileNumber,
+      kUnknownOldestAncesterTime, kUnknownFileCreationTime,
+      300 /* epoch_number */, kUnknownFileChecksum,
+      kUnknownFileChecksumFuncName, kNullUniqueId64x2,
+      0 /* compensated_range_deletion_size */, 0 /* tail_size */,
+      false /* user_defined_timestamps_persisted */);
+  edit.AddFile(3, 300, 3, 100,
+               InternalKey(smallest + min_ts, kBig + 500, kTypeValue),
+               InternalKey(largest + min_ts, kBig + 600, kTypeDeletion),
+               kBig + 500, kBig + 600, true, Temperature::kUnknown,
+               kInvalidBlobFileNumber, kUnknownOldestAncesterTime,
+               kUnknownFileCreationTime, 300 /* epoch_number */,
+               kUnknownFileChecksum, kUnknownFileChecksumFuncName,
+               kNullUniqueId64x2, 0 /* compensated_range_deletion_size */,
+               0 /* tail_size */, true /* user_defined_timestamps_persisted */);
+
+  std::string encoded;
+  edit.EncodeTo(&encoded, ts_sz);
+  VersionEdit parsed;
+  Status s = parsed.DecodeFrom(encoded);
+  ASSERT_TRUE(s.ok()) << s.ToString();
+  auto& new_files = parsed.GetNewFiles();
+  ASSERT_TRUE(new_files.size() == 2);
+  ASSERT_FALSE(new_files[0].second.user_defined_timestamps_persisted);
+  // First file's boundaries do not contain user-defined timestamps.
+  ASSERT_EQ(InternalKey(smallest, kBig + 500, kTypeValue).Encode(),
+            new_files[0].second.smallest.Encode());
+  ASSERT_EQ(InternalKey(largest, kBig + 600, kTypeDeletion).Encode(),
+            new_files[0].second.largest.Encode());
+  ASSERT_TRUE(new_files[1].second.user_defined_timestamps_persisted);
+  // Second file's boundaries contain user-defined timestamps.
+  ASSERT_EQ(InternalKey(smallest + min_ts, kBig + 500, kTypeValue).Encode(),
+            new_files[1].second.smallest.Encode());
+  ASSERT_EQ(InternalKey(largest + min_ts, kBig + 600, kTypeDeletion).Encode(),
+            new_files[1].second.largest.Encode());
+}
+
 TEST_F(VersionEditTest, ForwardCompatibleNewFile4) {
   static const uint64_t kBig = 1ull << 50;
   VersionEdit edit;
@@ -158,7 +217,7 @@ TEST_F(VersionEditTest, ForwardCompatibleNewFile4) {
         }
       });
   ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing();
-  edit.EncodeTo(&encoded);
+  edit.EncodeTo(&encoded, 0 /* ts_sz */);
   ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->DisableProcessing();
 
   VersionEdit parsed;
@@ -198,7 +257,7 @@ TEST_F(VersionEditTest, NewFile4NotSupportedField) {
         PutLengthPrefixedSlice(str, str1);
       });
   ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing();
-  edit.EncodeTo(&encoded);
+  edit.EncodeTo(&encoded, 0 /* ts_sz */);
   ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->DisableProcessing();
 
   VersionEdit parsed;
@@ -214,7 +273,7 @@ TEST_F(VersionEditTest, EncodeEmptyFile) {
                1 /*epoch_number*/, kUnknownFileChecksum,
                kUnknownFileChecksumFuncName, kNullUniqueId64x2, 0, 0, true);
   std::string buffer;
-  ASSERT_TRUE(!edit.EncodeTo(&buffer));
+  ASSERT_TRUE(!edit.EncodeTo(&buffer, 0 /* ts_sz */));
 }
 
 TEST_F(VersionEditTest, ColumnFamilyTest) {
@@ -583,7 +642,7 @@ TEST_F(VersionEditTest, IgnorableTags) {
   edit.SetColumnFamily(kColumnFamilyId);
 
   std::string encoded;
-  ASSERT_TRUE(edit.EncodeTo(&encoded));
+  ASSERT_TRUE(edit.EncodeTo(&encoded, 0 /* ts_sz */));
 
   VersionEdit decoded;
   ASSERT_OK(decoded.DecodeFrom(encoded));

db/version_set.cc

@@ -5118,6 +5118,12 @@ Status VersionSet::ProcessManifestWrites(
   assert(manifest_writers_.front() == &first_writer);
 
   autovector<VersionEdit*> batch_edits;
+  // This vector keeps track of the corresponding user-defined timestamp size
+  // for `batch_edits` side by side, which is only needed for encoding a
+  // `VersionEdit` that adds new SST files.
+  // Note that anytime `batch_edits` has new element added or get existing
+  // element removed, `batch_edits_ts_sz` should be updated too.
+  autovector<std::optional<size_t>> batch_edits_ts_sz;
   autovector<Version*> versions;
   autovector<const MutableCFOptions*> mutable_cf_options_ptrs;
   std::vector<std::unique_ptr<BaseReferencedVersionBuilder>> builder_guards;
@@ -5133,6 +5139,7 @@ Status VersionSet::ProcessManifestWrites(
     // No group commits for column family add or drop
     LogAndApplyCFHelper(first_writer.edit_list.front(), &max_last_sequence);
     batch_edits.push_back(first_writer.edit_list.front());
+    batch_edits_ts_sz.push_back(std::nullopt);
   } else {
     auto it = manifest_writers_.cbegin();
     size_t group_start = std::numeric_limits<size_t>::max();
@@ -5209,6 +5216,9 @@ Status VersionSet::ProcessManifestWrites(
         TEST_SYNC_POINT_CALLBACK("VersionSet::ProcessManifestWrites:NewVersion",
                                  version);
       }
+      const Comparator* ucmp = last_writer->cfd->user_comparator();
+      assert(ucmp);
+      std::optional<size_t> edit_ts_sz = ucmp->timestamp_size();
       for (const auto& e : last_writer->edit_list) {
         if (e->is_in_atomic_group_) {
           if (batch_edits.empty() || !batch_edits.back()->is_in_atomic_group_ ||
@@ -5229,6 +5239,7 @@ Status VersionSet::ProcessManifestWrites(
           return s;
         }
         batch_edits.push_back(e);
+        batch_edits_ts_sz.push_back(edit_ts_sz);
       }
     }
     for (int i = 0; i < static_cast<int>(versions.size()); ++i) {
@@ -5394,9 +5405,11 @@ Status VersionSet::ProcessManifestWrites(
 #ifndef NDEBUG
       size_t idx = 0;
 #endif
-      for (auto& e : batch_edits) {
+      assert(batch_edits.size() == batch_edits_ts_sz.size());
+      for (size_t bidx = 0; bidx < batch_edits.size(); bidx++) {
+        auto& e = batch_edits[bidx];
         std::string record;
-        if (!e->EncodeTo(&record)) {
+        if (!e->EncodeTo(&record, batch_edits_ts_sz[bidx])) {
           s = Status::Corruption("Unable to encode VersionEdit:" +
                                  e->DebugString(true));
           break;
@@ -6289,8 +6302,9 @@ Status VersionSet::GetLiveFilesChecksumInfo(FileChecksumList* checksum_list) {
   return s;
 }
 
-Status VersionSet::DumpManifest(Options& options, std::string& dscname,
-                                bool verbose, bool hex, bool json) {
+Status VersionSet::DumpManifest(
+    Options& options, std::string& dscname, bool verbose, bool hex, bool json,
+    const std::vector<ColumnFamilyDescriptor>& cf_descs) {
   assert(options.env);
   // TODO: plumb Env::IOActivity
   const ReadOptions read_options;
@@ -6316,13 +6330,22 @@ Status VersionSet::DumpManifest(Options& options, std::string& dscname,
         std::move(file), dscname, db_options_->log_readahead_size, io_tracer_);
   }
 
-  std::vector<ColumnFamilyDescriptor> cf_descs;
+  std::map<std::string, const ColumnFamilyDescriptor*> cf_name_to_desc;
+  for (const auto& cf_desc : cf_descs) {
+    cf_name_to_desc[cf_desc.name] = &cf_desc;
+  }
+  std::vector<ColumnFamilyDescriptor> final_cf_descs;
   for (const auto& cf : column_families) {
-    cf_descs.emplace_back(cf, options);
+    const auto iter = cf_name_to_desc.find(cf);
+    if (iter != cf_name_to_desc.cend()) {
+      final_cf_descs.push_back(*iter->second);
+    } else {
+      final_cf_descs.emplace_back(cf, options);
+    }
   }
 
-  DumpManifestHandler handler(cf_descs, this, io_tracer_, read_options, verbose,
-                              hex, json);
+  DumpManifestHandler handler(final_cf_descs, this, io_tracer_, read_options,
+                              verbose, hex, json);
   {
     VersionSet::LogReporter reporter;
     reporter.status = &s;
@@ -6505,8 +6528,10 @@ Status VersionSet::WriteCurrentStateToManifest(
 
       edit.SetLastSequence(descriptor_last_sequence_);
 
+      const Comparator* ucmp = cfd->user_comparator();
+      assert(ucmp);
       std::string record;
-      if (!edit.EncodeTo(&record)) {
+      if (!edit.EncodeTo(&record, ucmp->timestamp_size())) {
         return Status::Corruption("Unable to Encode VersionEdit:" +
                                   edit.DebugString(true));
       }
@@ -6858,7 +6883,7 @@ InternalIterator* VersionSet::MakeInputIterator(
           const FileMetaData& fmd = *flevel->files[i].file_metadata;
           if (start.has_value() &&
               cfd->user_comparator()->CompareWithoutTimestamp(
-                  start.value(), fmd.largest.user_key()) > 0) {
+                  *start, fmd.largest.user_key()) > 0) {
             continue;
           }
           // We should be able to filter out the case where the end key
@@ -6866,7 +6891,7 @@ InternalIterator* VersionSet::MakeInputIterator(
           // We try to be extra safe here.
           if (end.has_value() &&
               cfd->user_comparator()->CompareWithoutTimestamp(
-                  end.value(), fmd.smallest.user_key()) < 0) {
+                  *end, fmd.smallest.user_key()) < 0) {
             continue;
           }
           TruncatedRangeDelIterator* range_tombstone_iter = nullptr;

db/version_set.h

@@ -1273,8 +1273,8 @@ class VersionSet {
 
   // printf contents (for debugging)
   Status DumpManifest(Options& options, std::string& manifestFileName,
-                      bool verbose, bool hex = false, bool json = false);
-
+                      bool verbose, bool hex = false, bool json = false,
+                      const std::vector<ColumnFamilyDescriptor>& cf_descs = {});
 
   const std::string& DbSessionId() const { return db_session_id_; }
 

db/version_set_test.cc

@@ -3320,7 +3320,7 @@ class VersionSetTestMissingFiles : public VersionSetTestBase,
     ++last_seqno_;
     assert(log_writer_.get() != nullptr);
     std::string record;
-    ASSERT_TRUE(edit.EncodeTo(&record));
+    ASSERT_TRUE(edit.EncodeTo(&record, 0 /* ts_sz */));
     Status s = log_writer_->AddRecord(record);
     ASSERT_OK(s);
   }

env/env_encryption.cc

@@ -30,14 +30,6 @@ std::shared_ptr<EncryptionProvider> EncryptionProvider::NewCTRProvider(
   return std::make_shared<CTREncryptionProvider>(cipher);
 }
 
-// Read up to "n" bytes from the file.  "scratch[0..n-1]" may be
-// written by this routine.  Sets "*result" to the data that was
-// read (including if fewer than "n" bytes were successfully read).
-// May set "*result" to point at data in "scratch[0..n-1]", so
-// "scratch[0..n-1]" must be live when "*result" is used.
-// If an error was encountered, returns a non-OK status.
-//
-// REQUIRES: External synchronization
 IOStatus EncryptedSequentialFile::Read(size_t n, const IOOptions& options,
                                        Slice* result, char* scratch,
                                        IODebugContext* dbg) {
@@ -52,19 +44,12 @@ IOStatus EncryptedSequentialFile::Read(size_t n, const IOOptions& options,
         stream_->Decrypt(offset_, (char*)result->data(), result->size()));
   }
   if (io_s.ok()) {
-    offset_ += result->size();  // We've already ready data from disk, so update
+    offset_ += result->size();  // We've already read data from disk, so update
                                 // offset_ even if decryption fails.
   }
   return io_s;
 }
 
-// Skip "n" bytes from the file. This is guaranteed to be no
-// slower that reading the same data, but may be faster.
-//
-// If end of file is reached, skipping will stop at the end of the
-// file, and Skip will return OK.
-//
-// REQUIRES: External synchronization
 IOStatus EncryptedSequentialFile::Skip(uint64_t n) {
   auto status = file_->Skip(n);
   if (!status.ok()) {
@@ -74,28 +59,19 @@ IOStatus EncryptedSequentialFile::Skip(uint64_t n) {
   return status;
 }
 
-// Indicates the upper layers if the current SequentialFile implementation
-// uses direct IO.
 bool EncryptedSequentialFile::use_direct_io() const {
   return file_->use_direct_io();
 }
 
-// Use the returned alignment value to allocate
-// aligned buffer for Direct I/O
 size_t EncryptedSequentialFile::GetRequiredBufferAlignment() const {
   return file_->GetRequiredBufferAlignment();
 }
 
-// Remove any kind of caching of data from the offset to offset+length
-// of this file. If the length is 0, then it refers to the end of file.
-// If the system is not caching the file contents, then this is a noop.
 IOStatus EncryptedSequentialFile::InvalidateCache(size_t offset,
                                                   size_t length) {
   return file_->InvalidateCache(offset + prefixLength_, length);
 }
 
-// Positioned Read for direct I/O
-// If Direct I/O enabled, offset, n, and scratch should be properly aligned
 IOStatus EncryptedSequentialFile::PositionedRead(uint64_t offset, size_t n,
                                                  const IOOptions& options,
                                                  Slice* result, char* scratch,
@@ -115,16 +91,6 @@ IOStatus EncryptedSequentialFile::PositionedRead(uint64_t offset, size_t n,
   return io_s;
 }
 
-// Read up to "n" bytes from the file starting at "offset".
-// "scratch[0..n-1]" may be written by this routine.  Sets "*result"
-// to the data that was read (including if fewer than "n" bytes were
-// successfully read).  May set "*result" to point at data in
-// "scratch[0..n-1]", so "scratch[0..n-1]" must be live when
-// "*result" is used.  If an error was encountered, returns a non-OK
-// status.
-//
-// Safe for concurrent use by multiple threads.
-// If Direct I/O enabled, offset, n, and scratch should be aligned properly.
 IOStatus EncryptedRandomAccessFile::Read(uint64_t offset, size_t n,
                                          const IOOptions& options,
                                          Slice* result, char* scratch,
@@ -143,29 +109,12 @@ IOStatus EncryptedRandomAccessFile::Read(uint64_t offset, size_t n,
   return io_s;
 }
 
-// Readahead the file starting from offset by n bytes for caching.
 IOStatus EncryptedRandomAccessFile::Prefetch(uint64_t offset, size_t n,
                                              const IOOptions& options,
                                              IODebugContext* dbg) {
-  // return Status::OK();
   return file_->Prefetch(offset + prefixLength_, n, options, dbg);
 }
 
-// Tries to get an unique ID for this file that will be the same each time
-// the file is opened (and will stay the same while the file is open).
-// Furthermore, it tries to make this ID at most "max_size" bytes. If such an
-// ID can be created this function returns the length of the ID and places it
-// in "id"; otherwise, this function returns 0, in which case "id"
-// may not have been modified.
-//
-// This function guarantees, for IDs from a given environment, two unique ids
-// cannot be made equal to each other by adding arbitrary bytes to one of
-// them. That is, no unique ID is the prefix of another.
-//
-// This function guarantees that the returned ID will not be interpretable as
-// a single varint.
-//
-// Note: these IDs are only valid for the duration of the process.
 size_t EncryptedRandomAccessFile::GetUniqueId(char* id, size_t max_size) const {
   return file_->GetUniqueId(id, max_size);
 };
@@ -174,29 +123,19 @@ void EncryptedRandomAccessFile::Hint(AccessPattern pattern) {
   file_->Hint(pattern);
 }
 
-// Indicates the upper layers if the current RandomAccessFile implementation
-// uses direct IO.
 bool EncryptedRandomAccessFile::use_direct_io() const {
   return file_->use_direct_io();
 }
 
-// Use the returned alignment value to allocate
-// aligned buffer for Direct I/O
 size_t EncryptedRandomAccessFile::GetRequiredBufferAlignment() const {
   return file_->GetRequiredBufferAlignment();
 }
 
-// Remove any kind of caching of data from the offset to offset+length
-// of this file. If the length is 0, then it refers to the end of file.
-// If the system is not caching the file contents, then this is a noop.
 IOStatus EncryptedRandomAccessFile::InvalidateCache(size_t offset,
                                                     size_t length) {
   return file_->InvalidateCache(offset + prefixLength_, length);
 }
 
-// A file abstraction for sequential writing.  The implementation
-// must provide buffering since callers may append small fragments
-// at a time to the file.
 IOStatus EncryptedWritableFile::Append(const Slice& data,
                                        const IOOptions& options,
                                        IODebugContext* dbg) {
@@ -252,67 +191,39 @@ IOStatus EncryptedWritableFile::PositionedAppend(const Slice& data,
   return file_->PositionedAppend(dataToAppend, offset, options, dbg);
 }
 
-// Indicates the upper layers if the current WritableFile implementation
-// uses direct IO.
 bool EncryptedWritableFile::use_direct_io() const {
   return file_->use_direct_io();
 }
 
-// true if Sync() and Fsync() are safe to call concurrently with Append()
-// and Flush().
 bool EncryptedWritableFile::IsSyncThreadSafe() const {
   return file_->IsSyncThreadSafe();
 }
 
-// Use the returned alignment value to allocate
-// aligned buffer for Direct I/O
 size_t EncryptedWritableFile::GetRequiredBufferAlignment() const {
   return file_->GetRequiredBufferAlignment();
 }
 
-/*
- * Get the size of valid data in the file.
- */
 uint64_t EncryptedWritableFile::GetFileSize(const IOOptions& options,
                                             IODebugContext* dbg) {
   return file_->GetFileSize(options, dbg) - prefixLength_;
 }
 
-// Truncate is necessary to trim the file to the correct size
-// before closing. It is not always possible to keep track of the file
-// size due to whole pages writes. The behavior is undefined if called
-// with other writes to follow.
 IOStatus EncryptedWritableFile::Truncate(uint64_t size,
                                          const IOOptions& options,
                                          IODebugContext* dbg) {
   return file_->Truncate(size + prefixLength_, options, dbg);
 }
 
-// Remove any kind of caching of data from the offset to offset+length
-// of this file. If the length is 0, then it refers to the end of file.
-// If the system is not caching the file contents, then this is a noop.
-// This call has no effect on dirty pages in the cache.
 IOStatus EncryptedWritableFile::InvalidateCache(size_t offset, size_t length) {
   return file_->InvalidateCache(offset + prefixLength_, length);
 }
 
-// Sync a file range with disk.
-// offset is the starting byte of the file range to be synchronized.
-// nbytes specifies the length of the range to be synchronized.
-// This asks the OS to initiate flushing the cached data to disk,
-// without waiting for completion.
-// Default implementation does nothing.
 IOStatus EncryptedWritableFile::RangeSync(uint64_t offset, uint64_t nbytes,
                                           const IOOptions& options,
                                           IODebugContext* dbg) {
   return file_->RangeSync(offset + prefixLength_, nbytes, options, dbg);
 }
 
-// PrepareWrite performs any necessary preparation for a write
-// before the write actually occurs.  This allows for pre-allocation
-// of space on devices where it can result in less file
-// fragmentation and/or less waste from over-zealous filesystem
-// pre-allocation.
 void EncryptedWritableFile::PrepareWrite(size_t offset, size_t len,
                                          const IOOptions& options,
                                          IODebugContext* dbg) {
@@ -330,7 +241,6 @@ void EncryptedWritableFile::GetPreallocationStatus(
   file_->GetPreallocationStatus(block_size, last_allocated_block);
 }
 
-// Pre-allocates space for a file.
 IOStatus EncryptedWritableFile::Allocate(uint64_t offset, uint64_t len,
                                          const IOOptions& options,
                                          IODebugContext* dbg) {
@@ -352,22 +262,14 @@ IOStatus EncryptedWritableFile::Close(const IOOptions& options,
   return file_->Close(options, dbg);
 }
 
-// A file abstraction for random reading and writing.
-
-// Indicates if the class makes use of direct I/O
-// If false you must pass aligned buffer to Write()
 bool EncryptedRandomRWFile::use_direct_io() const {
   return file_->use_direct_io();
 }
 
-// Use the returned alignment value to allocate
-// aligned buffer for Direct I/O
 size_t EncryptedRandomRWFile::GetRequiredBufferAlignment() const {
   return file_->GetRequiredBufferAlignment();
 }
 
-// Write bytes in `data` at  offset `offset`, Returns Status::OK() on success.
-// Pass aligned buffer when use_direct_io() returns true.
 IOStatus EncryptedRandomRWFile::Write(uint64_t offset, const Slice& data,
                                       const IOOptions& options,
                                       IODebugContext* dbg) {
@@ -394,9 +296,6 @@ IOStatus EncryptedRandomRWFile::Write(uint64_t offset, const Slice& data,
   return file_->Write(offset, dataToWrite, options, dbg);
 }
 
-// Read up to `n` bytes starting from offset `offset` and store them in
-// result, provided `scratch` size should be at least `n`.
-// Returns Status::OK() on success.
 IOStatus EncryptedRandomRWFile::Read(uint64_t offset, size_t n,
                                      const IOOptions& options, Slice* result,
                                      char* scratch, IODebugContext* dbg) const {
@@ -678,7 +577,6 @@ class EncryptedFileSystemImpl : public EncryptedFileSystem {
     return provider_->AddCipher(descriptor, cipher, len, for_write);
   }
 
-  // NewSequentialFile opens a file for sequential reading.
   IOStatus NewSequentialFile(const std::string& fname,
                              const FileOptions& options,
                              std::unique_ptr<FSSequentialFile>* result,
@@ -716,7 +614,6 @@ class EncryptedFileSystemImpl : public EncryptedFileSystem {
     return status;
   }
 
-  // NewRandomAccessFile opens a file for random read access.
   IOStatus NewRandomAccessFile(const std::string& fname,
                                const FileOptions& options,
                                std::unique_ptr<FSRandomAccessFile>* result,
@@ -747,7 +644,6 @@ class EncryptedFileSystemImpl : public EncryptedFileSystem {
     return status;
   }
 
-  // NewWritableFile opens a file for sequential writing.
   IOStatus NewWritableFile(const std::string& fname, const FileOptions& options,
                            std::unique_ptr<FSWritableFile>* result,
                            IODebugContext* dbg) override {
@@ -765,13 +661,6 @@ class EncryptedFileSystemImpl : public EncryptedFileSystem {
     return CreateWritableEncryptedFile(fname, underlying, options, result, dbg);
   }
 
-  // Create an object that writes to a new file with the specified
-  // name.  Deletes any existing file with the same name and creates a
-  // new file.  On success, stores a pointer to the new file in
-  // *result and returns OK.  On failure stores nullptr in *result and
-  // returns non-OK.
-  //
-  // The returned file will only be accessed by one thread at a time.
   IOStatus ReopenWritableFile(const std::string& fname,
                               const FileOptions& options,
                               std::unique_ptr<FSWritableFile>* result,
@@ -790,7 +679,6 @@ class EncryptedFileSystemImpl : public EncryptedFileSystem {
     return CreateWritableEncryptedFile(fname, underlying, options, result, dbg);
   }
 
-  // Reuse an existing file by renaming it and opening it as writable.
   IOStatus ReuseWritableFile(const std::string& fname,
                              const std::string& old_fname,
                              const FileOptions& options,
@@ -810,11 +698,6 @@ class EncryptedFileSystemImpl : public EncryptedFileSystem {
     return CreateWritableEncryptedFile(fname, underlying, options, result, dbg);
   }
 
-  // Open `fname` for random read and write, if file doesn't exist the file
-  // will be created.  On success, stores a pointer to the new file in
-  // *result and returns OK.  On failure returns non-OK.
-  //
-  // The returned file will only be accessed by one thread at a time.
   IOStatus NewRandomRWFile(const std::string& fname, const FileOptions& options,
                            std::unique_ptr<FSRandomRWFile>* result,
                            IODebugContext* dbg) override {
@@ -854,20 +737,6 @@ class EncryptedFileSystemImpl : public EncryptedFileSystem {
     return status;
   }
 
-  // Store in *result the attributes of the children of the specified
-  // directory.
-  // In case the implementation lists the directory prior to iterating the
-  // files
-  // and files are concurrently deleted, the deleted files will be omitted
-  // from
-  // result.
-  // The name attributes are relative to "dir".
-  // Original contents of *results are dropped.
-  // Returns OK if "dir" exists and "*result" contains its children.
-  //         NotFound if "dir" does not exist, the calling process does not
-  //         have
-  //                  permission to access "dir", or if "dir" is invalid.
-  //         IOError if an IO Error was encountered
   IOStatus GetChildrenFileAttributes(const std::string& dir,
                                      const IOOptions& options,
                                      std::vector<FileAttributes>* result,
@@ -894,7 +763,6 @@ class EncryptedFileSystemImpl : public EncryptedFileSystem {
     return IOStatus::OK();
   }
 
-  // Store the size of fname in *file_size.
   IOStatus GetFileSize(const std::string& fname, const IOOptions& options,
                        uint64_t* file_size, IODebugContext* dbg) override {
     auto status =
@@ -940,16 +808,13 @@ std::shared_ptr<FileSystem> NewEncryptedFS(
     return nullptr;
   }
 }
-// Returns an Env that encrypts data when stored on disk and decrypts data when
-// read from disk.
+
 Env* NewEncryptedEnv(Env* base_env,
                      const std::shared_ptr<EncryptionProvider>& provider) {
   return new CompositeEnvWrapper(
       base_env, NewEncryptedFS(base_env->GetFileSystem(), provider));
 }
 
-// Encrypt one or more (partial) blocks of data at the file offset.
-// Length of data is given in dataSize.
 Status BlockAccessCipherStream::Encrypt(uint64_t fileOffset, char* data,
                                         size_t dataSize) {
   // Calculate block index
@@ -968,7 +833,7 @@ Status BlockAccessCipherStream::Encrypt(uint64_t fileOffset, char* data,
     if (n != blockSize) {
       // We're not encrypting a full block.
       // Copy data to blockBuffer
-      if (!blockBuffer.get()) {
+      if (!blockBuffer) {
         // Allocate buffer
         blockBuffer = std::unique_ptr<char[]>(new char[blockSize]);
       }
@@ -994,8 +859,6 @@ Status BlockAccessCipherStream::Encrypt(uint64_t fileOffset, char* data,
   }
 }
 
-// Decrypt one or more (partial) blocks of data at the file offset.
-// Length of data is given in dataSize.
 Status BlockAccessCipherStream::Decrypt(uint64_t fileOffset, char* data,
                                         size_t dataSize) {
   // Calculate block index
@@ -1014,7 +877,7 @@ Status BlockAccessCipherStream::Decrypt(uint64_t fileOffset, char* data,
     if (n != blockSize) {
       // We're not decrypting a full block.
       // Copy data to blockBuffer
-      if (!blockBuffer.get()) {
+      if (!blockBuffer) {
         // Allocate buffer
         blockBuffer = std::unique_ptr<char[]>(new char[blockSize]);
       }
@@ -1055,6 +918,7 @@ static std::unordered_map<std::string, OptionTypeInfo>
          {0 /* No offset, whole struct*/, OptionType::kInt,
           OptionVerificationType::kNormal, OptionTypeFlags::kNone}},
 };
+
 // Implements a BlockCipher using ROT13.
 //
 // Note: This is a sample implementation of BlockCipher,
@@ -1071,22 +935,17 @@ class ROT13BlockCipher : public BlockCipher {
 
   static const char* kClassName() { return "ROT13"; }
   const char* Name() const override { return kClassName(); }
-  // BlockSize returns the size of each block supported by this cipher stream.
-  size_t BlockSize() override { return blockSize_; }
 
-  // Encrypt a block of data.
-  // Length of data is equal to BlockSize().
+  size_t BlockSize() override { return blockSize_; }
   Status Encrypt(char* data) override {
     for (size_t i = 0; i < blockSize_; ++i) {
       data[i] += 13;
     }
     return Status::OK();
   }
-
-  // Decrypt a block of data.
-  // Length of data is equal to BlockSize().
   Status Decrypt(char* data) override { return Encrypt(data); }
 };
+
 static const std::unordered_map<std::string, OptionTypeInfo>
     ctr_encryption_provider_type_info = {
         {"cipher",
@@ -1096,14 +955,11 @@ static const std::unordered_map<std::string, OptionTypeInfo>
 };
 }  // anonymous namespace
 
-// Allocate scratch space which is passed to EncryptBlock/DecryptBlock.
 void CTRCipherStream::AllocateScratch(std::string& scratch) {
   auto blockSize = cipher_->BlockSize();
   scratch.reserve(blockSize);
 }
 
-// Encrypt a block of data at the given block index.
-// Length of data is equal to BlockSize();
 Status CTRCipherStream::EncryptBlock(uint64_t blockIndex, char* data,
                                      char* scratch) {
   // Create nonce + counter
@@ -1111,7 +967,7 @@ Status CTRCipherStream::EncryptBlock(uint64_t blockIndex, char* data,
   memmove(scratch, iv_.data(), blockSize);
   EncodeFixed64(scratch, blockIndex + initialCounter_);
 
-  // Encrypt nonce+counter
+  // Encrypt nonce + counter
   auto status = cipher_->Encrypt(scratch);
   if (!status.ok()) {
     return status;
@@ -1124,8 +980,6 @@ Status CTRCipherStream::EncryptBlock(uint64_t blockIndex, char* data,
   return Status::OK();
 }
 
-// Decrypt a block of data at the given block index.
-// Length of data is equal to BlockSize();
 Status CTRCipherStream::DecryptBlock(uint64_t blockIndex, char* data,
                                      char* scratch) {
   // For CTR decryption & encryption are the same
@@ -1147,10 +1001,6 @@ bool CTREncryptionProvider::IsInstanceOf(const std::string& name) const {
   }
 }
 
-// GetPrefixLength returns the length of the prefix that is added to every file
-// and used for storing encryption options.
-// For optimal performance, the prefix length should be a multiple of
-// the page size.
 size_t CTREncryptionProvider::GetPrefixLength() const {
   return defaultPrefixLength;
 }
@@ -1179,8 +1029,6 @@ static void decodeCTRParameters(const char* prefix, size_t blockSize,
   iv = Slice(prefix + blockSize, blockSize);
 }
 
-// CreateNewPrefix initialized an allocated block of prefix memory
-// for a new file.
 Status CTREncryptionProvider::CreateNewPrefix(const std::string& /*fname*/,
                                               char* prefix,
                                               size_t prefixLength) const {
@@ -1212,10 +1060,8 @@ Status CTREncryptionProvider::CreateNewPrefix(const std::string& /*fname*/,
     status = cipherStream.Encrypt(0, prefix + (2 * blockSize),
                                   prefixLength - (2 * blockSize));
   }
-  if (!status.ok()) {
-    return status;
-  }
-  return Status::OK();
+
+  return status;
 }
 
 // PopulateSecretPrefixPart initializes the data into a new prefix block
@@ -1267,7 +1113,7 @@ Status CTREncryptionProvider::CreateCipherStream(
 }
 
 // CreateCipherStreamFromPrefix creates a block access cipher stream for a file
-// given given name and options. The given prefix is already decrypted.
+// given name and options. The given prefix is already decrypted.
 Status CTREncryptionProvider::CreateCipherStreamFromPrefix(
     const std::string& /*fname*/, const EnvOptions& /*options*/,
     uint64_t initialCounter, const Slice& iv, const Slice& /*prefix*/,

env/env_encryption_ctr.h

@@ -27,19 +27,13 @@ class CTRCipherStream final : public BlockAccessCipherStream {
       : cipher_(c), iv_(iv, c->BlockSize()), initialCounter_(initialCounter){};
   virtual ~CTRCipherStream(){};
 
-  // BlockSize returns the size of each block supported by this cipher stream.
   size_t BlockSize() override { return cipher_->BlockSize(); }
 
  protected:
-  // Allocate scratch space which is passed to EncryptBlock/DecryptBlock.
   void AllocateScratch(std::string&) override;
 
-  // Encrypt a block of data at the given block index.
-  // Length of data is equal to BlockSize();
   Status EncryptBlock(uint64_t blockIndex, char* data, char* scratch) override;
 
-  // Decrypt a block of data at the given block index.
-  // Length of data is equal to BlockSize();
   Status DecryptBlock(uint64_t blockIndex, char* data, char* scratch) override;
 };
 
@@ -66,20 +60,9 @@ class CTREncryptionProvider : public EncryptionProvider {
   static const char* kClassName() { return "CTR"; }
   const char* Name() const override { return kClassName(); }
   bool IsInstanceOf(const std::string& name) const override;
-  // GetPrefixLength returns the length of the prefix that is added to every
-  // file
-  // and used for storing encryption options.
-  // For optimal performance when using direct IO, the prefix length should be a
-  // multiple of the page size.
   size_t GetPrefixLength() const override;
-
-  // CreateNewPrefix initialized an allocated block of prefix memory
-  // for a new file.
   Status CreateNewPrefix(const std::string& fname, char* prefix,
                          size_t prefixLength) const override;
-
-  // CreateCipherStream creates a block access cipher stream for a file given
-  // given name and options.
   Status CreateCipherStream(
       const std::string& fname, const EnvOptions& options, Slice& prefix,
       std::unique_ptr<BlockAccessCipherStream>* result) override;

env/unique_id_gen.cc

@@ -21,6 +21,7 @@
 #ifdef __SSE4_2__
 #ifdef _WIN32
 #include <intrin.h>
+#define _rdtsc() __rdtsc()
 #else
 #include <x86intrin.h>
 #endif

file/prefetch_test.cc

@@ -128,7 +128,14 @@ std::string BuildKey(int num, std::string postfix = "") {
   return "my_key_" + std::to_string(num) + postfix;
 }
 
-// This test verifies the basic functionality of prefetching.
+// This test verifies the following basic functionalities of prefetching:
+// (1) If underline file system supports prefetch, and directIO is not enabled
+// make sure prefetch() is called and FilePrefetchBuffer is not used.
+// (2) If underline file system doesn't support prefetch, or directIO is
+// enabled, make sure prefetch() is not called and FilePrefetchBuffer is
+// used.
+// (3) Measure read bytes, hit and miss of SST's tail prefetching during table
+// open.
 TEST_P(PrefetchTest, Basic) {
   // First param is if the mockFS support_prefetch or not
   bool support_prefetch =
@@ -144,6 +151,7 @@ TEST_P(PrefetchTest, Basic) {
   Options options;
   SetGenericOptions(env.get(), use_direct_io, options);
   options.statistics = CreateDBStatistics();
+  options.compaction_readahead_size = 2 * 1024 * 1024;
 
   const int kNumKeys = 1100;
   int buff_prefetch_count = 0;
@@ -165,6 +173,7 @@ TEST_P(PrefetchTest, Basic) {
     ASSERT_OK(batch.Put(BuildKey(i), "value for range 1 key"));
   }
   ASSERT_OK(db_->Write(WriteOptions(), &batch));
+  ASSERT_OK(dbfull()->TEST_WaitForFlushMemTable());
 
   // create second key range
   batch.Clear();
@@ -172,6 +181,7 @@ TEST_P(PrefetchTest, Basic) {
     ASSERT_OK(batch.Put(BuildKey(i, "key2"), "value for range 2 key"));
   }
   ASSERT_OK(db_->Write(WriteOptions(), &batch));
+  ASSERT_OK(dbfull()->TEST_WaitForFlushMemTable());
 
   // delete second key range
   batch.Clear();
@@ -179,6 +189,20 @@ TEST_P(PrefetchTest, Basic) {
     ASSERT_OK(batch.Delete(BuildKey(i, "key2")));
   }
   ASSERT_OK(db_->Write(WriteOptions(), &batch));
+  ASSERT_OK(db_->Flush(FlushOptions()));
+
+  // To verify SST file tail prefetch (once per file) during flush output
+  // verification
+  if (support_prefetch && !use_direct_io) {
+    ASSERT_TRUE(fs->IsPrefetchCalled());
+    ASSERT_EQ(3, fs->GetPrefetchCount());
+    ASSERT_EQ(0, buff_prefetch_count);
+    fs->ClearPrefetchCount();
+  } else {
+    ASSERT_FALSE(fs->IsPrefetchCalled());
+    ASSERT_EQ(buff_prefetch_count, 3);
+    buff_prefetch_count = 0;
+  }
 
   // compact database
   std::string start_key = BuildKey(0);
@@ -205,25 +229,27 @@ TEST_P(PrefetchTest, Basic) {
   const uint64_t cur_table_open_prefetch_tail_hit =
       options.statistics->getTickerCount(TABLE_OPEN_PREFETCH_TAIL_HIT);
 
+  // To verify prefetch during compaction input read
   if (support_prefetch && !use_direct_io) {
-    // If underline file system supports prefetch, and directIO is not enabled
-    // make sure prefetch() is called and FilePrefetchBuffer is not used.
     ASSERT_TRUE(fs->IsPrefetchCalled());
-    fs->ClearPrefetchCount();
+    // To rule out false positive by the SST file tail prefetch during
+    // compaction output verification
+    ASSERT_GT(fs->GetPrefetchCount(), 1);
     ASSERT_EQ(0, buff_prefetch_count);
+    fs->ClearPrefetchCount();
   } else {
-    // If underline file system doesn't support prefetch, or directIO is
-    // enabled, make sure prefetch() is not called and FilePrefetchBuffer is
-    // used.
     ASSERT_FALSE(fs->IsPrefetchCalled());
-    ASSERT_GT(buff_prefetch_count, 0);
+    // To rule out false positive by the SST file tail prefetch during
+    // compaction output verification
+    ASSERT_GT(buff_prefetch_count, 1);
+    buff_prefetch_count = 0;
+
     ASSERT_GT(cur_table_open_prefetch_tail_read.count,
               prev_table_open_prefetch_tail_read.count);
     ASSERT_GT(cur_table_open_prefetch_tail_hit,
               prev_table_open_prefetch_tail_hit);
     ASSERT_GE(cur_table_open_prefetch_tail_miss,
               prev_table_open_prefetch_tail_miss);
-    buff_prefetch_count = 0;
   }
 
   // count the keys
@@ -236,7 +262,7 @@ TEST_P(PrefetchTest, Basic) {
     (void)num_keys;
   }
 
-  // Make sure prefetch is called only if file system support prefetch.
+  // To verify prefetch during user scan
   if (support_prefetch && !use_direct_io) {
     ASSERT_TRUE(fs->IsPrefetchCalled());
     fs->ClearPrefetchCount();

file/random_access_file_reader.cc

@@ -97,6 +97,15 @@ IOStatus RandomAccessFileReader::Read(
 
   IOStatus io_s;
   uint64_t elapsed = 0;
+  size_t alignment = file_->GetRequiredBufferAlignment();
+  bool is_aligned = false;
+  if (scratch != nullptr) {
+    // Check if offset, length and buffer are aligned.
+    is_aligned = (offset & (alignment - 1)) == 0 &&
+                 (n & (alignment - 1)) == 0 &&
+                 (uintptr_t(scratch) & (alignment - 1)) == 0;
+  }
+
   {
     StopWatch sw(clock_, stats_, hist_type_,
                  (opts.io_activity != Env::IOActivity::kUnknown)
@@ -106,8 +115,7 @@ IOStatus RandomAccessFileReader::Read(
                  true /*delay_enabled*/);
     auto prev_perf_level = GetPerfLevel();
     IOSTATS_TIMER_GUARD(read_nanos);
-    if (use_direct_io()) {
-      size_t alignment = file_->GetRequiredBufferAlignment();
+    if (use_direct_io() && is_aligned == false) {
       size_t aligned_offset =
           TruncateToPageBoundary(alignment, static_cast<size_t>(offset));
       size_t offset_advance = static_cast<size_t>(offset) - aligned_offset;
@@ -182,9 +190,9 @@ IOStatus RandomAccessFileReader::Read(
           if (rate_limiter_->IsRateLimited(RateLimiter::OpType::kRead)) {
             sw.DelayStart();
           }
-          allowed = rate_limiter_->RequestToken(n - pos, 0 /* alignment */,
-                                                rate_limiter_priority, stats_,
-                                                RateLimiter::OpType::kRead);
+          allowed = rate_limiter_->RequestToken(
+              n - pos, (use_direct_io() ? alignment : 0), rate_limiter_priority,
+              stats_, RateLimiter::OpType::kRead);
           if (rate_limiter_->IsRateLimited(RateLimiter::OpType::kRead)) {
             sw.DelayStop();
           }

include/rocksdb/advanced_options.h

@@ -858,24 +858,23 @@ struct AdvancedColumnFamilyOptions {
   // Dynamically changeable through SetOptions() API
   bool report_bg_io_stats = false;
 
-  // Files containing updates older than TTL will go through the compaction
-  // process. This usually happens in a cascading way so that those entries
-  // will be compacted to bottommost level/file.
-  // The feature is used to remove stale entries that have been deleted or
-  // updated from the file system.
-  // Pre-req: This needs max_open_files to be set to -1.
-  // In Level: Non-bottom-level files older than TTL will go through the
-  //           compaction process.
-  // In FIFO: Files older than TTL will be deleted.
+  // This option has different meanings for different compaction styles:
+  //
+  // Leveled: Non-bottom-level files with all keys older than TTL will go
+  //    through the compaction process. This usually happens in a cascading
+  //    way so that those entries will be compacted to bottommost level/file.
+  //    The feature is used to remove stale entries that have been deleted or
+  //    updated from the file system.
+  //
+  // FIFO: Files with all keys older than TTL will be deleted. TTL is only
+  //    supported if option max_open_files is set to -1.
+  //
   // unit: seconds. Ex: 1 day = 1 * 24 * 60 * 60
-  // In FIFO, this option will have the same meaning as
-  // periodic_compaction_seconds. Whichever stricter will be used.
   // 0 means disabling.
   // UINT64_MAX - 1 (0xfffffffffffffffe) is special flag to allow RocksDB to
   // pick default.
   //
-  // Default: 30 days for leveled compaction + block based table. disable
-  //          otherwise.
+  // Default: 30 days if using block based table. 0 (disable) otherwise.
   //
   // Dynamically changeable through SetOptions() API
   uint64_t ttl = 0xfffffffffffffffe;
@@ -891,12 +890,9 @@ struct AdvancedColumnFamilyOptions {
   // age is based on the file's last modified time (given by the underlying
   // Env).
   //
-  // Supported in all compaction styles.
+  // Supported in leveled and universal compaction.
   // In Universal compaction, rocksdb will try to do a full compaction when
   // possible, see more in UniversalCompactionBuilder::PickPeriodicCompaction().
-  // In FIFO compaction, this option has the same meaning as TTL and whichever
-  // stricter will be used.
-  // Pre-req: max_open_file == -1.
   // unit: seconds. Ex: 7 days = 7 * 24 * 60 * 60
   //
   // Values:
@@ -905,9 +901,6 @@ struct AdvancedColumnFamilyOptions {
   //     as needed. For now, RocksDB will change this value to 30 days
   //     (i.e 30 * 24 * 60 * 60) so that every file goes through the compaction
   //     process at least once every 30 days if not compacted sooner.
-  //     In FIFO compaction, since the option has the same meaning as ttl,
-  //     when this value is left default, and ttl is left to 0, 30 days will be
-  //     used. Otherwise, min(ttl, periodic_compaction_seconds) will be used.
   //
   // Default: UINT64_MAX - 1 (allow RocksDB to auto-tune)
   //
@@ -1120,11 +1113,30 @@ struct AdvancedColumnFamilyOptions {
   //
   // When it's false, the user-defined timestamps will be removed from the user
   // keys when data is flushed from memtables to SST files. Other places that
-  // user keys can be persisted like WAL and blob files go through a similar
-  // process. Users should call `DB::IncreaseFullHistoryTsLow` to set a cutoff
-  // timestamp. RocksDB refrains from flushing a memtable with data still above
-  // the cutoff timestamp with best effort. When users try to read below the
-  // cutoff timestamp, an error will be returned.
+  // user keys can be persisted like file boundaries in file metadata and blob
+  // files go through a similar process. There are two major motivations
+  // for this flag:
+  // 1) backward compatibility: if the user later decides to
+  // disable the user-defined timestamp feature for the column family, these SST
+  // files can be handled by a user comparator that is not aware of user-defined
+  // timestamps.
+  // 2) enable user-defined timestamp feature for an existing column family
+  // while set this flag to be `false`: user keys in the newly generated SST
+  // files are of the same format as the existing SST files.
+  //
+  // When setting this flag to `false`, users should also call
+  // `DB::IncreaseFullHistoryTsLow` to set a cutoff timestamp for flush. RocksDB
+  // refrains from flushing a memtable with data still above
+  // the cutoff timestamp with best effort. Users can do user-defined
+  // multi-versioned read above the cutoff timestamp. When users try to read
+  // below the cutoff timestamp, an error will be returned.
+  //
+  // Note that if WAL is enabled, unlike SST files, user-defined timestamps are
+  // persisted to WAL even if this flag is set to `false`. The benefit of this
+  // is that user-defined timestamps can be recovered with the caveat that users
+  // should flush all memtables so there is no active WAL files before doing a
+  // downgrade or toggling on / off the user-defined timestamp feature on a
+  // column family.
   //
   // Default: true (user-defined timestamps are persisted)
   // Not dynamically changeable, change it requires db restart and

include/rocksdb/c.h

@@ -411,6 +411,16 @@ rocksdb_create_column_family(rocksdb_t* db,
                              const rocksdb_options_t* column_family_options,
                              const char* column_family_name, char** errptr);
 
+extern ROCKSDB_LIBRARY_API rocksdb_column_family_handle_t**
+rocksdb_create_column_families(rocksdb_t* db,
+                               const rocksdb_options_t* column_family_options,
+                               int num_column_families,
+                               const char* const* column_family_names,
+                               size_t* lencfs, char** errptr);
+
+extern ROCKSDB_LIBRARY_API void rocksdb_create_column_families_destroy(
+    rocksdb_column_family_handle_t** list);
+
 extern ROCKSDB_LIBRARY_API rocksdb_column_family_handle_t*
 rocksdb_create_column_family_with_ttl(
     rocksdb_t* db, const rocksdb_options_t* column_family_options,
@@ -1518,7 +1528,8 @@ extern ROCKSDB_LIBRARY_API void rocksdb_options_set_hash_skip_list_rep(
 extern ROCKSDB_LIBRARY_API void rocksdb_options_set_hash_link_list_rep(
     rocksdb_options_t*, size_t);
 extern ROCKSDB_LIBRARY_API void rocksdb_options_set_plain_table_factory(
-    rocksdb_options_t*, uint32_t, int, double, size_t);
+    rocksdb_options_t*, uint32_t, int, double, size_t, size_t, char,
+    unsigned char, unsigned char);
 
 extern ROCKSDB_LIBRARY_API void rocksdb_options_set_min_level_to_compress(
     rocksdb_options_t* opt, int level);
@@ -1615,6 +1626,9 @@ extern ROCKSDB_LIBRARY_API void rocksdb_options_set_row_cache(
 
 extern ROCKSDB_LIBRARY_API void
 rocksdb_options_add_compact_on_deletion_collector_factory(
+    rocksdb_options_t*, size_t window_size, size_t num_dels_trigger);
+extern ROCKSDB_LIBRARY_API void
+rocksdb_options_add_compact_on_deletion_collector_factory_del_ratio(
     rocksdb_options_t*, size_t window_size, size_t num_dels_trigger,
     double deletion_ratio);
 extern ROCKSDB_LIBRARY_API void rocksdb_options_set_manual_wal_flush(
@@ -2443,6 +2457,12 @@ extern ROCKSDB_LIBRARY_API char* rocksdb_transactiondb_property_value(
 extern ROCKSDB_LIBRARY_API int rocksdb_transactiondb_property_int(
     rocksdb_transactiondb_t* db, const char* propname, uint64_t* out_val);
 
+extern ROCKSDB_LIBRARY_API rocksdb_t* rocksdb_transactiondb_get_base_db(
+    rocksdb_transactiondb_t* txn_db);
+
+extern ROCKSDB_LIBRARY_API void rocksdb_transactiondb_close_base_db(
+    rocksdb_t* base_db);
+
 extern ROCKSDB_LIBRARY_API rocksdb_transaction_t* rocksdb_transaction_begin(
     rocksdb_transactiondb_t* txn_db,
     const rocksdb_writeoptions_t* write_options,

include/rocksdb/cache.h

@@ -32,18 +32,18 @@ class SecondaryCache;
 // example, HyperClockCache is not usable as a general cache because it expects
 // only fixed-size block cache keys, but this limitation is not yet reflected
 // in the API function signatures.
-// * Phase 1 (done) - Make both BlockCache and GeneralCache aliases for Cache,
-// and make a factory function for general caches. Encourage users of row_cache
-// (not common) to switch to the factory function for general caches.
-// * Phase 2 - Split off GenericCache as its own class, removing secondary
+// * Phase 1 (done) - Make both BlockCache and RowCache aliases for Cache,
+// and make a factory function for row caches. Encourage users of row_cache
+// (not common) to switch to the factory function for row caches.
+// * Phase 2 - Split off RowCache as its own class, removing secondary
 // cache support features and more from the API to simplify it. Between Phase 1
 // and Phase 2 users of row_cache will need to update their code. Any time
-// after Phase 2, the block cache API can become more specialized in ways
-// incompatible with general caches.
+// after Phase 2, the block cache and row cache APIs can become more specialized
+// in ways incompatible with general caches.
 // * Phase 3 - Move existing RocksDB uses of Cache to BlockCache, and deprecate
 // (but not yet remove) Cache as an alias for BlockCache.
 using BlockCache = Cache;
-using GeneralCache = Cache;
+using RowCache = Cache;
 
 // Classifications of block cache entries.
 //
@@ -155,7 +155,7 @@ struct ShardedCacheOptions {
   CacheMetadataChargePolicy metadata_charge_policy =
       kDefaultCacheMetadataChargePolicy;
 
-  // A SecondaryCache instance to use the non-volatile tier. For a GeneralCache
+  // A SecondaryCache instance to use the non-volatile tier. For a RowCache
   // this option must be kept as default empty.
   std::shared_ptr<SecondaryCache> secondary_cache;
 
@@ -260,9 +260,9 @@ struct LRUCacheOptions : public ShardedCacheOptions {
   // Construct an instance of LRUCache using these options
   std::shared_ptr<Cache> MakeSharedCache() const;
 
-  // Construct an instance of LRUCache for use as a general cache (e.g. for
-  // row_cache). Some options are not relevant to general caches.
-  std::shared_ptr<GeneralCache> MakeSharedGeneralCache() const;
+  // Construct an instance of LRUCache for use as a row cache, typically for
+  // `DBOptions::row_cache`. Some options are not relevant to row caches.
+  std::shared_ptr<RowCache> MakeSharedRowCache() const;
 };
 
 // DEPRECATED wrapper function

include/rocksdb/env_encryption.h

@@ -67,7 +67,7 @@ class BlockAccessCipherStream {
 // including data loss, unreported corruption, deadlocks, and more.
 class BlockCipher : public Customizable {
  public:
-  virtual ~BlockCipher(){};
+  virtual ~BlockCipher() {}
 
   // Creates a new BlockCipher from the input config_options and value
   // The value describes the type of provider (and potentially optional
@@ -114,13 +114,13 @@ class BlockCipher : public Customizable {
 // including data loss, unreported corruption, deadlocks, and more.
 class EncryptionProvider : public Customizable {
  public:
-  virtual ~EncryptionProvider(){};
+  virtual ~EncryptionProvider() {}
 
-  // Creates a new EncryptionProvider from the input config_options and value
+  // Creates a new EncryptionProvider from the input config_options and value.
   // The value describes the type of provider (and potentially optional
   // configuration parameters) used to create this provider.
   // For example, if the value is "CTR", a CTREncryptionProvider will be
-  // created. If the value is ends with "://test" (e.g CTR://test"), the
+  // created. If the value is end with "://test" (e.g CTR://test"), the
   // provider will be initialized in "TEST" mode prior to being returned.
   //
   // @param config_options  Options to control how this provider is created
@@ -153,7 +153,7 @@ class EncryptionProvider : public Customizable {
                                  size_t prefixLength) const = 0;
 
   // Method to add a new cipher key for use by the EncryptionProvider.
-  // @param description  Descriptor for this key.
+  // @param descriptor   Descriptor for this key
   // @param cipher       The cryptographic key to use
   // @param len          The length of the cipher key
   // @param for_write If true, this cipher should be used for writing files.
@@ -165,15 +165,15 @@ class EncryptionProvider : public Customizable {
                            size_t len, bool for_write) = 0;
 
   // CreateCipherStream creates a block access cipher stream for a file given
-  // given name and options.
+  // name and options.
   virtual Status CreateCipherStream(
       const std::string& fname, const EnvOptions& options, Slice& prefix,
       std::unique_ptr<BlockAccessCipherStream>* result) = 0;
 
   // Returns a string representing an encryption marker prefix for this
   // provider. If a marker is provided, this marker can be used to tell whether
-  // or not a file is encrypted by this provider.  The maker will also be part
-  // of any encryption prefix for this provider.
+  // a file is encrypted by this provider.  The marker will also be part of any
+  // encryption prefix for this provider.
   virtual std::string GetMarker() const { return ""; }
 };
 
@@ -182,7 +182,7 @@ class EncryptedSequentialFile : public FSSequentialFile {
   std::unique_ptr<FSSequentialFile> file_;
   std::unique_ptr<BlockAccessCipherStream> stream_;
   uint64_t offset_;
-  size_t prefixLength_;
+  const size_t prefixLength_;
 
  public:
   // Default ctor. Given underlying sequential file is supposed to be at
@@ -195,47 +195,22 @@ class EncryptedSequentialFile : public FSSequentialFile {
         offset_(prefixLength),
         prefixLength_(prefixLength) {}
 
-  // Read up to "n" bytes from the file.  "scratch[0..n-1]" may be
-  // written by this routine.  Sets "*result" to the data that was
-  // read (including if fewer than "n" bytes were successfully read).
-  // May set "*result" to point at data in "scratch[0..n-1]", so
-  // "scratch[0..n-1]" must be live when "*result" is used.
-  // If an error was encountered, returns a non-OK status.
-  //
-  // REQUIRES: External synchronization
   IOStatus Read(size_t n, const IOOptions& options, Slice* result,
                 char* scratch, IODebugContext* dbg) override;
 
-  // Skip "n" bytes from the file. This is guaranteed to be no
-  // slower that reading the same data, but may be faster.
-  //
-  // If end of file is reached, skipping will stop at the end of the
-  // file, and Skip will return OK.
-  //
-  // REQUIRES: External synchronization
   IOStatus Skip(uint64_t n) override;
 
-  // Indicates the upper layers if the current SequentialFile implementation
-  // uses direct IO.
   bool use_direct_io() const override;
 
-  // Use the returned alignment value to allocate
-  // aligned buffer for Direct I/O
   size_t GetRequiredBufferAlignment() const override;
 
-  // Remove any kind of caching of data from the offset to offset+length
-  // of this file. If the length is 0, then it refers to the end of file.
-  // If the system is not caching the file contents, then this is a noop.
   IOStatus InvalidateCache(size_t offset, size_t length) override;
 
-  // Positioned Read for direct I/O
-  // If Direct I/O enabled, offset, n, and scratch should be properly aligned
   IOStatus PositionedRead(uint64_t offset, size_t n, const IOOptions& options,
                           Slice* result, char* scratch,
                           IODebugContext* dbg) override;
 };
 
-// A file abstraction for randomly reading the contents of a file.
 class EncryptedRandomAccessFile : public FSRandomAccessFile {
  protected:
   std::unique_ptr<FSRandomAccessFile> file_;
@@ -250,60 +225,24 @@ class EncryptedRandomAccessFile : public FSRandomAccessFile {
         stream_(std::move(s)),
         prefixLength_(prefixLength) {}
 
-  // Read up to "n" bytes from the file starting at "offset".
-  // "scratch[0..n-1]" may be written by this routine.  Sets "*result"
-  // to the data that was read (including if fewer than "n" bytes were
-  // successfully read).  May set "*result" to point at data in
-  // "scratch[0..n-1]", so "scratch[0..n-1]" must be live when
-  // "*result" is used.  If an error was encountered, returns a non-OK
-  // status.
-  //
-  // Safe for concurrent use by multiple threads.
-  // If Direct I/O enabled, offset, n, and scratch should be aligned properly.
   IOStatus Read(uint64_t offset, size_t n, const IOOptions& options,
                 Slice* result, char* scratch,
                 IODebugContext* dbg) const override;
 
-  // Readahead the file starting from offset by n bytes for caching.
   IOStatus Prefetch(uint64_t offset, size_t n, const IOOptions& options,
                     IODebugContext* dbg) override;
 
-  // Tries to get an unique ID for this file that will be the same each time
-  // the file is opened (and will stay the same while the file is open).
-  // Furthermore, it tries to make this ID at most "max_size" bytes. If such an
-  // ID can be created this function returns the length of the ID and places it
-  // in "id"; otherwise, this function returns 0, in which case "id"
-  // may not have been modified.
-  //
-  // This function guarantees, for IDs from a given environment, two unique ids
-  // cannot be made equal to each other by adding arbitrary bytes to one of
-  // them. That is, no unique ID is the prefix of another.
-  //
-  // This function guarantees that the returned ID will not be interpretable as
-  // a single varint.
-  //
-  // Note: these IDs are only valid for the duration of the process.
   size_t GetUniqueId(char* id, size_t max_size) const override;
 
   void Hint(AccessPattern pattern) override;
 
-  // Indicates the upper layers if the current RandomAccessFile implementation
-  // uses direct IO.
   bool use_direct_io() const override;
 
-  // Use the returned alignment value to allocate
-  // aligned buffer for Direct I/O
   size_t GetRequiredBufferAlignment() const override;
 
-  // Remove any kind of caching of data from the offset to offset+length
-  // of this file. If the length is 0, then it refers to the end of file.
-  // If the system is not caching the file contents, then this is a noop.
   IOStatus InvalidateCache(size_t offset, size_t length) override;
 };
 
-// A file abstraction for sequential writing.  The implementation
-// must provide buffering since callers may append small fragments
-// at a time to the file.
 class EncryptedWritableFile : public FSWritableFile {
  protected:
   std::unique_ptr<FSWritableFile> file_;
@@ -328,50 +267,22 @@ class EncryptedWritableFile : public FSWritableFile {
                             const IOOptions& options,
                             IODebugContext* dbg) override;
 
-  // true if Sync() and Fsync() are safe to call concurrently with Append()
-  // and Flush().
   bool IsSyncThreadSafe() const override;
 
-  // Indicates the upper layers if the current WritableFile implementation
-  // uses direct IO.
   bool use_direct_io() const override;
 
-  // Use the returned alignment value to allocate
-  // aligned buffer for Direct I/O
   size_t GetRequiredBufferAlignment() const override;
 
-  /*
-   * Get the size of valid data in the file.
-   */
   uint64_t GetFileSize(const IOOptions& options, IODebugContext* dbg) override;
 
-  // Truncate is necessary to trim the file to the correct size
-  // before closing. It is not always possible to keep track of the file
-  // size due to whole pages writes. The behavior is undefined if called
-  // with other writes to follow.
   IOStatus Truncate(uint64_t size, const IOOptions& options,
                     IODebugContext* dbg) override;
 
-  // Remove any kind of caching of data from the offset to offset+length
-  // of this file. If the length is 0, then it refers to the end of file.
-  // If the system is not caching the file contents, then this is a noop.
-  // This call has no effect on dirty pages in the cache.
   IOStatus InvalidateCache(size_t offset, size_t length) override;
 
-  // Sync a file range with disk.
-  // offset is the starting byte of the file range to be synchronized.
-  // nbytes specifies the length of the range to be synchronized.
-  // This asks the OS to initiate flushing the cached data to disk,
-  // without waiting for completion.
-  // Default implementation does nothing.
   IOStatus RangeSync(uint64_t offset, uint64_t nbytes, const IOOptions& options,
                      IODebugContext* dbg) override;
 
-  // PrepareWrite performs any necessary preparation for a write
-  // before the write actually occurs.  This allows for pre-allocation
-  // of space on devices where it can result in less file
-  // fragmentation and/or less waste from over-zealous filesystem
-  // pre-allocation.
   void PrepareWrite(size_t offset, size_t len, const IOOptions& options,
                     IODebugContext* dbg) override;
 
@@ -380,7 +291,6 @@ class EncryptedWritableFile : public FSWritableFile {
   void GetPreallocationStatus(size_t* block_size,
                               size_t* last_allocated_block) override;
 
-  // Pre-allocates space for a file.
   IOStatus Allocate(uint64_t offset, uint64_t len, const IOOptions& options,
                     IODebugContext* dbg) override;
 
@@ -391,7 +301,6 @@ class EncryptedWritableFile : public FSWritableFile {
   IOStatus Close(const IOOptions& options, IODebugContext* dbg) override;
 };
 
-// A file abstraction for random reading and writing.
 class EncryptedRandomRWFile : public FSRandomRWFile {
  protected:
   std::unique_ptr<FSRandomRWFile> file_;
@@ -406,22 +315,13 @@ class EncryptedRandomRWFile : public FSRandomRWFile {
         stream_(std::move(s)),
         prefixLength_(prefixLength) {}
 
-  // Indicates if the class makes use of direct I/O
-  // If false you must pass aligned buffer to Write()
   bool use_direct_io() const override;
 
-  // Use the returned alignment value to allocate
-  // aligned buffer for Direct I/O
   size_t GetRequiredBufferAlignment() const override;
 
-  // Write bytes in `data` at  offset `offset`, Returns Status::OK() on success.
-  // Pass aligned buffer when use_direct_io() returns true.
   IOStatus Write(uint64_t offset, const Slice& data, const IOOptions& options,
                  IODebugContext* dbg) override;
 
-  // Read up to `n` bytes starting from offset `offset` and store them in
-  // result, provided `scratch` size should be at least `n`.
-  // Returns Status::OK() on success.
   IOStatus Read(uint64_t offset, size_t n, const IOOptions& options,
                 Slice* result, char* scratch,
                 IODebugContext* dbg) const override;

include/rocksdb/options.h

@@ -1071,7 +1071,7 @@ struct DBOptions {
   //
   // By default, i.e., when it is false, rocksdb does not advance the sequence
   // number for new snapshots unless all the writes with lower sequence numbers
-  // are already finished. This provides the immutability that we except from
+  // are already finished. This provides the immutability that we expect from
   // snapshots. Moreover, since Iterator and MultiGet internally depend on
   // snapshots, the snapshot immutability results into Iterator and MultiGet
   // offering consistent-point-in-time view. If set to true, although
@@ -1157,7 +1157,7 @@ struct DBOptions {
 
   // A global cache for table-level rows.
   // Default: nullptr (disabled)
-  std::shared_ptr<GeneralCache> row_cache = nullptr;
+  std::shared_ptr<RowCache> row_cache = nullptr;
 
   // A filter object supplied to be invoked while processing write-ahead-logs
   // (WALs) during recovery. The filter provides a way to inspect log

include/rocksdb/version.h

@@ -12,8 +12,8 @@
 // NOTE: in 'main' development branch, this should be the *next*
 // minor or major version number planned for release.
 #define ROCKSDB_MAJOR 8
-#define ROCKSDB_MINOR 4
-#define ROCKSDB_PATCH 0
+#define ROCKSDB_MINOR 5
+#define ROCKSDB_PATCH 4
 
 // Do not use these. We made the mistake of declaring macros starting with
 // double underscore. Now we have to live with our choice. We'll deprecate these

port/stack_trace.cc

@@ -31,6 +31,10 @@ void* SaveStack(int* /*num_frames*/, int /*first_frames_to_skip*/) {
 #include <string.h>
 #include <unistd.h>
 
+#ifdef OS_OPENBSD
+#include <sys/wait.h>
+#include <sys/sysctl.h>
+#endif  // OS_OPENBSD
 #ifdef OS_FREEBSD
 #include <sys/sysctl.h>
 #endif  // OS_FREEBSD
@@ -51,21 +55,11 @@ namespace port {
 
 namespace {
 
-#if defined(OS_LINUX) || defined(OS_FREEBSD) || defined(OS_GNU_KFREEBSD)
+#if defined(OS_LINUX) || defined(OS_FREEBSD) || defined(OS_OPENBSD) || defined(OS_GNU_KFREEBSD)
 const char* GetExecutableName() {
   static char name[1024];
 
-#if !defined(OS_FREEBSD)
-  char link[1024];
-  snprintf(link, sizeof(link), "/proc/%d/exe", getpid());
-  auto read = readlink(link, name, sizeof(name) - 1);
-  if (-1 == read) {
-    return nullptr;
-  } else {
-    name[read] = 0;
-    return name;
-  }
-#else
+#if defined(OS_FREEBSD)
   int mib[4] = {CTL_KERN, KERN_PROC, KERN_PROC_PATHNAME, -1};
   size_t namesz = sizeof(name);
 
@@ -75,6 +69,27 @@ const char* GetExecutableName() {
   } else {
     return name;
   }
+#elif defined(OS_OPENBSD)
+  int mib[4] = {CTL_KERN, KERN_PROC_ARGS, getpid(), KERN_PROC_ARGV};
+  size_t namesz = sizeof(name);
+  char* bin[namesz];
+
+  auto ret = sysctl(mib, 4, bin, &namesz, nullptr, 0);
+  if (-1 == ret) {
+    return nullptr;
+  } else {
+    return bin[0];
+  }
+#else
+  char link[1024];
+  snprintf(link, sizeof(link), "/proc/%d/exe", getpid());
+  auto read = readlink(link, name, sizeof(name) - 1);
+  if (-1 == read) {
+    return nullptr;
+  } else {
+    name[read] = 0;
+    return name;
+  }
 #endif
 }
 
@@ -271,7 +286,7 @@ void PrintStack(int first_frames_to_skip) {
   const int kMaxFrames = 100;
   void* frames[kMaxFrames];
 
-  auto num_frames = backtrace(frames, kMaxFrames);
+  int num_frames = (int) backtrace(frames, kMaxFrames);
   PrintStack(&frames[first_frames_to_skip], num_frames - first_frames_to_skip);
 }
 
@@ -284,7 +299,7 @@ void* SaveStack(int* num_frames, int first_frames_to_skip) {
   const int kMaxFrames = 100;
   void* frames[kMaxFrames];
 
-  auto count = backtrace(frames, kMaxFrames);
+  int count = (int) backtrace(frames, kMaxFrames);
   *num_frames = count - first_frames_to_skip;
   void* callstack = malloc(sizeof(void*) * *num_frames);
   memcpy(callstack, &frames[first_frames_to_skip], sizeof(void*) * *num_frames);

table/block_based/block_based_table_factory.cc

@@ -83,7 +83,7 @@ size_t TailPrefetchStats::GetSuggestedPrefetchSize() {
   //
   // and we use every of the value as a candidate, and estimate how much we
   // wasted, compared to read. For example, when we use the 3rd record
-  // as candiate. This area is what we read:
+  // as candidate. This area is what we read:
   //                                     +---+
   //                             +---+   |   |
   //                             |   |   |   |
@@ -123,7 +123,7 @@ size_t TailPrefetchStats::GetSuggestedPrefetchSize() {
   //  +---+    +---+    +---+    +---+   +---+
   //
   // Which can be calculated iteratively.
-  // The difference between wasted using 4st and 3rd record, will
+  // The difference between wasted using 4th and 3rd record, will
   // be following area:
   //                                     +---+
   //  +--+  +-+   ++  +-+  +-+   +---+   |   |
@@ -143,8 +143,8 @@ size_t TailPrefetchStats::GetSuggestedPrefetchSize() {
   //  |   |    |   |    |   |    |   |   |   |
   //  +---+    +---+    +---+    +---+   +---+
   //
-  // which will be the size difference between 4st and 3rd record,
-  // times 3, which is number of records before the 4st.
+  // which will be the size difference between 4th and 3rd record,
+  // times 3, which is number of records before the 4th.
   // Here we assume that all data within the prefetch range will be useful. In
   // reality, it may not be the case when a partial block is inside the range,
   // or there are data in the middle that is not read. We ignore those cases

table/block_based/block_builder.cc

@@ -53,8 +53,7 @@ BlockBuilder::BlockBuilder(
     : block_restart_interval_(block_restart_interval),
       use_delta_encoding_(use_delta_encoding),
       use_value_delta_encoding_(use_value_delta_encoding),
-      ts_sz_(ts_sz),
-      persist_user_defined_timestamps_(persist_user_defined_timestamps),
+      strip_ts_sz_(persist_user_defined_timestamps ? 0 : ts_sz),
       is_user_key_(is_user_key),
       restarts_(1, 0),  // First restart point is at offset 0
       counter_(0),
@@ -100,8 +99,8 @@ size_t BlockBuilder::EstimateSizeAfterKV(const Slice& key,
   // Note: this is an imprecise estimate as it accounts for the whole key size
   // instead of non-shared key size.
   estimate += key.size();
-  if (ts_sz_ > 0 && !persist_user_defined_timestamps_) {
-    estimate -= ts_sz_;
+  if (strip_ts_sz_ > 0) {
+    estimate -= strip_ts_sz_;
   }
   // In value delta encoding we estimate the value delta size as half the full
   // value size since only the size field of block handle is encoded.
@@ -175,13 +174,13 @@ void BlockBuilder::AddWithLastKey(const Slice& key, const Slice& value,
   // or Reset. This is more convenient for the caller and we can be more
   // clever inside BlockBuilder. On this hot code path, we want to avoid
   // conditional jumps like `buffer_.empty() ? ... : ...` so we can use a
-  // fast min operation instead, with an assertion to be sure our logic is
-  // sound.
+  // fast arithmetic operation instead, with an assertion to be sure our logic
+  // is sound.
   size_t buffer_size = buffer_.size();
   size_t last_key_size = last_key_param.size();
-  assert(buffer_size == 0 || buffer_size >= last_key_size);
+  assert(buffer_size == 0 || buffer_size >= last_key_size - strip_ts_sz_);
 
-  Slice last_key(last_key_param.data(), std::min(buffer_size, last_key_size));
+  Slice last_key(last_key_param.data(), last_key_size * (buffer_size > 0));
 
   AddWithLastKeyImpl(key, value, last_key, delta_value, buffer_size);
 }
@@ -255,11 +254,11 @@ inline void BlockBuilder::AddWithLastKeyImpl(const Slice& key,
 const Slice BlockBuilder::MaybeStripTimestampFromKey(std::string* key_buf,
                                                      const Slice& key) {
   Slice stripped_key = key;
-  if (ts_sz_ > 0 && !persist_user_defined_timestamps_) {
+  if (strip_ts_sz_ > 0) {
     if (is_user_key_) {
-      stripped_key.remove_suffix(ts_sz_);
+      stripped_key.remove_suffix(strip_ts_sz_);
     } else {
-      StripTimestampFromInternalKey(key_buf, key, ts_sz_);
+      StripTimestampFromInternalKey(key_buf, key, strip_ts_sz_);
       stripped_key = *key_buf;
     }
   }

table/block_based/block_builder.h

@@ -94,11 +94,10 @@ class BlockBuilder {
   const bool use_delta_encoding_;
   // Refer to BlockIter::DecodeCurrentValue for format of delta encoded values
   const bool use_value_delta_encoding_;
-  // Size in bytes for the user-defined timestamp in a user key.
-  const size_t ts_sz_;
-  // Whether the user-defined timestamp part in user keys should be persisted.
-  // If false, it will be stripped from the key before it's encoded.
-  const bool persist_user_defined_timestamps_;
+  // Size in bytes for the user-defined timestamp to strip in a user key.
+  // This is non-zero if there is user-defined timestamp in the user key and it
+  // should not be persisted.
+  const size_t strip_ts_sz_;
   // Whether the keys provided to build this block are user keys. If not,
   // the keys are internal keys. This will affect how timestamp stripping is
   // done for the key if `persisted_user_defined_timestamps_` is false and

table/block_based/block_prefetcher.cc

@@ -17,9 +17,30 @@ void BlockPrefetcher::PrefetchIfNeeded(
     const size_t readahead_size, bool is_for_compaction,
     const bool no_sequential_checking,
     const Env::IOPriority rate_limiter_priority) {
-  // num_file_reads is used  by FilePrefetchBuffer only when
-  // implicit_auto_readahead is set.
+  const size_t len = BlockBasedTable::BlockSizeWithTrailer(handle);
+  const size_t offset = handle.offset();
+
   if (is_for_compaction) {
+    if (!rep->file->use_direct_io() && compaction_readahead_size_ > 0) {
+      // If FS supports prefetching (readahead_limit_ will be non zero in that
+      // case) and current block exists in prefetch buffer then return.
+      if (offset + len <= readahead_limit_) {
+        return;
+      }
+      Status s = rep->file->Prefetch(offset, len + compaction_readahead_size_,
+                                     rate_limiter_priority);
+      if (s.ok()) {
+        readahead_limit_ = offset + len + compaction_readahead_size_;
+        return;
+      } else if (!s.IsNotSupported()) {
+        return;
+      }
+    }
+    // If FS prefetch is not supported, fall back to use internal prefetch
+    // buffer.
+    //
+    // num_file_reads is used  by FilePrefetchBuffer only when
+    // implicit_auto_readahead is set.
     rep->CreateFilePrefetchBufferIfNotExists(
         compaction_readahead_size_, compaction_readahead_size_,
         &prefetch_buffer_, /*implicit_auto_readahead=*/false,
@@ -60,9 +81,6 @@ void BlockPrefetcher::PrefetchIfNeeded(
     return;
   }
 
-  size_t len = BlockBasedTable::BlockSizeWithTrailer(handle);
-  size_t offset = handle.offset();
-
   // If FS supports prefetching (readahead_limit_ will be non zero in that case)
   // and current block exists in prefetch buffer then return.
   if (offset + len <= readahead_limit_) {
@@ -98,8 +116,6 @@ void BlockPrefetcher::PrefetchIfNeeded(
   }
 
   // If prefetch is not supported, fall back to use internal prefetch buffer.
-  // Discarding other return status of Prefetch calls intentionally, as
-  // we can fallback to reading from disk if Prefetch fails.
   Status s = rep->file->Prefetch(
       handle.offset(),
       BlockBasedTable::BlockSizeWithTrailer(handle) + readahead_size_,

table/block_based/block_prefetcher.h

@@ -58,11 +58,12 @@ class BlockPrefetcher {
   // lookup_context_.caller = kCompaction.
   size_t compaction_readahead_size_;
 
-  // readahead_size_ is used if underlying FS supports prefetching.
+  // readahead_size_ is used in non-compaction read if underlying FS supports
+  // prefetching.
   size_t readahead_size_;
   size_t readahead_limit_ = 0;
-  // initial_auto_readahead_size_ is used if RocksDB uses internal prefetch
-  // buffer.
+  // initial_auto_readahead_size_ is used in non-compaction read if RocksDB uses
+  // internal prefetch buffer.
   uint64_t initial_auto_readahead_size_;
   uint64_t num_file_reads_ = 0;
   uint64_t prev_offset_ = 0;

table/block_based/index_builder.h

@@ -9,8 +9,6 @@
 
 #pragma once
 
-#include <assert.h>
-
 #include <cinttypes>
 #include <list>
 #include <string>
@@ -39,9 +37,8 @@ class IndexBuilder {
       BlockBasedTableOptions::IndexType index_type,
       const ROCKSDB_NAMESPACE::InternalKeyComparator* comparator,
       const InternalKeySliceTransform* int_key_slice_transform,
-      const bool use_value_delta_encoding,
-      const BlockBasedTableOptions& table_opt, size_t ts_sz,
-      const bool persist_user_defined_timestamps);
+      bool use_value_delta_encoding, const BlockBasedTableOptions& table_opt,
+      size_t ts_sz, bool persist_user_defined_timestamps);
 
   // Index builder will construct a set of blocks which contain:
   //  1. One primary index block.
@@ -51,13 +48,13 @@ class IndexBuilder {
     Slice index_block_contents;
     std::unordered_map<std::string, Slice> meta_blocks;
   };
-  explicit IndexBuilder(const InternalKeyComparator* comparator, size_t ts_sz,
-                        bool persist_user_defined_timestamps)
+  IndexBuilder(const InternalKeyComparator* comparator, size_t ts_sz,
+               bool persist_user_defined_timestamps)
       : comparator_(comparator),
         ts_sz_(ts_sz),
         persist_user_defined_timestamps_(persist_user_defined_timestamps) {}
 
-  virtual ~IndexBuilder() {}
+  virtual ~IndexBuilder() = default;
 
   // Add a new index entry to index block.
   // To allow further optimization, we provide `last_key_in_current_block` and
@@ -132,7 +129,7 @@ class IndexBuilder {
 //     substitute key that serves the same function.
 class ShortenedIndexBuilder : public IndexBuilder {
  public:
-  explicit ShortenedIndexBuilder(
+  ShortenedIndexBuilder(
       const InternalKeyComparator* comparator,
       const int index_block_restart_interval, const uint32_t format_version,
       const bool use_value_delta_encoding,
@@ -159,15 +156,15 @@ class ShortenedIndexBuilder : public IndexBuilder {
     seperator_is_key_plus_seq_ = (format_version <= 2);
   }
 
-  virtual void OnKeyAdded(const Slice& key) override {
+  void OnKeyAdded(const Slice& key) override {
     if (include_first_key_ && current_block_first_internal_key_.empty()) {
       current_block_first_internal_key_.assign(key.data(), key.size());
     }
   }
 
-  virtual void AddIndexEntry(std::string* last_key_in_current_block,
-                             const Slice* first_key_in_next_block,
-                             const BlockHandle& block_handle) override {
+  void AddIndexEntry(std::string* last_key_in_current_block,
+                     const Slice* first_key_in_next_block,
+                     const BlockHandle& block_handle) override {
     if (first_key_in_next_block != nullptr) {
       if (shortening_mode_ !=
           BlockBasedTableOptions::IndexShorteningMode::kNoShortening) {
@@ -236,9 +233,8 @@ class ShortenedIndexBuilder : public IndexBuilder {
   }
 
   using IndexBuilder::Finish;
-  virtual Status Finish(
-      IndexBlocks* index_blocks,
-      const BlockHandle& /*last_partition_block_handle*/) override {
+  Status Finish(IndexBlocks* index_blocks,
+                const BlockHandle& /*last_partition_block_handle*/) override {
     if (seperator_is_key_plus_seq_) {
       index_blocks->index_block_contents = index_block_builder_.Finish();
     } else {
@@ -249,9 +245,9 @@ class ShortenedIndexBuilder : public IndexBuilder {
     return Status::OK();
   }
 
-  virtual size_t IndexSize() const override { return index_size_; }
+  size_t IndexSize() const override { return index_size_; }
 
-  virtual bool seperator_is_key_plus_seq() override {
+  bool seperator_is_key_plus_seq() override {
     return seperator_is_key_plus_seq_;
   }
 
@@ -306,13 +302,12 @@ class ShortenedIndexBuilder : public IndexBuilder {
 // data copy or small heap allocations for prefixes.
 class HashIndexBuilder : public IndexBuilder {
  public:
-  explicit HashIndexBuilder(
-      const InternalKeyComparator* comparator,
-      const SliceTransform* hash_key_extractor,
-      int index_block_restart_interval, int format_version,
-      bool use_value_delta_encoding,
-      BlockBasedTableOptions::IndexShorteningMode shortening_mode, size_t ts_sz,
-      const bool persist_user_defined_timestamps)
+  HashIndexBuilder(const InternalKeyComparator* comparator,
+                   const SliceTransform* hash_key_extractor,
+                   int index_block_restart_interval, int format_version,
+                   bool use_value_delta_encoding,
+                   BlockBasedTableOptions::IndexShorteningMode shortening_mode,
+                   size_t ts_sz, const bool persist_user_defined_timestamps)
       : IndexBuilder(comparator, ts_sz, persist_user_defined_timestamps),
         primary_index_builder_(comparator, index_block_restart_interval,
                                format_version, use_value_delta_encoding,
@@ -320,15 +315,15 @@ class HashIndexBuilder : public IndexBuilder {
                                ts_sz, persist_user_defined_timestamps),
         hash_key_extractor_(hash_key_extractor) {}
 
-  virtual void AddIndexEntry(std::string* last_key_in_current_block,
-                             const Slice* first_key_in_next_block,
-                             const BlockHandle& block_handle) override {
+  void AddIndexEntry(std::string* last_key_in_current_block,
+                     const Slice* first_key_in_next_block,
+                     const BlockHandle& block_handle) override {
     ++current_restart_index_;
     primary_index_builder_.AddIndexEntry(last_key_in_current_block,
                                          first_key_in_next_block, block_handle);
   }
 
-  virtual void OnKeyAdded(const Slice& key) override {
+  void OnKeyAdded(const Slice& key) override {
     auto key_prefix = hash_key_extractor_->Transform(key);
     bool is_first_entry = pending_block_num_ == 0;
 
@@ -355,9 +350,8 @@ class HashIndexBuilder : public IndexBuilder {
     }
   }
 
-  virtual Status Finish(
-      IndexBlocks* index_blocks,
-      const BlockHandle& last_partition_block_handle) override {
+  Status Finish(IndexBlocks* index_blocks,
+                const BlockHandle& last_partition_block_handle) override {
     if (pending_block_num_ != 0) {
       FlushPendingPrefix();
     }
@@ -370,12 +364,12 @@ class HashIndexBuilder : public IndexBuilder {
     return s;
   }
 
-  virtual size_t IndexSize() const override {
+  size_t IndexSize() const override {
     return primary_index_builder_.IndexSize() + prefix_block_.size() +
            prefix_meta_block_.size();
   }
 
-  virtual bool seperator_is_key_plus_seq() override {
+  bool seperator_is_key_plus_seq() override {
     return primary_index_builder_.seperator_is_key_plus_seq();
   }
 
@@ -421,27 +415,24 @@ class PartitionedIndexBuilder : public IndexBuilder {
  public:
   static PartitionedIndexBuilder* CreateIndexBuilder(
       const ROCKSDB_NAMESPACE::InternalKeyComparator* comparator,
-      const bool use_value_delta_encoding,
-      const BlockBasedTableOptions& table_opt, size_t ts_sz,
-      const bool persist_user_defined_timestamps);
+      bool use_value_delta_encoding, const BlockBasedTableOptions& table_opt,
+      size_t ts_sz, bool persist_user_defined_timestamps);
 
-  explicit PartitionedIndexBuilder(const InternalKeyComparator* comparator,
-                                   const BlockBasedTableOptions& table_opt,
-                                   const bool use_value_delta_encoding,
-                                   size_t ts_sz,
-                                   const bool persist_user_defined_timestamps);
+  PartitionedIndexBuilder(const InternalKeyComparator* comparator,
+                          const BlockBasedTableOptions& table_opt,
+                          bool use_value_delta_encoding, size_t ts_sz,
+                          bool persist_user_defined_timestamps);
 
-  virtual ~PartitionedIndexBuilder();
+  ~PartitionedIndexBuilder() override;
 
-  virtual void AddIndexEntry(std::string* last_key_in_current_block,
-                             const Slice* first_key_in_next_block,
-                             const BlockHandle& block_handle) override;
+  void AddIndexEntry(std::string* last_key_in_current_block,
+                     const Slice* first_key_in_next_block,
+                     const BlockHandle& block_handle) override;
 
-  virtual Status Finish(
-      IndexBlocks* index_blocks,
-      const BlockHandle& last_partition_block_handle) override;
+  Status Finish(IndexBlocks* index_blocks,
+                const BlockHandle& last_partition_block_handle) override;
 
-  virtual size_t IndexSize() const override { return index_size_; }
+  size_t IndexSize() const override { return index_size_; }
   size_t TopLevelIndexSize(uint64_t) const { return top_level_index_size_; }
   size_t NumPartitions() const;
 
@@ -460,11 +451,13 @@ class PartitionedIndexBuilder : public IndexBuilder {
   // cutting the next partition
   void RequestPartitionCut();
 
-  virtual bool seperator_is_key_plus_seq() override {
+  bool seperator_is_key_plus_seq() override {
     return seperator_is_key_plus_seq_;
   }
 
-  bool get_use_value_delta_encoding() { return use_value_delta_encoding_; }
+  bool get_use_value_delta_encoding() const {
+    return use_value_delta_encoding_;
+  }
 
  private:
   // Set after ::Finish is called

table/compaction_merging_iterator.cc

@@ -329,6 +329,7 @@ void CompactionMergingIterator::FindNextVisibleKey() {
       assert(current->iter.status().ok());
       minHeap_.replace_top(current);
     } else {
+      considerStatus(current->iter.status());
       minHeap_.pop();
     }
     if (range_tombstone_iters_[current->level]) {

table/merging_iterator.cc

@@ -308,6 +308,7 @@ class MergingIterator : public InternalIterator {
     // holds after this call, and minHeap_.top().iter points to the
     // first key >= target among children_ that is not covered by any range
     // tombstone.
+    status_ = Status::OK();
     SeekImpl(target);
     FindNextVisibleKey();
 
@@ -321,6 +322,7 @@ class MergingIterator : public InternalIterator {
   void SeekForPrev(const Slice& target) override {
     assert(range_tombstone_iters_.empty() ||
            range_tombstone_iters_.size() == children_.size());
+    status_ = Status::OK();
     SeekForPrevImpl(target);
     FindPrevVisibleKey();
 
@@ -798,7 +800,6 @@ void MergingIterator::SeekImpl(const Slice& target, size_t starting_level,
     active_.erase(active_.lower_bound(starting_level), active_.end());
   }
 
-  status_ = Status::OK();
   IterKey current_search_key;
   current_search_key.SetInternalKey(target, false /* copy */);
   // Seek target might change to some range tombstone end key, so
@@ -931,6 +932,7 @@ bool MergingIterator::SkipNextDeleted() {
       InsertRangeTombstoneToMinHeap(current->level, true /* start_key */,
                                     true /* replace_top */);
     } else {
+      // TruncatedRangeDelIterator does not have status
       minHeap_.pop();
     }
     return true /* current key deleted */;
@@ -988,6 +990,9 @@ bool MergingIterator::SkipNextDeleted() {
     if (current->iter.Valid()) {
       assert(current->iter.status().ok());
       minHeap_.push(current);
+    } else {
+      // TODO(cbi): check status and early return if non-ok.
+      considerStatus(current->iter.status());
     }
     // Invariants (rti) and (phi)
     if (range_tombstone_iters_[current->level] &&
@@ -1027,6 +1032,7 @@ bool MergingIterator::SkipNextDeleted() {
         if (current->iter.Valid()) {
           minHeap_.replace_top(current);
         } else {
+          considerStatus(current->iter.status());
           minHeap_.pop();
         }
         return true /* current key deleted */;
@@ -1078,7 +1084,6 @@ void MergingIterator::SeekForPrevImpl(const Slice& target,
     active_.erase(active_.lower_bound(starting_level), active_.end());
   }
 
-  status_ = Status::OK();
   IterKey current_search_key;
   current_search_key.SetInternalKey(target, false /* copy */);
   // Seek target might change to some range tombstone end key, so
@@ -1199,6 +1204,8 @@ bool MergingIterator::SkipPrevDeleted() {
     if (current->iter.Valid()) {
       assert(current->iter.status().ok());
       maxHeap_->push(current);
+    } else {
+      considerStatus(current->iter.status());
     }
 
     if (range_tombstone_iters_[current->level] &&
@@ -1241,6 +1248,7 @@ bool MergingIterator::SkipPrevDeleted() {
         if (current->iter.Valid()) {
           maxHeap_->replace_top(current);
         } else {
+          considerStatus(current->iter.status());
           maxHeap_->pop();
         }
         return true /* current key deleted */;

table/plain/plain_table_builder.h

@@ -85,7 +85,16 @@ class PlainTableBuilder : public TableBuilder {
   // Finish() call, returns the size of the final generated file.
   uint64_t FileSize() const override;
 
-  TableProperties GetTableProperties() const override { return properties_; }
+  TableProperties GetTableProperties() const override {
+    TableProperties ret = properties_;
+    for (const auto& collector : table_properties_collectors_) {
+      for (const auto& prop : collector->GetReadableProperties()) {
+        ret.readable_properties.insert(prop);
+      }
+      collector->Finish(&ret.user_collected_properties).PermitUncheckedError();
+    }
+    return ret;
+  }
 
   bool SaveIndexInFile() const { return store_index_in_file_; }
 

table/sst_file_dumper.cc

@@ -196,6 +196,7 @@ Status SstFileDumper::NewTableReader(
 }
 
 Status SstFileDumper::VerifyChecksum() {
+  assert(read_options_.verify_checksums);
   // We could pass specific readahead setting into read options if needed.
   return table_reader_->VerifyChecksum(read_options_,
                                        TableReaderCaller::kSSTDumpTool);

tools/benchmark.sh

@@ -6,6 +6,7 @@
 EXIT_INVALID_ARGS=1
 EXIT_NOT_COMPACTION_TEST=2
 EXIT_UNKNOWN_JOB=3
+EXIT_INVALID_PATH=4
 
 # Size Constants
 K=1024
@@ -114,6 +115,11 @@ fi
 
 job_id=${JOB_ID}
 
+if [ ! -x ./db_bench ]; then
+  echo "./db_bench not found. Please make sure it exists in the current directory."
+  exit $EXIT_INVALID_PATH
+fi
+
 # Make it easier to run only the compaction test. Getting valid data requires
 # a number of iterations and having an ability to run the test separately from
 # rest of the benchmarks helps.

tools/check_format_compatible.sh

@@ -125,7 +125,7 @@ EOF
 
 # To check for DB forward compatibility with loading options (old version
 # reading data from new), as well as backward compatibility
-declare -a db_forward_with_options_refs=("6.27.fb" "6.28.fb" "6.29.fb" "7.0.fb" "7.1.fb" "7.2.fb" "7.3.fb" "7.4.fb" "7.5.fb" "7.6.fb" "7.7.fb" "7.8.fb" "7.9.fb" "7.10.fb" "8.0.fb" "8.1.fb" "8.2.fb" "8.3.fb")
+declare -a db_forward_with_options_refs=("6.27.fb" "6.28.fb" "6.29.fb" "7.0.fb" "7.1.fb" "7.2.fb" "7.3.fb" "7.4.fb" "7.5.fb" "7.6.fb" "7.7.fb" "7.8.fb" "7.9.fb" "7.10.fb" "8.0.fb" "8.1.fb" "8.2.fb" "8.3.fb" "8.4.fb")
 # To check for DB forward compatibility without loading options (in addition
 # to the "with loading options" set), as well as backward compatibility
 declare -a db_forward_no_options_refs=() # N/A at the moment

tools/db_crashtest.py

@@ -734,6 +734,7 @@ def gen_cmd(params, unknown_params):
                 "stress_cmd",
                 "test_tiered_storage",
                 "cleanup_cmd",
+                "skip_tmpdir_check"
             }
             and v is not None
         ]
@@ -1016,6 +1017,7 @@ def main():
     parser.add_argument("--stress_cmd")
     parser.add_argument("--test_tiered_storage", action="store_true")
     parser.add_argument("--cleanup_cmd")
+    parser.add_argument("--skip_tmpdir_check", action="store_true")
 
     all_params = dict(
         list(default_params.items())
@@ -1042,13 +1044,13 @@ def main():
     args, unknown_args = parser.parse_known_args()
 
     test_tmpdir = os.environ.get(_TEST_DIR_ENV_VAR)
-    if test_tmpdir is not None:
+    if test_tmpdir is not None and not args.skip_tmpdir_check:
         isdir = False
         try:
             isdir = os.path.isdir(test_tmpdir)
             if not isdir:
                 print(
-                    "%s env var is set to a non-existent directory: %s"
+                    "ERROR: %s env var is set to a non-existent directory: %s. Update it to correct directory path."
                     % (_TEST_DIR_ENV_VAR, test_tmpdir)
                 )
                 sys.exit(1)

tools/ldb_cmd.cc

@@ -1310,7 +1310,8 @@ void DBLoaderCommand::DoCommand() {
 namespace {
 
 void DumpManifestFile(Options options, std::string file, bool verbose, bool hex,
-                      bool json) {
+                      bool json,
+                      const std::vector<ColumnFamilyDescriptor>& cf_descs) {
   EnvOptions sopt;
   std::string dbname("dummy");
   std::shared_ptr<Cache> tc(NewLRUCache(options.max_open_files - 10,
@@ -1326,7 +1327,7 @@ void DumpManifestFile(Options options, std::string file, bool verbose, bool hex,
   VersionSet versions(dbname, &immutable_db_options, sopt, tc.get(), &wb, &wc,
                       /*block_cache_tracer=*/nullptr, /*io_tracer=*/nullptr,
                       /*db_id*/ "", /*db_session_id*/ "");
-  Status s = versions.DumpManifest(options, file, verbose, hex, json);
+  Status s = versions.DumpManifest(options, file, verbose, hex, json, cf_descs);
   if (!s.ok()) {
     fprintf(stderr, "Error in processing file %s %s\n", file.c_str(),
             s.ToString().c_str());
@@ -1438,7 +1439,8 @@ void ManifestDumpCommand::DoCommand() {
     fprintf(stdout, "Processing Manifest file %s\n", manifestfile.c_str());
   }
 
-  DumpManifestFile(options_, manifestfile, verbose_, is_key_hex_, json_);
+  DumpManifestFile(options_, manifestfile, verbose_, is_key_hex_, json_,
+                   column_families_);
 
   if (verbose_) {
     fprintf(stdout, "Processing Manifest file %s done\n", manifestfile.c_str());
@@ -2046,7 +2048,7 @@ void DBDumperCommand::DoCommand() {
         break;
       case kDescriptorFile:
         DumpManifestFile(options_, path_, /* verbose_ */ false, is_key_hex_,
-                         /*  json_ */ false);
+                         /*  json_ */ false, column_families_);
         break;
       case kBlobFile:
         DumpBlobFile(path_, is_key_hex_, is_value_hex_,
@@ -3706,7 +3708,8 @@ void DBFileDumperCommand::DoCommand() {
   manifest_filepath = NormalizePath(manifest_filepath);
 
   std::cout << manifest_filepath << std::endl;
-  DumpManifestFile(options_, manifest_filepath, false, false, false);
+  DumpManifestFile(options_, manifest_filepath, false, false, false,
+                   column_families_);
   std::cout << std::endl;
 
   std::vector<ColumnFamilyMetaData> column_families;

tools/run_blob_bench.sh

@@ -19,6 +19,7 @@
 
 # Exit Codes
 EXIT_INVALID_ARGS=1
+EXIT_INVALID_PATH=2
 
 # Size constants
 K=1024
@@ -74,6 +75,11 @@ if [ $# -ge 1 ]; then
   fi
 fi
 
+if [ ! -f tools/benchmark.sh ]; then
+  echo "tools/benchmark.sh not found"
+  exit $EXIT_INVALID_PATH
+fi
+
 # shellcheck disable=SC2153
 if [ -z "$DB_DIR" ]; then
   echo "DB_DIR is not defined"

tools/sst_dump_tool.cc

@@ -419,6 +419,10 @@ int SSTDumpTool::Run(int argc, char const* const* argv, Options options) {
       filename = std::string(dir_or_file) + "/" + filename;
     }
 
+    if (command == "verify") {
+      verify_checksum = true;
+    }
+
     ROCKSDB_NAMESPACE::SstFileDumper dumper(
         options, filename, Temperature::kUnknown, readahead_size,
         verify_checksum, output_hex, decode_blob_index);

unreleased_history/behavior_changes/010_default_level_dynamic.md

@@ -1 +0,0 @@
-Change the default value for option `level_compaction_dynamic_level_bytes` to true. This affects users who use leveled compaction and do not set this option explicitly. These users may see additional background compactions following DB open. These compactions help to shape the LSM according to `level_compaction_dynamic_level_bytes` such that the size of each level Ln is approximately size of Ln-1 * `max_bytes_for_level_multiplier`. Turning on this option has other benefits too: see more detail in wiki: https://github.com/facebook/rocksdb/wiki/Leveled-Compaction#option-level_compaction_dynamic_level_bytes-and-levels-target-size and in option comment in advanced_options.h (#11525).

unreleased_history/behavior_changes/compact_range_to_bottom.md

@@ -1,2 +0,0 @@
-For Leveled Compaction users, `CompactRange()` will now always try to compact to the last non-empty level. (#11468)
-For Leveled Compaction users, `CompactRange()` with `bottommost_level_compaction = BottommostLevelCompaction::kIfHaveCompactionFilter` will behave similar to `kForceOptimized` in that it will skip files created during this manual compaction when compacting files in the bottommost level. (#11468)

unreleased_history/behavior_changes/drop_range_tombstone.md

@@ -1 +0,0 @@
-RocksDB will try to drop range tombstones during non-bottommost compaction when it is safe to do so. (#11459)

unreleased_history/behavior_changes/ingest_behind_universal.md

@@ -1 +0,0 @@
-When a DB is openend with `allow_ingest_behind=true` (currently only Universal compaction is supported), files in the last level, i.e. the ingested files,  will not be included in any compaction. (#11489)

unreleased_history/behavior_changes/sst_read_micros.md

@@ -1 +0,0 @@
-* Statistics `rocksdb.sst.read.micros` scope is expanded to all SST reads except for file ingestion and column family import (some compaction reads were previously excluded).

unreleased_history/bug_fixes/destroy_posix_env.md

@@ -1 +0,0 @@
-Reduced cases of illegally using Env::Default() during static destruction by never destroying the internal PosixEnv itself (except for builds checking for memory leaks). (#11538)

unreleased_history/bug_fixes/seek_extraprefetch.md

@@ -1 +0,0 @@
-Fix extra prefetching during seek in async_io when BlockBasedTableOptions.num_file_reads_for_auto_readahead_ = 1 leading to extra reads than required.

unreleased_history/bug_fixes/subcompaction_2.md

@@ -1 +0,0 @@
-Fix a bug where compactions that are qualified to be run as 2 subcompactions were only run as one subcompaction. 

unreleased_history/bug_fixes/use_after_move_in_block.md

@@ -1 +0,0 @@
-*Fix a use-after-move bug in block.cc.

unreleased_history/new_features/fs_provided_buffer.md

@@ -1 +0,0 @@
-Add FSReadRequest::fs_scratch which is a data buffer allocated and provided by underlying FileSystem to RocksDB during reads, when FS wants to provide its own buffer with data instead of using RocksDB provided FSReadRequest::scratch. This can help in cpu optimization by avoiding copy from file system's buffer to RocksDB buffer. More details on how to use/enable it in file_system.h. Right now its supported only for MultiReads(async + sync) with non direct io.

unreleased_history/new_features/logging_udt_sizes.md

@@ -1 +0,0 @@
-Start logging non-zero user-defined timestamp sizes in WAL to signal user key format in subsequent records and use it during recovery. This change will break recovery from WAL files written by early versions that contain user-defined timestamps. The workaround is to ensure there are no WAL files to recover (i.e. by flushing before close) before upgrade.

unreleased_history/new_features/obsolete_sst_files_size.md

@@ -1 +0,0 @@
-Added new property "rocksdb.obsolete-sst-files-size-property" that reports the size of SST files that have become obsolete but have not yet been deleted or scheduled for deletion

unreleased_history/new_features/persist_udt_flag_in_manifest.md

@@ -1 +0,0 @@
-*Start to record the value of the flag `AdvancedColumnFamilyOptions.persist_user_defined_timestamps` in the Manifest and table properties for a SST file when it is created. And use the recorded flag when creating a table reader for the SST file. This flag is only explicitly record if it's false. 

unreleased_history/new_features/shared_lock_buckets.md

@@ -1 +0,0 @@
-* Add a new option OptimisticTransactionDBOptions::shared_lock_buckets that enables sharing mutexes for validating transactions between DB instances, for better balancing memory efficiency and validation contention across DB instances. Different column families and DBs also now use different hash seeds in this validation, so that the same set of key names will not contend across DBs or column families.

unreleased_history/new_features/ticker_for_deleted_trash_files.md

@@ -1 +0,0 @@
-*Add a new ticker `rocksdb.files.marked.trash.deleted` to track the number of trash files deleted by background thread from the trash queue.

unreleased_history/new_features/tiered_volatile_cache.md

@@ -1 +0,0 @@
-Add an API NewTieredVolatileCache() in include/rocksdb/cache.h to allocate an instance of a block cache with a primary block cache tier and a compressed secondary cache tier. A cache of this type distributes memory reservations against the block cache, such as WriteBufferManager, table reader memory etc., proportionally across both the primary and compressed secondary cache.

unreleased_history/new_features/wait_for_compact.md

@@ -1 +0,0 @@
-Add `WaitForCompact()` to wait for all flush and compactions jobs to finish. Jobs to wait include the unscheduled (queued, but not scheduled yet).

unreleased_history/new_features/writebatchrelease.md

@@ -1 +0,0 @@
-Add `WriteBatch::Release()` that releases the batch's serialized data to the caller.

unreleased_history/public_api_changes/add_new_compaction_parameter.md

@@ -1 +0,0 @@
-Add parameter `deletion_ratio` to C API `rocksdb_options_add_compact_on_deletion_collector_factory`.

unreleased_history/public_api_changes/change_async_io_api.md

@@ -1 +0,0 @@
-change the FileSystem::use_async_io() API to SupportedOps API in order to extend it to various operations supported by underlying FileSystem. Right now it contains FSSupportedOps::kAsyncIO and FSSupportedOps::kFSBuffer. More details about FSSupportedOps in filesystem.h

unreleased_history/public_api_changes/fix_tickers_typo.md

@@ -1 +0,0 @@
-Add new tickers: `rocksdb.error.handler.bg.error.count`, `rocksdb.error.handler.bg.io.error.count`, `rocksdb.error.handler.bg.retryable.io.error.count` to replace the misspelled ones: `rocksdb.error.handler.bg.errro.count`, `rocksdb.error.handler.bg.io.errro.count`, `rocksdb.error.handler.bg.retryable.io.errro.count` ('error' instead of 'errro'). Users should switch to use the new tickers before 9.0 release as the misspelled old tickers will be completely removed then.

unreleased_history/public_api_changes/support_create_cf_with_import_multiple_cf.md

@@ -1 +0,0 @@
-Overload the API CreateColumnFamilyWithImport() to support creating ColumnFamily by importing multiple ColumnFamilies It requires that CFs should not overlap in user key range.

unreleased_history/release.sh

@@ -31,7 +31,7 @@ awk '/#define ROCKSDB_MAJOR/ { major = $3 }
      /#define ROCKSDB_MINOR/ { minor = $3 }
      /#define ROCKSDB_PATCH/ { patch = $3 }
      END { printf "## " major "." minor "." patch }' < include/rocksdb/version.h >> HISTORY.new
-echo " (`date +%x`)" >> HISTORY.new
+echo " (`git log -n1 --date=format:"%m/%d/%Y" --format="%ad"`)" >> HISTORY.new
 
 function process_file () {
   # use awk to correct extra or missing newlines, missing '* ' on first line

util/async_file_reader.cc

@@ -26,6 +26,11 @@ bool AsyncFileReader::MultiReadAsyncImpl(ReadAwaiter* awaiter) {
           FSReadRequest* read_req = static_cast<FSReadRequest*>(cb_arg);
           read_req->status = req.status;
           read_req->result = req.result;
+          if (req.fs_scratch != nullptr) {
+            // TODO akanksha: Revisit to remove the const in the callback.
+            FSReadRequest& req_tmp = const_cast<FSReadRequest&>(req);
+            read_req->fs_scratch = std::move(req_tmp.fs_scratch);
+          }
         },
         &awaiter->read_reqs_[i], &awaiter->io_handle_[i], &awaiter->del_fn_[i],
         /*aligned_buf=*/nullptr);

util/crc32c.cc

@@ -1117,7 +1117,13 @@ static inline Function Choose_Extend() {
   }
 #elif defined(__SSE4_2__) && defined(__PCLMUL__) && !defined NO_THREEWAY_CRC32C
   // NOTE: runtime detection no longer supported on x86
-  (void)ExtendImpl<DefaultCRC32>;  // suppress unused warning
+#ifdef _MSC_VER
+#pragma warning(disable: 4551)
+#endif
+  (void)ExtendImpl<DefaultCRC32>; // suppress unused warning
+#ifdef _MSC_VER
+#pragma warning(default: 4551)
+#endif
   return crc32c_3way;
 #else
   return ExtendImpl<DefaultCRC32>;

util/filelock_test.cc

@@ -11,6 +11,9 @@
 #include <sys/types.h>
 #include <sys/wait.h>
 #endif
+#ifdef __OpenBSD__
+#include <sys/wait.h>
+#endif
 #include <vector>
 
 #include "test_util/testharness.h"

util/rate_limiter.cc

@@ -179,16 +179,16 @@ void GenericRateLimiter::Request(int64_t bytes, const Env::IOPriority pri,
       // Whichever thread reaches here first performs duty (2) as described
       // above.
       RefillBytesAndGrantRequestsLocked();
-      if (r.request_bytes == 0) {
-        // If there is any remaining requests, make sure there exists at least
-        // one candidate is awake for future duties by signaling a front request
-        // of a queue.
-        for (int i = Env::IO_TOTAL - 1; i >= Env::IO_LOW; --i) {
-          std::deque<Req*> queue = queue_[i];
-          if (!queue.empty()) {
-            queue.front()->cv.Signal();
-            break;
-          }
+    }
+    if (r.request_bytes == 0) {
+      // If there is any remaining requests, make sure there exists at least
+      // one candidate is awake for future duties by signaling a front request
+      // of a queue.
+      for (int i = Env::IO_TOTAL - 1; i >= Env::IO_LOW; --i) {
+        auto& queue = queue_[i];
+        if (!queue.empty()) {
+          queue.front()->cv.Signal();
+          break;
         }
       }
     }

util/single_thread_executor.h

@@ -8,6 +8,7 @@
 
 #if USE_COROUTINES
 #include <atomic>
+#include <queue>
 
 #include "folly/CPortability.h"
 #include "folly/CppAttributes.h"

util/udt_util.cc

@@ -36,7 +36,7 @@ RecoveryType GetRecoveryType(const size_t running_ts_sz,
     return RecoveryType::kPadTimestamp;
   }
 
-  if (running_ts_sz != recorded_ts_sz.value()) {
+  if (running_ts_sz != *recorded_ts_sz) {
     return RecoveryType::kUnrecoverable;
   }
 
@@ -214,7 +214,7 @@ Status TimestampRecoveryHandler::ReconcileTimestampDiscrepancy(
       break;
     case RecoveryType::kStripTimestamp:
       assert(record_ts_sz.has_value());
-      *new_key = StripTimestampFromUserKey(key, record_ts_sz.value());
+      *new_key = StripTimestampFromUserKey(key, *record_ts_sz);
       new_batch_diff_from_orig_batch_ = true;
       break;
     case RecoveryType::kPadTimestamp: