StackableDB optionally take shared ownership of the underlying DB

Summary:
Allow StackableDB optionally takes a shared_ptr on construction and thus hold shared ownership of the underlying DB.
Closes https://github.com/facebook/rocksdb/pull/3423

Differential Revision: D6824163

Pulled By: yiwu-arbug

fbshipit-source-id: dbdc30c42e007533a987ef413785e192340f03eb

CMakeLists.txt

@@ -169,7 +169,7 @@ if(PORTABLE)
   # MSVC does not need a separate compiler flag to enable SSE4.2; if nmmintrin.h
   # is available, it is available by default.
   if(FORCE_SSE42 AND NOT MSVC)
-    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -msse4.2")
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -msse4.2 -mpclmul")
   endif()
 else()
   if(MSVC)
@@ -181,13 +181,18 @@ endif()
 
 include(CheckCXXSourceCompiles)
 if(NOT MSVC)
-  set(CMAKE_REQUIRED_FLAGS "-msse4.2")
+  set(CMAKE_REQUIRED_FLAGS "-msse4.2 -mpclmul")
 endif()
 CHECK_CXX_SOURCE_COMPILES("
 #include <cstdint>
 #include <nmmintrin.h>
+#include <wmmintrin.h>
 int main() {
   volatile uint32_t x = _mm_crc32_u32(0, 0);
+  const auto a = _mm_set_epi64x(0, 0);
+  const auto b = _mm_set_epi64x(0, 0);
+  const auto c = _mm_clmulepi64_si128(a, b, 0x00);
+  auto d = _mm_cvtsi128_si64(c);
 }
 " HAVE_SSE42)
 unset(CMAKE_REQUIRED_FLAGS)
@@ -485,7 +490,6 @@ set(SOURCES
         table/block_based_table_factory.cc
         table/block_based_table_reader.cc
         table/block_builder.cc
-        table/block_fetcher.cc
         table/block_prefix_index.cc
         table/bloom_block.cc
         table/cuckoo_table_builder.cc
@@ -609,7 +613,7 @@ if(HAVE_SSE42 AND NOT FORCE_SSE42)
   if(NOT MSVC)
 set_source_files_properties(
   util/crc32c.cc
-      PROPERTIES COMPILE_FLAGS "-msse4.2")
+      PROPERTIES COMPILE_FLAGS "-msse4.2 -mpclmul")
   endif()
 endif()
 

HISTORY.md

@@ -6,6 +6,7 @@
 * When running `make` with environment variable `USE_SSE` set and `PORTABLE` unset, will use all machine features available locally. Previously this combination only compiled SSE-related features.
 
 ### New Features
+* CRC32C is now using the 3-way pipelined SSE algorithm `crc32c_3way` on supported platforms to improve performance. The system will choose to use this algorithm on supported platforms automatically whenever possible. If PCLMULQDQ is not supported it will fall back to the old Fast_CRC32 algorithm.
 * Provide lifetime hints when writing files on Linux. This reduces hardware write-amp on storage devices supporting multiple streams.
 * Add a DB stat, `NUMBER_ITER_SKIP`, which returns how many internal keys were skipped during iterations (e.g., due to being tombstones or duplicate versions of a key).
 * Add PerfContext counters, `key_lock_wait_count` and `key_lock_wait_time`, which measure the number of times transactions wait on key locks and total amount of time waiting.

Makefile

@@ -305,6 +305,9 @@ LDFLAGS += $(LUA_LIB)
 
 endif
 
+ifeq ($(NO_THREEWAY_CRC32C), 1)
+	CXXFLAGS += -DNO_THREEWAY_CRC32C
+endif
 
 CFLAGS += $(WARNING_FLAGS) -I. -I./include $(PLATFORM_CCFLAGS) $(OPT)
 CXXFLAGS += $(WARNING_FLAGS) -I. -I./include $(PLATFORM_CXXFLAGS) $(OPT) -Woverloaded-virtual -Wnon-virtual-dtor -Wno-missing-field-initializers

TARGETS

@@ -157,7 +157,6 @@ cpp_library(
         "table/block_based_table_factory.cc",
         "table/block_based_table_reader.cc",
         "table/block_builder.cc",
-        "table/block_fetcher.cc",
         "table/block_prefix_index.cc",
         "table/bloom_block.cc",
         "table/cuckoo_table_builder.cc",

build_tools/build_detect_platform

@@ -484,10 +484,10 @@ if test -z "$PORTABLE"; then
   elif [ "$TARGET_OS" != AIX ] && [ "$TARGET_OS" != SunOS ]; then
     COMMON_FLAGS="$COMMON_FLAGS -march=native "
   elif test "$USE_SSE"; then
-    COMMON_FLAGS="$COMMON_FLAGS -msse4.2"
+    COMMON_FLAGS="$COMMON_FLAGS -msse4.2 -mpclmul"
   fi
 elif test "$USE_SSE"; then
-  COMMON_FLAGS="$COMMON_FLAGS -msse4.2"
+  COMMON_FLAGS="$COMMON_FLAGS -msse4.2 -mpclmul"
 fi
 
 $CXX $PLATFORM_CXXFLAGS $COMMON_FLAGS -x c++ - -o /dev/null 2>/dev/null <<EOF
@@ -501,6 +501,24 @@ if [ "$?" = 0 ]; then
   COMMON_FLAGS="$COMMON_FLAGS -DHAVE_SSE42"
 elif test "$USE_SSE"; then
   echo "warning: USE_SSE specified but compiler could not use SSE intrinsics, disabling"
+  exit 1
+fi
+
+$CXX $PLATFORM_CXXFLAGS $COMMON_FLAGS -x c++ - -o /dev/null 2>/dev/null <<EOF
+  #include <cstdint>
+  #include <wmmintrin.h>
+  int main() {
+    const auto a = _mm_set_epi64x(0, 0);
+    const auto b = _mm_set_epi64x(0, 0);
+    const auto c = _mm_clmulepi64_si128(a, b, 0x00);
+    auto d = _mm_cvtsi128_si64(c);
+  }
+EOF
+if [ "$?" = 0 ]; then
+  COMMON_FLAGS="$COMMON_FLAGS -DHAVE_PCLMUL"
+elif test "$USE_SSE"; then
+  echo "warning: USE_SSE specified but compiler could not use PCLMUL intrinsics, disabling"
+  exit 1
 fi
 
 # iOS doesn't support thread-local storage, but this check would erroneously

db/compaction_iterator.cc

@@ -184,11 +184,8 @@ void CompactionIterator::InvokeFilterIfNeeded(bool* need_skip,
                                               Slice* skip_until) {
   if (compaction_filter_ != nullptr &&
       (ikey_.type == kTypeValue || ikey_.type == kTypeBlobIndex) &&
-      (visible_at_tip_ || ignore_snapshots_ ||
-       ikey_.sequence > latest_snapshot_ ||
-       (snapshot_checker_ != nullptr &&
-        UNLIKELY(!snapshot_checker_->IsInSnapshot(ikey_.sequence,
-                                                  latest_snapshot_))))) {
+      (visible_at_tip_ || ikey_.sequence > latest_snapshot_ ||
+       ignore_snapshots_)) {
     // If the user has specified a compaction filter and the sequence
     // number is greater than any external snapshot, then invoke the
     // filter. If the return value of the compaction filter is true,
@@ -414,10 +411,7 @@ void CompactionIterator::NextFromInput() {
           cmp_->Equal(ikey_.user_key, next_ikey.user_key)) {
         // Check whether the next key belongs to the same snapshot as the
         // SingleDelete.
-        if (prev_snapshot == 0 || next_ikey.sequence > prev_snapshot ||
-            (snapshot_checker_ != nullptr &&
-             UNLIKELY(!snapshot_checker_->IsInSnapshot(next_ikey.sequence,
-                                                       prev_snapshot)))) {
+        if (prev_snapshot == 0 || next_ikey.sequence > prev_snapshot) {
           if (next_ikey.type == kTypeSingleDeletion) {
             // We encountered two SingleDeletes in a row.  This could be due to
             // unexpected user input.
@@ -428,12 +422,8 @@ void CompactionIterator::NextFromInput() {
             // input_->Next().
             ++iter_stats_.num_record_drop_obsolete;
             ++iter_stats_.num_single_del_mismatch;
-          } else if (has_outputted_key_ ||
-                     (ikey_.sequence <= earliest_write_conflict_snapshot_ &&
-                      (snapshot_checker_ == nullptr ||
-                       LIKELY(snapshot_checker_->IsInSnapshot(
-                           ikey_.sequence,
-                           earliest_write_conflict_snapshot_))))) {
+          } else if ((ikey_.sequence <= earliest_write_conflict_snapshot_) ||
+                     has_outputted_key_) {
             // Found a matching value, we can drop the single delete and the
             // value.  It is safe to drop both records since we've already
             // outputted a key in this snapshot, or there is no earlier
@@ -481,9 +471,6 @@ void CompactionIterator::NextFromInput() {
         // comparison, so the value of has_current_user_key does not matter.
         has_current_user_key_ = false;
         if (compaction_ != nullptr && ikey_.sequence <= earliest_snapshot_ &&
-            (snapshot_checker_ == nullptr ||
-             LIKELY(snapshot_checker_->IsInSnapshot(ikey_.sequence,
-                                                    earliest_snapshot_))) &&
             compaction_->KeyNotExistsBeyondOutputLevel(ikey_.user_key,
                                                        &level_ptrs_)) {
           // Key doesn't exist outside of this range.
@@ -517,9 +504,6 @@ void CompactionIterator::NextFromInput() {
       input_->Next();
     } else if (compaction_ != nullptr && ikey_.type == kTypeDeletion &&
                ikey_.sequence <= earliest_snapshot_ &&
-               (snapshot_checker_ == nullptr ||
-                LIKELY(snapshot_checker_->IsInSnapshot(ikey_.sequence,
-                                                       earliest_snapshot_))) &&
                ikeyNotNeededForIncrementalSnapshot() &&
                compaction_->KeyNotExistsBeyondOutputLevel(ikey_.user_key,
                                                           &level_ptrs_)) {
@@ -554,10 +538,6 @@ void CompactionIterator::NextFromInput() {
       // have hit (A)
       // We encapsulate the merge related state machine in a different
       // object to minimize change to the existing flow.
-      // In case snapshot_checker is present, we can probably merge further
-      // beyond prev_snapshot, since there could be more keys with sequence
-      // smaller than prev_snapshot, but reported by snapshot_checker as not
-      // visible by prev_snapshot. But it will make the logic more complicated.
       Status s = merge_helper_->MergeUntil(input_, range_del_agg_,
                                            prev_snapshot, bottommost_level_);
       merge_out_iter_.SeekToFirst();

db/db_impl.cc

@@ -1682,12 +1682,6 @@ void DBImpl::ReleaseSnapshot(const Snapshot* s) {
   delete casted_s;
 }
 
-bool DBImpl::HasActiveSnapshotInRange(SequenceNumber lower_bound,
-                                      SequenceNumber upper_bound) {
-  InstrumentedMutexLock l(&mutex_);
-  return snapshots_.HasSnapshotInRange(lower_bound, upper_bound);
-}
-
 #ifndef ROCKSDB_LITE
 Status DBImpl::GetPropertiesOfAllTables(ColumnFamilyHandle* column_family,
                                         TablePropertiesCollection* props) {

db/db_impl.h

@@ -229,10 +229,6 @@ class DBImpl : public DB {
 
   virtual bool SetPreserveDeletesSequenceNumber(SequenceNumber seqnum) override;
 
-  // Whether there is an active snapshot in range [lower_bound, upper_bound).
-  bool HasActiveSnapshotInRange(SequenceNumber lower_bound,
-                                SequenceNumber upper_bound);
-
 #ifndef ROCKSDB_LITE
   using DB::ResetStats;
   virtual Status ResetStats() override;

db/db_impl_write.cc

@@ -521,6 +521,9 @@ Status DBImpl::WriteImplWALOnly(const WriteOptions& write_options,
   PERF_TIMER_GUARD(write_pre_and_post_process_time);
   WriteThread::Writer w(write_options, my_batch, callback, log_ref,
                         true /* disable_memtable */, pre_release_callback);
+  if (write_options.disableWAL) {
+    return status;
+  }
   RecordTick(stats_, WRITE_WITH_WAL);
   StopWatch write_sw(env_, immutable_db_options_.statistics.get(), DB_WRITE);
 
@@ -577,13 +580,7 @@ Status DBImpl::WriteImplWALOnly(const WriteOptions& write_options,
   // LastAllocatedSequence is increased inside WriteToWAL under
   // wal_write_mutex_ to ensure ordered events in WAL
   size_t seq_inc = seq_per_batch_ ? write_group.size : 0 /*total_count*/;
-  if (!write_options.disableWAL) {
-    status =
-        ConcurrentWriteToWAL(write_group, log_used, &last_sequence, seq_inc);
-  } else {
-    // Otherwise we inc seq number to do solely the seq allocation
-    last_sequence = versions_->FetchAddLastAllocatedSequence(seq_inc);
-  }
+  status = ConcurrentWriteToWAL(write_group, log_used, &last_sequence, seq_inc);
   auto curr_seq = last_sequence + 1;
   for (auto* writer : write_group) {
     if (writer->CallbackFailed()) {
@@ -596,7 +593,6 @@ Status DBImpl::WriteImplWALOnly(const WriteOptions& write_options,
     // else seq advances only by memtable writes
   }
   if (status.ok() && write_options.sync) {
-    assert(!write_options.disableWAL);
     // Requesting sync with two_write_queues_ is expected to be very rare. We
     // hance provide a simple implementation that is not necessarily efficient.
     if (manual_wal_flush_) {

db/db_sst_test.cc

@@ -376,30 +376,6 @@ TEST_F(DBSSTTest, RateLimitedDelete) {
   rocksdb::SyncPoint::GetInstance()->DisableProcessing();
 }
 
-TEST_F(DBSSTTest, OpenDBWithExistingTrash) {
-  Options options = CurrentOptions();
-
-  options.sst_file_manager.reset(
-      NewSstFileManager(env_, nullptr, "", 1024 * 1024 /* 1 MB/sec */));
-  auto sfm = static_cast<SstFileManagerImpl*>(options.sst_file_manager.get());
-
-  Destroy(last_options_);
-
-  // Add some trash files to the db directory so the DB can clean them up
-  env_->CreateDirIfMissing(dbname_);
-  ASSERT_OK(WriteStringToFile(env_, "abc", dbname_ + "/" + "001.sst.trash"));
-  ASSERT_OK(WriteStringToFile(env_, "abc", dbname_ + "/" + "002.sst.trash"));
-  ASSERT_OK(WriteStringToFile(env_, "abc", dbname_ + "/" + "003.sst.trash"));
-
-  // Reopen the DB and verify that it deletes existing trash files
-  ASSERT_OK(TryReopen(options));
-  sfm->WaitForEmptyTrash();
-  ASSERT_NOK(env_->FileExists(dbname_ + "/" + "001.sst.trash"));
-  ASSERT_NOK(env_->FileExists(dbname_ + "/" + "002.sst.trash"));
-  ASSERT_NOK(env_->FileExists(dbname_ + "/" + "003.sst.trash"));
-}
-
-
 // Create a DB with 2 db_paths, and generate multiple files in the 2
 // db_paths using CompactRangeOptions, make sure that files that were
 // deleted from first db_path were deleted using DeleteScheduler and

db/db_test.cc

@@ -4375,6 +4375,7 @@ TEST_F(DBTest, DynamicFIFOCompactionOptions) {
   ASSERT_EQ(dbfull()->GetOptions().compaction_options_fifo.allow_compaction,
             true);
 }
+#endif  // ROCKSDB_LITE
 
 TEST_F(DBTest, DynamicUniversalCompactionOptions) {
   Options options;
@@ -4443,7 +4444,6 @@ TEST_F(DBTest, DynamicUniversalCompactionOptions) {
       dbfull()->GetOptions().compaction_options_universal.allow_trivial_move,
       false);
 }
-#endif  // ROCKSDB_LITE
 
 TEST_F(DBTest, FileCreationRandomFailure) {
   Options options;

db/db_write_test.cc

@@ -39,52 +39,6 @@ TEST_P(DBWriteTest, SyncAndDisableWAL) {
   ASSERT_TRUE(dbfull()->Write(write_options, &batch).IsInvalidArgument());
 }
 
-// Sequence number should be return through input write batch.
-TEST_P(DBWriteTest, ReturnSeuqneceNumber) {
-  Random rnd(4422);
-  Open();
-  for (int i = 0; i < 100; i++) {
-    WriteBatch batch;
-    batch.Put("key" + ToString(i), test::RandomHumanReadableString(&rnd, 10));
-    ASSERT_OK(dbfull()->Write(WriteOptions(), &batch));
-    ASSERT_EQ(dbfull()->GetLatestSequenceNumber(),
-              WriteBatchInternal::Sequence(&batch));
-  }
-}
-
-TEST_P(DBWriteTest, ReturnSeuqneceNumberMultiThreaded) {
-  constexpr size_t kThreads = 16;
-  constexpr size_t kNumKeys = 1000;
-  Open();
-  ASSERT_EQ(0, dbfull()->GetLatestSequenceNumber());
-  // Check each sequence is used once and only once.
-  std::vector<std::atomic_flag> flags(kNumKeys * kThreads + 1);
-  for (size_t i = 0; i < flags.size(); i++) {
-    flags[i].clear();
-  }
-  auto writer = [&](size_t id) {
-    Random rnd(4422 + static_cast<uint32_t>(id));
-    for (size_t k = 0; k < kNumKeys; k++) {
-      WriteBatch batch;
-      batch.Put("key" + ToString(id) + "-" + ToString(k),
-                test::RandomHumanReadableString(&rnd, 10));
-      ASSERT_OK(dbfull()->Write(WriteOptions(), &batch));
-      SequenceNumber sequence = WriteBatchInternal::Sequence(&batch);
-      ASSERT_GT(sequence, 0);
-      ASSERT_LE(sequence, kNumKeys * kThreads);
-      // The sequence isn't consumed by someone else.
-      ASSERT_FALSE(flags[sequence].test_and_set());
-    }
-  };
-  std::vector<port::Thread> threads;
-  for (size_t i = 0; i < kThreads; i++) {
-    threads.emplace_back(writer, i);
-  }
-  for (size_t i = 0; i < kThreads; i++) {
-    threads[i].join();
-  }
-}
-
 TEST_P(DBWriteTest, IOErrorOnWALWritePropagateToWriteThreadFollower) {
   constexpr int kNumThreads = 5;
   std::unique_ptr<FaultInjectionTestEnv> mock_env(

db/snapshot_impl.h

@@ -108,22 +108,6 @@ class SnapshotList {
     return ret;
   }
 
-  // Whether there is an active snapshot in range [lower_bound, upper_bound).
-  bool HasSnapshotInRange(SequenceNumber lower_bound,
-                          SequenceNumber upper_bound) {
-    if (empty()) {
-      return false;
-    }
-    const SnapshotImpl* s = &list_;
-    while (s->next_ != &list_) {
-      if (s->next_->number_ >= lower_bound) {
-        return s->next_->number_ < upper_bound;
-      }
-      s = s->next_;
-    }
-    return false;
-  }
-
   // get the sequence number of the most recent snapshot
   SequenceNumber GetNewest() {
     if (empty()) {

include/rocksdb/statistics.h

@@ -415,7 +415,7 @@ const std::vector<std::pair<Tickers, std::string>> TickersNameMap = {
     {BLOB_DB_WRITE_BLOB, "rocksdb.blobdb.write.blob"},
     {BLOB_DB_WRITE_BLOB_TTL, "rocksdb.blobdb.write.blob.ttl"},
     {BLOB_DB_BLOB_FILE_BYTES_WRITTEN, "rocksdb.blobdb.blob.file.bytes.written"},
-    {BLOB_DB_BLOB_FILE_BYTES_READ, "rocksdb.blobdb.blob.file,bytes.read"},
+    {BLOB_DB_BLOB_FILE_BYTES_READ, "rocksdb.blobdb.blob.file.bytes.read"},
     {BLOB_DB_BLOB_FILE_SYNCED, "rocksdb.blobdb.blob.file.synced"},
     {BLOB_DB_BLOB_INDEX_EXPIRED, "rocksdb.blobdb.blob.index.expired"},
     {BLOB_DB_GC_NUM_FILES, "rocksdb.blobdb.gc.num.files"},

include/rocksdb/utilities/stackable_db.h

@@ -4,6 +4,7 @@
 
 #pragma once
 #include <map>
+#include <memory>
 #include <string>
 #include "rocksdb/db.h"
 
@@ -18,11 +19,20 @@ namespace rocksdb {
 // This class contains APIs to stack rocksdb wrappers.Eg. Stack TTL over base d
 class StackableDB : public DB {
  public:
-  // StackableDB is the owner of db now!
+  // StackableDB take sole ownership of the underlying db.
   explicit StackableDB(DB* db) : db_(db) {}
 
+  // StackableDB take shared ownership of the underlying db.
+  explicit StackableDB(std::shared_ptr<DB> db)
+      : db_(db.get()), shared_db_ptr_(db) {}
+
   ~StackableDB() {
-    delete db_;
+    if (shared_db_ptr_ == nullptr) {
+      delete db_;
+    } else {
+      assert(shared_db_ptr_.get() == db_);
+    }
+    db_ = nullptr;
   }
 
   virtual DB* GetBaseDB() {
@@ -373,6 +383,7 @@ class StackableDB : public DB {
 
  protected:
   DB* db_;
+  std::shared_ptr<DB> shared_db_ptr_;
 };
 
 } //  namespace rocksdb

src.mk

@@ -96,7 +96,6 @@ LIB_SOURCES =                                                   \
   table/block_based_table_factory.cc                            \
   table/block_based_table_reader.cc                             \
   table/block_builder.cc                                        \
-  table/block_fetcher.cc                                        \
   table/block_prefix_index.cc                                   \
   table/bloom_block.cc                                          \
   table/cuckoo_table_builder.cc                                 \

table/block_based_table_reader.cc

@@ -30,7 +30,6 @@
 #include "table/block.h"
 #include "table/block_based_filter_block.h"
 #include "table/block_based_table_factory.h"
-#include "table/block_fetcher.h"
 #include "table/block_prefix_index.h"
 #include "table/filter_block.h"
 #include "table/format.h"
@@ -79,10 +78,9 @@ Status ReadBlockFromFile(
     const PersistentCacheOptions& cache_options, SequenceNumber global_seqno,
     size_t read_amp_bytes_per_bit) {
   BlockContents contents;
-  BlockFetcher block_fetcher(file, prefetch_buffer, footer, options, handle,
-                             &contents, ioptions, do_uncompress,
-                             compression_dict, cache_options);
-  Status s = block_fetcher.ReadBlockContents();
+  Status s = ReadBlockContents(file, prefetch_buffer, footer, options, handle,
+                               &contents, ioptions, do_uncompress,
+                               compression_dict, cache_options);
   if (s.ok()) {
     result->reset(new Block(std::move(contents), global_seqno,
                             read_amp_bytes_per_bit, ioptions.statistics));
@@ -427,23 +425,20 @@ class HashIndexReader : public IndexReader {
       return Status::OK();
     }
 
-    Slice dummy_comp_dict;
     // Read contents for the blocks
     BlockContents prefixes_contents;
-    BlockFetcher prefixes_block_fetcher(
-        file, prefetch_buffer, footer, ReadOptions(), prefixes_handle,
-        &prefixes_contents, ioptions, true /* decompress */,
-        dummy_comp_dict /*compression dict*/, cache_options);
-    s = prefixes_block_fetcher.ReadBlockContents();
+    s = ReadBlockContents(file, prefetch_buffer, footer, ReadOptions(),
+                          prefixes_handle, &prefixes_contents, ioptions,
+                          true /* decompress */, Slice() /*compression dict*/,
+                          cache_options);
     if (!s.ok()) {
       return s;
     }
     BlockContents prefixes_meta_contents;
-    BlockFetcher prefixes_meta_block_fetcher(
-        file, prefetch_buffer, footer, ReadOptions(), prefixes_meta_handle,
-        &prefixes_meta_contents, ioptions, true /* decompress */,
-        dummy_comp_dict /*compression dict*/, cache_options);
-    prefixes_meta_block_fetcher.ReadBlockContents();
+    s = ReadBlockContents(file, prefetch_buffer, footer, ReadOptions(),
+                          prefixes_meta_handle, &prefixes_meta_contents,
+                          ioptions, true /* decompress */,
+                          Slice() /*compression dict*/, cache_options);
     if (!s.ok()) {
       // TODO: log error
       return Status::OK();
@@ -1198,16 +1193,11 @@ FilterBlockReader* BlockBasedTable::ReadFilter(
     return nullptr;
   }
   BlockContents block;
-
-  Slice dummy_comp_dict;
-
-  BlockFetcher block_fetcher(rep->file.get(), prefetch_buffer, rep->footer,
-                             ReadOptions(), filter_handle, &block,
-                             rep->ioptions, false /* decompress */,
-                             dummy_comp_dict, rep->persistent_cache_options);
-  Status s = block_fetcher.ReadBlockContents();
-
-  if (!s.ok()) {
+  if (!ReadBlockContents(rep->file.get(), prefetch_buffer, rep->footer,
+                         ReadOptions(), filter_handle, &block, rep->ioptions,
+                         false /* decompress */, Slice() /*compression dict*/,
+                         rep->persistent_cache_options)
+           .ok()) {
     // Error reading the block
     return nullptr;
   }
@@ -1993,13 +1983,11 @@ Status BlockBasedTable::VerifyChecksumInBlocks(InternalIterator* index_iter) {
       break;
     }
     BlockContents contents;
-    Slice dummy_comp_dict;
-    BlockFetcher block_fetcher(rep_->file.get(), nullptr /* prefetch buffer */,
-                               rep_->footer, ReadOptions(), handle, &contents,
-                               rep_->ioptions, false /* decompress */,
-                               dummy_comp_dict /*compression dict*/,
-                               rep_->persistent_cache_options);
-    s = block_fetcher.ReadBlockContents();
+    s = ReadBlockContents(rep_->file.get(), nullptr /* prefetch buffer */,
+                          rep_->footer, ReadOptions(), handle, &contents,
+                          rep_->ioptions, false /* decompress */,
+                          Slice() /*compression dict*/,
+                          rep_->persistent_cache_options);
     if (!s.ok()) {
       break;
     }
@@ -2284,14 +2272,12 @@ Status BlockBasedTable::DumpTable(WritableFile* out_file) {
       BlockHandle handle;
       if (FindMetaBlock(meta_iter.get(), filter_block_key, &handle).ok()) {
         BlockContents block;
-        Slice dummy_comp_dict;
-        BlockFetcher block_fetcher(
-            rep_->file.get(), nullptr /* prefetch_buffer */, rep_->footer,
-            ReadOptions(), handle, &block, rep_->ioptions, false /*decompress*/,
-            dummy_comp_dict /*compression dict*/,
-            rep_->persistent_cache_options);
-        s = block_fetcher.ReadBlockContents();
-        if (!s.ok()) {
+        if (ReadBlockContents(rep_->file.get(), nullptr /* prefetch_buffer */,
+                              rep_->footer, ReadOptions(), handle, &block,
+                              rep_->ioptions, false /*decompress*/,
+                              Slice() /*compression dict*/,
+                              rep_->persistent_cache_options)
+                .ok()) {
           rep_->filter.reset(new BlockBasedFilterBlockReader(
               rep_->ioptions.prefix_extractor, table_options,
               table_options.whole_key_filtering, std::move(block),

table/block_fetcher.cc

@@ -1,233 +0,0 @@
-//  Copyright (c) 2011-present, Facebook, Inc.  All rights reserved.
-//  This source code is licensed under both the GPLv2 (found in the
-//  COPYING file in the root directory) and Apache 2.0 License
-//  (found in the LICENSE.Apache file in the root directory).
-//
-// Copyright (c) 2011 The LevelDB Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file. See the AUTHORS file for names of contributors.
-
-#include "table/block_fetcher.h"
-
-#include <string>
-#include <inttypes.h>
-
-#include "monitoring/perf_context_imp.h"
-#include "monitoring/statistics.h"
-#include "rocksdb/env.h"
-#include "table/block.h"
-#include "table/block_based_table_reader.h"
-#include "table/persistent_cache_helper.h"
-#include "table/format.h"
-#include "util/coding.h"
-#include "util/compression.h"
-#include "util/crc32c.h"
-#include "util/file_reader_writer.h"
-#include "util/logging.h"
-#include "util/stop_watch.h"
-#include "util/string_util.h"
-#include "util/xxhash.h"
-
-namespace rocksdb {
-
-void BlockFetcher::CheckBlockChecksum() {
-  // Check the crc of the type and the block contents
-  if (read_options_.verify_checksums) {
-    const char* data = slice_.data();  // Pointer to where Read put the data
-    PERF_TIMER_GUARD(block_checksum_time);
-    uint32_t value = DecodeFixed32(data + block_size_ + 1);
-    uint32_t actual = 0;
-    switch (footer_.checksum()) {
-      case kNoChecksum:
-        break;
-      case kCRC32c:
-        value = crc32c::Unmask(value);
-        actual = crc32c::Value(data, block_size_ + 1);
-        break;
-      case kxxHash:
-        actual = XXH32(data, static_cast<int>(block_size_) + 1, 0);
-        break;
-      default:
-        status_ = Status::Corruption(
-            "unknown checksum type " + ToString(footer_.checksum()) + " in " +
-            file_->file_name() + " offset " + ToString(handle_.offset()) +
-            " size " + ToString(block_size_));
-    }
-    if (status_.ok() && actual != value) {
-      status_ = Status::Corruption(
-          "block checksum mismatch: expected " + ToString(actual) + ", got " +
-          ToString(value) + "  in " + file_->file_name() + " offset " +
-          ToString(handle_.offset()) + " size " + ToString(block_size_));
-    }
-  }
-}
-
-bool BlockFetcher::TryGetUncompressBlockFromPersistentCache() {
-  if (cache_options_.persistent_cache &&
-      !cache_options_.persistent_cache->IsCompressed()) {
-    Status status = PersistentCacheHelper::LookupUncompressedPage(
-        cache_options_, handle_, contents_);
-    if (status.ok()) {
-      // uncompressed page is found for the block handle
-      return true;
-    } else {
-      // uncompressed page is not found
-      if (ioptions_.info_log && !status.IsNotFound()) {
-        assert(!status.ok());
-        ROCKS_LOG_INFO(ioptions_.info_log,
-                       "Error reading from persistent cache. %s",
-                       status.ToString().c_str());
-      }
-    }
-  }
-  return false;
-}
-
-bool BlockFetcher::TryGetFromPrefetchBuffer() {
-  if (prefetch_buffer_ != nullptr &&
-      prefetch_buffer_->TryReadFromCache(
-          handle_.offset(),
-          static_cast<size_t>(handle_.size()) + kBlockTrailerSize, &slice_)) {
-    block_size_ = static_cast<size_t>(handle_.size());
-    CheckBlockChecksum();
-    if (!status_.ok()) {
-      return true;
-    }
-    got_from_prefetch_buffer_ = true;
-    used_buf_ = const_cast<char*>(slice_.data());
-  }
-  return got_from_prefetch_buffer_;
-}
-
-bool BlockFetcher::TryGetCompressedBlockFromPersistentCache() {
-  if (cache_options_.persistent_cache &&
-      cache_options_.persistent_cache->IsCompressed()) {
-    // lookup uncompressed cache mode p-cache
-    status_ = PersistentCacheHelper::LookupRawPage(
-        cache_options_, handle_, &heap_buf_, block_size_ + kBlockTrailerSize);
-    if (status_.ok()) {
-      used_buf_ = heap_buf_.get();
-      slice_ = Slice(heap_buf_.get(), block_size_);
-      return true;
-    } else if (!status_.IsNotFound() && ioptions_.info_log) {
-      assert(!status_.ok());
-      ROCKS_LOG_INFO(ioptions_.info_log,
-                     "Error reading from persistent cache. %s",
-                     status_.ToString().c_str());
-    }
-  }
-  return false;
-}
-
-void BlockFetcher::PrepareBufferForBlockFromFile() {
-  // cache miss read from device
-  if (do_uncompress_ &&
-      block_size_ + kBlockTrailerSize < kDefaultStackBufferSize) {
-    // If we've got a small enough hunk of data, read it in to the
-    // trivially allocated stack buffer instead of needing a full malloc()
-    used_buf_ = &stack_buf_[0];
-  } else {
-    heap_buf_ =
-        std::unique_ptr<char[]>(new char[block_size_ + kBlockTrailerSize]);
-    used_buf_ = heap_buf_.get();
-  }
-}
-
-void BlockFetcher::InsertCompressedBlockToPersistentCacheIfNeeded() {
-  if (status_.ok() && read_options_.fill_cache &&
-      cache_options_.persistent_cache &&
-      cache_options_.persistent_cache->IsCompressed()) {
-    // insert to raw cache
-    PersistentCacheHelper::InsertRawPage(cache_options_, handle_, used_buf_,
-                                         block_size_ + kBlockTrailerSize);
-  }
-}
-
-void BlockFetcher::InsertUncompressedBlockToPersistentCacheIfNeeded() {
-  if (status_.ok() && !got_from_prefetch_buffer_ && read_options_.fill_cache &&
-      cache_options_.persistent_cache &&
-      !cache_options_.persistent_cache->IsCompressed()) {
-    // insert to uncompressed cache
-    PersistentCacheHelper::InsertUncompressedPage(cache_options_, handle_,
-                                                  *contents_);
-  }
-}
-
-void BlockFetcher::GetBlockContents() {
-  if (slice_.data() != used_buf_) {
-    // the slice content is not the buffer provided
-    *contents_ = BlockContents(Slice(slice_.data(), block_size_), false,
-                               compression_type);
-  } else {
-    // page is uncompressed, the buffer either stack or heap provided
-    if (got_from_prefetch_buffer_ || used_buf_ == &stack_buf_[0]) {
-      heap_buf_ = std::unique_ptr<char[]>(new char[block_size_]);
-      memcpy(heap_buf_.get(), used_buf_, block_size_);
-    }
-    *contents_ = BlockContents(std::move(heap_buf_), block_size_, true,
-                               compression_type);
-  }
-}
-
-Status BlockFetcher::ReadBlockContents() {
-  block_size_ = static_cast<size_t>(handle_.size());
-
-  if (TryGetUncompressBlockFromPersistentCache()) {
-    return Status::OK();
-  }
-  if (TryGetFromPrefetchBuffer()) {
-    if (!status_.ok()) {
-      return status_;
-    }
-  } else if (!TryGetCompressedBlockFromPersistentCache()) {
-    PrepareBufferForBlockFromFile();
-    Status s;
-
-    {
-      PERF_TIMER_GUARD(block_read_time);
-      // Actual file read
-      status_ = file_->Read(handle_.offset(), block_size_ + kBlockTrailerSize,
-                            &slice_, used_buf_);
-    }
-    PERF_COUNTER_ADD(block_read_count, 1);
-    PERF_COUNTER_ADD(block_read_byte, block_size_ + kBlockTrailerSize);
-    if (!status_.ok()) {
-      return status_;
-    }
-
-    if (slice_.size() != block_size_ + kBlockTrailerSize) {
-      return Status::Corruption("truncated block read from " +
-                                file_->file_name() + " offset " +
-                                ToString(handle_.offset()) + ", expected " +
-                                ToString(block_size_ + kBlockTrailerSize) +
-                                " bytes, got " + ToString(slice_.size()));
-    }
-
-    CheckBlockChecksum();
-    if (status_.ok()) {
-      InsertCompressedBlockToPersistentCacheIfNeeded();
-    } else {
-      return status_;
-    }
-  }
-
-  PERF_TIMER_GUARD(block_decompress_time);
-
-  compression_type =
-      static_cast<rocksdb::CompressionType>(slice_.data()[block_size_]);
-
-  if (do_uncompress_ && compression_type != kNoCompression) {
-    // compressed page, uncompress, update cache
-    status_ = UncompressBlockContents(slice_.data(), block_size_, contents_,
-                                      footer_.version(), compression_dict_,
-                                      ioptions_);
-  } else {
-    GetBlockContents();
-  }
-
-  InsertUncompressedBlockToPersistentCacheIfNeeded();
-
-  return status_;
-}
-
-}  // namespace rocksdb

table/block_fetcher.h

@@ -1,75 +0,0 @@
-//  Copyright (c) 2011-present, Facebook, Inc.  All rights reserved.
-//  This source code is licensed under both the GPLv2 (found in the
-//  COPYING file in the root directory) and Apache 2.0 License
-//  (found in the LICENSE.Apache file in the root directory).
-//
-// Copyright (c) 2011 The LevelDB Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file. See the AUTHORS file for names of contributors.
-
-#pragma once
-#include "table/block.h"
-#include "table/format.h"
-
-namespace rocksdb {
-class BlockFetcher {
- public:
-  // Read the block identified by "handle" from "file".
-  // The only relevant option is options.verify_checksums for now.
-  // On failure return non-OK.
-  // On success fill *result and return OK - caller owns *result
-  // @param compression_dict Data for presetting the compression library's
-  //    dictionary.
-  BlockFetcher(RandomAccessFileReader* file,
-               FilePrefetchBuffer* prefetch_buffer, const Footer& footer,
-               const ReadOptions& read_options, const BlockHandle& handle,
-               BlockContents* contents,
-               const ImmutableCFOptions& ioptions,
-               bool do_uncompress, const Slice& compression_dict,
-               const PersistentCacheOptions& cache_options)
-      : file_(file),
-        prefetch_buffer_(prefetch_buffer),
-        footer_(footer),
-        read_options_(read_options),
-        handle_(handle),
-        contents_(contents),
-        ioptions_(ioptions),
-        do_uncompress_(do_uncompress),
-        compression_dict_(compression_dict),
-        cache_options_(cache_options) {}
-  Status ReadBlockContents();
-
- private:
-  static const uint32_t kDefaultStackBufferSize = 5000;
-
-  RandomAccessFileReader* file_;
-  FilePrefetchBuffer* prefetch_buffer_;
-  const Footer& footer_;
-  const ReadOptions read_options_;
-  const BlockHandle& handle_;
-  BlockContents* contents_;
-  const ImmutableCFOptions& ioptions_;
-  bool do_uncompress_;
-  const Slice& compression_dict_;
-  const PersistentCacheOptions& cache_options_;
-  Status status_;
-  Slice slice_;
-  char* used_buf_ = nullptr;
-  size_t block_size_;
-  std::unique_ptr<char[]> heap_buf_;
-  char stack_buf_[kDefaultStackBufferSize];
-  bool got_from_prefetch_buffer_ = false;
-  rocksdb::CompressionType compression_type;
-
-  // return true if found
-  bool TryGetUncompressBlockFromPersistentCache();
-  // return true if found
-  bool TryGetFromPrefetchBuffer();
-  bool TryGetCompressedBlockFromPersistentCache();
-  void PrepareBufferForBlockFromFile();
-  void GetBlockContents();
-  void InsertCompressedBlockToPersistentCacheIfNeeded();
-  void InsertUncompressedBlockToPersistentCacheIfNeeded();
-  void CheckBlockChecksum();
-};
-}  // namespace rocksdb

table/format.cc

@@ -17,7 +17,6 @@
 #include "rocksdb/env.h"
 #include "table/block.h"
 #include "table/block_based_table_reader.h"
-#include "table/block_fetcher.h"
 #include "table/persistent_cache_helper.h"
 #include "util/coding.h"
 #include "util/compression.h"
@@ -41,6 +40,7 @@ extern const uint64_t kPlainTableMagicNumber;
 const uint64_t kLegacyPlainTableMagicNumber = 0;
 const uint64_t kPlainTableMagicNumber = 0;
 #endif
+const uint32_t DefaultStackBufferSize = 5000;
 
 bool ShouldReportDetailedTime(Env* env, Statistics* stats) {
   return env != nullptr && stats != nullptr &&
@@ -264,6 +264,205 @@ Status ReadFooterFromFile(RandomAccessFileReader* file,
   return Status::OK();
 }
 
+// Without anonymous namespace here, we fail the warning -Wmissing-prototypes
+namespace {
+Status CheckBlockChecksum(const ReadOptions& options, const Footer& footer,
+                          const Slice& contents, size_t block_size,
+                          RandomAccessFileReader* file,
+                          const BlockHandle& handle) {
+  Status s;
+  // Check the crc of the type and the block contents
+  if (options.verify_checksums) {
+    const char* data = contents.data();  // Pointer to where Read put the data
+    PERF_TIMER_GUARD(block_checksum_time);
+    uint32_t value = DecodeFixed32(data + block_size + 1);
+    uint32_t actual = 0;
+    switch (footer.checksum()) {
+      case kNoChecksum:
+        break;
+      case kCRC32c:
+        value = crc32c::Unmask(value);
+        actual = crc32c::Value(data, block_size + 1);
+        break;
+      case kxxHash:
+        actual = XXH32(data, static_cast<int>(block_size) + 1, 0);
+        break;
+      default:
+        s = Status::Corruption(
+            "unknown checksum type " + ToString(footer.checksum()) + " in " +
+            file->file_name() + " offset " + ToString(handle.offset()) +
+            " size " + ToString(block_size));
+    }
+    if (s.ok() && actual != value) {
+      s = Status::Corruption(
+          "block checksum mismatch: expected " + ToString(actual) + ", got " +
+          ToString(value) + "  in " + file->file_name() + " offset " +
+          ToString(handle.offset()) + " size " + ToString(block_size));
+    }
+    if (!s.ok()) {
+      return s;
+    }
+  }
+  return s;
+}
+
+// Read a block and check its CRC
+// contents is the result of reading.
+// According to the implementation of file->Read, contents may not point to buf
+Status ReadBlock(RandomAccessFileReader* file, const Footer& footer,
+                 const ReadOptions& options, const BlockHandle& handle,
+                 Slice* contents, /* result of reading */ char* buf) {
+  size_t n = static_cast<size_t>(handle.size());
+  Status s;
+
+  {
+    PERF_TIMER_GUARD(block_read_time);
+    s = file->Read(handle.offset(), n + kBlockTrailerSize, contents, buf);
+  }
+
+  PERF_COUNTER_ADD(block_read_count, 1);
+  PERF_COUNTER_ADD(block_read_byte, n + kBlockTrailerSize);
+
+  if (!s.ok()) {
+    return s;
+  }
+  if (contents->size() != n + kBlockTrailerSize) {
+    return Status::Corruption("truncated block read from " + file->file_name() +
+                              " offset " + ToString(handle.offset()) +
+                              ", expected " + ToString(n + kBlockTrailerSize) +
+                              " bytes, got " + ToString(contents->size()));
+  }
+  return CheckBlockChecksum(options, footer, *contents, n, file, handle);
+}
+
+}  // namespace
+
+Status ReadBlockContents(RandomAccessFileReader* file,
+                         FilePrefetchBuffer* prefetch_buffer,
+                         const Footer& footer, const ReadOptions& read_options,
+                         const BlockHandle& handle, BlockContents* contents,
+                         const ImmutableCFOptions& ioptions,
+                         bool decompression_requested,
+                         const Slice& compression_dict,
+                         const PersistentCacheOptions& cache_options) {
+  Status status;
+  Slice slice;
+  size_t n = static_cast<size_t>(handle.size());
+  std::unique_ptr<char[]> heap_buf;
+  char stack_buf[DefaultStackBufferSize];
+  char* used_buf = nullptr;
+  rocksdb::CompressionType compression_type;
+
+  if (cache_options.persistent_cache &&
+      !cache_options.persistent_cache->IsCompressed()) {
+    status = PersistentCacheHelper::LookupUncompressedPage(cache_options,
+                                                           handle, contents);
+    if (status.ok()) {
+      // uncompressed page is found for the block handle
+      return status;
+    } else {
+      // uncompressed page is not found
+      if (ioptions.info_log && !status.IsNotFound()) {
+        assert(!status.ok());
+        ROCKS_LOG_INFO(ioptions.info_log,
+                       "Error reading from persistent cache. %s",
+                       status.ToString().c_str());
+      }
+    }
+  }
+
+  bool got_from_prefetch_buffer = false;
+  if (prefetch_buffer != nullptr &&
+      prefetch_buffer->TryReadFromCache(
+          handle.offset(),
+          static_cast<size_t>(handle.size()) + kBlockTrailerSize, &slice)) {
+    status =
+        CheckBlockChecksum(read_options, footer, slice,
+                           static_cast<size_t>(handle.size()), file, handle);
+    if (!status.ok()) {
+      return status;
+    }
+    got_from_prefetch_buffer = true;
+    used_buf = const_cast<char*>(slice.data());
+  } else if (cache_options.persistent_cache &&
+             cache_options.persistent_cache->IsCompressed()) {
+    // lookup uncompressed cache mode p-cache
+    status = PersistentCacheHelper::LookupRawPage(
+        cache_options, handle, &heap_buf, n + kBlockTrailerSize);
+  } else {
+    status = Status::NotFound();
+  }
+
+  if (!got_from_prefetch_buffer) {
+    if (status.ok()) {
+      // cache hit
+      used_buf = heap_buf.get();
+      slice = Slice(heap_buf.get(), n);
+    } else {
+      if (ioptions.info_log && !status.IsNotFound()) {
+        assert(!status.ok());
+        ROCKS_LOG_INFO(ioptions.info_log,
+                       "Error reading from persistent cache. %s",
+                       status.ToString().c_str());
+      }
+      // cache miss read from device
+      if (decompression_requested &&
+          n + kBlockTrailerSize < DefaultStackBufferSize) {
+        // If we've got a small enough hunk of data, read it in to the
+        // trivially allocated stack buffer instead of needing a full malloc()
+        used_buf = &stack_buf[0];
+      } else {
+        heap_buf = std::unique_ptr<char[]>(new char[n + kBlockTrailerSize]);
+        used_buf = heap_buf.get();
+      }
+
+      status = ReadBlock(file, footer, read_options, handle, &slice, used_buf);
+      if (status.ok() && read_options.fill_cache &&
+          cache_options.persistent_cache &&
+          cache_options.persistent_cache->IsCompressed()) {
+        // insert to raw cache
+        PersistentCacheHelper::InsertRawPage(cache_options, handle, used_buf,
+                                             n + kBlockTrailerSize);
+      }
+    }
+
+    if (!status.ok()) {
+      return status;
+    }
+  }
+
+  PERF_TIMER_GUARD(block_decompress_time);
+
+  compression_type = static_cast<rocksdb::CompressionType>(slice.data()[n]);
+
+  if (decompression_requested && compression_type != kNoCompression) {
+    // compressed page, uncompress, update cache
+    status = UncompressBlockContents(slice.data(), n, contents,
+                                     footer.version(), compression_dict,
+                                     ioptions);
+  } else if (slice.data() != used_buf) {
+    // the slice content is not the buffer provided
+    *contents = BlockContents(Slice(slice.data(), n), false, compression_type);
+  } else {
+    // page is uncompressed, the buffer either stack or heap provided
+    if (got_from_prefetch_buffer || used_buf == &stack_buf[0]) {
+      heap_buf = std::unique_ptr<char[]>(new char[n]);
+      memcpy(heap_buf.get(), used_buf, n);
+    }
+    *contents = BlockContents(std::move(heap_buf), n, true, compression_type);
+  }
+
+  if (status.ok() && !got_from_prefetch_buffer && read_options.fill_cache &&
+      cache_options.persistent_cache &&
+      !cache_options.persistent_cache->IsCompressed()) {
+    // insert to uncompressed cache
+    PersistentCacheHelper::InsertUncompressedPage(cache_options, handle,
+                                                  *contents);
+  }
+
+  return status;
+}
+
 Status UncompressBlockContentsForCompressionType(
     const char* data, size_t n, BlockContents* contents,
     uint32_t format_version, const Slice& compression_dict,

table/format.h

@@ -22,6 +22,7 @@
 
 namespace rocksdb {
 
+class Block;
 class RandomAccessFile;
 struct ReadOptions;
 

table/meta_blocks.cc

@@ -11,7 +11,6 @@
 #include "rocksdb/table.h"
 #include "rocksdb/table_properties.h"
 #include "table/block.h"
-#include "table/block_fetcher.h"
 #include "table/format.h"
 #include "table/internal_iterator.h"
 #include "table/persistent_cache_helper.h"
@@ -177,13 +176,8 @@ Status ReadProperties(const Slice& handle_value, RandomAccessFileReader* file,
   ReadOptions read_options;
   read_options.verify_checksums = false;
   Status s;
-  Slice compression_dict;
-  PersistentCacheOptions cache_options;
-
-  BlockFetcher block_fetcher(
-      file, prefetch_buffer, footer, read_options, handle, &block_contents,
-      ioptions, false /* decompress */, compression_dict, cache_options);
-  s = block_fetcher.ReadBlockContents();
+  s = ReadBlockContents(file, prefetch_buffer, footer, read_options, handle,
+                        &block_contents, ioptions, false /* decompress */);
 
   if (!s.ok()) {
     return s;
@@ -298,14 +292,9 @@ Status ReadTableProperties(RandomAccessFileReader* file, uint64_t file_size,
   BlockContents metaindex_contents;
   ReadOptions read_options;
   read_options.verify_checksums = false;
-  Slice compression_dict;
-  PersistentCacheOptions cache_options;
-
-  BlockFetcher block_fetcher(
-      file, nullptr /* prefetch_buffer */, footer, read_options,
-      metaindex_handle, &metaindex_contents, ioptions, false /* decompress */,
-      compression_dict, cache_options);
-  s = block_fetcher.ReadBlockContents();
+  s = ReadBlockContents(file, nullptr /* prefetch_buffer */, footer,
+                        read_options, metaindex_handle, &metaindex_contents,
+                        ioptions, false /* decompress */);
   if (!s.ok()) {
     return s;
   }
@@ -361,13 +350,9 @@ Status FindMetaBlock(RandomAccessFileReader* file, uint64_t file_size,
   BlockContents metaindex_contents;
   ReadOptions read_options;
   read_options.verify_checksums = false;
-  Slice compression_dict;
-  PersistentCacheOptions cache_options;
-  BlockFetcher block_fetcher(
-      file, nullptr /* prefetch_buffer */, footer, read_options,
-      metaindex_handle, &metaindex_contents, ioptions,
-      false /* do decompression */, compression_dict, cache_options);
-  s = block_fetcher.ReadBlockContents();
+  s = ReadBlockContents(file, nullptr /* prefetch_buffer */, footer,
+                        read_options, metaindex_handle, &metaindex_contents,
+                        ioptions, false /* do decompression */);
   if (!s.ok()) {
     return s;
   }
@@ -399,14 +384,9 @@ Status ReadMetaBlock(RandomAccessFileReader* file,
   BlockContents metaindex_contents;
   ReadOptions read_options;
   read_options.verify_checksums = false;
-  Slice compression_dict;
-  PersistentCacheOptions cache_options;
-
-  BlockFetcher block_fetcher(file, prefetch_buffer, footer, read_options,
+  status = ReadBlockContents(file, prefetch_buffer, footer, read_options,
                              metaindex_handle, &metaindex_contents, ioptions,
-                             false /* decompress */, compression_dict,
-                             cache_options);
-  status = block_fetcher.ReadBlockContents();
+                             false /* decompress */);
   if (!status.ok()) {
     return status;
   }
@@ -426,10 +406,9 @@ Status ReadMetaBlock(RandomAccessFileReader* file,
   }
 
   // Reading metablock
-  BlockFetcher block_fetcher2(
-      file, prefetch_buffer, footer, read_options, block_handle, contents,
-      ioptions, false /* decompress */, compression_dict, cache_options);
-  return block_fetcher2.ReadBlockContents();
+  return ReadBlockContents(file, prefetch_buffer, footer, read_options,
+                           block_handle, contents, ioptions,
+                           false /* decompress */);
 }
 
 }  // namespace rocksdb

tools/db_bench_tool.cc

@@ -2752,8 +2752,10 @@ void VerifyDBFromDB(std::string& truth_db_name) {
 
   void Crc32c(ThreadState* thread) {
     // Checksum about 500MB of data total
-    const int size = 4096;
-    const char* label = "(4K per op)";
+    const int size = FLAGS_block_size; // use --block_size option for db_bench
+    std::string labels = "(" + ToString(FLAGS_block_size) + " per op)";
+    const char* label = labels.c_str();
+
     std::string data(size, 'x');
     int64_t bytes = 0;
     uint32_t crc = 0;

tools/db_stress.cc

@@ -51,8 +51,6 @@ int main() {
 #include "rocksdb/slice_transform.h"
 #include "rocksdb/statistics.h"
 #include "rocksdb/utilities/db_ttl.h"
-#include "rocksdb/utilities/transaction.h"
-#include "rocksdb/utilities/transaction_db.h"
 #include "rocksdb/write_batch.h"
 #include "util/coding.h"
 #include "util/compression.h"
@@ -352,10 +350,6 @@ DEFINE_uint64(rate_limiter_bytes_per_sec, 0, "Set options.rate_limiter value.");
 DEFINE_bool(rate_limit_bg_reads, false,
             "Use options.rate_limiter on compaction reads");
 
-DEFINE_bool(use_txn, false,
-            "Use TransactionDB. Currently the default write policy is "
-            "TxnDBWritePolicy::WRITE_PREPARED");
-
 // Temporarily disable this to allows it to detect new bugs
 DEFINE_int32(compact_files_one_in, 0,
              "If non-zero, then CompactFiles() will be called one for every N "
@@ -987,22 +981,11 @@ const uint32_t SharedState::SENTINEL = 0xffffffff;
 
 // Per-thread state for concurrent executions of the same benchmark.
 struct ThreadState {
-  uint32_t tid;  // 0..n-1
-  Random rand;   // Has different seeds for different threads
+  uint32_t tid; // 0..n-1
+  Random rand;  // Has different seeds for different threads
   SharedState* shared;
   Stats stats;
-  struct SnapshotState {
-    const Snapshot* snapshot;
-    // The cf from which we did a Get at this stapshot
-    int cf_at;
-    // The key with which we did a Get at this stapshot
-    std::string key;
-    // The status of the Get
-    Status status;
-    // The value of the Get
-    std::string value;
-  };
-  std::queue<std::pair<uint64_t, SnapshotState> > snapshot_queue;
+  std::queue<std::pair<uint64_t, const Snapshot*> > snapshot_queue;
 
   ThreadState(uint32_t index, SharedState* _shared)
       : tid(index), rand(1000 + index + _shared->GetSeed()), shared(_shared) {}
@@ -1126,9 +1109,6 @@ class StressTest {
                                  : NewBloomFilterPolicy(FLAGS_bloom_bits, false)
                            : nullptr),
         db_(nullptr),
-#ifndef ROCKSDB_LITE
-        txn_db_(nullptr),
-#endif
         new_column_family_name_(1),
         num_times_reopened_(0) {
     if (FLAGS_destroy_db_initially) {
@@ -1336,32 +1316,6 @@ class StressTest {
   }
 
  private:
-  Status AssertSame(DB* db, ColumnFamilyHandle* cf,
-                    ThreadState::SnapshotState& snap_state) {
-    ReadOptions ropt;
-    ropt.snapshot = snap_state.snapshot;
-    PinnableSlice exp_v(&snap_state.value);
-    exp_v.PinSelf();
-    Status s;
-    PinnableSlice v;
-    s = db->Get(ropt, cf, snap_state.key, &v);
-    if (!s.ok() && !s.IsNotFound()) {
-      return s;
-    }
-    if (snap_state.status != s) {
-      return Status::Corruption("The snapshot gave inconsistent results: (" +
-                                snap_state.status.ToString() + ") vs. (" +
-                                s.ToString() + ")");
-    }
-    if (s.ok()) {
-      if (exp_v != v) {
-        return Status::Corruption("The snapshot gave inconsistent values: (" +
-                                  exp_v.ToString() + ") vs. (" + v.ToString() +
-                                  ")");
-      }
-    }
-    return Status::OK();
-  }
 
   static void ThreadBody(void* v) {
     ThreadState* thread = reinterpret_cast<ThreadState*>(v);
@@ -1687,32 +1641,6 @@ class StressTest {
     return db_->SetOptions(cfh, opts);
   }
 
-#ifndef ROCKSDB_LITE
-  Status NewTxn(WriteOptions& write_opts, Transaction** txn) {
-    if (!FLAGS_use_txn) {
-      return Status::InvalidArgument("NewTxn when FLAGS_use_txn is not set");
-    }
-    static std::atomic<uint64_t> txn_id = {0};
-    TransactionOptions txn_options;
-    *txn = txn_db_->BeginTransaction(write_opts, txn_options);
-    auto istr = std::to_string(txn_id.fetch_add(1));
-    Status s = (*txn)->SetName("xid" + istr);
-    return s;
-  }
-
-  Status CommitTxn(Transaction* txn) {
-    if (!FLAGS_use_txn) {
-      return Status::InvalidArgument("CommitTxn when FLAGS_use_txn is not set");
-    }
-    Status s = txn->Prepare();
-    if (s.ok()) {
-      s = txn->Commit();
-    }
-    delete txn;
-    return s;
-  }
-#endif
-
   void OperateDb(ThreadState* thread) {
     ReadOptions read_opts(FLAGS_verify_checksum, true);
     WriteOptions write_opts;
@@ -1739,8 +1667,7 @@ class StressTest {
           thread->stats.FinishedSingleOp();
           MutexLock l(thread->shared->GetMutex());
           while (!thread->snapshot_queue.empty()) {
-            db_->ReleaseSnapshot(
-                thread->snapshot_queue.front().second.snapshot);
+            db_->ReleaseSnapshot(thread->snapshot_queue.front().second);
             thread->snapshot_queue.pop();
           }
           thread->shared->IncVotedReopen();
@@ -1854,6 +1781,19 @@ class StressTest {
         }
       }
 #endif                // !ROCKSDB_LITE
+      if (FLAGS_acquire_snapshot_one_in > 0 &&
+          thread->rand.Uniform(FLAGS_acquire_snapshot_one_in) == 0) {
+        thread->snapshot_queue.emplace(
+            std::min(FLAGS_ops_per_thread - 1, i + FLAGS_snapshot_hold_ops),
+            db_->GetSnapshot());
+      }
+      if (!thread->snapshot_queue.empty()) {
+        while (i == thread->snapshot_queue.front().first) {
+          db_->ReleaseSnapshot(thread->snapshot_queue.front().second);
+          thread->snapshot_queue.pop();
+        }
+      }
+
       const double completed_ratio =
           static_cast<double>(i) / FLAGS_ops_per_thread;
       const int64_t base_key = static_cast<int64_t>(
@@ -1867,38 +1807,8 @@ class StressTest {
         l.reset(new MutexLock(
             shared->GetMutexForKey(rand_column_family, rand_key)));
       }
-
       auto column_family = column_families_[rand_column_family];
 
-      if (FLAGS_acquire_snapshot_one_in > 0 &&
-          thread->rand.Uniform(FLAGS_acquire_snapshot_one_in) == 0) {
-        auto snapshot = db_->GetSnapshot();
-        ReadOptions ropt;
-        ropt.snapshot = snapshot;
-        std::string value_at;
-        // When taking a snapshot, we also read a key from that snapshot. We
-        // will later read the same key before releasing the snapshot and verify
-        // that the results are the same.
-        auto status_at = db_->Get(ropt, column_family, key, &value_at);
-        ThreadState::SnapshotState snap_state = {snapshot, rand_column_family,
-                                                 keystr, status_at, value_at};
-        thread->snapshot_queue.emplace(
-            std::min(FLAGS_ops_per_thread - 1, i + FLAGS_snapshot_hold_ops),
-            snap_state);
-      }
-      while (!thread->snapshot_queue.empty() &&
-             i == thread->snapshot_queue.front().first) {
-        auto snap_state = thread->snapshot_queue.front().second;
-        assert(snap_state.snapshot);
-        Status s =
-            AssertSame(db_, column_families_[snap_state.cf_at], snap_state);
-        if (!s.ok()) {
-          VerificationAbort(shared, "Snapshot gave inconsistent state", s);
-        }
-        db_->ReleaseSnapshot(snap_state.snapshot);
-        thread->snapshot_queue.pop();
-      }
-
       int prob_op = thread->rand.Uniform(100);
       if (prob_op >= 0 && prob_op < (int)FLAGS_readpercent) {
         // OPERATION read
@@ -1975,35 +1885,9 @@ class StressTest {
           shared->Put(rand_column_family, rand_key, value_base);
           Status s;
           if (FLAGS_use_merge) {
-            if (!FLAGS_use_txn) {
-              s = db_->Merge(write_opts, column_family, key, v);
-            } else {
-#ifndef ROCKSDB_LITE
-              Transaction* txn;
-              s = NewTxn(write_opts, &txn);
-              if (s.ok()) {
-                s = txn->Merge(column_family, key, v);
-                if (s.ok()) {
-                  s = CommitTxn(txn);
-                }
-              }
-#endif
-            }
+            s = db_->Merge(write_opts, column_family, key, v);
           } else {
-            if (!FLAGS_use_txn) {
-              s = db_->Put(write_opts, column_family, key, v);
-            } else {
-#ifndef ROCKSDB_LITE
-              Transaction* txn;
-              s = NewTxn(write_opts, &txn);
-              if (s.ok()) {
-                s = txn->Put(column_family, key, v);
-                if (s.ok()) {
-                  s = CommitTxn(txn);
-                }
-              }
-#endif
-            }
+            s = db_->Put(write_opts, column_family, key, v);
           }
           if (!s.ok()) {
             fprintf(stderr, "put or merge error: %s\n", s.ToString().c_str());
@@ -2037,21 +1921,7 @@ class StressTest {
           // otherwise.
           if (shared->AllowsOverwrite(rand_column_family, rand_key)) {
             shared->Delete(rand_column_family, rand_key);
-            Status s;
-            if (!FLAGS_use_txn) {
-              s = db_->Delete(write_opts, column_family, key);
-            } else {
-#ifndef ROCKSDB_LITE
-              Transaction* txn;
-              s = NewTxn(write_opts, &txn);
-              if (s.ok()) {
-                s = txn->Delete(column_family, key);
-                if (s.ok()) {
-                  s = CommitTxn(txn);
-                }
-              }
-#endif
-            }
+            Status s = db_->Delete(write_opts, column_family, key);
             thread->stats.AddDeletes(1);
             if (!s.ok()) {
               fprintf(stderr, "delete error: %s\n", s.ToString().c_str());
@@ -2059,21 +1929,7 @@ class StressTest {
             }
           } else {
             shared->SingleDelete(rand_column_family, rand_key);
-            Status s;
-            if (!FLAGS_use_txn) {
-              s = db_->SingleDelete(write_opts, column_family, key);
-            } else {
-#ifndef ROCKSDB_LITE
-              Transaction* txn;
-              s = NewTxn(write_opts, &txn);
-              if (s.ok()) {
-                s = txn->SingleDelete(column_family, key);
-                if (s.ok()) {
-                  s = CommitTxn(txn);
-                }
-              }
-#endif
-            }
+            Status s = db_->SingleDelete(write_opts, column_family, key);
             thread->stats.AddSingleDeletes(1);
             if (!s.ok()) {
               fprintf(stderr, "single delete error: %s\n",
@@ -2211,12 +2067,6 @@ class StressTest {
     }
   }
 
-  void VerificationAbort(SharedState* shared, std::string msg, Status s) const {
-    printf("Verification failed: %s. Status is %s\n", msg.c_str(),
-           s.ToString().c_str());
-    shared->SetVerificationFailure();
-  }
-
   void VerificationAbort(SharedState* shared, std::string msg, int cf,
                          int64_t key) const {
     printf("Verification failed for column family %d key %" PRIi64 ": %s\n", cf, key,
@@ -2288,8 +2138,6 @@ class StressTest {
   void PrintEnv() const {
     fprintf(stdout, "RocksDB version           : %d.%d\n", kMajorVersion,
             kMinorVersion);
-    fprintf(stdout, "TransactionDB             : %s\n",
-            FLAGS_use_txn ? "true" : "false");
     fprintf(stdout, "Column families           : %d\n", FLAGS_column_families);
     if (!FLAGS_test_batches_snapshots) {
       fprintf(stdout, "Clear CFs one in          : %d\n",
@@ -2362,9 +2210,6 @@ class StressTest {
 
   void Open() {
     assert(db_ == nullptr);
-#ifndef ROCKSDB_LITE
-    assert(txn_db_ == nullptr);
-#endif
     BlockBasedTableOptions block_based_options;
     block_based_options.block_cache = cache_;
     block_based_options.block_cache_compressed = compressed_cache_;
@@ -2537,19 +2382,8 @@ class StressTest {
       options_.listeners.emplace_back(
           new DbStressListener(FLAGS_db, options_.db_paths));
       options_.create_missing_column_families = true;
-      if (!FLAGS_use_txn) {
-        s = DB::Open(DBOptions(options_), FLAGS_db, cf_descriptors,
-                     &column_families_, &db_);
-      } else {
-#ifndef ROCKSDB_LITE
-        TransactionDBOptions txn_db_options;
-        // For the moment it is sufficient to test WRITE_PREPARED policy
-        txn_db_options.write_policy = TxnDBWritePolicy::WRITE_PREPARED;
-        s = TransactionDB::Open(options_, txn_db_options, FLAGS_db,
-                                cf_descriptors, &column_families_, &txn_db_);
-        db_ = txn_db_;
-#endif
-      }
+      s = DB::Open(DBOptions(options_), FLAGS_db, cf_descriptors,
+                   &column_families_, &db_);
       assert(!s.ok() || column_families_.size() ==
                             static_cast<size_t>(FLAGS_column_families));
     } else {
@@ -2575,9 +2409,6 @@ class StressTest {
     column_families_.clear();
     delete db_;
     db_ = nullptr;
-#ifndef ROCKSDB_LITE
-    txn_db_ = nullptr;
-#endif
 
     num_times_reopened_++;
     auto now = FLAGS_env->NowMicros();
@@ -2598,9 +2429,6 @@ class StressTest {
   std::shared_ptr<Cache> compressed_cache_;
   std::shared_ptr<const FilterPolicy> filter_policy_;
   DB* db_;
-#ifndef ROCKSDB_LITE
-  TransactionDB* txn_db_;
-#endif
   Options options_;
   std::vector<ColumnFamilyHandle*> column_families_;
   std::vector<std::string> column_family_names_;

util/crc32c.cc

@@ -9,12 +9,11 @@
 //
 // A portable implementation of crc32c, optimized to handle
 // four bytes at a time.
-
 #include "util/crc32c.h"
-
 #include <stdint.h>
 #ifdef HAVE_SSE42
 #include <nmmintrin.h>
+#include <wmmintrin.h>
 #endif
 #include "util/coding.h"
 
@@ -352,6 +351,7 @@ static inline void Fast_CRC32(uint64_t* l, uint8_t const **p) {
 
 template<void (*CRC32)(uint64_t*, uint8_t const**)>
 uint32_t ExtendImpl(uint32_t crc, const char* buf, size_t size) {
+
   const uint8_t *p = reinterpret_cast<const uint8_t *>(buf);
   const uint8_t *e = p + size;
   uint64_t l = crc ^ 0xffffffffu;
@@ -395,13 +395,14 @@ uint32_t ExtendImpl(uint32_t crc, const char* buf, size_t size) {
 
 // Detect if SS42 or not.
 #ifndef HAVE_POWER8
+
 static bool isSSE42() {
 #ifndef HAVE_SSE42
   return false;
 #elif defined(__GNUC__) && defined(__x86_64__) && !defined(IOS_CROSS_COMPILE)
   uint32_t c_;
   __asm__("cpuid" : "=c"(c_) : "a"(1) : "ebx", "edx");
-  return c_ & (1U << 20);  // copied from CpuId.h in Folly.
+  return c_ & (1U << 20);  // copied from CpuId.h in Folly. Test SSE42
 #elif defined(_WIN64)
   int info[4];
   __cpuidex(info, 0x00000001, 0);
@@ -410,7 +411,26 @@ static bool isSSE42() {
   return false;
 #endif
 }
+
+static bool isPCLMULQDQ() {
+#ifndef HAVE_SSE42
+// in build_detect_platform we set this macro when both SSE42 and PCLMULQDQ are
+// supported by compiler
+  return false;
+#elif defined(__GNUC__) && defined(__x86_64__) && !defined(IOS_CROSS_COMPILE)
+  uint32_t c_;
+  __asm__("cpuid" : "=c"(c_) : "a"(1) : "ebx", "edx");
+  return c_ & (1U << 1);  // PCLMULQDQ is in bit 1 (not bit 0)
+#elif defined(_WIN64)
+  int info[4];
+  __cpuidex(info, 0x00000001, 0);
+  return (info[2] & ((int)1 << 1)) != 0;
+#else
+  return false;
 #endif
+}
+
+#endif  // HAVE_POWER8
 
 typedef uint32_t (*Function)(uint32_t, const char*, size_t);
 
@@ -440,13 +460,6 @@ static bool isAltiVec() {
 }
 #endif
 
-static inline Function Choose_Extend() {
-#ifndef HAVE_POWER8
-  return isSSE42() ? ExtendImpl<Fast_CRC32> : ExtendImpl<Slow_CRC32>;
-#else
-  return isAltiVec() ? ExtendPPCImpl : ExtendImpl<Slow_CRC32>;
-#endif
-}
 
 std::string IsFastCrc32Supported() {
   bool has_fast_crc = false;
@@ -475,11 +488,572 @@ std::string IsFastCrc32Supported() {
   return fast_zero_msg;
 }
 
-static Function ChosenExtend = Choose_Extend();
 
+/*
+ * Copyright 2016 Ferry Toth, Exalon Delft BV, The Netherlands
+ *  This software is provided 'as-is', without any express or implied
+ * warranty.  In no event will the author be held liable for any damages
+ * arising from the use of this software.
+ *  Permission is granted to anyone to use this software for any purpose,
+ * including commercial applications, and to alter it and redistribute it
+ * freely, subject to the following restrictions:
+ *  1. The origin of this software must not be misrepresented; you must not
+ *   claim that you wrote the original software. If you use this software
+ *   in a product, an acknowledgment in the product documentation would be
+ *   appreciated but is not required.
+ * 2. Altered source versions must be plainly marked as such, and must not be
+ *   misrepresented as being the original software.
+ * 3. This notice may not be removed or altered from any source distribution.
+ *  Ferry Toth
+ * ftoth@exalondelft.nl
+ *
+ * https://github.com/htot/crc32c
+ *
+ * Modified by Facebook
+ *
+ * Original intel whitepaper:
+ * "Fast CRC Computation for iSCSI Polynomial Using CRC32 Instruction"
+ * https://www.intel.com/content/dam/www/public/us/en/documents/white-papers/crc-iscsi-polynomial-crc32-instruction-paper.pdf
+ *
+ * This version is from the folly library, created by Dave Watson <davejwatson@fb.com>
+ *
+*/
+#if defined HAVE_SSE42 && defined HAVE_PCLMUL
+
+#define CRCtriplet(crc, buf, offset)                  \
+  crc##0 = _mm_crc32_u64(crc##0, *(buf##0 + offset)); \
+  crc##1 = _mm_crc32_u64(crc##1, *(buf##1 + offset)); \
+  crc##2 = _mm_crc32_u64(crc##2, *(buf##2 + offset));
+
+#define CRCduplet(crc, buf, offset)                   \
+  crc##0 = _mm_crc32_u64(crc##0, *(buf##0 + offset)); \
+  crc##1 = _mm_crc32_u64(crc##1, *(buf##1 + offset));
+
+#define CRCsinglet(crc, buf, offset)                    \
+  crc = _mm_crc32_u64(crc, *(uint64_t*)(buf + offset));
+
+
+// Numbers taken directly from intel whitepaper.
+// clang-format off
+const uint64_t clmul_constants[] = {
+    0x14cd00bd6, 0x105ec76f0, 0x0ba4fc28e, 0x14cd00bd6,
+    0x1d82c63da, 0x0f20c0dfe, 0x09e4addf8, 0x0ba4fc28e,
+    0x039d3b296, 0x1384aa63a, 0x102f9b8a2, 0x1d82c63da,
+    0x14237f5e6, 0x01c291d04, 0x00d3b6092, 0x09e4addf8,
+    0x0c96cfdc0, 0x0740eef02, 0x18266e456, 0x039d3b296,
+    0x0daece73e, 0x0083a6eec, 0x0ab7aff2a, 0x102f9b8a2,
+    0x1248ea574, 0x1c1733996, 0x083348832, 0x14237f5e6,
+    0x12c743124, 0x02ad91c30, 0x0b9e02b86, 0x00d3b6092,
+    0x018b33a4e, 0x06992cea2, 0x1b331e26a, 0x0c96cfdc0,
+    0x17d35ba46, 0x07e908048, 0x1bf2e8b8a, 0x18266e456,
+    0x1a3e0968a, 0x11ed1f9d8, 0x0ce7f39f4, 0x0daece73e,
+    0x061d82e56, 0x0f1d0f55e, 0x0d270f1a2, 0x0ab7aff2a,
+    0x1c3f5f66c, 0x0a87ab8a8, 0x12ed0daac, 0x1248ea574,
+    0x065863b64, 0x08462d800, 0x11eef4f8e, 0x083348832,
+    0x1ee54f54c, 0x071d111a8, 0x0b3e32c28, 0x12c743124,
+    0x0064f7f26, 0x0ffd852c6, 0x0dd7e3b0c, 0x0b9e02b86,
+    0x0f285651c, 0x0dcb17aa4, 0x010746f3c, 0x018b33a4e,
+    0x1c24afea4, 0x0f37c5aee, 0x0271d9844, 0x1b331e26a,
+    0x08e766a0c, 0x06051d5a2, 0x093a5f730, 0x17d35ba46,
+    0x06cb08e5c, 0x11d5ca20e, 0x06b749fb2, 0x1bf2e8b8a,
+    0x1167f94f2, 0x021f3d99c, 0x0cec3662e, 0x1a3e0968a,
+    0x19329634a, 0x08f158014, 0x0e6fc4e6a, 0x0ce7f39f4,
+    0x08227bb8a, 0x1a5e82106, 0x0b0cd4768, 0x061d82e56,
+    0x13c2b89c4, 0x188815ab2, 0x0d7a4825c, 0x0d270f1a2,
+    0x10f5ff2ba, 0x105405f3e, 0x00167d312, 0x1c3f5f66c,
+    0x0f6076544, 0x0e9adf796, 0x026f6a60a, 0x12ed0daac,
+    0x1a2adb74e, 0x096638b34, 0x19d34af3a, 0x065863b64,
+    0x049c3cc9c, 0x1e50585a0, 0x068bce87a, 0x11eef4f8e,
+    0x1524fa6c6, 0x19f1c69dc, 0x16cba8aca, 0x1ee54f54c,
+    0x042d98888, 0x12913343e, 0x1329d9f7e, 0x0b3e32c28,
+    0x1b1c69528, 0x088f25a3a, 0x02178513a, 0x0064f7f26,
+    0x0e0ac139e, 0x04e36f0b0, 0x0170076fa, 0x0dd7e3b0c,
+    0x141a1a2e2, 0x0bd6f81f8, 0x16ad828b4, 0x0f285651c,
+    0x041d17b64, 0x19425cbba, 0x1fae1cc66, 0x010746f3c,
+    0x1a75b4b00, 0x18db37e8a, 0x0f872e54c, 0x1c24afea4,
+    0x01e41e9fc, 0x04c144932, 0x086d8e4d2, 0x0271d9844,
+    0x160f7af7a, 0x052148f02, 0x05bb8f1bc, 0x08e766a0c,
+    0x0a90fd27a, 0x0a3c6f37a, 0x0b3af077a, 0x093a5f730,
+    0x04984d782, 0x1d22c238e, 0x0ca6ef3ac, 0x06cb08e5c,
+    0x0234e0b26, 0x063ded06a, 0x1d88abd4a, 0x06b749fb2,
+    0x04597456a, 0x04d56973c, 0x0e9e28eb4, 0x1167f94f2,
+    0x07b3ff57a, 0x19385bf2e, 0x0c9c8b782, 0x0cec3662e,
+    0x13a9cba9e, 0x0e417f38a, 0x093e106a4, 0x19329634a,
+    0x167001a9c, 0x14e727980, 0x1ddffc5d4, 0x0e6fc4e6a,
+    0x00df04680, 0x0d104b8fc, 0x02342001e, 0x08227bb8a,
+    0x00a2a8d7e, 0x05b397730, 0x168763fa6, 0x0b0cd4768,
+    0x1ed5a407a, 0x0e78eb416, 0x0d2c3ed1a, 0x13c2b89c4,
+    0x0995a5724, 0x1641378f0, 0x19b1afbc4, 0x0d7a4825c,
+    0x109ffedc0, 0x08d96551c, 0x0f2271e60, 0x10f5ff2ba,
+    0x00b0bf8ca, 0x00bf80dd2, 0x123888b7a, 0x00167d312,
+    0x1e888f7dc, 0x18dcddd1c, 0x002ee03b2, 0x0f6076544,
+    0x183e8d8fe, 0x06a45d2b2, 0x133d7a042, 0x026f6a60a,
+    0x116b0f50c, 0x1dd3e10e8, 0x05fabe670, 0x1a2adb74e,
+    0x130004488, 0x0de87806c, 0x000bcf5f6, 0x19d34af3a,
+    0x18f0c7078, 0x014338754, 0x017f27698, 0x049c3cc9c,
+    0x058ca5f00, 0x15e3e77ee, 0x1af900c24, 0x068bce87a,
+    0x0b5cfca28, 0x0dd07448e, 0x0ded288f8, 0x1524fa6c6,
+    0x059f229bc, 0x1d8048348, 0x06d390dec, 0x16cba8aca,
+    0x037170390, 0x0a3e3e02c, 0x06353c1cc, 0x042d98888,
+    0x0c4584f5c, 0x0d73c7bea, 0x1f16a3418, 0x1329d9f7e,
+    0x0531377e2, 0x185137662, 0x1d8d9ca7c, 0x1b1c69528,
+    0x0b25b29f2, 0x18a08b5bc, 0x19fb2a8b0, 0x02178513a,
+    0x1a08fe6ac, 0x1da758ae0, 0x045cddf4e, 0x0e0ac139e,
+    0x1a91647f2, 0x169cf9eb0, 0x1a0f717c4, 0x0170076fa,
+};
+
+// Compute the crc32c value for buffer smaller than 8
+inline void align_to_8(
+    size_t len,
+    uint64_t& crc0, // crc so far, updated on return
+    const unsigned char*& next) { // next data pointer, updated on return
+  uint32_t crc32bit = static_cast<uint32_t>(crc0);
+  if (len & 0x04) {
+    crc32bit = _mm_crc32_u32(crc32bit, *(uint32_t*)next);
+    next += sizeof(uint32_t);
+  }
+  if (len & 0x02) {
+    crc32bit = _mm_crc32_u16(crc32bit, *(uint16_t*)next);
+    next += sizeof(uint16_t);
+  }
+  if (len & 0x01) {
+    crc32bit = _mm_crc32_u8(crc32bit, *(next));
+    next++;
+  }
+  crc0 = crc32bit;
+}
+
+//
+// CombineCRC performs pclmulqdq multiplication of 2 partial CRC's and a well
+// chosen constant and xor's these with the remaining CRC.
+//
+inline uint64_t CombineCRC(
+    size_t block_size,
+    uint64_t crc0,
+    uint64_t crc1,
+    uint64_t crc2,
+    const uint64_t* next2) {
+  const auto multiplier =
+      *(reinterpret_cast<const __m128i*>(clmul_constants) + block_size - 1);
+  const auto crc0_xmm = _mm_set_epi64x(0, crc0);
+  const auto res0 = _mm_clmulepi64_si128(crc0_xmm, multiplier, 0x00);
+  const auto crc1_xmm = _mm_set_epi64x(0, crc1);
+  const auto res1 = _mm_clmulepi64_si128(crc1_xmm, multiplier, 0x10);
+  const auto res = _mm_xor_si128(res0, res1);
+  crc0 = _mm_cvtsi128_si64(res);
+  crc0 = crc0 ^ *((uint64_t*)next2 - 1);
+  crc2 = _mm_crc32_u64(crc2, crc0);
+  return crc2;
+}
+
+// Compute CRC-32C using the Intel hardware instruction.
+uint32_t crc32c_3way(uint32_t crc, const char* buf, size_t len) {
+  const unsigned char* next = (const unsigned char*)buf;
+  uint64_t count;
+  uint64_t crc0, crc1, crc2;
+  crc0 = crc ^ 0xffffffffu;
+
+
+  if (len >= 8) {
+    // if len > 216 then align and use triplets
+    if (len > 216) {
+      {
+        // Work on the bytes (< 8) before the first 8-byte alignment addr starts
+        uint64_t align_bytes = (8 - (uintptr_t)next) & 7;
+        len -= align_bytes;
+        align_to_8(align_bytes, crc0, next);
+      }
+
+      // Now work on the remaining blocks
+      count = len / 24; // number of triplets
+      len %= 24; // bytes remaining
+      uint64_t n = count >> 7; // #blocks = first block + full blocks
+      uint64_t block_size = count & 127;
+      if (block_size == 0) {
+        block_size = 128;
+      } else {
+        n++;
+      }
+      // points to the first byte of the next block
+      const uint64_t* next0 = (uint64_t*)next + block_size;
+      const uint64_t* next1 = next0 + block_size;
+      const uint64_t* next2 = next1 + block_size;
+
+      crc1 = crc2 = 0;
+      // Use Duff's device, a for() loop inside a switch()
+      // statement. This needs to execute at least once, round len
+      // down to nearest triplet multiple
+      switch (block_size) {
+        case 128:
+          do {
+            // jumps here for a full block of len 128
+            CRCtriplet(crc, next, -128);
+            case 127:
+              // jumps here or below for the first block smaller
+              CRCtriplet(crc, next, -127);
+            case 126:
+              CRCtriplet(crc, next, -126); // than 128
+            case 125:
+              CRCtriplet(crc, next, -125);
+            case 124:
+              CRCtriplet(crc, next, -124);
+            case 123:
+              CRCtriplet(crc, next, -123);
+            case 122:
+              CRCtriplet(crc, next, -122);
+            case 121:
+              CRCtriplet(crc, next, -121);
+            case 120:
+              CRCtriplet(crc, next, -120);
+            case 119:
+              CRCtriplet(crc, next, -119);
+            case 118:
+              CRCtriplet(crc, next, -118);
+            case 117:
+              CRCtriplet(crc, next, -117);
+            case 116:
+              CRCtriplet(crc, next, -116);
+            case 115:
+              CRCtriplet(crc, next, -115);
+            case 114:
+              CRCtriplet(crc, next, -114);
+            case 113:
+              CRCtriplet(crc, next, -113);
+            case 112:
+              CRCtriplet(crc, next, -112);
+            case 111:
+              CRCtriplet(crc, next, -111);
+            case 110:
+              CRCtriplet(crc, next, -110);
+            case 109:
+              CRCtriplet(crc, next, -109);
+            case 108:
+              CRCtriplet(crc, next, -108);
+            case 107:
+              CRCtriplet(crc, next, -107);
+            case 106:
+              CRCtriplet(crc, next, -106);
+            case 105:
+              CRCtriplet(crc, next, -105);
+            case 104:
+              CRCtriplet(crc, next, -104);
+            case 103:
+              CRCtriplet(crc, next, -103);
+            case 102:
+              CRCtriplet(crc, next, -102);
+            case 101:
+              CRCtriplet(crc, next, -101);
+            case 100:
+              CRCtriplet(crc, next, -100);
+            case 99:
+              CRCtriplet(crc, next, -99);
+            case 98:
+              CRCtriplet(crc, next, -98);
+            case 97:
+              CRCtriplet(crc, next, -97);
+            case 96:
+              CRCtriplet(crc, next, -96);
+            case 95:
+              CRCtriplet(crc, next, -95);
+            case 94:
+              CRCtriplet(crc, next, -94);
+            case 93:
+              CRCtriplet(crc, next, -93);
+            case 92:
+              CRCtriplet(crc, next, -92);
+            case 91:
+              CRCtriplet(crc, next, -91);
+            case 90:
+              CRCtriplet(crc, next, -90);
+            case 89:
+              CRCtriplet(crc, next, -89);
+            case 88:
+              CRCtriplet(crc, next, -88);
+            case 87:
+              CRCtriplet(crc, next, -87);
+            case 86:
+              CRCtriplet(crc, next, -86);
+            case 85:
+              CRCtriplet(crc, next, -85);
+            case 84:
+              CRCtriplet(crc, next, -84);
+            case 83:
+              CRCtriplet(crc, next, -83);
+            case 82:
+              CRCtriplet(crc, next, -82);
+            case 81:
+              CRCtriplet(crc, next, -81);
+            case 80:
+              CRCtriplet(crc, next, -80);
+            case 79:
+              CRCtriplet(crc, next, -79);
+            case 78:
+              CRCtriplet(crc, next, -78);
+            case 77:
+              CRCtriplet(crc, next, -77);
+            case 76:
+              CRCtriplet(crc, next, -76);
+            case 75:
+              CRCtriplet(crc, next, -75);
+            case 74:
+              CRCtriplet(crc, next, -74);
+            case 73:
+              CRCtriplet(crc, next, -73);
+            case 72:
+              CRCtriplet(crc, next, -72);
+            case 71:
+              CRCtriplet(crc, next, -71);
+            case 70:
+              CRCtriplet(crc, next, -70);
+            case 69:
+              CRCtriplet(crc, next, -69);
+            case 68:
+              CRCtriplet(crc, next, -68);
+            case 67:
+              CRCtriplet(crc, next, -67);
+            case 66:
+              CRCtriplet(crc, next, -66);
+            case 65:
+              CRCtriplet(crc, next, -65);
+            case 64:
+              CRCtriplet(crc, next, -64);
+            case 63:
+              CRCtriplet(crc, next, -63);
+            case 62:
+              CRCtriplet(crc, next, -62);
+            case 61:
+              CRCtriplet(crc, next, -61);
+            case 60:
+              CRCtriplet(crc, next, -60);
+            case 59:
+              CRCtriplet(crc, next, -59);
+            case 58:
+              CRCtriplet(crc, next, -58);
+            case 57:
+              CRCtriplet(crc, next, -57);
+            case 56:
+              CRCtriplet(crc, next, -56);
+            case 55:
+              CRCtriplet(crc, next, -55);
+            case 54:
+              CRCtriplet(crc, next, -54);
+            case 53:
+              CRCtriplet(crc, next, -53);
+            case 52:
+              CRCtriplet(crc, next, -52);
+            case 51:
+              CRCtriplet(crc, next, -51);
+            case 50:
+              CRCtriplet(crc, next, -50);
+            case 49:
+              CRCtriplet(crc, next, -49);
+            case 48:
+              CRCtriplet(crc, next, -48);
+            case 47:
+              CRCtriplet(crc, next, -47);
+            case 46:
+              CRCtriplet(crc, next, -46);
+            case 45:
+              CRCtriplet(crc, next, -45);
+            case 44:
+              CRCtriplet(crc, next, -44);
+            case 43:
+              CRCtriplet(crc, next, -43);
+            case 42:
+              CRCtriplet(crc, next, -42);
+            case 41:
+              CRCtriplet(crc, next, -41);
+            case 40:
+              CRCtriplet(crc, next, -40);
+            case 39:
+              CRCtriplet(crc, next, -39);
+            case 38:
+              CRCtriplet(crc, next, -38);
+            case 37:
+              CRCtriplet(crc, next, -37);
+            case 36:
+              CRCtriplet(crc, next, -36);
+            case 35:
+              CRCtriplet(crc, next, -35);
+            case 34:
+              CRCtriplet(crc, next, -34);
+            case 33:
+              CRCtriplet(crc, next, -33);
+            case 32:
+              CRCtriplet(crc, next, -32);
+            case 31:
+              CRCtriplet(crc, next, -31);
+            case 30:
+              CRCtriplet(crc, next, -30);
+            case 29:
+              CRCtriplet(crc, next, -29);
+            case 28:
+              CRCtriplet(crc, next, -28);
+            case 27:
+              CRCtriplet(crc, next, -27);
+            case 26:
+              CRCtriplet(crc, next, -26);
+            case 25:
+              CRCtriplet(crc, next, -25);
+            case 24:
+              CRCtriplet(crc, next, -24);
+            case 23:
+              CRCtriplet(crc, next, -23);
+            case 22:
+              CRCtriplet(crc, next, -22);
+            case 21:
+              CRCtriplet(crc, next, -21);
+            case 20:
+              CRCtriplet(crc, next, -20);
+            case 19:
+              CRCtriplet(crc, next, -19);
+            case 18:
+              CRCtriplet(crc, next, -18);
+            case 17:
+              CRCtriplet(crc, next, -17);
+            case 16:
+              CRCtriplet(crc, next, -16);
+            case 15:
+              CRCtriplet(crc, next, -15);
+            case 14:
+              CRCtriplet(crc, next, -14);
+            case 13:
+              CRCtriplet(crc, next, -13);
+            case 12:
+              CRCtriplet(crc, next, -12);
+            case 11:
+              CRCtriplet(crc, next, -11);
+            case 10:
+              CRCtriplet(crc, next, -10);
+            case 9:
+              CRCtriplet(crc, next, -9);
+            case 8:
+              CRCtriplet(crc, next, -8);
+            case 7:
+              CRCtriplet(crc, next, -7);
+            case 6:
+              CRCtriplet(crc, next, -6);
+            case 5:
+              CRCtriplet(crc, next, -5);
+            case 4:
+              CRCtriplet(crc, next, -4);
+            case 3:
+              CRCtriplet(crc, next, -3);
+            case 2:
+              CRCtriplet(crc, next, -2);
+            case 1:
+              CRCduplet(crc, next, -1); // the final triplet is actually only 2
+              //{ CombineCRC(); }
+              crc0 = CombineCRC(block_size, crc0, crc1, crc2, next2);
+              if (--n > 0) {
+                crc1 = crc2 = 0;
+                block_size = 128;
+                // points to the first byte of the next block
+                next0 = next2 + 128;
+                next1 = next0 + 128; // from here on all blocks are 128 long
+                next2 = next1 + 128;
+              }
+            case 0:;
+          } while (n > 0);
+      }
+      next = (const unsigned char*)next2;
+    }
+    uint64_t count2 = len >> 3; // 216 of less bytes is 27 or less singlets
+    len = len & 7;
+    next += (count2 * 8);
+    switch (count2) {
+      case 27:
+        CRCsinglet(crc0, next, -27 * 8);
+      case 26:
+        CRCsinglet(crc0, next, -26 * 8);
+      case 25:
+        CRCsinglet(crc0, next, -25 * 8);
+      case 24:
+        CRCsinglet(crc0, next, -24 * 8);
+      case 23:
+        CRCsinglet(crc0, next, -23 * 8);
+      case 22:
+        CRCsinglet(crc0, next, -22 * 8);
+      case 21:
+        CRCsinglet(crc0, next, -21 * 8);
+      case 20:
+        CRCsinglet(crc0, next, -20 * 8);
+      case 19:
+        CRCsinglet(crc0, next, -19 * 8);
+      case 18:
+        CRCsinglet(crc0, next, -18 * 8);
+      case 17:
+        CRCsinglet(crc0, next, -17 * 8);
+      case 16:
+        CRCsinglet(crc0, next, -16 * 8);
+      case 15:
+        CRCsinglet(crc0, next, -15 * 8);
+      case 14:
+        CRCsinglet(crc0, next, -14 * 8);
+      case 13:
+        CRCsinglet(crc0, next, -13 * 8);
+      case 12:
+        CRCsinglet(crc0, next, -12 * 8);
+      case 11:
+        CRCsinglet(crc0, next, -11 * 8);
+      case 10:
+        CRCsinglet(crc0, next, -10 * 8);
+      case 9:
+        CRCsinglet(crc0, next, -9 * 8);
+      case 8:
+        CRCsinglet(crc0, next, -8 * 8);
+      case 7:
+        CRCsinglet(crc0, next, -7 * 8);
+      case 6:
+        CRCsinglet(crc0, next, -6 * 8);
+      case 5:
+        CRCsinglet(crc0, next, -5 * 8);
+      case 4:
+        CRCsinglet(crc0, next, -4 * 8);
+      case 3:
+        CRCsinglet(crc0, next, -3 * 8);
+      case 2:
+        CRCsinglet(crc0, next, -2 * 8);
+      case 1:
+        CRCsinglet(crc0, next, -1 * 8);
+      case 0:;
+    }
+  }
+  {
+    align_to_8(len, crc0, next);
+    return (uint32_t)crc0 ^ 0xffffffffu;
+  }
+}
+
+#endif //HAVE_SSE42 && HAVE_PCLMUL
+
+static inline Function Choose_Extend() {
+#ifndef HAVE_POWER8
+  if (isSSE42()) {
+    if (isPCLMULQDQ()) {
+#if defined HAVE_SSE42  && defined HAVE_PCLMUL && !defined NO_THREEWAY_CRC32C
+      return crc32c_3way;
+#else
+    return ExtendImpl<Fast_CRC32>; // Fast_CRC32 will check HAVE_SSE42 itself
+#endif
+    }
+    else {  // no runtime PCLMULQDQ support but has SSE42 support
+      return ExtendImpl<Fast_CRC32>;
+    }
+  } // end of isSSE42()
+  else {
+    return ExtendImpl<Slow_CRC32>;
+  }
+#else  //HAVE_POWER8
+  return isAltiVec() ? ExtendPPCImpl : ExtendImpl<Slow_CRC32>;
+#endif
+}
+
+static Function ChosenExtend = Choose_Extend();
 uint32_t Extend(uint32_t crc, const char* buf, size_t size) {
   return ChosenExtend(crc, buf, size);
 }
 
+
 }  // namespace crc32c
 }  // namespace rocksdb

util/crc32c_test.cc

@@ -6,7 +6,6 @@
 // Copyright (c) 2011 The LevelDB Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file. See the AUTHORS file for names of contributors.
-
 #include "util/crc32c.h"
 #include "util/testharness.h"
 
@@ -15,7 +14,57 @@ namespace crc32c {
 
 class CRC { };
 
+
+// Tests for 3-way crc32c algorithm. We need these tests because it uses
+// different lookup tables than the original Fast_CRC32
+const unsigned int BUFFER_SIZE = 512 * 1024 * sizeof(uint64_t);
+char buffer[BUFFER_SIZE];
+
+struct ExpectedResult {
+  size_t offset;
+  size_t length;
+  uint32_t crc32c;
+};
+
+ExpectedResult expectedResults[] = {
+    // Zero-byte input
+    { 0, 0, ~0U },
+    // Small aligned inputs to test special cases in SIMD implementations
+    { 8, 1, 1543413366 },
+    { 8, 2, 523493126 },
+    { 8, 3, 1560427360 },
+    { 8, 4, 3422504776 },
+    { 8, 5, 447841138 },
+    { 8, 6, 3910050499 },
+    { 8, 7, 3346241981 },
+    // Small unaligned inputs
+    { 9, 1, 3855826643 },
+    { 10, 2, 560880875 },
+    { 11, 3, 1479707779 },
+    { 12, 4, 2237687071 },
+    { 13, 5, 4063855784 },
+    { 14, 6, 2553454047 },
+    { 15, 7, 1349220140 },
+    // Larger inputs to test leftover chunks at the end of aligned blocks
+    { 8, 8, 627613930 },
+    { 8, 9, 2105929409 },
+    { 8, 10, 2447068514 },
+    { 8, 11, 863807079 },
+    { 8, 12, 292050879 },
+    { 8, 13, 1411837737 },
+    { 8, 14, 2614515001 },
+    { 8, 15, 3579076296 },
+    { 8, 16, 2897079161 },
+    { 8, 17, 675168386 },
+    // // Much larger inputs
+    { 0, BUFFER_SIZE, 2096790750 },
+    { 1, BUFFER_SIZE / 2, 3854797577 },
+
+};
+
 TEST(CRC, StandardResults) {
+
+  // Original Fast_CRC32 tests.
   // From rfc3720 section B.4.
   char buf[32];
 
@@ -50,6 +99,24 @@ TEST(CRC, StandardResults) {
     0x00, 0x00, 0x00, 0x00,
   };
   ASSERT_EQ(0xd9963a56, Value(reinterpret_cast<char*>(data), sizeof(data)));
+
+  // 3-Way Crc32c tests ported from folly.
+  // Test 1: single computation
+  for (auto expected : expectedResults) {
+    uint32_t result = Value(buffer + expected.offset, expected.length);
+    EXPECT_EQ(~expected.crc32c, result);
+  }
+
+  // Test 2: stitching two computations
+  for (auto expected : expectedResults) {
+    size_t partialLength = expected.length / 2;
+    uint32_t partialChecksum = Value(buffer + expected.offset, partialLength);
+    uint32_t result = Extend(partialChecksum,
+        buffer + expected.offset + partialLength,
+        expected.length - partialLength);
+    EXPECT_EQ(~expected.crc32c, result);
+  }
+
 }
 
 TEST(CRC, Values) {
@@ -72,7 +139,36 @@ TEST(CRC, Mask) {
 }  // namespace crc32c
 }  // namespace rocksdb
 
+// copied from folly
+const uint64_t FNV_64_HASH_START = 14695981039346656037ULL;
+inline uint64_t fnv64_buf(const void* buf,
+                          size_t n,
+                          uint64_t hash = FNV_64_HASH_START) {
+  // forcing signed char, since other platforms can use unsigned
+  const signed char* char_buf = reinterpret_cast<const signed char*>(buf);
+
+  for (size_t i = 0; i < n; ++i) {
+    hash += (hash << 1) + (hash << 4) + (hash << 5) + (hash << 7) +
+      (hash << 8) + (hash << 40);
+    hash ^= char_buf[i];
+  }
+  return hash;
+}
+
 int main(int argc, char** argv) {
   ::testing::InitGoogleTest(&argc, argv);
+
+  // Populate a buffer with a deterministic pattern
+  // on which to compute checksums
+
+  const uint8_t* src = (uint8_t*)rocksdb::crc32c::buffer;
+  uint64_t* dst = (uint64_t*)rocksdb::crc32c::buffer;
+  const uint64_t* end = (const uint64_t*)(rocksdb::crc32c::buffer + rocksdb::crc32c::BUFFER_SIZE);
+  *dst++ = 0;
+  while (dst < end) {
+    *dst++ = fnv64_buf((const char*)src, sizeof(uint64_t));
+    src += sizeof(uint64_t);
+  }
+
   return RUN_ALL_TESTS();
 }

util/delete_scheduler.cc

@@ -151,7 +151,6 @@ Status DeleteScheduler::MarkAsTrash(const std::string& file_path,
   Status s;
   if (DeleteScheduler::IsTrashFile(file_path)) {
     // This is already a trash file
-    *trash_file = file_path;
     return s;
   }
 

util/transaction_test_util.cc

@@ -80,7 +80,6 @@ Status RandomTransactionInserter::DBGet(
   // four digits and zero end char
   char prefix_buf[5];
   // Pad prefix appropriately so we can iterate over each set
-  assert(set_i + 1 <= 9999);
   snprintf(prefix_buf, sizeof(prefix_buf), "%.4u", set_i + 1);
   // key format:  [SET#][random#]
   std::string skey = ToString(ikey);
@@ -125,7 +124,7 @@ bool RandomTransactionInserter::DoInsert(DB* db, Transaction* txn,
   bool unexpected_error = false;
 
   std::vector<uint16_t> set_vec(num_sets_);
-  std::iota(set_vec.begin(), set_vec.end(), 0);
+  std::iota(set_vec.begin(), set_vec.end(), static_cast<uint16_t>(0));
   std::random_shuffle(set_vec.begin(), set_vec.end(),
                       [&](uint64_t r) { return rand_->Uniform(r); });
   // For each set, pick a key at random and increment it
@@ -255,7 +254,7 @@ Status RandomTransactionInserter::Verify(DB* db, uint16_t num_sets,
   }
 
   std::vector<uint16_t> set_vec(num_sets);
-  std::iota(set_vec.begin(), set_vec.end(), 0);
+  std::iota(set_vec.begin(), set_vec.end(), static_cast<uint16_t>(0));
   if (rand) {
     std::random_shuffle(set_vec.begin(), set_vec.end(),
                         [&](uint64_t r) { return rand->Uniform(r); });
@@ -264,7 +263,6 @@ Status RandomTransactionInserter::Verify(DB* db, uint16_t num_sets,
   for (uint16_t set_i : set_vec) {
     // four digits and zero end char
     char prefix_buf[5];
-    assert(set_i + 1 <= 9999);
     snprintf(prefix_buf, sizeof(prefix_buf), "%.4u", set_i + 1);
     uint64_t total = 0;
 

utilities/blob_db/blob_db.cc

@@ -73,6 +73,8 @@ void BlobDBOptions::Dump(Logger* log) const {
                    blob_dir_size);
   ROCKS_LOG_HEADER(log, "           blob_db_options.ttl_range_secs: %" PRIu32,
                    ttl_range_secs);
+  ROCKS_LOG_HEADER(log, "            blob_db_options.min_blob_size: %" PRIu64,
+                   min_blob_size);
   ROCKS_LOG_HEADER(log, "           blob_db_options.bytes_per_sync: %" PRIu64,
                    bytes_per_sync);
   ROCKS_LOG_HEADER(log, "           blob_db_options.blob_file_size: %" PRIu64,

utilities/blob_db/blob_db.h

@@ -56,8 +56,10 @@ struct BlobDBOptions {
   // will be inlined in base DB together with the key.
   uint64_t min_blob_size = 0;
 
-  // at what bytes will the blob files be synced to blob log.
-  uint64_t bytes_per_sync = 0;
+  // Allows OS to incrementally sync blob files to disk for every
+  // bytes_per_sync bytes written. Users shouldn't rely on it for
+  // persistency guarantee.
+  uint64_t bytes_per_sync = 512 * 1024;
 
   // the target size of each blob file. File will become immutable
   // after it exceeds that size
@@ -76,6 +78,13 @@ struct BlobDBOptions {
   // blob files will be cleanup based on TTL.
   bool enable_garbage_collection = false;
 
+  // Time interval to trigger garbage collection, in seconds.
+  uint64_t garbage_collection_interval_secs = 60;
+
+  // If garbage collection is enabled, blob files with deleted size no less
+  // than this ratio will become candidates to be cleanup.
+  double garbage_collection_deletion_size_threshold = 0.75;
+
   // Disable all background job. Used for test only.
   bool disable_background_tasks = false;
 
@@ -203,6 +212,8 @@ class BlobDB : public StackableDB {
 
   virtual BlobDBOptions GetBlobDBOptions() const = 0;
 
+  virtual Status SyncBlobFiles() = 0;
+
   virtual ~BlobDB() {}
 
  protected:

utilities/blob_db/blob_db_impl.cc

@@ -111,7 +111,6 @@ BlobDBImpl::BlobDBImpl(const std::string& dbname,
       cf_options_(cf_options),
       env_options_(db_options),
       statistics_(db_options_.statistics.get()),
-      dir_change_(false),
       next_file_number_(1),
       epoch_of_(0),
       shutdown_(false),
@@ -124,6 +123,7 @@ BlobDBImpl::BlobDBImpl(const std::string& dbname,
   blob_dir_ = (bdb_options_.path_relative)
                   ? dbname + "/" + bdb_options_.blob_dir
                   : bdb_options_.blob_dir;
+  env_options_.bytes_per_sync = blob_db_options.bytes_per_sync;
 }
 
 BlobDBImpl::~BlobDBImpl() {
@@ -202,6 +202,7 @@ Status BlobDBImpl::Open(std::vector<ColumnFamilyHandle*>* handles) {
   }
 
   ROCKS_LOG_INFO(db_options_.info_log, "BlobDB pointer %p", this);
+  bdb_options_.Dump(db_options_.info_log.get());
   return s;
 }
 
@@ -210,7 +211,8 @@ void BlobDBImpl::StartBackgroundTasks() {
   tqueue_.add(
       kReclaimOpenFilesPeriodMillisecs,
       std::bind(&BlobDBImpl::ReclaimOpenFiles, this, std::placeholders::_1));
-  tqueue_.add(kGCCheckPeriodMillisecs,
+  tqueue_.add(static_cast<int64_t>(
+                  bdb_options_.garbage_collection_interval_secs * 1000),
               std::bind(&BlobDBImpl::RunGC, this, std::placeholders::_1));
   if (bdb_options_.enable_garbage_collection) {
     tqueue_.add(
@@ -225,8 +227,6 @@ void BlobDBImpl::StartBackgroundTasks() {
       std::bind(&BlobDBImpl::DeleteObsoleteFiles, this, std::placeholders::_1));
   tqueue_.add(kSanityCheckPeriodMillisecs,
               std::bind(&BlobDBImpl::SanityCheck, this, std::placeholders::_1));
-  tqueue_.add(kFSyncFilesPeriodMillisecs,
-              std::bind(&BlobDBImpl::FsyncFiles, this, std::placeholders::_1));
   tqueue_.add(
       kCheckSeqFilesPeriodMillisecs,
       std::bind(&BlobDBImpl::CheckSeqFiles, this, std::placeholders::_1));
@@ -472,7 +472,6 @@ std::shared_ptr<BlobFile> BlobDBImpl::SelectBlobFile() {
     return nullptr;
   }
 
-  dir_change_.store(true);
   blob_files_.insert(std::make_pair(bfile->BlobFileNumber(), bfile));
   open_non_ttl_file_ = bfile;
   return bfile;
@@ -547,7 +546,6 @@ std::shared_ptr<BlobFile> BlobDBImpl::SelectBlobFileTTL(uint64_t expiration) {
     return nullptr;
   }
 
-  dir_change_.store(true);
   blob_files_.insert(std::make_pair(bfile->BlobFileNumber(), bfile));
   open_ttl_files_.insert(bfile);
   epoch_of_++;
@@ -571,18 +569,14 @@ class BlobDBImpl::BlobInserter : public WriteBatch::Handler {
   const WriteOptions& options_;
   BlobDBImpl* blob_db_impl_;
   uint32_t default_cf_id_;
-  SequenceNumber sequence_;
   WriteBatch batch_;
 
  public:
   BlobInserter(const WriteOptions& options, BlobDBImpl* blob_db_impl,
-               uint32_t default_cf_id, SequenceNumber seq)
+               uint32_t default_cf_id)
       : options_(options),
         blob_db_impl_(blob_db_impl),
-        default_cf_id_(default_cf_id),
-        sequence_(seq) {}
-
-  SequenceNumber sequence() { return sequence_; }
+        default_cf_id_(default_cf_id) {}
 
   WriteBatch* batch() { return &batch_; }
 
@@ -597,8 +591,7 @@ class BlobDBImpl::BlobInserter : public WriteBatch::Handler {
     uint64_t expiration =
         blob_db_impl_->ExtractExpiration(key, value, &value_slice, &new_value);
     Status s = blob_db_impl_->PutBlobValue(options_, key, value_slice,
-                                           expiration, sequence_, &batch_);
-    sequence_++;
+                                           expiration, &batch_);
     return s;
   }
 
@@ -609,7 +602,6 @@ class BlobDBImpl::BlobInserter : public WriteBatch::Handler {
           "Blob DB doesn't support non-default column family.");
     }
     Status s = WriteBatchInternal::Delete(&batch_, column_family_id, key);
-    sequence_++;
     return s;
   }
 
@@ -621,7 +613,6 @@ class BlobDBImpl::BlobInserter : public WriteBatch::Handler {
     }
     Status s = WriteBatchInternal::DeleteRange(&batch_, column_family_id,
                                                begin_key, end_key);
-    sequence_++;
     return s;
   }
 
@@ -643,12 +634,8 @@ Status BlobDBImpl::Write(const WriteOptions& options, WriteBatch* updates) {
   RecordTick(statistics_, BLOB_DB_NUM_WRITE);
   uint32_t default_cf_id =
       reinterpret_cast<ColumnFamilyHandleImpl*>(DefaultColumnFamily())->GetID();
-  // TODO(yiwu): In case there are multiple writers the latest sequence would
-  // not be the actually sequence we are writting. Need to get the sequence
-  // from write batch after DB write instead.
-  SequenceNumber current_seq = GetLatestSequenceNumber() + 1;
   Status s;
-  BlobInserter blob_inserter(options, this, default_cf_id, current_seq);
+  BlobInserter blob_inserter(options, this, default_cf_id);
   {
     // Release write_mutex_ before DB write to avoid race condition with
     // flush begin listener, which also require write_mutex_ to sync
@@ -693,6 +680,8 @@ Status BlobDBImpl::Write(const WriteOptions& options, WriteBatch* updates) {
 
   if (bdb_options_.enable_garbage_collection) {
     // add deleted key to list of keys that have been deleted for book-keeping
+    SequenceNumber current_seq =
+        WriteBatchInternal::Sequence(blob_inserter.batch());
     DeleteBookkeeper delete_bookkeeper(this, current_seq);
     s = updates->Iterate(&delete_bookkeeper);
   }
@@ -761,11 +750,7 @@ Status BlobDBImpl::PutUntil(const WriteOptions& options, const Slice& key,
     // flush begin listener, which also require write_mutex_ to sync
     // blob files.
     MutexLock l(&write_mutex_);
-    // TODO(yiwu): In case there are multiple writers the latest sequence would
-    // not be the actually sequence we are writting. Need to get the sequence
-    // from write batch after DB write instead.
-    SequenceNumber sequence = GetLatestSequenceNumber() + 1;
-    s = PutBlobValue(options, key, value, expiration, sequence, &batch);
+    s = PutBlobValue(options, key, value, expiration, &batch);
   }
   if (s.ok()) {
     s = db_->Write(options, &batch);
@@ -776,7 +761,7 @@ Status BlobDBImpl::PutUntil(const WriteOptions& options, const Slice& key,
 
 Status BlobDBImpl::PutBlobValue(const WriteOptions& options, const Slice& key,
                                 const Slice& value, uint64_t expiration,
-                                SequenceNumber sequence, WriteBatch* batch) {
+                                WriteBatch* batch) {
   Status s;
   std::string index_entry;
   uint32_t column_family_id =
@@ -817,7 +802,6 @@ Status BlobDBImpl::PutBlobValue(const WriteOptions& options, const Slice& key,
     }
 
     if (s.ok()) {
-      bfile->ExtendSequenceRange(sequence);
       if (expiration != kNoExpiration) {
         bfile->ExtendExpirationRange(expiration);
       }
@@ -876,6 +860,9 @@ std::shared_ptr<BlobFile> BlobDBImpl::GetOldestBlobFile() {
   CopyBlobFiles(&blob_files, [](const std::shared_ptr<BlobFile>& f) {
     return !f->Obsolete() && f->Immutable();
   });
+  if (blob_files.empty()) {
+    return nullptr;
+  }
   blobf_compare_ttl compare;
   return *std::min_element(blob_files.begin(), blob_files.end(), compare);
 }
@@ -898,7 +885,7 @@ bool BlobDBImpl::EvictOldestBlobFile() {
                    total_blob_space_.load(), bdb_options_.blob_dir_size,
                    oldest_file->BlobFileNumber(), expiration_range.first,
                    expiration_range.second);
-    oldest_file->MarkObsolete(oldest_file->GetSequenceRange().second);
+    oldest_file->MarkObsolete(GetLatestSequenceNumber());
     obsolete_files_.push_back(oldest_file);
     oldest_file_evicted_.store(true);
     RecordTick(statistics_, BLOB_DB_FIFO_NUM_FILES_EVICTED);
@@ -906,6 +893,7 @@ bool BlobDBImpl::EvictOldestBlobFile() {
                oldest_file->BlobCount());
     RecordTick(statistics_, BLOB_DB_FIFO_BYTES_EVICTED,
                oldest_file->GetFileSize());
+    TEST_SYNC_POINT("BlobDBImpl::EvictOldestBlobFile:Evicted");
     return true;
   }
 
@@ -1072,58 +1060,58 @@ Status BlobDBImpl::GetBlobValue(const Slice& key, const Slice& index_entry,
   std::shared_ptr<RandomAccessFileReader> reader =
       GetOrOpenRandomAccessReader(bfile, env_, env_options_);
 
-  std::string* valueptr = value->GetSelf();
-  std::string value_c;
-  if (bdb_options_.compression != kNoCompression) {
-    valueptr = &value_c;
-  }
+  assert(blob_index.offset() > key.size() + sizeof(uint32_t));
+  uint64_t record_offset = blob_index.offset() - key.size() - sizeof(uint32_t);
+  uint64_t record_size = sizeof(uint32_t) + key.size() + blob_index.size();
 
   // Allocate the buffer. This is safe in C++11
-  // Note that std::string::reserved() does not work, since previous value
-  // of the buffer can be larger than blob_index.size().
-  valueptr->resize(blob_index.size());
-  char* buffer = &(*valueptr)[0];
+  std::string buffer_str(record_size, static_cast<char>(0));
+  char* buffer = &buffer_str[0];
 
-  Slice blob_value;
+  // A partial blob record contain checksum, key and value.
+  Slice blob_record;
   {
     StopWatch read_sw(env_, statistics_, BLOB_DB_BLOB_FILE_READ_MICROS);
-    s = reader->Read(blob_index.offset(), blob_index.size(), &blob_value,
-                     buffer);
-    RecordTick(statistics_, BLOB_DB_BLOB_FILE_BYTES_READ, blob_value.size());
+    s = reader->Read(record_offset, record_size, &blob_record, buffer);
+    RecordTick(statistics_, BLOB_DB_BLOB_FILE_BYTES_READ, blob_record.size());
   }
-  if (!s.ok() || blob_value.size() != blob_index.size()) {
-    if (debug_level_ >= 2) {
-      ROCKS_LOG_ERROR(db_options_.info_log,
-                      "Failed to read blob from file: %s blob_offset: %" PRIu64
-                      " blob_size: %" PRIu64 " read: %d key: %s status: '%s'",
-                      bfile->PathName().c_str(), blob_index.offset(),
-                      blob_index.size(), static_cast<int>(blob_value.size()),
-                      key.data(), s.ToString().c_str());
-    }
-    return Status::NotFound("Blob Not Found as couldnt retrieve Blob");
+  if (!s.ok()) {
+    ROCKS_LOG_DEBUG(db_options_.info_log,
+                    "Failed to read blob from blob file %" PRIu64
+                    ", blob_offset: %" PRIu64 ", blob_size: %" PRIu64
+                    ", key_size: " PRIu64 ", read " PRIu64
+                    "bytes, status: '%s'",
+                    bfile->BlobFileNumber(), blob_index.offset(),
+                    blob_index.size(), key.size(), s.ToString().c_str());
+    return s;
   }
+  if (blob_record.size() != record_size) {
+    ROCKS_LOG_DEBUG(db_options_.info_log,
+                    "Failed to read blob from blob file %" PRIu64
+                    ", blob_offset: %" PRIu64 ", blob_size: %" PRIu64
+                    ", key_size: " PRIu64 ", read " PRIu64
+                    "bytes, status: '%s'",
+                    bfile->BlobFileNumber(), blob_index.offset(),
+                    blob_index.size(), key.size(), s.ToString().c_str());
 
-  // TODO(yiwu): Add an option to skip crc checking.
-  Slice crc_slice;
+    return Status::Corruption("Failed to retrieve blob from blob index.");
+  }
+  Slice crc_slice(blob_record.data(), sizeof(uint32_t));
+  Slice blob_value(blob_record.data() + sizeof(uint32_t) + key.size(),
+                   blob_index.size());
   uint32_t crc_exp;
-  std::string crc_str;
-  crc_str.resize(sizeof(uint32_t));
-  char* crc_buffer = &(crc_str[0]);
-  s = reader->Read(blob_index.offset() - (key.size() + sizeof(uint32_t)),
-                   sizeof(uint32_t), &crc_slice, crc_buffer);
-  if (!s.ok() || !GetFixed32(&crc_slice, &crc_exp)) {
-    if (debug_level_ >= 2) {
-      ROCKS_LOG_ERROR(db_options_.info_log,
-                      "Failed to fetch blob crc file: %s blob_offset: %" PRIu64
-                      " blob_size: %" PRIu64 " key: %s status: '%s'",
-                      bfile->PathName().c_str(), blob_index.offset(),
-                      blob_index.size(), key.data(), s.ToString().c_str());
-    }
-    return Status::NotFound("Blob Not Found as couldnt retrieve CRC");
+  if (!GetFixed32(&crc_slice, &crc_exp)) {
+    ROCKS_LOG_DEBUG(db_options_.info_log,
+                    "Unable to decode CRC from blob file %" PRIu64
+                    ", blob_offset: %" PRIu64 ", blob_size: %" PRIu64
+                    ", key size: %" PRIu64 ", status: '%s'",
+                    bfile->BlobFileNumber(), blob_index.offset(),
+                    blob_index.size(), key.size(), s.ToString().c_str());
+    return Status::Corruption("Unable to decode checksum.");
   }
 
-  uint32_t crc = crc32c::Value(key.data(), key.size());
-  crc = crc32c::Extend(crc, blob_value.data(), blob_value.size());
+  uint32_t crc = crc32c::Value(blob_record.data() + sizeof(uint32_t),
+                               blob_record.size() - sizeof(uint32_t));
   crc = crc32c::Mask(crc);  // Adjust for storage
   if (crc != crc_exp) {
     if (debug_level_ >= 2) {
@@ -1136,7 +1124,9 @@ Status BlobDBImpl::GetBlobValue(const Slice& key, const Slice& index_entry,
     return Status::Corruption("Corruption. Blob CRC mismatch");
   }
 
-  if (bfile->compression() != kNoCompression) {
+  if (bfile->compression() == kNoCompression) {
+    value->PinSelf(blob_value);
+  } else {
     BlockContents contents;
     auto cfh = reinterpret_cast<ColumnFamilyHandleImpl*>(DefaultColumnFamily());
     {
@@ -1147,11 +1137,9 @@ Status BlobDBImpl::GetBlobValue(const Slice& key, const Slice& index_entry,
           kBlockBasedTableVersionFormat, Slice(), bfile->compression(),
           *(cfh->cfd()->ioptions()));
     }
-    *(value->GetSelf()) = contents.data.ToString();
+    value->PinSelf(contents.data);
   }
 
-  value->PinSelf();
-
   return s;
 }
 
@@ -1271,9 +1259,26 @@ Status BlobDBImpl::CloseBlobFileIfNeeded(std::shared_ptr<BlobFile>& bfile) {
 bool BlobDBImpl::VisibleToActiveSnapshot(
     const std::shared_ptr<BlobFile>& bfile) {
   assert(bfile->Obsolete());
-  SequenceNumber first_sequence = bfile->GetSequenceRange().first;
+
+  // We check whether the oldest snapshot is no less than the last sequence
+  // by the time the blob file become obsolete. If so, the blob file is not
+  // visible to all existing snapshots.
+  //
+  // If we keep track of the earliest sequence of the keys in the blob file,
+  // we could instead check if there's a snapshot falls in range
+  // [earliest_sequence, obsolete_sequence). But doing so will make the
+  // implementation more complicated.
   SequenceNumber obsolete_sequence = bfile->GetObsoleteSequence();
-  return db_impl_->HasActiveSnapshotInRange(first_sequence, obsolete_sequence);
+  SequenceNumber oldest_snapshot = 0;
+  {
+    // Need to lock DBImpl mutex before access snapshot list.
+    InstrumentedMutexLock l(db_impl_->mutex());
+    auto snapshots = db_impl_->snapshots();
+    if (!snapshots.empty()) {
+      oldest_snapshot = snapshots.oldest()->GetSequenceNumber();
+    }
+  }
+  return oldest_snapshot < obsolete_sequence;
 }
 
 bool BlobDBImpl::FindFileAndEvictABlob(uint64_t file_number, uint64_t key_size,
@@ -1431,14 +1436,6 @@ std::pair<bool, int64_t> BlobDBImpl::CheckSeqFiles(bool aborted) {
   return std::make_pair(true, -1);
 }
 
-std::pair<bool, int64_t> BlobDBImpl::FsyncFiles(bool aborted) {
-  if (aborted || shutdown_) {
-    return std::make_pair(false, -1);
-  }
-  SyncBlobFiles();
-  return std::make_pair(true, -1);
-}
-
 Status BlobDBImpl::SyncBlobFiles() {
   MutexLock l(&write_mutex_);
 
@@ -1446,35 +1443,30 @@ Status BlobDBImpl::SyncBlobFiles() {
   {
     ReadLock rl(&mutex_);
     for (auto fitr : open_ttl_files_) {
-      if (fitr->NeedsFsync(true, bdb_options_.bytes_per_sync))
-        process_files.push_back(fitr);
+      process_files.push_back(fitr);
     }
-
-    if (open_non_ttl_file_ != nullptr &&
-        open_non_ttl_file_->NeedsFsync(true, bdb_options_.bytes_per_sync)) {
+    if (open_non_ttl_file_ != nullptr) {
       process_files.push_back(open_non_ttl_file_);
     }
   }
 
   Status s;
-
   for (auto& blob_file : process_files) {
-    if (blob_file->NeedsFsync(true, bdb_options_.bytes_per_sync)) {
-      s = blob_file->Fsync();
-      if (!s.ok()) {
-        ROCKS_LOG_ERROR(db_options_.info_log,
-                        "Failed to sync blob file %" PRIu64 ", status: %s",
-                        blob_file->BlobFileNumber(), s.ToString().c_str());
-        return s;
-      }
+    s = blob_file->Fsync();
+    if (!s.ok()) {
+      ROCKS_LOG_ERROR(db_options_.info_log,
+                      "Failed to sync blob file %" PRIu64 ", status: %s",
+                      blob_file->BlobFileNumber(), s.ToString().c_str());
+      return s;
     }
   }
 
-  bool expected = true;
-  if (dir_change_.compare_exchange_weak(expected, false)) {
-    s = dir_ent_->Fsync();
+  s = dir_ent_->Fsync();
+  if (!s.ok()) {
+    ROCKS_LOG_ERROR(db_options_.info_log,
+                    "Failed to sync blob directory, status: %s",
+                    s.ToString().c_str());
   }
-
   return s;
 }
 
@@ -1729,7 +1721,6 @@ Status BlobDBImpl::GCFileAndUpdateLSM(const std::shared_ptr<BlobFile>& bfptr,
 
       WriteLock wl(&mutex_);
 
-      dir_change_.store(true);
       blob_files_.insert(std::make_pair(newfile->BlobFileNumber(), newfile));
     }
 
@@ -1757,8 +1748,6 @@ Status BlobDBImpl::GCFileAndUpdateLSM(const std::shared_ptr<BlobFile>& bfptr,
                                                    &rewrite_batch, &callback);
     }
     if (rewrite_status.ok()) {
-      newfile->ExtendSequenceRange(
-          WriteBatchInternal::Sequence(&rewrite_batch));
       gc_stats->num_keys_relocated++;
       gc_stats->bytes_relocated += record.record_size();
     } else if (rewrite_status.IsBusy()) {
@@ -1775,10 +1764,7 @@ Status BlobDBImpl::GCFileAndUpdateLSM(const std::shared_ptr<BlobFile>& bfptr,
   }  // end of ReadRecord loop
 
   if (s.ok()) {
-    SequenceNumber obsolete_sequence =
-        newfile == nullptr ? bfptr->GetSequenceRange().second + 1
-                           : newfile->GetSequenceRange().second;
-    bfptr->MarkObsolete(obsolete_sequence);
+    bfptr->MarkObsolete(GetLatestSequenceNumber());
     if (!first_gc) {
       WriteLock wl(&mutex_);
       obsolete_files_.push_back(bfptr);
@@ -1788,8 +1774,9 @@ Status BlobDBImpl::GCFileAndUpdateLSM(const std::shared_ptr<BlobFile>& bfptr,
   ROCKS_LOG_INFO(
       db_options_.info_log,
       "%s blob file %" PRIu64 ". Total blob records: %" PRIu64
-      ", Expired: %" PRIu64 " keys/%" PRIu64 " bytes, Overwritten: %" PRIu64
-      " keys/%" PRIu64 " bytes.",
+      ", expired: %" PRIu64 " keys/%" PRIu64
+      " bytes, updated or deleted by user: %" PRIu64 " keys/%" PRIu64
+      " bytes, rewrite to new file: %" PRIu64 " keys/%" PRIu64 " bytes.",
       s.ok() ? "Successfully garbage collected" : "Failed to garbage collect",
       bfptr->BlobFileNumber(), gc_stats->blob_count, gc_stats->num_keys_expired,
       gc_stats->bytes_expired, gc_stats->num_keys_overwritten,
@@ -1843,14 +1830,9 @@ bool BlobDBImpl::ShouldGCFile(std::shared_ptr<BlobFile> bfile, uint64_t now,
       return true;
     }
 
-    if (bdb_options_.ttl_range_secs < kPartialExpirationGCRangeSecs) {
-      *reason = "has ttl but partial expiration not turned on";
-      return false;
-    }
-
     ReadLock lockbfile_r(&bfile->mutex_);
-    bool ret = ((bfile->deleted_size_ * 100.0 / bfile->file_size_.load()) >
-                kPartialExpirationPercentage);
+    bool ret = ((bfile->deleted_size_ / bfile->file_size_.load()) >
+                bdb_options_.garbage_collection_deletion_size_threshold);
     if (ret) {
       *reason = "deleted blobs beyond threshold";
     } else {
@@ -1869,8 +1851,8 @@ bool BlobDBImpl::ShouldGCFile(std::shared_ptr<BlobFile> bfile, uint64_t now,
   ReadLock lockbfile_r(&bfile->mutex_);
 
   if (bdb_options_.enable_garbage_collection) {
-    if ((bfile->deleted_size_ * 100.0 / bfile->file_size_.load()) >
-        kPartialExpirationPercentage) {
+    if ((bfile->deleted_size_ / bfile->file_size_.load()) >
+        bdb_options_.garbage_collection_deletion_size_threshold) {
       *reason = "deleted simple blobs beyond threshold";
       return true;
     }

utilities/blob_db/blob_db_impl.h

@@ -144,23 +144,6 @@ class BlobDBImpl : public BlobDB {
   // how many random access open files can we tolerate
   static constexpr uint32_t kOpenFilesTrigger = 100;
 
-  // how many periods of stats do we keep.
-  static constexpr uint32_t kWriteAmplificationStatsPeriods = 24;
-
-  // we will garbage collect blob files in
-  // which entire files have expired. However if the
-  // ttl_range of files is very large say a day, we
-  // would have to wait for the entire day, before we
-  // recover most of the space.
-  static constexpr uint32_t kPartialExpirationGCRangeSecs = 4 * 3600;
-
-  // this should be based on allowed Write Amplification
-  // if 50% of the space of a blob file has been deleted/expired,
-  static constexpr uint32_t kPartialExpirationPercentage = 75;
-
-  // how often should we schedule a job to fsync open files
-  static constexpr uint32_t kFSyncFilesPeriodMillisecs = 10 * 1000;
-
   // how often to schedule reclaim open files.
   static constexpr uint32_t kReclaimOpenFilesPeriodMillisecs = 1 * 1000;
 
@@ -228,7 +211,7 @@ class BlobDBImpl : public BlobDB {
 
   Status Open(std::vector<ColumnFamilyHandle*>* handles);
 
-  Status SyncBlobFiles();
+  Status SyncBlobFiles() override;
 
 #ifndef NDEBUG
   Status TEST_GetBlobValue(const Slice& key, const Slice& index_entry,
@@ -284,7 +267,7 @@ class BlobDBImpl : public BlobDB {
 
   Status PutBlobValue(const WriteOptions& options, const Slice& key,
                       const Slice& value, uint64_t expiration,
-                      SequenceNumber sequence, WriteBatch* batch);
+                      WriteBatch* batch);
 
   Status AppendBlob(const std::shared_ptr<BlobFile>& bfile,
                     const std::string& headerbuf, const Slice& key,
@@ -313,9 +296,6 @@ class BlobDBImpl : public BlobDB {
   // Major task to garbage collect expired and deleted blobs
   std::pair<bool, int64_t> RunGC(bool aborted);
 
-  // asynchronous task to fsync/fdatasync the open blob files
-  std::pair<bool, int64_t> FsyncFiles(bool aborted);
-
   // periodically check if open blob files and their TTL's has expired
   // if expired, close the sequential writer and make the file immutable
   std::pair<bool, int64_t> CheckSeqFiles(bool aborted);
@@ -420,8 +400,6 @@ class BlobDBImpl : public BlobDB {
   // pointer to directory
   std::unique_ptr<Directory> dir_ent_;
 
-  std::atomic<bool> dir_change_;
-
   // Read Write Mutex, which protects all the data structures
   // HEAVILY TRAFFICKED
   mutable port::RWMutex mutex_;

utilities/blob_db/blob_db_test.cc

@@ -901,20 +901,18 @@ TEST_F(BlobDBTest, SnapshotAndGarbageCollection) {
       ASSERT_EQ(1, gc_stats.blob_count);
       if (delete_key) {
         ASSERT_EQ(0, gc_stats.num_keys_relocated);
-        ASSERT_EQ(bfile->GetSequenceRange().second + 1,
-                  bfile->GetObsoleteSequence());
       } else {
         ASSERT_EQ(1, gc_stats.num_keys_relocated);
-        ASSERT_EQ(blob_db_->GetLatestSequenceNumber(),
-                  bfile->GetObsoleteSequence());
       }
+      ASSERT_EQ(blob_db_->GetLatestSequenceNumber(),
+                bfile->GetObsoleteSequence());
       if (i == 3) {
         snapshot = blob_db_->GetSnapshot();
       }
       size_t num_files = delete_key ? 3 : 4;
       ASSERT_EQ(num_files, blob_db_impl()->TEST_GetBlobFiles().size());
       blob_db_impl()->TEST_DeleteObsoleteFiles();
-      if (i == 0 || i == 3 || (i == 2 && delete_key)) {
+      if (i == 3) {
         // The snapshot shouldn't see data in bfile
         ASSERT_EQ(num_files - 1, blob_db_impl()->TEST_GetBlobFiles().size());
         blob_db_->ReleaseSnapshot(snapshot);
@@ -1063,15 +1061,19 @@ TEST_F(BlobDBTest, OutOfSpace) {
 }
 
 TEST_F(BlobDBTest, EvictOldestFileWhenCloseToSpaceLimit) {
-  // Use mock env to stop wall clock.
-  Options options;
   BlobDBOptions bdb_options;
   bdb_options.blob_dir_size = 270;
   bdb_options.blob_file_size = 100;
-  bdb_options.disable_background_tasks = true;
   bdb_options.is_fifo = true;
+  bdb_options.disable_background_tasks = true;
   Open(bdb_options);
 
+  std::atomic<int> evict_count{0};
+  SyncPoint::GetInstance()->SetCallBack(
+      "BlobDBImpl::EvictOldestBlobFile:Evicted",
+      [&](void *) { evict_count++; });
+  SyncPoint::GetInstance()->EnableProcessing();
+
   // Each stored blob has an overhead of 32 bytes currently.
   // So a 100 byte blob should take up 132 bytes.
   std::string value(100, 'v');
@@ -1095,6 +1097,28 @@ TEST_F(BlobDBTest, EvictOldestFileWhenCloseToSpaceLimit) {
   bdb_impl->TEST_DeleteObsoleteFiles();
   obsolete_files = bdb_impl->TEST_GetObsoleteFiles();
   ASSERT_TRUE(obsolete_files.empty());
+  ASSERT_EQ(1, evict_count);
+}
+
+TEST_F(BlobDBTest, NoOldestFileToEvict) {
+  Options options;
+  BlobDBOptions bdb_options;
+  bdb_options.blob_dir_size = 1000;
+  bdb_options.blob_file_size = 5000;
+  bdb_options.is_fifo = true;
+  bdb_options.disable_background_tasks = true;
+  Open(bdb_options);
+
+  std::atomic<int> evict_count{0};
+  SyncPoint::GetInstance()->SetCallBack(
+      "BlobDBImpl::EvictOldestBlobFile:Evicted",
+      [&](void *) { evict_count++; });
+  SyncPoint::GetInstance()->EnableProcessing();
+
+  std::string value(2000, 'v');
+  ASSERT_OK(Put("foo", std::string(2000, 'v')));
+  ASSERT_OK(Put("bar", std::string(2000, 'v')));
+  ASSERT_EQ(0, evict_count);
 }
 
 TEST_F(BlobDBTest, InlineSmallValues) {
@@ -1111,10 +1135,6 @@ TEST_F(BlobDBTest, InlineSmallValues) {
   Open(bdb_options, options);
   std::map<std::string, std::string> data;
   std::map<std::string, KeyVersion> versions;
-  SequenceNumber first_non_ttl_seq = kMaxSequenceNumber;
-  SequenceNumber first_ttl_seq = kMaxSequenceNumber;
-  SequenceNumber last_non_ttl_seq = 0;
-  SequenceNumber last_ttl_seq = 0;
   for (size_t i = 0; i < 1000; i++) {
     bool is_small_value = rnd.Next() % 2;
     bool has_ttl = rnd.Next() % 2;
@@ -1134,15 +1154,6 @@ TEST_F(BlobDBTest, InlineSmallValues) {
     versions[key] =
         KeyVersion(key, value, sequence,
                    (is_small_value && !has_ttl) ? kTypeValue : kTypeBlobIndex);
-    if (!is_small_value) {
-      if (!has_ttl) {
-        first_non_ttl_seq = std::min(first_non_ttl_seq, sequence);
-        last_non_ttl_seq = std::max(last_non_ttl_seq, sequence);
-      } else {
-        first_ttl_seq = std::min(first_ttl_seq, sequence);
-        last_ttl_seq = std::max(last_ttl_seq, sequence);
-      }
-    }
   }
   VerifyDB(data);
   VerifyBaseDB(versions);
@@ -1159,11 +1170,7 @@ TEST_F(BlobDBTest, InlineSmallValues) {
     ttl_file = blob_files[1];
   }
   ASSERT_FALSE(non_ttl_file->HasTTL());
-  ASSERT_EQ(first_non_ttl_seq, non_ttl_file->GetSequenceRange().first);
-  ASSERT_EQ(last_non_ttl_seq, non_ttl_file->GetSequenceRange().second);
   ASSERT_TRUE(ttl_file->HasTTL());
-  ASSERT_EQ(first_ttl_seq, ttl_file->GetSequenceRange().first);
-  ASSERT_EQ(last_ttl_seq, ttl_file->GetSequenceRange().second);
 }
 
 TEST_F(BlobDBTest, CompactionFilterNotSupported) {

utilities/blob_db/blob_dump_tool.cc

@@ -142,8 +142,6 @@ Status BlobDumpTool::DumpBlobLogFooter(uint64_t file_size,
   fprintf(stdout, "  Blob count       : %" PRIu64 "\n", footer.blob_count);
   fprintf(stdout, "  Expiration Range : %s\n",
           GetString(footer.expiration_range).c_str());
-  fprintf(stdout, "  Sequence Range   : %s\n",
-          GetString(footer.sequence_range).c_str());
   return s;
 }
 

utilities/blob_db/blob_file.cc

@@ -43,7 +43,6 @@ BlobFile::BlobFile()
       obsolete_(false),
       gc_once_after_open_(false),
       expiration_range_({0, 0}),
-      sequence_range_({kMaxSequenceNumber, 0}),
       last_access_(-1),
       last_fsync_(0),
       header_valid_(false),
@@ -67,7 +66,6 @@ BlobFile::BlobFile(const BlobDBImpl* p, const std::string& bdir, uint64_t fn,
       obsolete_(false),
       gc_once_after_open_(false),
       expiration_range_({0, 0}),
-      sequence_range_({kMaxSequenceNumber, 0}),
       last_access_(-1),
       last_fsync_(0),
       header_valid_(false),
@@ -116,12 +114,11 @@ std::string BlobFile::DumpState() const {
            " file_size: %" PRIu64 " deleted_count: %" PRIu64
            " deleted_size: %" PRIu64
            " closed: %d obsolete: %d expiration_range: (%" PRIu64 ", %" PRIu64
-           ") sequence_range: (%" PRIu64 " %" PRIu64 "), writer: %d reader: %d",
+           "), writer: %d reader: %d",
            path_to_dir_.c_str(), file_number_, blob_count_.load(),
            gc_epoch_.load(), file_size_.load(), deleted_count_, deleted_size_,
            closed_.load(), obsolete_.load(), expiration_range_.first,
-           expiration_range_.second, sequence_range_.first,
-           sequence_range_.second, (!!log_writer_), (!!ra_file_reader_));
+           expiration_range_.second, (!!log_writer_), (!!ra_file_reader_));
   return str;
 }
 
@@ -144,8 +141,6 @@ Status BlobFile::WriteFooterAndCloseLocked() {
     footer.expiration_range = expiration_range_;
   }
 
-  footer.sequence_range = sequence_range_;
-
   // this will close the file and reset the Writable File Pointer.
   Status s = log_writer_->AppendFooter(footer);
   if (s.ok()) {
@@ -185,7 +180,6 @@ Status BlobFile::SetFromFooterLocked(const BlobLogFooter& footer) {
   last_fsync_.store(file_size_);
   blob_count_ = footer.blob_count;
   expiration_range_ = footer.expiration_range;
-  sequence_range_ = footer.sequence_range;
   closed_ = true;
   return Status::OK();
 }

utilities/blob_db/blob_file.h

@@ -84,8 +84,6 @@ class BlobFile {
 
   ExpirationRange expiration_range_;
 
-  SequenceRange sequence_range_;
-
   // Sequential/Append writer for blobs
   std::shared_ptr<Writer> log_writer_;
 
@@ -177,17 +175,6 @@ class BlobFile {
     expiration_range_.second = std::max(expiration_range_.second, expiration);
   }
 
-  SequenceRange GetSequenceRange() const { return sequence_range_; }
-
-  void SetSequenceRange(SequenceRange sequence_range) {
-    sequence_range_ = sequence_range;
-  }
-
-  void ExtendSequenceRange(SequenceNumber sequence) {
-    sequence_range_.first = std::min(sequence_range_.first, sequence);
-    sequence_range_.second = std::max(sequence_range_.second, sequence);
-  }
-
   bool HasTTL() const { return has_ttl_; }
 
   void SetHasTTL(bool has_ttl) { has_ttl_ = has_ttl; }

utilities/blob_db/blob_log_format.cc

@@ -67,8 +67,6 @@ void BlobLogFooter::EncodeTo(std::string* dst) {
   PutFixed64(dst, blob_count);
   PutFixed64(dst, expiration_range.first);
   PutFixed64(dst, expiration_range.second);
-  PutFixed64(dst, sequence_range.first);
-  PutFixed64(dst, sequence_range.second);
   crc = crc32c::Value(dst->c_str(), dst->size());
   crc = crc32c::Mask(crc);
   PutFixed32(dst, crc);
@@ -82,14 +80,12 @@ Status BlobLogFooter::DecodeFrom(Slice src) {
                               "Unexpected blob file footer size");
   }
   uint32_t src_crc = 0;
-  src_crc = crc32c::Value(src.data(), BlobLogFooter::kSize - 4);
+  src_crc = crc32c::Value(src.data(), BlobLogFooter::kSize - sizeof(uint32_t));
   src_crc = crc32c::Mask(src_crc);
   uint32_t magic_number;
   if (!GetFixed32(&src, &magic_number) || !GetFixed64(&src, &blob_count) ||
       !GetFixed64(&src, &expiration_range.first) ||
-      !GetFixed64(&src, &expiration_range.second) ||
-      !GetFixed64(&src, &sequence_range.first) ||
-      !GetFixed64(&src, &sequence_range.second) || !GetFixed32(&src, &crc)) {
+      !GetFixed64(&src, &expiration_range.second) || !GetFixed32(&src, &crc)) {
     return Status::Corruption(kErrorMessage, "Error decoding content");
   }
   if (magic_number != kMagicNumber) {

utilities/blob_db/blob_log_format.h

@@ -24,7 +24,6 @@ constexpr uint32_t kVersion1 = 1;
 constexpr uint64_t kNoExpiration = std::numeric_limits<uint64_t>::max();
 
 using ExpirationRange = std::pair<uint64_t, uint64_t>;
-using SequenceRange = std::pair<uint64_t, uint64_t>;
 
 // Format of blob log file header (30 bytes):
 //
@@ -53,24 +52,23 @@ struct BlobLogHeader {
   Status DecodeFrom(Slice slice);
 };
 
-// Format of blob log file footer (48 bytes):
+// Format of blob log file footer (32 bytes):
 //
-//    +--------------+------------+-------------------+-------------------+------------+
-//    | magic number | blob count | expiration range  |  sequence range   | footer CRC |
-//    +--------------+------------+-------------------+-------------------+------------+
-//    |   Fixed32    |  Fixed64   | Fixed64 + Fixed64 | Fixed64 + Fixed64 |   Fixed32  |
-//    +--------------+------------+-------------------+-------------------+------------+
+//    +--------------+------------+-------------------+------------+
+//    | magic number | blob count | expiration range  | footer CRC |
+//    +--------------+------------+-------------------+------------+
+//    |   Fixed32    |  Fixed64   | Fixed64 + Fixed64 |   Fixed32  |
+//    +--------------+------------+-------------------+------------+
 //
 // The footer will be presented only when the blob file is properly closed.
 //
 // Unlike the same field in file header, expiration range in the footer is the
 // range of smallest and largest expiration of the data in this file.
 struct BlobLogFooter {
-  static constexpr size_t kSize = 48;
+  static constexpr size_t kSize = 32;
 
   uint64_t blob_count = 0;
   ExpirationRange expiration_range = std::make_pair(0, 0);
-  SequenceRange sequence_range = std::make_pair(0, 0);
   uint32_t crc = 0;
 
   void EncodeTo(std::string* dst);

utilities/cassandra/cassandra_format_test.cc

@@ -317,10 +317,10 @@ TEST(RowValueTest, PurgeTtlShouldRemvoeAllColumnsExpired) {
   int64_t now = time(nullptr);
 
   auto row_value = CreateTestRowValue({
-    std::make_tuple(kColumn, 0, ToMicroSeconds(now)),
-    std::make_tuple(kExpiringColumn, 1, ToMicroSeconds(now - kTtl - 10)), //expired
-    std::make_tuple(kExpiringColumn, 2, ToMicroSeconds(now)), // not expired
-    std::make_tuple(kTombstone, 3, ToMicroSeconds(now))
+    CreateTestColumnSpec(kColumn, 0, ToMicroSeconds(now)),
+    CreateTestColumnSpec(kExpiringColumn, 1, ToMicroSeconds(now - kTtl - 10)), //expired
+    CreateTestColumnSpec(kExpiringColumn, 2, ToMicroSeconds(now)), // not expired
+    CreateTestColumnSpec(kTombstone, 3, ToMicroSeconds(now))
   });
 
   bool changed = false;
@@ -339,10 +339,10 @@ TEST(RowValueTest, ExpireTtlShouldConvertExpiredColumnsToTombstones) {
   int64_t now = time(nullptr);
 
   auto row_value = CreateTestRowValue({
-    std::make_tuple(kColumn, 0, ToMicroSeconds(now)),
-    std::make_tuple(kExpiringColumn, 1, ToMicroSeconds(now - kTtl - 10)), //expired
-    std::make_tuple(kExpiringColumn, 2, ToMicroSeconds(now)), // not expired
-    std::make_tuple(kTombstone, 3, ToMicroSeconds(now))
+    CreateTestColumnSpec(kColumn, 0, ToMicroSeconds(now)),
+    CreateTestColumnSpec(kExpiringColumn, 1, ToMicroSeconds(now - kTtl - 10)), //expired
+    CreateTestColumnSpec(kExpiringColumn, 2, ToMicroSeconds(now)), // not expired
+    CreateTestColumnSpec(kTombstone, 3, ToMicroSeconds(now))
   });
 
   bool changed = false;

utilities/cassandra/cassandra_functional_test.cc

@@ -145,21 +145,21 @@ TEST_F(CassandraFunctionalTest, SimpleMergeTest) {
   int64_t now = time(nullptr);
 
   store.Append("k1", CreateTestRowValue({
-    std::make_tuple(kTombstone, 0, ToMicroSeconds(now + 5)),
-    std::make_tuple(kColumn, 1, ToMicroSeconds(now + 8)),
-    std::make_tuple(kExpiringColumn, 2, ToMicroSeconds(now + 5)),
+    CreateTestColumnSpec(kTombstone, 0, ToMicroSeconds(now + 5)),
+    CreateTestColumnSpec(kColumn, 1, ToMicroSeconds(now + 8)),
+    CreateTestColumnSpec(kExpiringColumn, 2, ToMicroSeconds(now + 5)),
   }));
   store.Append("k1",CreateTestRowValue({
-    std::make_tuple(kColumn, 0, ToMicroSeconds(now + 2)),
-    std::make_tuple(kExpiringColumn, 1, ToMicroSeconds(now + 5)),
-    std::make_tuple(kTombstone, 2, ToMicroSeconds(now + 7)),
-    std::make_tuple(kExpiringColumn, 7, ToMicroSeconds(now + 17)),
+    CreateTestColumnSpec(kColumn, 0, ToMicroSeconds(now + 2)),
+    CreateTestColumnSpec(kExpiringColumn, 1, ToMicroSeconds(now + 5)),
+    CreateTestColumnSpec(kTombstone, 2, ToMicroSeconds(now + 7)),
+    CreateTestColumnSpec(kExpiringColumn, 7, ToMicroSeconds(now + 17)),
   }));
   store.Append("k1", CreateTestRowValue({
-    std::make_tuple(kExpiringColumn, 0, ToMicroSeconds(now + 6)),
-    std::make_tuple(kTombstone, 1, ToMicroSeconds(now + 5)),
-    std::make_tuple(kColumn, 2, ToMicroSeconds(now + 4)),
-    std::make_tuple(kTombstone, 11, ToMicroSeconds(now + 11)),
+    CreateTestColumnSpec(kExpiringColumn, 0, ToMicroSeconds(now + 6)),
+    CreateTestColumnSpec(kTombstone, 1, ToMicroSeconds(now + 5)),
+    CreateTestColumnSpec(kColumn, 2, ToMicroSeconds(now + 4)),
+    CreateTestColumnSpec(kTombstone, 11, ToMicroSeconds(now + 11)),
   }));
 
   auto ret = store.Get("k1");
@@ -180,16 +180,16 @@ TEST_F(CassandraFunctionalTest,
   int64_t now= time(nullptr);
 
   store.Append("k1", CreateTestRowValue({
-    std::make_tuple(kExpiringColumn, 0, ToMicroSeconds(now - kTtl - 20)), //expired
-    std::make_tuple(kExpiringColumn, 1, ToMicroSeconds(now - kTtl + 10)), // not expired
-    std::make_tuple(kTombstone, 3, ToMicroSeconds(now))
+    CreateTestColumnSpec(kExpiringColumn, 0, ToMicroSeconds(now - kTtl - 20)), //expired
+    CreateTestColumnSpec(kExpiringColumn, 1, ToMicroSeconds(now - kTtl + 10)), // not expired
+    CreateTestColumnSpec(kTombstone, 3, ToMicroSeconds(now))
   }));
 
   store.Flush();
 
   store.Append("k1",CreateTestRowValue({
-    std::make_tuple(kExpiringColumn, 0, ToMicroSeconds(now - kTtl - 10)), //expired
-    std::make_tuple(kColumn, 2, ToMicroSeconds(now))
+    CreateTestColumnSpec(kExpiringColumn, 0, ToMicroSeconds(now - kTtl - 10)), //expired
+    CreateTestColumnSpec(kColumn, 2, ToMicroSeconds(now))
   }));
 
   store.Flush();
@@ -213,16 +213,16 @@ TEST_F(CassandraFunctionalTest,
   int64_t now = time(nullptr);
 
   store.Append("k1", CreateTestRowValue({
-    std::make_tuple(kExpiringColumn, 0, ToMicroSeconds(now - kTtl - 20)), //expired
-    std::make_tuple(kExpiringColumn, 1, ToMicroSeconds(now)), // not expired
-    std::make_tuple(kTombstone, 3, ToMicroSeconds(now))
+    CreateTestColumnSpec(kExpiringColumn, 0, ToMicroSeconds(now - kTtl - 20)), //expired
+    CreateTestColumnSpec(kExpiringColumn, 1, ToMicroSeconds(now)), // not expired
+    CreateTestColumnSpec(kTombstone, 3, ToMicroSeconds(now))
   }));
 
   store.Flush();
 
   store.Append("k1",CreateTestRowValue({
-    std::make_tuple(kExpiringColumn, 0, ToMicroSeconds(now - kTtl - 10)), //expired
-    std::make_tuple(kColumn, 2, ToMicroSeconds(now))
+    CreateTestColumnSpec(kExpiringColumn, 0, ToMicroSeconds(now - kTtl - 10)), //expired
+    CreateTestColumnSpec(kColumn, 2, ToMicroSeconds(now))
   }));
 
   store.Flush();
@@ -244,14 +244,14 @@ TEST_F(CassandraFunctionalTest,
   int64_t now = time(nullptr);
 
   store.Append("k1", CreateTestRowValue({
-    std::make_tuple(kExpiringColumn, 0, ToMicroSeconds(now - kTtl - 20)),
-    std::make_tuple(kExpiringColumn, 1, ToMicroSeconds(now - kTtl - 20)),
+    CreateTestColumnSpec(kExpiringColumn, 0, ToMicroSeconds(now - kTtl - 20)),
+    CreateTestColumnSpec(kExpiringColumn, 1, ToMicroSeconds(now - kTtl - 20)),
   }));
 
   store.Flush();
 
   store.Append("k1",CreateTestRowValue({
-    std::make_tuple(kExpiringColumn, 0, ToMicroSeconds(now - kTtl - 10)),
+    CreateTestColumnSpec(kExpiringColumn, 0, ToMicroSeconds(now - kTtl - 10)),
   }));
 
   store.Flush();
@@ -266,18 +266,18 @@ TEST_F(CassandraFunctionalTest,
   int64_t now = time(nullptr);
 
   store.Append("k1", CreateTestRowValue({
-    std::make_tuple(kTombstone, 0, ToMicroSeconds(now - gc_grace_period_in_seconds_ - 1)),
-    std::make_tuple(kColumn, 1, ToMicroSeconds(now))
+    CreateTestColumnSpec(kTombstone, 0, ToMicroSeconds(now - gc_grace_period_in_seconds_ - 1)),
+    CreateTestColumnSpec(kColumn, 1, ToMicroSeconds(now))
   }));
 
   store.Append("k2", CreateTestRowValue({
-    std::make_tuple(kColumn, 0, ToMicroSeconds(now))
+    CreateTestColumnSpec(kColumn, 0, ToMicroSeconds(now))
   }));
 
   store.Flush();
 
   store.Append("k1",CreateTestRowValue({
-    std::make_tuple(kColumn, 1, ToMicroSeconds(now)),
+    CreateTestColumnSpec(kColumn, 1, ToMicroSeconds(now)),
   }));
 
   store.Flush();
@@ -296,7 +296,7 @@ TEST_F(CassandraFunctionalTest, CompactionShouldRemoveTombstoneFromPut) {
   int64_t now = time(nullptr);
 
   store.Put("k1", CreateTestRowValue({
-    std::make_tuple(kTombstone, 0, ToMicroSeconds(now - gc_grace_period_in_seconds_ - 1)),
+    CreateTestColumnSpec(kTombstone, 0, ToMicroSeconds(now - gc_grace_period_in_seconds_ - 1)),
   }));
 
   store.Flush();

utilities/cassandra/cassandra_row_merge_test.cc

@@ -15,27 +15,27 @@ TEST(RowValueMergeTest, Merge) {
   std::vector<RowValue> row_values;
   row_values.push_back(
     CreateTestRowValue({
-      std::make_tuple(kTombstone, 0, 5),
-      std::make_tuple(kColumn, 1, 8),
-      std::make_tuple(kExpiringColumn, 2, 5),
+      CreateTestColumnSpec(kTombstone, 0, 5),
+      CreateTestColumnSpec(kColumn, 1, 8),
+      CreateTestColumnSpec(kExpiringColumn, 2, 5),
     })
   );
 
   row_values.push_back(
     CreateTestRowValue({
-      std::make_tuple(kColumn, 0, 2),
-      std::make_tuple(kExpiringColumn, 1, 5),
-      std::make_tuple(kTombstone, 2, 7),
-      std::make_tuple(kExpiringColumn, 7, 17),
+      CreateTestColumnSpec(kColumn, 0, 2),
+      CreateTestColumnSpec(kExpiringColumn, 1, 5),
+      CreateTestColumnSpec(kTombstone, 2, 7),
+      CreateTestColumnSpec(kExpiringColumn, 7, 17),
     })
   );
 
   row_values.push_back(
     CreateTestRowValue({
-      std::make_tuple(kExpiringColumn, 0, 6),
-      std::make_tuple(kTombstone, 1, 5),
-      std::make_tuple(kColumn, 2, 4),
-      std::make_tuple(kTombstone, 11, 11),
+      CreateTestColumnSpec(kExpiringColumn, 0, 6),
+      CreateTestColumnSpec(kTombstone, 1, 5),
+      CreateTestColumnSpec(kColumn, 2, 4),
+      CreateTestColumnSpec(kTombstone, 11, 11),
     })
   );
 
@@ -60,24 +60,24 @@ TEST(RowValueMergeTest, MergeWithRowTombstone) {
   // This row's timestamp is smaller than tombstone.
   row_values.push_back(
     CreateTestRowValue({
-      std::make_tuple(kColumn, 0, 5),
-      std::make_tuple(kColumn, 1, 6),
+      CreateTestColumnSpec(kColumn, 0, 5),
+      CreateTestColumnSpec(kColumn, 1, 6),
     })
   );
 
   // Some of the column's row is smaller, some is larger.
   row_values.push_back(
     CreateTestRowValue({
-      std::make_tuple(kColumn, 2, 10),
-      std::make_tuple(kColumn, 3, 12),
+      CreateTestColumnSpec(kColumn, 2, 10),
+      CreateTestColumnSpec(kColumn, 3, 12),
     })
   );
 
   // All of the column's rows are larger than tombstone.
   row_values.push_back(
     CreateTestRowValue({
-      std::make_tuple(kColumn, 4, 13),
-      std::make_tuple(kColumn, 5, 14),
+      CreateTestColumnSpec(kColumn, 4, 13),
+      CreateTestColumnSpec(kColumn, 5, 14),
     })
   );
 

utilities/cassandra/format.cc

@@ -129,7 +129,7 @@ std::shared_ptr<Tombstone> ExpiringColumn::ToTombstone() const {
   int64_t marked_for_delete_at =
     std::chrono::duration_cast<std::chrono::microseconds>(expired_at).count();
   return std::make_shared<Tombstone>(
-    ColumnTypeMask::DELETION_MASK,
+    static_cast<int8_t>(ColumnTypeMask::DELETION_MASK),
     Index(),
     local_deletion_time,
     marked_for_delete_at);

utilities/cassandra/test_utils.cc

@@ -29,6 +29,12 @@ std::shared_ptr<ColumnBase> CreateTestColumn(int8_t mask,
   }
 }
 
+std::tuple<int8_t, int8_t, int64_t> CreateTestColumnSpec(int8_t mask,
+                                                         int8_t index,
+                                                         int64_t timestamp) {
+  return std::make_tuple(mask, index, timestamp);
+}
+
 RowValue CreateTestRowValue(
     std::vector<std::tuple<int8_t, int8_t, int64_t>> column_specs) {
   std::vector<std::shared_ptr<ColumnBase>> columns;

utilities/cassandra/test_utils.h

@@ -23,6 +23,10 @@ std::shared_ptr<ColumnBase> CreateTestColumn(int8_t mask,
                                              int8_t index,
                                              int64_t timestamp);
 
+std::tuple<int8_t, int8_t, int64_t> CreateTestColumnSpec(int8_t mask,
+                                                         int8_t index,
+                                                         int64_t timestamp);
+
 RowValue CreateTestRowValue(
     std::vector<std::tuple<int8_t, int8_t, int64_t>> column_specs);
 

utilities/transactions/write_prepared_transaction_test.cc

@@ -436,83 +436,6 @@ TEST_P(WritePreparedTransactionTest, MaybeUpdateOldCommitMap) {
   MaybeUpdateOldCommitMapTestWithNext(p, c, s, ns, false);
 }
 
-// Test that the entries in old_commit_map_ get garbage collected properly
-TEST_P(WritePreparedTransactionTest, OldCommitMapGC) {
-  const size_t snapshot_cache_bits = 0;
-  const size_t commit_cache_bits = 0;
-  DBImpl* mock_db = new DBImpl(options, dbname);
-  std::unique_ptr<WritePreparedTxnDBMock> wp_db(new WritePreparedTxnDBMock(
-      mock_db, txn_db_options, snapshot_cache_bits, commit_cache_bits));
-
-  SequenceNumber seq = 0;
-  // Take the first snapshot that overlaps with two txn
-  auto prep_seq = ++seq;
-  wp_db->AddPrepared(prep_seq);
-  auto prep_seq2 = ++seq;
-  wp_db->AddPrepared(prep_seq2);
-  auto snap_seq1 = seq;
-  wp_db->TakeSnapshot(snap_seq1);
-  auto commit_seq = ++seq;
-  wp_db->AddCommitted(prep_seq, commit_seq);
-  auto commit_seq2 = ++seq;
-  wp_db->AddCommitted(prep_seq2, commit_seq2);
-  // Take the 2nd and 3rd snapshot that overlap with the same txn
-  prep_seq = ++seq;
-  wp_db->AddPrepared(prep_seq);
-  auto snap_seq2 = seq;
-  wp_db->TakeSnapshot(snap_seq2);
-  seq++;
-  auto snap_seq3 = seq;
-  wp_db->TakeSnapshot(snap_seq3);
-  seq++;
-  commit_seq = ++seq;
-  wp_db->AddCommitted(prep_seq, commit_seq);
-  // Make sure max_evicted_seq_ will be larger than 2nd snapshot by evicting the
-  // only item in the commit_cache_ via another commit.
-  prep_seq = ++seq;
-  wp_db->AddPrepared(prep_seq);
-  commit_seq = ++seq;
-  wp_db->AddCommitted(prep_seq, commit_seq);
-
-  // Verify that the evicted commit entries for all snapshots are in the
-  // old_commit_map_
-  {
-    ASSERT_FALSE(wp_db->old_commit_map_empty_.load());
-    ReadLock rl(&wp_db->old_commit_map_mutex_);
-    ASSERT_EQ(3, wp_db->old_commit_map_.size());
-    ASSERT_EQ(2, wp_db->old_commit_map_[snap_seq1].size());
-    ASSERT_EQ(1, wp_db->old_commit_map_[snap_seq2].size());
-    ASSERT_EQ(1, wp_db->old_commit_map_[snap_seq3].size());
-  }
-
-  // Verify that the 2nd snapshot is cleaned up after the release
-  wp_db->ReleaseSnapshotInternal(snap_seq2);
-  {
-    ASSERT_FALSE(wp_db->old_commit_map_empty_.load());
-    ReadLock rl(&wp_db->old_commit_map_mutex_);
-    ASSERT_EQ(2, wp_db->old_commit_map_.size());
-    ASSERT_EQ(2, wp_db->old_commit_map_[snap_seq1].size());
-    ASSERT_EQ(1, wp_db->old_commit_map_[snap_seq3].size());
-  }
-
-  // Verify that the 1st snapshot is cleaned up after the release
-  wp_db->ReleaseSnapshotInternal(snap_seq1);
-  {
-    ASSERT_FALSE(wp_db->old_commit_map_empty_.load());
-    ReadLock rl(&wp_db->old_commit_map_mutex_);
-    ASSERT_EQ(1, wp_db->old_commit_map_.size());
-    ASSERT_EQ(1, wp_db->old_commit_map_[snap_seq3].size());
-  }
-
-  // Verify that the 3rd snapshot is cleaned up after the release
-  wp_db->ReleaseSnapshotInternal(snap_seq3);
-  {
-    ASSERT_TRUE(wp_db->old_commit_map_empty_.load());
-    ReadLock rl(&wp_db->old_commit_map_mutex_);
-    ASSERT_EQ(0, wp_db->old_commit_map_.size());
-  }
-}
-
 TEST_P(WritePreparedTransactionTest, CheckAgainstSnapshotsTest) {
   std::vector<SequenceNumber> snapshots = {100l, 200l, 300l, 400l, 500l,
                                            600l, 700l, 800l, 900l};
@@ -557,24 +480,20 @@ TEST_P(WritePreparedTransactionTest, SnapshotConcurrentAccessTest) {
   // We have a sync point in the method under test after checking each snapshot.
   // If you increase the max number of snapshots in this test, more sync points
   // in the methods must also be added.
-  const std::vector<SequenceNumber> snapshots = {10l, 20l, 30l, 40l, 50l, 60l};
-  // TODO(myabandeh): increase the snapshots list for pre-release tests
-  // const std::vector<SequenceNumber> snapshots = {10l, 20l, 30l, 40l, 50l,
-  //                                               60l, 70l, 80l, 90l, 100l};
+  const std::vector<SequenceNumber> snapshots = {10l, 20l, 30l, 40l, 50l,
+                                                 60l, 70l, 80l, 90l, 100l};
   const size_t snapshot_cache_bits = 2;
   // Safety check to express the intended size in the test. Can be adjusted if
   // the snapshots lists changed.
-  assert((1ul << snapshot_cache_bits) + 2 == snapshots.size());
+  assert((1ul << snapshot_cache_bits) * 2 + 2 == snapshots.size());
   SequenceNumber version = 1000l;
   // Choose the cache size so that the new snapshot list could replace all the
   // existing items in the cache and also have some overflow.
   DBImpl* mock_db = new DBImpl(options, dbname);
   std::unique_ptr<WritePreparedTxnDBMock> wp_db(
       new WritePreparedTxnDBMock(mock_db, txn_db_options, snapshot_cache_bits));
-  // TODO(myabandeh): increase this number for pre-release tests
-  const size_t extra = 1;
-  // Add up to extra items that do not fit into the cache
-  for (size_t old_size = 1; old_size <= wp_db->SNAPSHOT_CACHE_SIZE + extra;
+  // Add up to 2 items that do not fit into the cache
+  for (size_t old_size = 1; old_size <= wp_db->SNAPSHOT_CACHE_SIZE + 2;
        old_size++) {
     const std::vector<SequenceNumber> old_snapshots(
         snapshots.begin(), snapshots.begin() + old_size);

utilities/transactions/write_prepared_txn_db.cc

@@ -12,7 +12,6 @@
 #include "utilities/transactions/write_prepared_txn_db.h"
 
 #include <inttypes.h>
-#include <algorithm>
 #include <string>
 #include <unordered_set>
 #include <vector>
@@ -235,8 +234,8 @@ bool WritePreparedTxnDB::IsInSnapshot(uint64_t prep_seq,
   // delayed_prepared_. Also we move evicted entries from commit cache to
   // old_commit_map_ if it overlaps with any snapshot. Since prep_seq <=
   // max_evicted_seq_, we have three cases: i) in delayed_prepared_, ii) in
-  // old_commit_map_, iii) committed with no conflict with any snapshot. Case
-  // (i) delayed_prepared_ is checked above
+  // old_commit_map_, iii) committed with no conflict with any snapshot (i)
+  // delayed_prepared_ is checked above
   if (max_evicted_seq < snapshot_seq) {  // then (ii) cannot be the case
     // only (iii) is the case: committed
     // commit_seq <= max_evicted_seq_ < snapshot_seq => commit_seq <
@@ -256,17 +255,12 @@ bool WritePreparedTxnDB::IsInSnapshot(uint64_t prep_seq,
     return true;
   }
   {
-    // We should not normally reach here unless sapshot_seq is old. This is a
-    // rare case and it is ok to pay the cost of mutex ReadLock for such old,
-    // reading transactions.
+    // We should not normally reach here
+    // TODO(myabandeh): check only if snapshot_seq is in the list of snaphots
     ReadLock rl(&old_commit_map_mutex_);
-    auto prep_set_entry = old_commit_map_.find(snapshot_seq);
-    bool found = prep_set_entry != old_commit_map_.end();
-    if (found) {
-      auto& vec = prep_set_entry->second;
-      found = std::binary_search(vec.begin(), vec.end(), prep_seq);
-    }
-    if (!found) {
+    auto old_commit_entry = old_commit_map_.find(prep_seq);
+    if (old_commit_entry == old_commit_map_.end() ||
+        old_commit_entry->second <= snapshot_seq) {
       ROCKSDB_LOG_DETAILS(
           info_log_, "IsInSnapshot %" PRIu64 " in %" PRIu64 " returns %" PRId32,
           prep_seq, snapshot_seq, 1);
@@ -328,9 +322,6 @@ void WritePreparedTxnDB::AddCommitted(uint64_t prepare_seq,
   bool to_be_evicted = GetCommitEntry(indexed_seq, &evicted_64b, &evicted);
   if (to_be_evicted) {
     auto prev_max = max_evicted_seq_.load(std::memory_order_acquire);
-    ROCKSDB_LOG_DETAILS(info_log_,
-                        "Evicting %" PRIu64 ",%" PRIu64 " with max %" PRIu64,
-                        evicted.prep_seq, evicted.commit_seq, prev_max);
     if (prev_max < evicted.commit_seq) {
       // Inc max in larger steps to avoid frequent updates
       auto max_evicted_seq = evicted.commit_seq + INC_STEP_FOR_MAX_EVICTED;
@@ -440,36 +431,6 @@ const std::vector<SequenceNumber> WritePreparedTxnDB::GetSnapshotListFromDB(
   return db_impl_->snapshots().GetAll(nullptr, max);
 }
 
-void WritePreparedTxnDB::ReleaseSnapshot(const Snapshot* snapshot) {
-  auto snap_seq = snapshot->GetSequenceNumber();
-  ReleaseSnapshotInternal(snap_seq);
-  db_impl_->ReleaseSnapshot(snapshot);
-}
-
-void WritePreparedTxnDB::ReleaseSnapshotInternal(
-    const SequenceNumber snap_seq) {
-  // relax is enough since max increases monotonically, i.e., if snap_seq <
-  // old_max => snap_seq < new_max as well.
-  if (snap_seq < max_evicted_seq_.load(std::memory_order_relaxed)) {
-    // Then this is a rare case that transaction did not finish before max
-    // advances. It is expected for a few read-only backup snapshots. For such
-    // snapshots we might have kept around a couple of entries in the
-    // old_commit_map_. Check and do garbage collection if that is the case.
-    bool need_gc = false;
-    {
-      ReadLock rl(&old_commit_map_mutex_);
-      auto prep_set_entry = old_commit_map_.find(snap_seq);
-      need_gc = prep_set_entry != old_commit_map_.end();
-    }
-    if (need_gc) {
-      WriteLock wl(&old_commit_map_mutex_);
-      old_commit_map_.erase(snap_seq);
-      old_commit_map_empty_.store(old_commit_map_.empty(),
-                                  std::memory_order_release);
-    }
-  }
-}
-
 void WritePreparedTxnDB::UpdateSnapshots(
     const std::vector<SequenceNumber>& snapshots,
     const SequenceNumber& version) {
@@ -580,8 +541,8 @@ void WritePreparedTxnDB::CheckAgainstSnapshots(const CommitEntry& evicted) {
 bool WritePreparedTxnDB::MaybeUpdateOldCommitMap(
     const uint64_t& prep_seq, const uint64_t& commit_seq,
     const uint64_t& snapshot_seq, const bool next_is_larger = true) {
-  // If we do not store an entry in old_commit_map_ we assume it is committed in
-  // all snapshots. If commit_seq <= snapshot_seq, it is considered already in
+  // If we do not store an entry in old_commit_map we assume it is committed in
+  // all snapshots. if commit_seq <= snapshot_seq, it is considered already in
   // the snapshot so we need not to keep the entry around for this snapshot.
   if (commit_seq <= snapshot_seq) {
     // continue the search if the next snapshot could be smaller than commit_seq
@@ -591,11 +552,9 @@ bool WritePreparedTxnDB::MaybeUpdateOldCommitMap(
   if (prep_seq <= snapshot_seq) {  // overlapping range
     WriteLock wl(&old_commit_map_mutex_);
     old_commit_map_empty_.store(false, std::memory_order_release);
-    auto& vec = old_commit_map_[snapshot_seq];
-    vec.insert(std::upper_bound(vec.begin(), vec.end(), prep_seq), prep_seq);
-    // We need to store it once for each overlapping snapshot. Returning true to
-    // continue the search if there is more overlapping snapshot.
-    return true;
+    old_commit_map_[prep_seq] = commit_seq;
+    // Storing once is enough. No need to check it for other snapshots.
+    return false;
   }
   // continue the search if the next snapshot could be larger than prep_seq
   return next_is_larger;

utilities/transactions/write_prepared_txn_db.h

@@ -90,8 +90,6 @@ class WritePreparedTxnDB : public PessimisticTransactionDB {
       const std::vector<ColumnFamilyHandle*>& column_families,
       std::vector<Iterator*>* iterators) override;
 
-  virtual void ReleaseSnapshot(const Snapshot* snapshot) override;
-
   // Check whether the transaction that wrote the value with seqeunce number seq
   // is visible to the snapshot with sequence number snapshot_seq
   bool IsInSnapshot(uint64_t seq, uint64_t snapshot_seq) const;
@@ -200,7 +198,6 @@ class WritePreparedTxnDB : public PessimisticTransactionDB {
   friend class WritePreparedTransactionTest_AdvanceMaxEvictedSeqBasicTest_Test;
   friend class WritePreparedTransactionTest_BasicRecoveryTest_Test;
   friend class WritePreparedTransactionTest_IsInSnapshotEmptyMapTest_Test;
-  friend class WritePreparedTransactionTest_OldCommitMapGC_Test;
   friend class WritePreparedTransactionTest_RollbackTest_Test;
 
   void Init(const TransactionDBOptions& /* unused */);
@@ -272,10 +269,6 @@ class WritePreparedTxnDB : public PessimisticTransactionDB {
   virtual const std::vector<SequenceNumber> GetSnapshotListFromDB(
       SequenceNumber max);
 
-  // Will be called by the public ReleaseSnapshot method. Does the maintenance
-  // internal to WritePreparedTxnDB
-  void ReleaseSnapshotInternal(const SequenceNumber snap_seq);
-
   // Update the list of snapshots corresponding to the soon-to-be-updated
   // max_eviceted_seq_. Thread-safety: this function can be called concurrently.
   // The concurrent invocations of this function is equivalent to a serial
@@ -294,8 +287,9 @@ class WritePreparedTxnDB : public PessimisticTransactionDB {
 
   // Add a new entry to old_commit_map_ if prep_seq <= snapshot_seq <
   // commit_seq. Return false if checking the next snapshot(s) is not needed.
-  // This is the case if none of the next snapshots could satisfy the condition.
-  // next_is_larger: the next snapshot will be a larger value
+  // This is the case if the entry already added to old_commit_map_ or none of
+  // the next snapshots could satisfy the condition. next_is_larger: the next
+  // snapshot will be a larger value
   bool MaybeUpdateOldCommitMap(const uint64_t& prep_seq,
                                const uint64_t& commit_seq,
                                const uint64_t& snapshot_seq,
@@ -339,14 +333,10 @@ class WritePreparedTxnDB : public PessimisticTransactionDB {
   // it to be too large either as it would cause stalls by doing too much
   // maintenance work under the lock.
   size_t INC_STEP_FOR_MAX_EVICTED = 1;
-  // A map from old snapshots (expected to be used by a few read-only txns) to
-  // prpared sequence number of the evicted entries from commit_cache_ that
-  // overlaps with such snapshot. These are the prepared sequence numbers that
-  // the snapshot, to which they are mapped, cannot assume to be committed just
-  // because it is no longer in the commit_cache_. The vector must be sorted
-  // after each update.
+  // A map of the evicted entries from commit_cache_ that has to be kept around
+  // to service the old snapshots. This is expected to be empty normally.
   // Thread-safety is provided with old_commit_map_mutex_.
-  std::map<SequenceNumber, std::vector<SequenceNumber>> old_commit_map_;
+  std::map<uint64_t, uint64_t> old_commit_map_;
   // A set of long-running prepared transactions that are not finished by the
   // time max_evicted_seq_ advances their sequence number. This is expected to
   // be empty normally. Thread-safety is provided with prepared_mutex_.