Disable warning as error

Summary: N/A

Test Plan: N/A

Reviewers: yhchiang, ljin

Reviewed By: ljin

Subscribers: dhruba, leveldb, igor

Differential Revision: https://reviews.facebook.net/D22053

.gitignore

@@ -32,3 +32,4 @@ coverage/COVERAGE_REPORT
 tags
 java/*.log
 java/include/org_rocksdb_*.h
+unity.cc

HISTORY.md

@@ -2,8 +2,15 @@
 
 ### Unreleased
 ### New Features
+
+### Public API changes
+
+
+## 3.4.0 (8/18/2014)
+### New Features
 * Support Multiple DB paths in universal style compactions
 * Add feature of storing plain table index and bloom filter in SST file.
+* CompactRange() will never output compacted files to level 0. This used to be the case when all the compaction input files were at level 0.
 
 ### Public API changes
 * DBOptions.db_paths now is a vector of a DBPath structure which indicates both of path and target size

Makefile

@@ -49,7 +49,6 @@ else
 	PLATFORM_CCFLAGS += $(JEMALLOC_INCLUDE) -DHAVE_JEMALLOC
 endif
 
-WARNING_FLAGS = -Wall -Werror -Wsign-compare
 CFLAGS += $(WARNING_FLAGS) -I. -I./include $(PLATFORM_CCFLAGS) $(OPT)
 CXXFLAGS += $(WARNING_FLAGS) -I. -I./include $(PLATFORM_CXXFLAGS) $(OPT) -Woverloaded-virtual
 
@@ -234,8 +233,14 @@ valgrind_check: all $(PROGRAMS) $(TESTS)
 		echo $$t $$((etime - stime)) >> $(VALGRIND_DIR)/valgrind_tests_times; \
 	done
 
+unity.cc:	
+	$(shell (export ROCKSDB_ROOT="$(CURDIR)"; "$(CURDIR)/build_tools/unity" "$(CURDIR)/unity.cc"))
+
+unity: unity.cc unity.o
+	$(CXX) unity.o $(EXEC_LDFLAGS) -o $@ $(LDFLAGS) $(COVERAGEFLAGS)
+
 clean:
-	-rm -f $(PROGRAMS) $(TESTS) $(LIBRARY) $(SHARED) $(MEMENVLIBRARY) build_config.mk
+	-rm -f $(PROGRAMS) $(TESTS) $(LIBRARY) $(SHARED) $(MEMENVLIBRARY) build_config.mk unity.cc
 	-rm -rf ios-x86/* ios-arm/*
 	-find . -name "*.[od]" -exec rm {} \;
 	-find . -type f -regex ".*\.\(\(gcda\)\|\(gcno\)\)" -exec rm {} \;

build_tools/build_detect_platform

@@ -49,13 +49,7 @@ COMMON_FLAGS="-DROCKSDB_PLATFORM_POSIX"
 if [ -d /mnt/gvfs/third-party -a -z "$CXX" ]; then
     FBCODE_BUILD="true"
     if [ -z "$USE_CLANG" ]; then
-        CENTOS_VERSION=`rpm -q --qf "%{VERSION}" \
-          $(rpm -q --whatprovides redhat-release)`
-        if [ "$CENTOS_VERSION" = "6" ]; then
-          source "$PWD/build_tools/fbcode.gcc481.sh"
-        else
-          source "$PWD/build_tools/fbcode.gcc471.sh"
-        fi
+        source $PWD/build_tools/fbcode_config.sh
     else
         source "$PWD/build_tools/fbcode.clang31.sh"
     fi
@@ -242,7 +236,7 @@ EOF
       int main() {}
 EOF
     if [ "$?" = 0 ]; then
-        COMMON_FLAGS="$COMMON_FLAGS -DGFLAGS=google"
+        COMMON_FLAGS="$COMMON_FLAGS -DGFLAGS=gflags"
         PLATFORM_LDFLAGS="$PLATFORM_LDFLAGS -lgflags"
     fi
 

build_tools/dependencies.sh

@@ -0,0 +1,19 @@
+# Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
+GCC_BASE=/mnt/gvfs/third-party2/gcc/7331085db891a2ef4a88a48a751d834e8d68f4cb/7.x/centos7-native/b2ef2b6
+CLANG_BASE=/mnt/gvfs/third-party2/llvm-fb/963d9aeda70cc4779885b1277484fe7544a04e3e/9.0.0/platform007/9e92d53/
+LIBGCC_BASE=/mnt/gvfs/third-party2/libgcc/6ace84e956873d53638c738b6f65f3f469cca74c/7.x/platform007/5620abc
+GLIBC_BASE=/mnt/gvfs/third-party2/glibc/192b0f42d63dcf6210d6ceae387b49af049e6e0c/2.26/platform007/f259413
+SNAPPY_BASE=/mnt/gvfs/third-party2/snappy/7f9bdaada18f59bc27ec2b0871eb8a6144343aef/1.1.3/platform007/ca4da3d
+ZLIB_BASE=/mnt/gvfs/third-party2/zlib/2d9f0b9a4274cc21f61272a9e89bdb859bce8f1f/1.2.8/platform007/ca4da3d
+BZIP2_BASE=/mnt/gvfs/third-party2/bzip2/dc49a21c5fceec6456a7a28a94dcd16690af1337/1.0.6/platform007/ca4da3d
+LZ4_BASE=/mnt/gvfs/third-party2/lz4/0f607f8fc442ea7d6b876931b1898bb573d5e5da/1.9.1/platform007/ca4da3d
+ZSTD_BASE=/mnt/gvfs/third-party2/zstd/ca22bc441a4eb709e9e0b1f9fec9750fed7b31c5/1.4.x/platform007/15a3614
+GFLAGS_BASE=/mnt/gvfs/third-party2/gflags/0b9929d2588991c65a57168bf88aff2db87c5d48/2.2.0/platform007/ca4da3d
+JEMALLOC_BASE=/mnt/gvfs/third-party2/jemalloc/c26f08f47ac35fc31da2633b7da92d6b863246eb/master/platform007/c26c002
+NUMA_BASE=/mnt/gvfs/third-party2/numa/3f3fb57a5ccc5fd21c66416c0b83e0aa76a05376/2.0.11/platform007/ca4da3d
+LIBUNWIND_BASE=/mnt/gvfs/third-party2/libunwind/40c73d874898b386a71847f1b99115d93822d11f/1.4/platform007/6f3e0a9
+TBB_BASE=/mnt/gvfs/third-party2/tbb/4ce8e8dba77cdbd81b75d6f0c32fd7a1b76a11ec/2018_U5/platform007/ca4da3d
+KERNEL_HEADERS_BASE=/mnt/gvfs/third-party2/kernel-headers/fb251ecd2f5ae16f8671f7014c246e52a748fe0b/fb/platform007/da39a3e
+BINUTILS_BASE=/mnt/gvfs/third-party2/binutils/ab9f09bba370e7066cafd4eb59752db93f2e8312/2.29.1/platform007/15a3614
+VALGRIND_BASE=/mnt/gvfs/third-party2/valgrind/d42d152a15636529b0861ec493927200ebebca8e/3.15.0/platform007/ca4da3d
+LUA_BASE=/mnt/gvfs/third-party2/lua/f0cd714433206d5139df61659eb7b28b1dea6683/5.3.4/platform007/5007832

build_tools/fbcode_config.sh

@@ -0,0 +1,134 @@
+#!/bin/sh
+#
+# Set environment variables so that we can compile rocksdb using
+# fbcode settings.  It uses the latest g++ and clang compilers and also
+# uses jemalloc
+# Environment variables that change the behavior of this script:
+# PIC_BUILD -- if true, it will only take pic versions of libraries from fbcode. libraries that don't have pic variant will not be included
+
+
+BASEDIR=`dirname $BASH_SOURCE`
+source "$BASEDIR/dependencies.sh"
+
+CFLAGS=""
+
+# libgcc
+LIBGCC_INCLUDE="$LIBGCC_BASE/include/c++/7.3.0"
+LIBGCC_LIBS=" -L $LIBGCC_BASE/lib"
+
+# glibc
+GLIBC_INCLUDE="$GLIBC_BASE/include"
+GLIBC_LIBS=" -L $GLIBC_BASE/lib"
+
+# snappy
+SNAPPY_INCLUDE=" -I $SNAPPY_BASE/include/"
+if test -z $PIC_BUILD; then
+  SNAPPY_LIBS=" $SNAPPY_BASE/lib/libsnappy.a"
+else
+  SNAPPY_LIBS=" $SNAPPY_BASE/lib/libsnappy_pic.a"
+fi
+CFLAGS+=" -DSNAPPY"
+
+if test -z $PIC_BUILD; then
+  # location of zlib headers and libraries
+  ZLIB_INCLUDE=" -I $ZLIB_BASE/include/"
+  ZLIB_LIBS=" $ZLIB_BASE/lib/libz.a"
+  CFLAGS+=" -DZLIB"
+
+  LZ4_INCLUDE=" -I $LZ4_BASE/include/"
+  LZ4_LIBS=" $LZ4_BASE/lib/liblz4.a"
+  CFLAGS+=" -DLZ4"
+fi
+
+# location of gflags headers and libraries
+GFLAGS_INCLUDE=" -I $GFLAGS_BASE/include/"
+if test -z $PIC_BUILD; then
+  GFLAGS_LIBS=" $GFLAGS_BASE/lib/libgflags.a"
+else
+  GFLAGS_LIBS=" $GFLAGS_BASE/lib/libgflags_pic.a"
+fi
+CFLAGS+=" -DGFLAGS=gflags"
+
+# location of jemalloc
+JEMALLOC_INCLUDE=" -I $JEMALLOC_BASE/include/"
+JEMALLOC_LIB=" $JEMALLOC_BASE/lib/libjemalloc.a"
+
+if test -z $PIC_BUILD; then
+  # location of libunwind
+  LIBUNWIND="$LIBUNWIND_BASE/lib/libunwind.a"
+fi
+
+# location of TBB
+TBB_INCLUDE=" -isystem $TBB_BASE/include/"
+if test -z $PIC_BUILD; then
+  TBB_LIBS="$TBB_BASE/lib/libtbb.a"
+else
+  TBB_LIBS="$TBB_BASE/lib/libtbb_pic.a"
+fi
+CFLAGS+=" -DTBB"
+
+# use Intel SSE support for checksum calculations
+export USE_SSE=" -msse -msse4.2 "
+
+BINUTILS="$BINUTILS_BASE/bin"
+AR="$BINUTILS/ar"
+
+DEPS_INCLUDE="$SNAPPY_INCLUDE $ZLIB_INCLUDE $LZ4_INCLUDE $GFLAGS_INCLUDE"
+
+STDLIBS="-L $GCC_BASE/lib64"
+
+CLANG_BIN="$CLANG_BASE/bin"
+CLANG_LIB="$CLANG_BASE/lib"
+CLANG_SRC="$CLANG_BASE/../../src"
+
+CLANG_ANALYZER="$CLANG_BIN/clang++"
+CLANG_SCAN_BUILD="$CLANG_SRC/llvm/tools/clang/tools/scan-build/bin/scan-build"
+
+if [ -z "$USE_CLANG" ]; then
+  # gcc
+  CC="$GCC_BASE/bin/gcc"
+  CXX="$GCC_BASE/bin/g++"
+
+  CFLAGS+=" -B$BINUTILS/gold"
+  CFLAGS+=" -isystem $LIBGCC_INCLUDE"
+  CFLAGS+=" -isystem $GLIBC_INCLUDE"
+  JEMALLOC=1
+else
+  # clang
+  CLANG_INCLUDE="$CLANG_LIB/clang/stable/include"
+  CC="$CLANG_BIN/clang"
+  CXX="$CLANG_BIN/clang++"
+
+  KERNEL_HEADERS_INCLUDE="$KERNEL_HEADERS_BASE/include"
+
+  CFLAGS+=" -B$BINUTILS/gold -nostdinc -nostdlib"
+  CFLAGS+=" -isystem $LIBGCC_BASE/include/c++/7.x "
+  CFLAGS+=" -isystem $LIBGCC_BASE/include/c++/7.x/x86_64-facebook-linux "
+  CFLAGS+=" -isystem $GLIBC_INCLUDE"
+  CFLAGS+=" -isystem $LIBGCC_INCLUDE"
+  CFLAGS+=" -isystem $CLANG_INCLUDE"
+  CFLAGS+=" -isystem $KERNEL_HEADERS_INCLUDE/linux "
+  CFLAGS+=" -isystem $KERNEL_HEADERS_INCLUDE "
+  CFLAGS+=" -Wno-expansion-to-defined "
+  CXXFLAGS="-nostdinc++"
+fi
+
+CFLAGS+=" $DEPS_INCLUDE"
+CFLAGS+=" -DROCKSDB_PLATFORM_POSIX -DROCKSDB_LIB_IO_POSIX -DROCKSDB_FALLOCATE_PRESENT -DROCKSDB_MALLOC_USABLE_SIZE -DROCKSDB_RANGESYNC_PRESENT -DROCKSDB_SCHED_GETCPU_PRESENT -DROCKSDB_SUPPORT_THREAD_LOCAL -DHAVE_SSE42"
+CXXFLAGS+=" $CFLAGS"
+
+EXEC_LDFLAGS=" $SNAPPY_LIBS $ZLIB_LIBS $LZ4_LIBS $ZSTD_LIBS $GFLAGS_LIBS $NUMA_LIB $TBB_LIBS"
+EXEC_LDFLAGS+=" -B$BINUTILS/gold"
+EXEC_LDFLAGS+=" -Wl,--dynamic-linker,/usr/local/fbcode/platform007/lib/ld.so"
+EXEC_LDFLAGS+=" $LIBUNWIND"
+EXEC_LDFLAGS+=" -Wl,-rpath=/usr/local/fbcode/platform007/lib"
+# required by libtbb
+EXEC_LDFLAGS+=" -ldl"
+
+PLATFORM_LDFLAGS="$LIBGCC_LIBS $GLIBC_LIBS $STDLIBS -lgcc -lstdc++"
+
+EXEC_LDFLAGS_SHARED="$SNAPPY_LIBS $ZLIB_LIBS $LZ4_LIBS $ZSTD_LIBS $GFLAGS_LIBS $TBB_LIBS"
+
+VALGRIND_VER="$VALGRIND_BASE/bin/"
+
+export CC CXX AR CFLAGS CXXFLAGS EXEC_LDFLAGS EXEC_LDFLAGS_SHARED VALGRIND_VER JEMALLOC_LIB JEMALLOC_INCLUDE CLANG_ANALYZER CLANG_SCAN_BUILD 

build_tools/unity

@@ -0,0 +1,78 @@
+#!/bin/sh
+#
+# Create the unity file
+#
+
+OUTPUT=$1
+if test -z "$OUTPUT"; then
+  echo "usage: $0 <output-filename>" >&2
+  exit 1
+fi
+
+# Delete existing file, if it exists
+rm -f "$OUTPUT"
+touch "$OUTPUT"
+
+# Detect OS
+if test -z "$TARGET_OS"; then
+    TARGET_OS=`uname -s`
+fi
+
+# generic port files (working on all platform by #ifdef) go directly in /port
+GENERIC_PORT_FILES=`cd "$ROCKSDB_ROOT"; find port -name '*.cc' | tr "\n" " "`
+
+# On GCC, we pick libc's memcmp over GCC's memcmp via -fno-builtin-memcmp
+case "$TARGET_OS" in
+    Darwin)
+        # PORT_FILES=port/darwin/darwin_specific.cc
+        ;;
+    IOS)
+        ;;
+    Linux)
+        # PORT_FILES=port/linux/linux_specific.cc
+        ;;
+    SunOS)
+        # PORT_FILES=port/sunos/sunos_specific.cc
+        ;;
+    FreeBSD)
+        # PORT_FILES=port/freebsd/freebsd_specific.cc
+        ;;
+    NetBSD)
+        # PORT_FILES=port/netbsd/netbsd_specific.cc
+        ;;
+    OpenBSD)
+        # PORT_FILES=port/openbsd/openbsd_specific.cc
+        ;;
+    DragonFly)
+        # PORT_FILES=port/dragonfly/dragonfly_specific.cc
+        ;;
+    OS_ANDROID_CROSSCOMPILE)
+        # PORT_FILES=port/android/android.cc
+        ;;
+    *)
+        echo "Unknown platform!" >&2
+        exit 1
+esac
+
+# We want to make a list of all cc files within util, db, table, and helpers
+# except for the test and benchmark files. By default, find will output a list
+# of all files matching either rule, so we need to append -print to make the
+# prune take effect.
+DIRS="util db table utilities"
+
+set -f # temporarily disable globbing so that our patterns arent expanded
+PRUNE_TEST="-name *test*.cc -prune"
+PRUNE_BENCH="-name *bench*.cc -prune"
+PORTABLE_FILES=`cd "$ROCKSDB_ROOT"; find $DIRS $PRUNE_TEST -o $PRUNE_BENCH -o -name '*.cc' -print | sort`
+PORTABLE_CPP=`cd "$ROCKSDB_ROOT"; find $DIRS $PRUNE_TEST -o $PRUNE_BENCH -o -name '*.cpp' -print | sort`
+set +f # re-enable globbing
+
+# The sources consist of the portable files, plus the platform-specific port
+# file.
+for SOURCE_FILE in $PORTABLE_FILES $GENERIC_PORT_FILES $PORT_FILES $PORTABLE_CPP
+do
+	echo "#include <$SOURCE_FILE>" >> "$OUTPUT"
+done
+
+echo "int main(int argc, char** argv){ return 0; }" >> "$OUTPUT"
+

db/builder.cc

@@ -113,6 +113,13 @@ Status BuildTable(const std::string& dbname, Env* env, const Options& options,
           is_first_key = false;
 
           if (this_ikey.type == kTypeMerge) {
+            // TODO(tbd): Add a check here to prevent RocksDB from crash when
+            // reopening a DB w/o properly specifying the merge operator.  But
+            // currently we observed a memory leak on failing in RocksDB
+            // recovery, so we decide to let it crash instead of causing
+            // memory leak for now before we have identified the real cause
+            // of the memory leak.
+
             // Handle merge-type keys using the MergeHelper
             // TODO: pass statistics to MergeUntil
             merge.MergeUntil(iter, 0 /* don't worry about snapshot */);

db/c.cc

@@ -36,6 +36,9 @@ using rocksdb::ColumnFamilyHandle;
 using rocksdb::ColumnFamilyOptions;
 using rocksdb::CompactionFilter;
 using rocksdb::CompactionFilterFactory;
+using rocksdb::CompactionFilterV2;
+using rocksdb::CompactionFilterFactoryV2;
+using rocksdb::CompactionFilterContext;
 using rocksdb::CompactionOptionsFIFO;
 using rocksdb::Comparator;
 using rocksdb::CompressionType;
@@ -154,6 +157,104 @@ struct rocksdb_compactionfilterfactory_t : public CompactionFilterFactory {
   virtual const char* Name() const { return (*name_)(state_); }
 };
 
+struct rocksdb_compactionfilterv2_t : public CompactionFilterV2 {
+  void* state_;
+  void (*destructor_)(void*);
+  const char* (*name_)(void*);
+  void (*filter_)(void*, int level, size_t num_keys,
+                  const char* const* keys_list, const size_t* keys_list_sizes,
+                  const char* const* existing_values_list, const size_t* existing_values_list_sizes,
+                  char** new_values_list, size_t* new_values_list_sizes,
+                  unsigned char* to_delete_list);
+
+  virtual ~rocksdb_compactionfilterv2_t() {
+    (*destructor_)(state_);
+  }
+
+  virtual const char* Name() const {
+    return (*name_)(state_);
+  }
+
+  virtual std::vector<bool> Filter(int level,
+                                   const SliceVector& keys,
+                                   const SliceVector& existing_values,
+                                   std::vector<std::string>* new_values,
+                                   std::vector<bool>* values_changed) const {
+    // Make a vector pointing to the underlying key data.
+    size_t num_keys = keys.size();
+    std::vector<const char*> keys_list(num_keys);
+    std::vector<size_t> keys_list_sizes(num_keys);
+    for (size_t i = 0; i < num_keys; ++i) {
+      keys_list[i] = keys[i].data();
+      keys_list_sizes[i] = keys[i].size();
+    }
+    // Make a vector pointing to the underlying value data.
+    std::vector<const char*> existing_values_list(num_keys);
+    std::vector<size_t> existing_values_list_sizes(num_keys);
+    for (size_t i = 0; i < num_keys; ++i) {
+      existing_values_list[i] = existing_values[i].data();
+      existing_values_list_sizes[i] = existing_values[i].size();
+    }
+    // Make a vector which will accept newly-allocated char* arrays
+    // which we will take ownership of and assign to strings in new_values.
+    new_values->clear();
+    std::vector<char*> new_values_list(num_keys);
+    std::vector<size_t> new_values_list_sizes(num_keys);
+    // Resize values_changed to hold all keys.
+    values_changed->resize(num_keys);
+    // Make a vector for bools indicating a value should be deleted
+    // on compaction (true) or maintained (false).
+    std::vector<unsigned char> to_delete_list(num_keys);
+
+    (*filter_)(
+        state_, level, num_keys, &keys_list[0], &keys_list_sizes[0],
+        &existing_values_list[0], &existing_values_list_sizes[0],
+        &new_values_list[0], &new_values_list_sizes[0], &to_delete_list[0]);
+
+    // Now, we transfer any changed values, setting values_changed and
+    // initializing new_values in the event a value changed.
+    std::vector<bool> to_delete(num_keys);
+    for (size_t i = 0; i < num_keys; ++i) {
+      to_delete[i] = to_delete_list[i];
+      (*values_changed)[i] = new_values_list[i] != nullptr;
+      if ((*values_changed)[i]) {
+        new_values->push_back(std::string(new_values_list[i], new_values_list_sizes[i]));
+        free(new_values_list[i]);
+      }
+    }
+    return to_delete;
+  }
+};
+
+struct rocksdb_compactionfilterfactoryv2_t : public CompactionFilterFactoryV2 {
+  void* state_;
+  void (*destructor_)(void*);
+  const char* (*name_)(void*);
+  rocksdb_compactionfilterv2_t* (*create_compaction_filter_v2_)(
+      void* state, const rocksdb_compactionfiltercontext_t* context);
+
+  rocksdb_compactionfilterfactoryv2_t(const SliceTransform* prefix_extractor)
+      : CompactionFilterFactoryV2(prefix_extractor) {
+  }
+
+  virtual ~rocksdb_compactionfilterfactoryv2_t() {
+    (*destructor_)(state_);
+  }
+
+  virtual const char* Name() const {
+    return (*name_)(state_);
+  }
+
+  virtual std::unique_ptr<CompactionFilterV2> CreateCompactionFilterV2(
+      const CompactionFilterContext& context) {
+    struct rocksdb_compactionfiltercontext_t c_context;
+    c_context.rep.is_full_compaction = context.is_full_compaction;
+    c_context.rep.is_manual_compaction = context.is_manual_compaction;
+    return std::unique_ptr<CompactionFilterV2>(
+        (*create_compaction_filter_v2_)(state_, &c_context));
+  }
+};
+
 struct rocksdb_comparator_t : public Comparator {
   void* state_;
   void (*destructor_)(void*);
@@ -1004,6 +1105,12 @@ void rocksdb_options_set_merge_operator(
   opt->rep.merge_operator = std::shared_ptr<MergeOperator>(merge_operator);
 }
 
+void rocksdb_options_set_compaction_filter_factory_v2(
+    rocksdb_options_t* opt,
+    rocksdb_compactionfilterfactoryv2_t* compaction_filter_factory_v2) {
+  opt->rep.compaction_filter_factory_v2 = std::shared_ptr<CompactionFilterFactoryV2>(compaction_filter_factory_v2);
+}
+
 void rocksdb_options_set_filter_policy(
     rocksdb_options_t* opt,
     rocksdb_filterpolicy_t* policy) {
@@ -1550,6 +1657,46 @@ void rocksdb_compactionfilterfactory_destroy(
   delete factory;
 }
 
+rocksdb_compactionfilterv2_t* rocksdb_compactionfilterv2_create(
+    void* state,
+    void (*destructor)(void*),
+    void (*filter)(void*, int level, size_t num_keys,
+                   const char* const* keys_list, const size_t* keys_list_sizes,
+                   const char* const* existing_values_list, const size_t* existing_values_list_sizes,
+                   char** new_values_list, size_t* new_values_list_sizes,
+                   unsigned char* to_delete_list),
+    const char* (*name)(void*)) {
+  rocksdb_compactionfilterv2_t* result = new rocksdb_compactionfilterv2_t;
+  result->state_ = state;
+  result->destructor_ = destructor;
+  result->filter_ = filter;
+  result->name_ = name;
+  return result;
+}
+
+void rocksdb_compactionfilterv2_destroy(rocksdb_compactionfilterv2_t* filter) {
+  delete filter;
+}
+
+rocksdb_compactionfilterfactoryv2_t* rocksdb_compactionfilterfactoryv2_create(
+    void* state,
+    rocksdb_slicetransform_t* prefix_extractor,
+    void (*destructor)(void*),
+    rocksdb_compactionfilterv2_t* (*create_compaction_filter_v2)(
+        void* state, const rocksdb_compactionfiltercontext_t* context),
+    const char* (*name)(void*)) {
+  rocksdb_compactionfilterfactoryv2_t* result = new rocksdb_compactionfilterfactoryv2_t(prefix_extractor);
+  result->state_ = state;
+  result->destructor_ = destructor;
+  result->create_compaction_filter_v2_ = create_compaction_filter_v2;
+  result->name_ = name;
+  return result;
+}
+
+void rocksdb_compactionfilterfactoryv2_destroy(rocksdb_compactionfilterfactoryv2_t* factory) {
+  delete factory;
+}
+
 rocksdb_comparator_t* rocksdb_comparator_create(
     void* state,
     void (*destructor)(void*),

db/c_test.c

@@ -225,6 +225,56 @@ static rocksdb_t* CheckCompaction(rocksdb_t* db, rocksdb_options_t* options,
   return db;
 }
 
+// Custom compaction filter V2.
+static void CompactionFilterV2Destroy(void* arg) { }
+static const char* CompactionFilterV2Name(void* arg) {
+  return "TestCompactionFilterV2";
+}
+static void CompactionFilterV2Filter(
+    void* arg, int level, size_t num_keys,
+    const char* const* keys_list, const size_t* keys_list_sizes,
+    const char* const* existing_values_list, const size_t* existing_values_list_sizes,
+    char** new_values_list, size_t* new_values_list_sizes,
+    unsigned char* to_delete_list) {
+  size_t i;
+  for (i = 0; i < num_keys; i++) {
+    // If any value is "gc", it's removed.
+    if (existing_values_list_sizes[i] == 2 && memcmp(existing_values_list[i], "gc", 2) == 0) {
+      to_delete_list[i] = 1;
+    } else if (existing_values_list_sizes[i] == 6 && memcmp(existing_values_list[i], "gc all", 6) == 0) {
+      // If any value is "gc all", all keys are removed.
+      size_t j;
+      for (j = 0; j < num_keys; j++) {
+        to_delete_list[j] = 1;
+      }
+      return;
+    } else if (existing_values_list_sizes[i] == 6 && memcmp(existing_values_list[i], "change", 6) == 0) {
+      // If value is "change", set changed value to "changed".
+      size_t len;
+      len = strlen("changed");
+      new_values_list[i] = malloc(len);
+      memcpy(new_values_list[i], "changed", len);
+      new_values_list_sizes[i] = len;
+    } else {
+      // Otherwise, no keys are removed.
+    }
+  }
+}
+
+// Custom compaction filter factory V2.
+static void CompactionFilterFactoryV2Destroy(void* arg) {
+  rocksdb_slicetransform_destroy((rocksdb_slicetransform_t*)arg);
+}
+static const char* CompactionFilterFactoryV2Name(void* arg) {
+  return "TestCompactionFilterFactoryV2";
+}
+static rocksdb_compactionfilterv2_t* CompactionFilterFactoryV2Create(
+    void* state, const rocksdb_compactionfiltercontext_t* context) {
+  return rocksdb_compactionfilterv2_create(state, CompactionFilterV2Destroy,
+                                           CompactionFilterV2Filter,
+                                           CompactionFilterV2Name);
+}
+
 // Custom merge operator
 static void MergeOperatorDestroy(void* arg) { }
 static const char* MergeOperatorName(void* arg) {
@@ -531,6 +581,47 @@ int main(int argc, char** argv) {
     rocksdb_options_destroy(options);
   }
 
+  StartPhase("compaction_filter_v2");
+  {
+    rocksdb_compactionfilterfactoryv2_t* factory;
+    rocksdb_slicetransform_t* prefix_extractor;
+    prefix_extractor = rocksdb_slicetransform_create_fixed_prefix(3);
+    factory = rocksdb_compactionfilterfactoryv2_create(
+        prefix_extractor, prefix_extractor, CompactionFilterFactoryV2Destroy,
+        CompactionFilterFactoryV2Create, CompactionFilterFactoryV2Name);
+    // Create new database
+    rocksdb_close(db);
+    rocksdb_destroy_db(options, dbname, &err);
+    rocksdb_options_set_compaction_filter_factory_v2(options, factory);
+    db = rocksdb_open(options, dbname, &err);
+    CheckNoError(err);
+    // Only foo2 is GC'd, foo3 is changed.
+    rocksdb_put(db, woptions, "foo1", 4, "no gc", 5, &err);
+    CheckNoError(err);
+    rocksdb_put(db, woptions, "foo2", 4, "gc", 2, &err);
+    CheckNoError(err);
+    rocksdb_put(db, woptions, "foo3", 4, "change", 6, &err);
+    CheckNoError(err);
+    // All bars are GC'd.
+    rocksdb_put(db, woptions, "bar1", 4, "no gc", 5, &err);
+    CheckNoError(err);
+    rocksdb_put(db, woptions, "bar2", 4, "gc all", 6, &err);
+    CheckNoError(err);
+    rocksdb_put(db, woptions, "bar3", 4, "no gc", 5, &err);
+    CheckNoError(err);
+    // Compact the DB to garbage collect.
+    rocksdb_compact_range(db, NULL, 0, NULL, 0);
+
+    // Verify foo entries.
+    CheckGet(db, roptions, "foo1", "no gc");
+    CheckGet(db, roptions, "foo2", NULL);
+    CheckGet(db, roptions, "foo3", "changed");
+    // Verify bar entries were all deleted.
+    CheckGet(db, roptions, "bar1", NULL);
+    CheckGet(db, roptions, "bar2", NULL);
+    CheckGet(db, roptions, "bar3", NULL);
+  }
+
   StartPhase("merge_operator");
   {
     rocksdb_mergeoperator_t* merge_operator;

db/column_family.cc

@@ -49,15 +49,6 @@ ColumnFamilyHandleImpl::~ColumnFamilyHandleImpl() {
 
 uint32_t ColumnFamilyHandleImpl::GetID() const { return cfd()->GetID(); }
 
-namespace {
-// Fix user-supplied options to be reasonable
-template <class T, class V>
-static void ClipToRange(T* ptr, V minvalue, V maxvalue) {
-  if (static_cast<V>(*ptr) > maxvalue) *ptr = maxvalue;
-  if (static_cast<V>(*ptr) < minvalue) *ptr = minvalue;
-}
-}  // anonymous namespace
-
 ColumnFamilyOptions SanitizeOptions(const InternalKeyComparator* icmp,
                                     const InternalFilterPolicy* ipolicy,
                                     const ColumnFamilyOptions& src) {
@@ -191,8 +182,7 @@ void SuperVersionUnrefHandle(void* ptr) {
 }
 }  // anonymous namespace
 
-ColumnFamilyData::ColumnFamilyData(const std::string& dbname, uint32_t id,
-                                   const std::string& name,
+ColumnFamilyData::ColumnFamilyData(uint32_t id, const std::string& name,
                                    Version* dummy_versions, Cache* table_cache,
                                    const ColumnFamilyOptions& options,
                                    const DBOptions* db_options,
@@ -485,7 +475,7 @@ ColumnFamilySet::ColumnFamilySet(const std::string& dbname,
                                  const EnvOptions& storage_options,
                                  Cache* table_cache)
     : max_column_family_(0),
-      dummy_cfd_(new ColumnFamilyData(dbname, 0, "", nullptr, nullptr,
+      dummy_cfd_(new ColumnFamilyData(0, "", nullptr, nullptr,
                                       ColumnFamilyOptions(), db_options,
                                       storage_options_, nullptr)),
       default_cfd_cache_(nullptr),
@@ -556,8 +546,8 @@ ColumnFamilyData* ColumnFamilySet::CreateColumnFamily(
     const ColumnFamilyOptions& options) {
   assert(column_families_.find(name) == column_families_.end());
   ColumnFamilyData* new_cfd =
-      new ColumnFamilyData(db_name_, id, name, dummy_versions, table_cache_,
-                           options, db_options_, storage_options_, this);
+      new ColumnFamilyData(id, name, dummy_versions, table_cache_, options,
+                           db_options_, storage_options_, this);
   Lock();
   column_families_.insert({name, id});
   column_family_data_.insert({id, new_cfd});

db/column_family.h

@@ -248,9 +248,9 @@ class ColumnFamilyData {
 
  private:
   friend class ColumnFamilySet;
-  ColumnFamilyData(const std::string& dbname, uint32_t id,
-                   const std::string& name, Version* dummy_versions,
-                   Cache* table_cache, const ColumnFamilyOptions& options,
+  ColumnFamilyData(uint32_t id, const std::string& name,
+                   Version* dummy_versions, Cache* table_cache,
+                   const ColumnFamilyOptions& options,
                    const DBOptions* db_options,
                    const EnvOptions& storage_options,
                    ColumnFamilySet* column_family_set);

db/compaction.cc

@@ -18,7 +18,7 @@
 
 namespace rocksdb {
 
-static uint64_t TotalFileSize(const std::vector<FileMetaData*>& files) {
+uint64_t TotalFileSize(const std::vector<FileMetaData*>& files) {
   uint64_t sum = 0;
   for (size_t i = 0; i < files.size() && files[i]; i++) {
     sum += files[i]->fd.GetFileSize();

db/compaction.h

@@ -231,4 +231,7 @@ class Compaction {
   void ResetNextCompactionIndex();
 };
 
+// Utility function
+extern uint64_t TotalFileSize(const std::vector<FileMetaData*>& files);
+
 }  // namespace rocksdb

db/compaction_picker.cc

@@ -18,6 +18,14 @@
 
 namespace rocksdb {
 
+uint64_t TotalCompensatedFileSize(const std::vector<FileMetaData*>& files) {
+  uint64_t sum = 0;
+  for (size_t i = 0; i < files.size() && files[i]; i++) {
+    sum += files[i]->compensated_file_size;
+  }
+  return sum;
+}
+
 namespace {
 // Determine compression type, based on user options, level of the output
 // file and whether compression is disabled.
@@ -45,14 +53,6 @@ CompressionType GetCompressionType(const Options& options, int level,
   }
 }
 
-uint64_t TotalCompensatedFileSize(const std::vector<FileMetaData*>& files) {
-  uint64_t sum = 0;
-  for (size_t i = 0; i < files.size() && files[i]; i++) {
-    sum += files[i]->compensated_file_size;
-  }
-  return sum;
-}
-
 // Multiple two operands. If they overflow, return op1.
 uint64_t MultiplyCheckOverflow(uint64_t op1, int op2) {
   if (op1 == 0) {

db/compaction_picker.h

@@ -204,4 +204,7 @@ class FIFOCompactionPicker : public CompactionPicker {
   }
 };
 
+// Utility function
+extern uint64_t TotalCompensatedFileSize(const std::vector<FileMetaData*>& files);
+
 }  // namespace rocksdb

db/db_bench.cc

@@ -30,7 +30,6 @@ int main() {
 #include <gflags/gflags.h>
 #include "db/db_impl.h"
 #include "db/version_set.h"
-#include "rocksdb/statistics.h"
 #include "rocksdb/options.h"
 #include "rocksdb/cache.h"
 #include "rocksdb/db.h"
@@ -1703,6 +1702,11 @@ class Benchmark {
     } else {
       BlockBasedTableOptions block_based_options;
       if (FLAGS_use_hash_search) {
+        if (FLAGS_prefix_size == 0) {
+          fprintf(stderr,
+              "prefix_size not assigned when enable use_hash_search \n");
+          exit(1);
+        }
         block_based_options.index_type = BlockBasedTableOptions::kHashSearch;
       } else {
         block_based_options.index_type = BlockBasedTableOptions::kBinarySearch;

db/db_impl.cc

@@ -138,8 +138,6 @@ struct DBImpl::CompactionState {
     return context;
   }
 
-  std::vector<Slice> key_buf_;
-  std::vector<Slice> existing_value_buf_;
   std::vector<std::string> key_str_buf_;
   std::vector<std::string> existing_value_str_buf_;
   // new_value_buf_ will only be appended if a value changes
@@ -149,12 +147,7 @@ struct DBImpl::CompactionState {
   std::vector<bool> value_changed_buf_;
   // to_delete_buf_[i] is true iff key_buf_[i] is deleted
   std::vector<bool> to_delete_buf_;
-  // buffer for the parsed internal keys, the string buffer is backed
-  // by key_str_buf_
-  std::vector<ParsedInternalKey> ikey_buf_;
 
-  std::vector<Slice> other_key_buf_;
-  std::vector<Slice> other_value_buf_;
   std::vector<std::string> other_key_str_buf_;
   std::vector<std::string> other_value_str_buf_;
 
@@ -168,12 +161,6 @@ struct DBImpl::CompactionState {
   void BufferKeyValueSlices(const Slice& key, const Slice& value) {
     key_str_buf_.emplace_back(key.ToString());
     existing_value_str_buf_.emplace_back(value.ToString());
-    key_buf_.emplace_back(Slice(key_str_buf_.back()));
-    existing_value_buf_.emplace_back(Slice(existing_value_str_buf_.back()));
-
-    ParsedInternalKey ikey;
-    ParseInternalKey(key_buf_.back(), &ikey);
-    ikey_buf_.emplace_back(ikey);
   }
 
   // Buffers the kv-pair that will not be run through compaction filter V2
@@ -181,8 +168,6 @@ struct DBImpl::CompactionState {
   void BufferOtherKeyValueSlices(const Slice& key, const Slice& value) {
     other_key_str_buf_.emplace_back(key.ToString());
     other_value_str_buf_.emplace_back(value.ToString());
-    other_key_buf_.emplace_back(Slice(other_key_str_buf_.back()));
-    other_value_buf_.emplace_back(Slice(other_value_str_buf_.back()));
   }
 
   // Add a kv-pair to the combined buffer
@@ -196,24 +181,24 @@ struct DBImpl::CompactionState {
   void MergeKeyValueSliceBuffer(const InternalKeyComparator* comparator) {
     size_t i = 0;
     size_t j = 0;
-    size_t total_size = key_buf_.size() + other_key_buf_.size();
+    size_t total_size = key_str_buf_.size() + other_key_str_buf_.size();
     combined_key_buf_.reserve(total_size);
     combined_value_buf_.reserve(total_size);
 
     while (i + j < total_size) {
       int comp_res = 0;
-      if (i < key_buf_.size() && j < other_key_buf_.size()) {
-        comp_res = comparator->Compare(key_buf_[i], other_key_buf_[j]);
-      } else if (i >= key_buf_.size() && j < other_key_buf_.size()) {
+      if (i < key_str_buf_.size() && j < other_key_str_buf_.size()) {
+        comp_res = comparator->Compare(key_str_buf_[i], other_key_str_buf_[j]);
+      } else if (i >= key_str_buf_.size() && j < other_key_str_buf_.size()) {
         comp_res = 1;
-      } else if (j >= other_key_buf_.size() && i < key_buf_.size()) {
+      } else if (j >= other_key_str_buf_.size() && i < key_str_buf_.size()) {
         comp_res = -1;
       }
       if (comp_res > 0) {
-        AddToCombinedKeyValueSlices(other_key_buf_[j], other_value_buf_[j]);
+        AddToCombinedKeyValueSlices(other_key_str_buf_[j], other_value_str_buf_[j]);
         j++;
       } else if (comp_res < 0) {
-        AddToCombinedKeyValueSlices(key_buf_[i], existing_value_buf_[i]);
+        AddToCombinedKeyValueSlices(key_str_buf_[i], existing_value_str_buf_[i]);
         i++;
       }
     }
@@ -221,29 +206,19 @@ struct DBImpl::CompactionState {
 
   void CleanupBatchBuffer() {
     to_delete_buf_.clear();
-    key_buf_.clear();
-    existing_value_buf_.clear();
     key_str_buf_.clear();
     existing_value_str_buf_.clear();
     new_value_buf_.clear();
     value_changed_buf_.clear();
-    ikey_buf_.clear();
 
     to_delete_buf_.shrink_to_fit();
-    key_buf_.shrink_to_fit();
-    existing_value_buf_.shrink_to_fit();
     key_str_buf_.shrink_to_fit();
     existing_value_str_buf_.shrink_to_fit();
     new_value_buf_.shrink_to_fit();
     value_changed_buf_.shrink_to_fit();
-    ikey_buf_.shrink_to_fit();
 
-    other_key_buf_.clear();
-    other_value_buf_.clear();
     other_key_str_buf_.clear();
     other_value_str_buf_.clear();
-    other_key_buf_.shrink_to_fit();
-    other_value_buf_.shrink_to_fit();
     other_key_str_buf_.shrink_to_fit();
     other_value_str_buf_.shrink_to_fit();
   }
@@ -256,15 +231,6 @@ struct DBImpl::CompactionState {
   }
 };
 
-namespace {
-// Fix user-supplied options to be reasonable
-template <class T, class V>
-static void ClipToRange(T* ptr, V minvalue, V maxvalue) {
-  if (static_cast<V>(*ptr) > maxvalue) *ptr = maxvalue;
-  if (static_cast<V>(*ptr) < minvalue) *ptr = minvalue;
-}
-}  // anonymous namespace
-
 Options SanitizeOptions(const std::string& dbname,
                         const InternalKeyComparator* icmp,
                         const InternalFilterPolicy* ipolicy,
@@ -450,6 +416,7 @@ Status DBImpl::NewDB() {
   new_db.SetNextFile(2);
   new_db.SetLastSequence(0);
 
+  Log(options_.info_log, "Creating manifest 1 \n");
   const std::string manifest = DescriptorFileName(dbname_, 1);
   unique_ptr<WritableFile> file;
   Status s = env_->NewWritableFile(
@@ -513,17 +480,21 @@ void DBImpl::MaybeDumpStats() {
     // period in rare cases.
     last_stats_dump_time_microsec_ = now_micros;
 
-    DBPropertyType cf_property_type = GetPropertyType("rocksdb.cfstats");
-    DBPropertyType db_property_type = GetPropertyType("rocksdb.dbstats");
+    bool tmp1 = false;
+    bool tmp2 = false;
+    DBPropertyType cf_property_type =
+        GetPropertyType("rocksdb.cfstats", &tmp1, &tmp2);
+    DBPropertyType db_property_type =
+        GetPropertyType("rocksdb.dbstats", &tmp1, &tmp2);
     std::string stats;
     {
       MutexLock l(&mutex_);
       for (auto cfd : *versions_->GetColumnFamilySet()) {
-        cfd->internal_stats()->GetProperty(
-            cf_property_type, "rocksdb.cfstats", &stats);
+        cfd->internal_stats()->GetStringProperty(cf_property_type,
+                                                 "rocksdb.cfstats", &stats);
       }
-      default_cf_internal_stats_->GetProperty(
-          db_property_type, "rocksdb.dbstats", &stats);
+      default_cf_internal_stats_->GetStringProperty(db_property_type,
+                                                    "rocksdb.dbstats", &stats);
     }
     Log(options_.info_log, "------- DUMPING STATS -------");
     Log(options_.info_log, "%s", stats.c_str());
@@ -1139,7 +1110,6 @@ Status DBImpl::Recover(
 
     if (!env_->FileExists(CurrentFileName(dbname_))) {
       if (options_.create_if_missing) {
-        // TODO: add merge_operator name check
         s = NewDB();
         is_new_db = true;
         if (!s.ok()) {
@@ -1639,10 +1609,12 @@ Status DBImpl::CompactRange(ColumnFamilyHandle* column_family,
   }
   for (int level = 0; level <= max_level_with_files; level++) {
     // in case the compaction is unversal or if we're compacting the
-    // bottom-most level, the output level will be the same as input one
+    // bottom-most level, the output level will be the same as input one.
+    // level 0 can never be the bottommost level (i.e. if all files are in level
+    // 0, we will compact to level 1)
     if (cfd->options()->compaction_style == kCompactionStyleUniversal ||
         cfd->options()->compaction_style == kCompactionStyleFIFO ||
-        level == max_level_with_files) {
+        (level == max_level_with_files && level > 0)) {
       s = RunManualCompaction(cfd, level, level, target_path_id, begin, end);
     } else {
       s = RunManualCompaction(cfd, level, level + 1, target_path_id, begin,
@@ -1853,6 +1825,11 @@ Status DBImpl::RunManualCompaction(ColumnFamilyData* cfd, int input_level,
   assert(!manual.in_progress);
   assert(bg_manual_only_ > 0);
   --bg_manual_only_;
+  if (bg_manual_only_ == 0) {
+    // an automatic compaction should have been scheduled might have be
+    // preempted by the manual compactions. Need to schedule it back.
+    MaybeScheduleFlushOrCompaction();
+  }
   return manual.status;
 }
 
@@ -2041,13 +2018,13 @@ void DBImpl::BackgroundCallFlush() {
     if (madeProgress || bg_schedule_needed_) {
       MaybeScheduleFlushOrCompaction();
     }
+    RecordFlushIOStats();
     bg_cv_.SignalAll();
     // IMPORTANT: there should be no code after calling SignalAll. This call may
     // signal the DB destructor that it's OK to proceed with destruction. In
     // that case, all DB variables will be dealloacated and referencing them
     // will cause trouble.
   }
-  RecordFlushIOStats();
 }
 
 void DBImpl::BackgroundCallCompaction() {
@@ -2699,6 +2676,12 @@ Status DBImpl::ProcessKeyValueCompaction(
         drop = true;
         RecordTick(stats_, COMPACTION_KEY_DROP_OBSOLETE);
       } else if (ikey.type == kTypeMerge) {
+        if (!merge.HasOperator()) {
+          LogToBuffer(log_buffer, "Options::merge_operator is null.");
+          status = Status::InvalidArgument(
+              "merge_operator is not properly initialized.");
+          break;
+        }
         // We know the merge type entry is not hidden, otherwise we would
         // have hit (A)
         // We encapsulate the merge related state machine in a different
@@ -2850,9 +2833,22 @@ void DBImpl::CallCompactionFilterV2(CompactionState* compact,
     return;
   }
 
+  // Assemble slice vectors for user keys and existing values.
+  // We also keep track of our parsed internal key structs because
+  // we may need to access the sequence number in the event that
+  // keys are garbage collected during the filter process.
+  std::vector<ParsedInternalKey> ikey_buf;
   std::vector<Slice> user_key_buf;
-  for (const auto& key : compact->ikey_buf_) {
-    user_key_buf.emplace_back(key.user_key);
+  std::vector<Slice> existing_value_buf;
+
+  for (const auto& key : compact->key_str_buf_) {
+    ParsedInternalKey ikey;
+    ParseInternalKey(Slice(key), &ikey);
+    ikey_buf.emplace_back(ikey);
+    user_key_buf.emplace_back(ikey.user_key);
+  }
+  for (const auto& value : compact->existing_value_str_buf_) {
+    existing_value_buf.emplace_back(Slice(value));
   }
 
   // If the user has specified a compaction filter and the sequence
@@ -2862,16 +2858,16 @@ void DBImpl::CallCompactionFilterV2(CompactionState* compact,
   // the entry with a delete marker.
   compact->to_delete_buf_ = compaction_filter_v2->Filter(
       compact->compaction->level(),
-      user_key_buf, compact->existing_value_buf_,
+      user_key_buf, existing_value_buf,
       &compact->new_value_buf_,
       &compact->value_changed_buf_);
 
   // new_value_buf_.size() <= to_delete__buf_.size(). "=" iff all
   // kv-pairs in this compaction run needs to be deleted.
   assert(compact->to_delete_buf_.size() ==
-      compact->key_buf_.size());
+      compact->key_str_buf_.size());
   assert(compact->to_delete_buf_.size() ==
-      compact->existing_value_buf_.size());
+      compact->existing_value_str_buf_.size());
   assert(compact->to_delete_buf_.size() ==
       compact->value_changed_buf_.size());
 
@@ -2881,16 +2877,16 @@ void DBImpl::CallCompactionFilterV2(CompactionState* compact,
       // update the string buffer directly
       // the Slice buffer points to the updated buffer
       UpdateInternalKey(&compact->key_str_buf_[i][0],
-          compact->key_str_buf_[i].size(),
-          compact->ikey_buf_[i].sequence,
-          kTypeDeletion);
+                        compact->key_str_buf_[i].size(),
+                        ikey_buf[i].sequence,
+                        kTypeDeletion);
 
       // no value associated with delete
-      compact->existing_value_buf_[i].clear();
+      compact->existing_value_str_buf_[i].clear();
       RecordTick(stats_, COMPACTION_KEY_DROP_USER);
     } else if (compact->value_changed_buf_[i]) {
-      compact->existing_value_buf_[i] =
-        Slice(compact->new_value_buf_[new_value_idx++]);
+      compact->existing_value_str_buf_[i] =
+          compact->new_value_buf_[new_value_idx++];
     }
   }  // for
 }
@@ -3019,7 +3015,7 @@ Status DBImpl::DoCompactionWork(CompactionState* compact,
         } else {
           // Now prefix changes, this batch is done.
           // Call compaction filter on the buffered values to change the value
-          if (compact->key_buf_.size() > 0) {
+          if (compact->key_str_buf_.size() > 0) {
             CallCompactionFilterV2(compact, compaction_filter_v2);
           }
           compact->cur_prefix_ = key_prefix.ToString();
@@ -3062,7 +3058,7 @@ Status DBImpl::DoCompactionWork(CompactionState* compact,
       backup_input->Next();
       if (!backup_input->Valid()) {
         // If this is the single last value, we need to merge it.
-        if (compact->key_buf_.size() > 0) {
+        if (compact->key_str_buf_.size() > 0) {
           CallCompactionFilterV2(compact, compaction_filter_v2);
         }
         compact->MergeKeyValueSliceBuffer(&cfd->internal_comparator());
@@ -3085,7 +3081,7 @@ Status DBImpl::DoCompactionWork(CompactionState* compact,
       }
     }  // done processing all prefix batches
     // finish the last batch
-    if (compact->key_buf_.size() > 0) {
+    if (compact->key_str_buf_.size() > 0) {
       CallCompactionFilterV2(compact, compaction_filter_v2);
     }
     compact->MergeKeyValueSliceBuffer(&cfd->internal_comparator());
@@ -3313,15 +3309,7 @@ Status DBImpl::GetImpl(const ReadOptions& options,
   }
 
   // Acquire SuperVersion
-  SuperVersion* sv = nullptr;
-  // TODO(ljin): consider using GetReferencedSuperVersion() directly
-  if (LIKELY(options_.allow_thread_local)) {
-    sv = cfd->GetThreadLocalSuperVersion(&mutex_);
-  } else {
-    mutex_.Lock();
-    sv = cfd->GetSuperVersion()->Ref();
-    mutex_.Unlock();
-  }
+  SuperVersion* sv = GetAndRefSuperVersion(cfd);
 
   // Prepare to store a list of merge operations if merge occurs.
   MergeContext merge_context;
@@ -3348,22 +3336,7 @@ Status DBImpl::GetImpl(const ReadOptions& options,
 
   PERF_TIMER_START(get_post_process_time);
 
-  bool unref_sv = true;
-  if (LIKELY(options_.allow_thread_local)) {
-    unref_sv = !cfd->ReturnThreadLocalSuperVersion(sv);
-  }
-
-  if (unref_sv) {
-    // Release SuperVersion
-    if (sv->Unref()) {
-      mutex_.Lock();
-      sv->Cleanup();
-      mutex_.Unlock();
-      delete sv;
-      RecordTick(stats_, NUMBER_SUPERVERSION_CLEANUPS);
-    }
-    RecordTick(stats_, NUMBER_SUPERVERSION_RELEASES);
-  }
+  ReturnAndCleanupSuperVersion(cfd, sv);
 
   RecordTick(stats_, NUMBER_KEYS_READ);
   RecordTick(stats_, BYTES_READ, value->size());
@@ -4364,21 +4337,92 @@ const Options& DBImpl::GetOptions(ColumnFamilyHandle* column_family) const {
 
 bool DBImpl::GetProperty(ColumnFamilyHandle* column_family,
                          const Slice& property, std::string* value) {
+  bool is_int_property = false;
+  bool need_out_of_mutex = false;
+  DBPropertyType property_type =
+      GetPropertyType(property, &is_int_property, &need_out_of_mutex);
+
   value->clear();
-  auto cfh = reinterpret_cast<ColumnFamilyHandleImpl*>(column_family);
-  auto cfd = cfh->cfd();
-  DBPropertyType property_type = GetPropertyType(property);
-  MutexLock l(&mutex_);
-  return cfd->internal_stats()->GetProperty(property_type, property, value);
+  if (is_int_property) {
+    uint64_t int_value;
+    bool ret_value = GetIntPropertyInternal(column_family, property_type,
+                                            need_out_of_mutex, &int_value);
+    if (ret_value) {
+      *value = std::to_string(int_value);
+    }
+    return ret_value;
+  } else {
+    auto cfh = reinterpret_cast<ColumnFamilyHandleImpl*>(column_family);
+    auto cfd = cfh->cfd();
+    MutexLock l(&mutex_);
+    return cfd->internal_stats()->GetStringProperty(property_type, property,
+                                                    value);
+  }
 }
 
 bool DBImpl::GetIntProperty(ColumnFamilyHandle* column_family,
                             const Slice& property, uint64_t* value) {
+  bool is_int_property = false;
+  bool need_out_of_mutex = false;
+  DBPropertyType property_type =
+      GetPropertyType(property, &is_int_property, &need_out_of_mutex);
+  if (!is_int_property) {
+    return false;
+  }
+  return GetIntPropertyInternal(column_family, property_type, need_out_of_mutex,
+                                value);
+}
+
+bool DBImpl::GetIntPropertyInternal(ColumnFamilyHandle* column_family,
+                                    DBPropertyType property_type,
+                                    bool need_out_of_mutex, uint64_t* value) {
   auto cfh = reinterpret_cast<ColumnFamilyHandleImpl*>(column_family);
   auto cfd = cfh->cfd();
-  DBPropertyType property_type = GetPropertyType(property);
-  MutexLock l(&mutex_);
-  return cfd->internal_stats()->GetIntProperty(property_type, property, value);
+
+  if (!need_out_of_mutex) {
+    MutexLock l(&mutex_);
+    return cfd->internal_stats()->GetIntProperty(property_type, value);
+  } else {
+    SuperVersion* sv = GetAndRefSuperVersion(cfd);
+
+    bool ret = cfd->internal_stats()->GetIntPropertyOutOfMutex(
+        property_type, sv->current, value);
+
+    ReturnAndCleanupSuperVersion(cfd, sv);
+
+    return ret;
+  }
+}
+
+SuperVersion* DBImpl::GetAndRefSuperVersion(ColumnFamilyData* cfd) {
+  // TODO(ljin): consider using GetReferencedSuperVersion() directly
+  if (LIKELY(options_.allow_thread_local)) {
+    return cfd->GetThreadLocalSuperVersion(&mutex_);
+  } else {
+    MutexLock l(&mutex_);
+    return cfd->GetSuperVersion()->Ref();
+  }
+}
+
+void DBImpl::ReturnAndCleanupSuperVersion(ColumnFamilyData* cfd,
+                                          SuperVersion* sv) {
+  bool unref_sv = true;
+  if (LIKELY(options_.allow_thread_local)) {
+    unref_sv = !cfd->ReturnThreadLocalSuperVersion(sv);
+  }
+
+  if (unref_sv) {
+    // Release SuperVersion
+    if (sv->Unref()) {
+      {
+        MutexLock l(&mutex_);
+        sv->Cleanup();
+      }
+      delete sv;
+      RecordTick(stats_, NUMBER_SUPERVERSION_CLEANUPS);
+    }
+    RecordTick(stats_, NUMBER_SUPERVERSION_RELEASES);
+  }
 }
 
 void DBImpl::GetApproximateSizes(ColumnFamilyHandle* column_family,

db/db_impl.h

@@ -611,6 +611,16 @@ class DBImpl : public DB {
   void InstallSuperVersion(ColumnFamilyData* cfd,
                            DeletionState& deletion_state);
 
+  // Find Super version and reference it. Based on options, it might return
+  // the thread local cached one.
+  inline SuperVersion* GetAndRefSuperVersion(ColumnFamilyData* cfd);
+
+  // Un-reference the super version and return it to thread local cache if
+  // needed. If it is the last reference of the super version. Clean it up
+  // after un-referencing it.
+  inline void ReturnAndCleanupSuperVersion(ColumnFamilyData* cfd,
+                                           SuperVersion* sv);
+
 #ifndef ROCKSDB_LITE
   using DB::GetPropertiesOfAllTables;
   virtual Status GetPropertiesOfAllTables(ColumnFamilyHandle* column_family,
@@ -623,6 +633,10 @@ class DBImpl : public DB {
   Status GetImpl(const ReadOptions& options, ColumnFamilyHandle* column_family,
                  const Slice& key, std::string* value,
                  bool* value_found = nullptr);
+
+  bool GetIntPropertyInternal(ColumnFamilyHandle* column_family,
+                              DBPropertyType property_type,
+                              bool need_out_of_mutex, uint64_t* value);
 };
 
 // Sanitize db options.  The caller should delete result.info_log if
@@ -633,4 +647,11 @@ extern Options SanitizeOptions(const std::string& db,
                                const Options& src);
 extern DBOptions SanitizeOptions(const std::string& db, const DBOptions& src);
 
+// Fix user-supplied options to be reasonable
+template <class T, class V>
+static void ClipToRange(T* ptr, V minvalue, V maxvalue) {
+  if (static_cast<V>(*ptr) > maxvalue) *ptr = maxvalue;
+  if (static_cast<V>(*ptr) < minvalue) *ptr = minvalue;
+}
+
 }  // namespace rocksdb

db/db_impl_readonly.h

@@ -32,60 +32,62 @@ class DBImplReadOnly : public DBImpl {
   using DB::Get;
   virtual Status Get(const ReadOptions& options,
                      ColumnFamilyHandle* column_family, const Slice& key,
-                     std::string* value);
+                     std::string* value) override;
 
   // TODO: Implement ReadOnly MultiGet?
 
   using DBImpl::NewIterator;
   virtual Iterator* NewIterator(const ReadOptions&,
-                                ColumnFamilyHandle* column_family);
+                                ColumnFamilyHandle* column_family) override;
 
   virtual Status NewIterators(
       const ReadOptions& options,
       const std::vector<ColumnFamilyHandle*>& column_families,
-      std::vector<Iterator*>* iterators);
+      std::vector<Iterator*>* iterators) override;
 
   using DBImpl::Put;
   virtual Status Put(const WriteOptions& options,
                      ColumnFamilyHandle* column_family, const Slice& key,
-                     const Slice& value) {
+                     const Slice& value) override {
     return Status::NotSupported("Not supported operation in read only mode.");
   }
   using DBImpl::Merge;
   virtual Status Merge(const WriteOptions& options,
                        ColumnFamilyHandle* column_family, const Slice& key,
-                       const Slice& value) {
+                       const Slice& value) override {
     return Status::NotSupported("Not supported operation in read only mode.");
   }
   using DBImpl::Delete;
   virtual Status Delete(const WriteOptions& options,
-                        ColumnFamilyHandle* column_family, const Slice& key) {
+                        ColumnFamilyHandle* column_family,
+                        const Slice& key) override {
     return Status::NotSupported("Not supported operation in read only mode.");
   }
-  virtual Status Write(const WriteOptions& options, WriteBatch* updates) {
+  virtual Status Write(const WriteOptions& options,
+                       WriteBatch* updates) override {
     return Status::NotSupported("Not supported operation in read only mode.");
   }
   using DBImpl::CompactRange;
   virtual Status CompactRange(ColumnFamilyHandle* column_family,
                               const Slice* begin, const Slice* end,
-                              bool reduce_level = false,
-                              int target_level = -1) {
+                              bool reduce_level = false, int target_level = -1,
+                              uint32_t target_path_id = 0) override {
     return Status::NotSupported("Not supported operation in read only mode.");
   }
-  virtual Status DisableFileDeletions() {
+  virtual Status DisableFileDeletions() override {
     return Status::NotSupported("Not supported operation in read only mode.");
   }
-  virtual Status EnableFileDeletions(bool force) {
+  virtual Status EnableFileDeletions(bool force) override {
     return Status::NotSupported("Not supported operation in read only mode.");
   }
   virtual Status GetLiveFiles(std::vector<std::string>&,
                               uint64_t* manifest_file_size,
-                              bool flush_memtable = true) {
+                              bool flush_memtable = true) override {
     return Status::NotSupported("Not supported operation in read only mode.");
   }
   using DBImpl::Flush;
   virtual Status Flush(const FlushOptions& options,
-                       ColumnFamilyHandle* column_family) {
+                       ColumnFamilyHandle* column_family) override {
     return Status::NotSupported("Not supported operation in read only mode.");
   }
 

db/db_iter.cc

@@ -11,6 +11,7 @@
 #include <stdexcept>
 #include <deque>
 #include <string>
+#include <limits>
 
 #include "db/filename.h"
 #include "db/dbformat.h"
@@ -71,6 +72,7 @@ class DBIter: public Iterator {
         current_entry_is_merged_(false),
         statistics_(options.statistics.get()) {
     RecordTick(statistics_, NO_ITERATORS);
+    has_prefix_extractor_ = (options.prefix_extractor.get() != nullptr);
     max_skip_ = options.max_sequential_skip_in_iterations;
   }
   virtual ~DBIter() {
@@ -130,6 +132,7 @@ class DBIter: public Iterator {
     }
   }
 
+  bool has_prefix_extractor_;
   bool arena_mode_;
   Env* const env_;
   Logger* logger_;
@@ -565,6 +568,11 @@ void DBIter::Seek(const Slice& target) {
 }
 
 void DBIter::SeekToFirst() {
+  // Don't use iter_::Seek() if we set a prefix extractor
+  // because prefix seek wiil be used.
+  if (has_prefix_extractor_) {
+    max_skip_ = std::numeric_limits<uint64_t>::max();
+  }
   direction_ = kForward;
   ClearSavedValue();
   PERF_TIMER_AUTO(seek_internal_seek_time);
@@ -578,6 +586,11 @@ void DBIter::SeekToFirst() {
 }
 
 void DBIter::SeekToLast() {
+  // Don't use iter_::Seek() if we set a prefix extractor
+  // because prefix seek wiil be used.
+  if (has_prefix_extractor_) {
+    max_skip_ = std::numeric_limits<uint64_t>::max();
+  }
   direction_ = kReverse;
   ClearSavedValue();
   PERF_TIMER_AUTO(seek_internal_seek_time);

db/db_test.cc

@@ -309,31 +309,31 @@ class DBTest {
  protected:
   // Sequence of option configurations to try
   enum OptionConfig {
-    kBlockBasedTableWithWholeKeyHashIndex,
-    kDefault,
-    kBlockBasedTableWithPrefixHashIndex,
-    kPlainTableFirstBytePrefix,
-    kPlainTableAllBytesPrefix,
-    kVectorRep,
-    kHashLinkList,
-    kHashCuckoo,
-    kMergePut,
-    kFilter,
-    kUncompressed,
-    kNumLevel_3,
-    kDBLogDir,
-    kWalDir,
-    kManifestFileSize,
-    kCompactOnFlush,
-    kPerfOptions,
-    kDeletesFilterFirst,
-    kHashSkipList,
-    kUniversalCompaction,
-    kCompressedBlockCache,
-    kInfiniteMaxOpenFiles,
-    kxxHashChecksum,
-    kFIFOCompaction,
-    kEnd
+    kDefault = 0,
+    kBlockBasedTableWithPrefixHashIndex = 1,
+    kBlockBasedTableWithWholeKeyHashIndex = 2,
+    kPlainTableFirstBytePrefix = 3,
+    kPlainTableAllBytesPrefix = 4,
+    kVectorRep = 5,
+    kHashLinkList = 6,
+    kHashCuckoo = 7,
+    kMergePut = 8,
+    kFilter = 9,
+    kUncompressed = 10,
+    kNumLevel_3 = 11,
+    kDBLogDir = 12,
+    kWalDir = 13,
+    kManifestFileSize = 14,
+    kCompactOnFlush = 15,
+    kPerfOptions = 16,
+    kDeletesFilterFirst = 17,
+    kHashSkipList = 18,
+    kUniversalCompaction = 19,
+    kCompressedBlockCache = 20,
+    kInfiniteMaxOpenFiles = 21,
+    kxxHashChecksum = 22,
+    kFIFOCompaction = 23,
+    kEnd = 24
   };
   int option_config_;
 
@@ -405,7 +405,7 @@ class DBTest {
               || option_config_ == kPlainTableFirstBytePrefix)) {
         continue;
       }
-      if ((skip_mask & kSkipPlainTable) &&
+      if ((skip_mask & kSkipHashIndex) &&
           (option_config_ == kBlockBasedTableWithPrefixHashIndex ||
            option_config_ == kBlockBasedTableWithWholeKeyHashIndex)) {
         continue;
@@ -1186,6 +1186,10 @@ TEST(DBTest, IndexAndFilterBlocksOfNewTableAddedToCache) {
   ASSERT_EQ(2, /* only index/filter were added */
             TestGetTickerCount(options, BLOCK_CACHE_ADD));
   ASSERT_EQ(0, TestGetTickerCount(options, BLOCK_CACHE_DATA_MISS));
+  uint64_t int_num;
+  ASSERT_TRUE(
+      dbfull()->GetIntProperty("rocksdb.estimate-table-readers-mem", &int_num));
+  ASSERT_EQ(int_num, 0U);
 
   // Make sure filter block is in cache.
   std::string value;
@@ -2489,6 +2493,10 @@ TEST(DBTest, GetProperty) {
   uint64_t int_num;
   SetPerfLevel(kEnableTime);
 
+  ASSERT_TRUE(
+      dbfull()->GetIntProperty("rocksdb.estimate-table-readers-mem", &int_num));
+  ASSERT_EQ(int_num, 0U);
+
   ASSERT_OK(dbfull()->Put(writeOpt, "k1", big_value));
   ASSERT_TRUE(dbfull()->GetProperty("rocksdb.num-immutable-mem-table", &num));
   ASSERT_EQ(num, "0");
@@ -2521,9 +2529,13 @@ TEST(DBTest, GetProperty) {
       dbfull()->GetIntProperty("rocksdb.mem-table-flush-pending", &int_num));
   ASSERT_EQ(int_num, 1U);
   ASSERT_TRUE(dbfull()->GetIntProperty("rocksdb.compaction-pending", &int_num));
-  ASSERT_EQ(int_num, 0);
+  ASSERT_EQ(int_num, 0U);
   ASSERT_TRUE(dbfull()->GetIntProperty("rocksdb.estimate-num-keys", &int_num));
-  ASSERT_EQ(int_num, 4);
+  ASSERT_EQ(int_num, 4U);
+
+  ASSERT_TRUE(
+      dbfull()->GetIntProperty("rocksdb.estimate-table-readers-mem", &int_num));
+  ASSERT_EQ(int_num, 0U);
 
   sleeping_task_high.WakeUp();
   sleeping_task_high.WaitUntilDone();
@@ -2538,8 +2550,29 @@ TEST(DBTest, GetProperty) {
   ASSERT_EQ(num, "1");
   ASSERT_TRUE(dbfull()->GetProperty("rocksdb.estimate-num-keys", &num));
   ASSERT_EQ(num, "4");
+
+  ASSERT_TRUE(
+      dbfull()->GetIntProperty("rocksdb.estimate-table-readers-mem", &int_num));
+  ASSERT_GT(int_num, 0U);
+
   sleeping_task_low.WakeUp();
   sleeping_task_low.WaitUntilDone();
+
+  dbfull()->TEST_WaitForFlushMemTable();
+  options.max_open_files = 10;
+  Reopen(&options);
+  // After reopening, no table reader is loaded, so no memory for table readers
+  ASSERT_TRUE(
+      dbfull()->GetIntProperty("rocksdb.estimate-table-readers-mem", &int_num));
+  ASSERT_EQ(int_num, 0U);
+  ASSERT_TRUE(dbfull()->GetIntProperty("rocksdb.estimate-num-keys", &int_num));
+  ASSERT_GT(int_num, 0U);
+
+  // After reading a key, at least one table reader is loaded.
+  Get("k5");
+  ASSERT_TRUE(
+      dbfull()->GetIntProperty("rocksdb.estimate-table-readers-mem", &int_num));
+  ASSERT_GT(int_num, 0U);
 }
 
 TEST(DBTest, FLUSH) {
@@ -4776,7 +4809,7 @@ TEST(DBTest, ApproximateSizes) {
     }
     // ApproximateOffsetOf() is not yet implemented in plain table format.
   } while (ChangeOptions(kSkipUniversalCompaction | kSkipFIFOCompaction |
-                         kSkipPlainTable));
+                         kSkipPlainTable | kSkipHashIndex));
 }
 
 TEST(DBTest, ApproximateSizes_MixOfSmallAndLarge) {
@@ -6726,7 +6759,14 @@ class ModelDB: public DB {
       iter_ = map_->lower_bound(k.ToString());
     }
     virtual void Next() { ++iter_; }
-    virtual void Prev() { --iter_; }
+    virtual void Prev() {
+      if (iter_ == map_->begin()) {
+        iter_ = map_->end();
+        return;
+      }
+      --iter_;
+    }
+
     virtual Slice key() const { return iter_->first; }
     virtual Slice value() const { return iter_->second; }
     virtual Status status() const { return Status::OK(); }
@@ -6854,8 +6894,14 @@ TEST(DBTest, Randomized) {
       }
 
       if ((step % 100) == 0) {
-        ASSERT_TRUE(CompareIterators(step, &model, db_, nullptr, nullptr));
-        ASSERT_TRUE(CompareIterators(step, &model, db_, model_snap, db_snap));
+        // For DB instances that use the hash index + block-based table, the
+        // iterator will be invalid right when seeking a non-existent key, right
+        // than return a key that is close to it.
+        if (option_config_ != kBlockBasedTableWithWholeKeyHashIndex &&
+            option_config_ != kBlockBasedTableWithPrefixHashIndex) {
+          ASSERT_TRUE(CompareIterators(step, &model, db_, nullptr, nullptr));
+          ASSERT_TRUE(CompareIterators(step, &model, db_, model_snap, db_snap));
+        }
 
         // Save a snapshot from each DB this time that we'll use next
         // time we compare things, to make sure the current state is
@@ -6869,12 +6915,18 @@ TEST(DBTest, Randomized) {
         model_snap = model.GetSnapshot();
         db_snap = db_->GetSnapshot();
       }
+
+      if ((step % 2000) == 0) {
+        fprintf(stdout,
+                "DBTest.Randomized, option ID: %d, step: %d out of %d\n",
+                option_config_, step, N);
+      }
     }
     if (model_snap != nullptr) model.ReleaseSnapshot(model_snap);
     if (db_snap != nullptr) db_->ReleaseSnapshot(db_snap);
     // skip cuckoo hash as it does not support snapshot.
-  } while (ChangeOptions(kSkipDeletesFilterFirst |
-                         kSkipNoSeekToLast | kSkipHashCuckoo));
+  } while (ChangeOptions(kSkipDeletesFilterFirst | kSkipNoSeekToLast |
+                         kSkipHashCuckoo));
 }
 
 TEST(DBTest, MultiGetSimple) {

db/internal_stats.cc

@@ -86,10 +86,17 @@ void PrintLevelStats(char* buf, size_t len, const std::string& name,
 
 }
 
-DBPropertyType GetPropertyType(const Slice& property) {
+DBPropertyType GetPropertyType(const Slice& property, bool* is_int_property,
+                               bool* need_out_of_mutex) {
+  assert(is_int_property != nullptr);
+  assert(need_out_of_mutex != nullptr);
   Slice in = property;
   Slice prefix("rocksdb.");
-  if (!in.starts_with(prefix)) return kUnknown;
+  *need_out_of_mutex = false;
+  *is_int_property = false;
+  if (!in.starts_with(prefix)) {
+    return kUnknown;
+  }
   in.remove_prefix(prefix.size());
 
   if (in.starts_with("num-files-at-level")) {
@@ -104,7 +111,10 @@ DBPropertyType GetPropertyType(const Slice& property) {
     return kDBStats;
   } else if (in == "sstables") {
     return kSsTables;
-  } else if (in == "num-immutable-mem-table") {
+  }
+
+  *is_int_property = true;
+  if (in == "num-immutable-mem-table") {
     return kNumImmutableMemTable;
   } else if (in == "mem-table-flush-pending") {
     return kMemtableFlushPending;
@@ -120,21 +130,32 @@ DBPropertyType GetPropertyType(const Slice& property) {
     return kNumEntriesInImmutableMemtable;
   } else if (in == "estimate-num-keys") {
     return kEstimatedNumKeys;
+  } else if (in == "estimate-table-readers-mem") {
+    *need_out_of_mutex = true;
+    return kEstimatedUsageByTableReaders;
   }
   return kUnknown;
 }
 
-bool InternalStats::GetProperty(DBPropertyType property_type,
-                                const Slice& property, std::string* value) {
-  if (property_type > kStartIntTypes) {
-    uint64_t int_value;
-    bool ret_value = GetIntProperty(property_type, property, &int_value);
-    if (ret_value) {
-      *value = std::to_string(int_value);
-    }
-    return ret_value;
+bool InternalStats::GetIntPropertyOutOfMutex(DBPropertyType property_type,
+                                             Version* version,
+                                             uint64_t* value) const {
+  assert(value != nullptr);
+  if (property_type != kEstimatedUsageByTableReaders) {
+    return false;
   }
+  if (version == nullptr) {
+    *value = 0;
+  } else {
+    *value = version->GetMemoryUsageByTableReaders();
+  }
+  return true;
+}
 
+bool InternalStats::GetStringProperty(DBPropertyType property_type,
+                                      const Slice& property,
+                                      std::string* value) {
+  assert(value != nullptr);
   Version* current = cfd_->current();
   Slice in = property;
 
@@ -169,10 +190,10 @@ bool InternalStats::GetProperty(DBPropertyType property_type,
       return true;
     }
     case kStats: {
-      if (!GetProperty(kCFStats, "rocksdb.cfstats", value)) {
+      if (!GetStringProperty(kCFStats, "rocksdb.cfstats", value)) {
         return false;
       }
-      if (!GetProperty(kDBStats, "rocksdb.dbstats", value)) {
+      if (!GetStringProperty(kDBStats, "rocksdb.dbstats", value)) {
         return false;
       }
       return true;
@@ -194,7 +215,6 @@ bool InternalStats::GetProperty(DBPropertyType property_type,
 }
 
 bool InternalStats::GetIntProperty(DBPropertyType property_type,
-                                   const Slice& property,
                                    uint64_t* value) const {
   Version* current = cfd_->current();
 

db/internal_stats.h

@@ -41,9 +41,12 @@ enum DBPropertyType : uint32_t {
   kNumEntriesInImmutableMemtable,  // Return sum of number of entries in all
                                    // the immutable mem tables.
   kEstimatedNumKeys,  // Estimated total number of keys in the database.
+  kEstimatedUsageByTableReaders,  // Estimated memory by table readers.
 };
 
-extern DBPropertyType GetPropertyType(const Slice& property);
+extern DBPropertyType GetPropertyType(const Slice& property,
+                                      bool* is_int_property,
+                                      bool* need_out_of_mutex);
 
 class InternalStats {
  public:
@@ -191,11 +194,13 @@ class InternalStats {
 
   uint64_t BumpAndGetBackgroundErrorCount() { return ++bg_error_count_; }
 
-  bool GetProperty(DBPropertyType property_type, const Slice& property,
-                   std::string* value);
+  bool GetStringProperty(DBPropertyType property_type, const Slice& property,
+                         std::string* value);
 
-  bool GetIntProperty(DBPropertyType property_type, const Slice& property,
-                      uint64_t* value) const;
+  bool GetIntProperty(DBPropertyType property_type, uint64_t* value) const;
+
+  bool GetIntPropertyOutOfMutex(DBPropertyType property_type, Version* version,
+                                uint64_t* value) const;
 
  private:
   void DumpDBStats(std::string* value);

db/log_and_apply_bench.cc

@@ -42,6 +42,11 @@ void BM_LogAndApply(int iters, int num_base_files) {
 
     Options options;
     EnvOptions sopt;
+    // Notice we are using the default options not through SanitizeOptions().
+    // We might want to initialize some options manually if needed.
+    options.db_paths.emplace_back(dbname, 0);
+    // The parameter of table cache is passed in as null, so any file I/O
+    // operation is likely to fail.
     vset = new VersionSet(dbname, &options, sopt, nullptr);
     std::vector<ColumnFamilyDescriptor> dummy;
     dummy.push_back(ColumnFamilyDescriptor());

db/memtable.cc

@@ -390,6 +390,16 @@ static bool SaveValue(void* arg, const char* entry) {
         return false;
       }
       case kTypeMerge: {
+        if (!merge_operator) {
+          *(s->status) = Status::InvalidArgument(
+              "merge_operator is not properly initialized.");
+          // Normally we continue the loop (return true) when we see a merge
+          // operand.  But in case of an error, we should stop the loop
+          // immediately and pretend we have found the value to stop further
+          // seek.  Otherwise, the later call will override this error status.
+          *(s->found_final_value) = true;
+          return false;
+        }
         std::string merge_result;  // temporary area for merge results later
         Slice v = GetLengthPrefixedSlice(key_ptr + key_length);
         *(s->merge_in_progress) = true;

db/merge_helper.cc

@@ -24,10 +24,12 @@ void MergeHelper::MergeUntil(Iterator* iter, SequenceNumber stop_before,
                              bool at_bottom, Statistics* stats, int* steps) {
   // Get a copy of the internal key, before it's invalidated by iter->Next()
   // Also maintain the list of merge operands seen.
+  assert(HasOperator());
   keys_.clear();
   operands_.clear();
   keys_.push_front(iter->key().ToString());
   operands_.push_front(iter->value().ToString());
+  assert(user_merge_operator_);
 
   success_ = false;   // Will become true if we hit Put/Delete or bottom
 

db/merge_helper.h

@@ -78,13 +78,16 @@ class MergeHelper {
   //   IMPORTANT 2: The entries were traversed in order from BACK to FRONT.
   //                So keys().back() was the first key seen by iterator.
   // TODO: Re-style this comment to be like the first one
-  bool IsSuccess() { return success_; }
-  Slice key() { assert(success_); return Slice(keys_.back()); }
-  Slice value() { assert(success_); return Slice(operands_.back()); }
-  const std::deque<std::string>& keys() { assert(!success_); return keys_; }
-  const std::deque<std::string>& values() {
+  bool IsSuccess() const { return success_; }
+  Slice key() const { assert(success_); return Slice(keys_.back()); }
+  Slice value() const { assert(success_); return Slice(operands_.back()); }
+  const std::deque<std::string>& keys() const {
+    assert(!success_); return keys_;
+  }
+  const std::deque<std::string>& values() const {
     assert(!success_); return operands_;
   }
+  bool HasOperator() const { return user_merge_operator_ != nullptr; }
 
  private:
   const Comparator* user_comparator_;

db/merge_test.cc

@@ -212,7 +212,7 @@ class Counters {
   void assert_add(const string& key, uint64_t value) {
     int result = add(key, value);
     assert(result);
-    if (result == 0) exit(1); // Disable unused variable warning. 
+    if (result == 0) exit(1); // Disable unused variable warning.
   }
 };
 
@@ -460,6 +460,41 @@ void runTest(int argc, const string& dbname, const bool use_ttl = false) {
       }
     }
   }
+
+  {
+    cout << "Test merge-operator not set after reopen\n";
+    {
+      auto db = OpenDb(dbname);
+      MergeBasedCounters counters(db, 0);
+      counters.add("test-key", 1);
+      counters.add("test-key", 1);
+      counters.add("test-key", 1);
+      db->CompactRange(nullptr, nullptr);
+    }
+
+    DB* reopen_db;
+    ASSERT_OK(DB::Open(Options(), dbname, &reopen_db));
+    std::string value;
+    ASSERT_TRUE(!(reopen_db->Get(ReadOptions(), "test-key", &value).ok()));
+    delete reopen_db;
+    DestroyDB(dbname, Options());
+  }
+
+  /* Temporary remove this test
+  {
+    cout << "Test merge-operator not set after reopen (recovery case)\n";
+    {
+      auto db = OpenDb(dbname);
+      MergeBasedCounters counters(db, 0);
+      counters.add("test-key", 1);
+      counters.add("test-key", 1);
+      counters.add("test-key", 1);
+    }
+
+    DB* reopen_db;
+    ASSERT_TRUE(DB::Open(Options(), dbname, &reopen_db).IsInvalidArgument());
+  }
+  */
 }
 }  // namespace
 

db/plain_table_db_test.cc

@@ -347,11 +347,20 @@ TEST(PlainTableDBTest, Flush) {
                 NewPlainTableFactory(plain_table_options));
           }
           DestroyAndReopen(&options);
+          uint64_t int_num;
+          ASSERT_TRUE(dbfull()->GetIntProperty(
+              "rocksdb.estimate-table-readers-mem", &int_num));
+          ASSERT_EQ(int_num, 0U);
+
           ASSERT_OK(Put("1000000000000foo", "v1"));
           ASSERT_OK(Put("0000000000000bar", "v2"));
           ASSERT_OK(Put("1000000000000foo", "v3"));
           dbfull()->TEST_FlushMemTable();
 
+          ASSERT_TRUE(dbfull()->GetIntProperty(
+              "rocksdb.estimate-table-readers-mem", &int_num));
+          ASSERT_GT(int_num, 0U);
+
           TablePropertiesCollection ptc;
           reinterpret_cast<DB*>(dbfull())->GetPropertiesOfAllTables(&ptc);
           ASSERT_EQ(1U, ptc.size());

db/simple_table_db_test.cc

@@ -92,6 +92,8 @@ public:
 
   uint64_t ApproximateOffsetOf(const Slice& key) override;
 
+  virtual size_t ApproximateMemoryUsage() const override { return 0; }
+
   void SetupForCompaction() override;
 
   std::shared_ptr<const TableProperties> GetTableProperties() const override;

db/table_cache.cc

@@ -185,6 +185,29 @@ Status TableCache::GetTableProperties(
   return s;
 }
 
+size_t TableCache::GetMemoryUsageByTableReader(
+    const EnvOptions& toptions,
+    const InternalKeyComparator& internal_comparator,
+    const FileDescriptor& fd) {
+  Status s;
+  auto table_reader = fd.table_reader;
+  // table already been pre-loaded?
+  if (table_reader) {
+    return table_reader->ApproximateMemoryUsage();
+  }
+
+  Cache::Handle* table_handle = nullptr;
+  s = FindTable(toptions, internal_comparator, fd, &table_handle, true);
+  if (!s.ok()) {
+    return 0;
+  }
+  assert(table_handle);
+  auto table = GetTableReaderFromHandle(table_handle);
+  auto ret = table->ApproximateMemoryUsage();
+  ReleaseHandle(table_handle);
+  return ret;
+}
+
 void TableCache::Evict(Cache* cache, uint64_t file_number) {
   cache->Erase(GetSliceForFileNumber(&file_number));
 }

db/table_cache.h

@@ -80,6 +80,13 @@ class TableCache {
                             std::shared_ptr<const TableProperties>* properties,
                             bool no_io = false);
 
+  // Return total memory usage of the table reader of the file.
+  // 0 of table reader of the file is not loaded.
+  size_t GetMemoryUsageByTableReader(
+      const EnvOptions& toptions,
+      const InternalKeyComparator& internal_comparator,
+      const FileDescriptor& fd);
+
   // Release the handle from a cache
   void ReleaseHandle(Cache::Handle* handle);
 

db/version_set.cc

@@ -301,23 +301,6 @@ class FilePicker {
 };
 }  // anonymous namespace
 
-static uint64_t TotalFileSize(const std::vector<FileMetaData*>& files) {
-  uint64_t sum = 0;
-  for (size_t i = 0; i < files.size() && files[i]; i++) {
-    sum += files[i]->fd.GetFileSize();
-  }
-  return sum;
-}
-
-static uint64_t TotalCompensatedFileSize(
-    const std::vector<FileMetaData*>& files) {
-  uint64_t sum = 0;
-  for (size_t i = 0; i < files.size() && files[i]; i++) {
-    sum += files[i]->compensated_file_size;
-  }
-  return sum;
-}
-
 Version::~Version() {
   assert(refs_ == 0);
 
@@ -593,6 +576,18 @@ Status Version::GetPropertiesOfAllTables(TablePropertiesCollection* props) {
   return Status::OK();
 }
 
+size_t Version::GetMemoryUsageByTableReaders() {
+  size_t total_usage = 0;
+  for (auto& file_level : file_levels_) {
+    for (size_t i = 0; i < file_level.num_files; i++) {
+      total_usage += cfd_->table_cache()->GetMemoryUsageByTableReader(
+          vset_->storage_options_, cfd_->internal_comparator(),
+          file_level.files[i].fd);
+    }
+  }
+  return total_usage;
+}
+
 uint64_t Version::GetEstimatedActiveKeys() {
   // Estimation will be not accurate when:
   // (1) there is merge keys
@@ -654,7 +649,6 @@ void Version::AddIterators(const ReadOptions& read_options,
 }
 
 // Callback from TableCache::Get()
-namespace {
 enum SaverState {
   kNotFound,
   kFound,
@@ -662,6 +656,8 @@ enum SaverState {
   kCorrupt,
   kMerge // saver contains the current merge result (the operands)
 };
+
+namespace version_set {
 struct Saver {
   SaverState state;
   const Comparator* ucmp;
@@ -674,7 +670,7 @@ struct Saver {
   Logger* logger;
   Statistics* statistics;
 };
-}
+} // namespace version_set
 
 // Called from TableCache::Get and Table::Get when file/block in which
 // key may  exist are not there in TableCache/BlockCache respectively. In this
@@ -682,7 +678,7 @@ struct Saver {
 // IO to be  certain.Set the status=kFound and value_found=false to let the
 // caller know that key may exist but is not there in memory
 static void MarkKeyMayExist(void* arg) {
-  Saver* s = reinterpret_cast<Saver*>(arg);
+  version_set::Saver* s = reinterpret_cast<version_set::Saver*>(arg);
   s->state = kFound;
   if (s->value_found != nullptr) {
     *(s->value_found) = false;
@@ -691,7 +687,7 @@ static void MarkKeyMayExist(void* arg) {
 
 static bool SaveValue(void* arg, const ParsedInternalKey& parsed_key,
                       const Slice& v) {
-  Saver* s = reinterpret_cast<Saver*>(arg);
+  version_set::Saver* s = reinterpret_cast<version_set::Saver*>(arg);
   MergeContext* merge_contex = s->merge_context;
   std::string merge_result;  // temporary area for merge results later
 
@@ -805,7 +801,7 @@ void Version::Get(const ReadOptions& options,
   Slice user_key = k.user_key();
 
   assert(status->ok() || status->IsMergeInProgress());
-  Saver saver;
+  version_set::Saver saver;
   saver.state = status->ok()? kNotFound : kMerge;
   saver.ucmp = user_comparator_;
   saver.user_key = user_key;
@@ -845,6 +841,11 @@ void Version::Get(const ReadOptions& options,
   }
 
   if (kMerge == saver.state) {
+    if (!merge_operator_) {
+      *status =  Status::InvalidArgument(
+          "merge_operator is not properly initialized.");
+      return;
+    }
     // merge_operands are in saver and we hit the beginning of the key history
     // do a final merge of nullptr and operands;
     if (merge_operator_->FullMerge(user_key, nullptr,
@@ -1876,6 +1877,9 @@ Status VersionSet::LogAndApply(ColumnFamilyData* column_family_data,
     // This is fine because everything inside of this block is serialized --
     // only one thread can be here at the same time
     if (new_descriptor_log) {
+      // create manifest file
+      Log(options_->info_log,
+          "Creating manifest %" PRIu64 "\n", pending_manifest_file_number_);
       unique_ptr<WritableFile> descriptor_file;
       s = env_->NewWritableFile(
           DescriptorFileName(dbname_, pending_manifest_file_number_),
@@ -1997,6 +2001,9 @@ Status VersionSet::LogAndApply(ColumnFamilyData* column_family_data,
         column_family_data->GetName().c_str());
     delete v;
     if (new_descriptor_log) {
+      Log(options_->info_log,
+        "Deleting manifest %" PRIu64 " current manifest %" PRIu64 "\n",
+        manifest_file_number_, pending_manifest_file_number_);
       descriptor_log_.reset();
       env_->DeleteFile(
           DescriptorFileName(dbname_, pending_manifest_file_number_));

db/version_set.h

@@ -234,6 +234,8 @@ class Version {
 
   uint64_t GetEstimatedActiveKeys();
 
+  size_t GetMemoryUsageByTableReaders();
+
   // used to sort files by size
   struct Fsize {
     int index;

include/rocksdb/c.h

@@ -6,8 +6,8 @@
   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.
 
-  C bindings for leveldb.  May be useful as a stable ABI that can be
-  used by programs that keep leveldb in a shared library, or for
+  C bindings for rocksdb.  May be useful as a stable ABI that can be
+  used by programs that keep rocksdb in a shared library, or for
   a JNI api.
 
   Does not support:
@@ -61,6 +61,10 @@ typedef struct rocksdb_compactionfiltercontext_t
     rocksdb_compactionfiltercontext_t;
 typedef struct rocksdb_compactionfilterfactory_t
     rocksdb_compactionfilterfactory_t;
+typedef struct rocksdb_compactionfilterv2_t
+    rocksdb_compactionfilterv2_t;
+typedef struct rocksdb_compactionfilterfactoryv2_t
+    rocksdb_compactionfilterfactoryv2_t;
 typedef struct rocksdb_comparator_t      rocksdb_comparator_t;
 typedef struct rocksdb_env_t             rocksdb_env_t;
 typedef struct rocksdb_fifo_compaction_options_t rocksdb_fifo_compaction_options_t;
@@ -359,11 +363,15 @@ extern void rocksdb_options_set_compaction_filter(
     rocksdb_compactionfilter_t*);
 extern void rocksdb_options_set_compaction_filter_factory(
     rocksdb_options_t*, rocksdb_compactionfilterfactory_t*);
+extern void rocksdb_options_set_compaction_filter_factory_v2(
+    rocksdb_options_t*,
+    rocksdb_compactionfilterfactoryv2_t*);
 extern void rocksdb_options_set_comparator(
     rocksdb_options_t*,
     rocksdb_comparator_t*);
-extern void rocksdb_options_set_merge_operator(rocksdb_options_t*,
-                                               rocksdb_mergeoperator_t*);
+extern void rocksdb_options_set_merge_operator(
+    rocksdb_options_t*,
+    rocksdb_mergeoperator_t*);
 extern void rocksdb_options_set_compression_per_level(
   rocksdb_options_t* opt,
   int* level_values,
@@ -571,6 +579,35 @@ extern rocksdb_compactionfilterfactory_t*
 extern void rocksdb_compactionfilterfactory_destroy(
     rocksdb_compactionfilterfactory_t*);
 
+/* Compaction Filter V2 */
+
+extern rocksdb_compactionfilterv2_t* rocksdb_compactionfilterv2_create(
+    void* state,
+    void (*destructor)(void*),
+    // num_keys specifies the number of array entries in every *list parameter.
+    // New values added to the new_values_list should be malloc'd and will be
+    // freed by the caller. Specify true in the to_delete_list to remove an
+    // entry during compaction; false to keep it.
+    void (*filter)(
+        void*, int level, size_t num_keys,
+        const char* const* keys_list, const size_t* keys_list_sizes,
+        const char* const* existing_values_list, const size_t* existing_values_list_sizes,
+        char** new_values_list, size_t* new_values_list_sizes,
+        unsigned char* to_delete_list),
+    const char* (*name)(void*));
+extern void rocksdb_compactionfilterv2_destroy(rocksdb_compactionfilterv2_t*);
+
+/* Compaction Filter Factory V2 */
+
+extern rocksdb_compactionfilterfactoryv2_t* rocksdb_compactionfilterfactoryv2_create(
+    void* state,
+    rocksdb_slicetransform_t* prefix_extractor,
+    void (*destructor)(void*),
+    rocksdb_compactionfilterv2_t* (*create_compaction_filter_v2)(
+        void*, const rocksdb_compactionfiltercontext_t* context),
+    const char* (*name)(void*));
+extern void rocksdb_compactionfilterfactoryv2_destroy(rocksdb_compactionfilterfactoryv2_t*);
+
 /* Comparator */
 
 extern rocksdb_comparator_t* rocksdb_comparator_create(

include/rocksdb/options.h

@@ -487,7 +487,9 @@ struct ColumnFamilyOptions {
       TablePropertiesCollectorFactories;
   TablePropertiesCollectorFactories table_properties_collector_factories;
 
-  // Allows thread-safe inplace updates.
+  // Allows thread-safe inplace updates. If this is true, there is no way to
+  // achieve point-in-time consistency using snapshot or iterator (assuming
+  // concurrent updates).
   // If inplace_callback function is not set,
   //   Put(key, new_value) will update inplace the existing_value iff
   //   * key exists in current memtable

include/rocksdb/statistics.h

@@ -265,7 +265,10 @@ class Statistics {
   virtual void measureTime(uint32_t histogramType, uint64_t time) = 0;
 
   // String representation of the statistic object.
-  virtual std::string ToString() const = 0;
+  virtual std::string ToString() const {
+    // Do nothing by default
+    return std::string("ToString(): not implemented");
+  }
 
   // Override this function to disable particular histogram collection
   virtual bool HistEnabledForType(uint32_t type) const {

include/rocksdb/utilities/spatial_db.h

@@ -120,11 +120,14 @@ class FeatureSet {
   iterator end() const { return map_.end(); }
 
   void Clear();
+  size_t Size() const { return map_.size(); }
 
   void Serialize(std::string* output) const;
   // REQUIRED: empty FeatureSet
   bool Deserialize(const Slice& input);
 
+  std::string DebugString() const;
+
  private:
   map map_;
 };

port/stack_trace.cc

@@ -20,7 +20,9 @@ void PrintStack(int first_frames_to_skip) {}
 #include <execinfo.h>
 #include <signal.h>
 #include <stdio.h>
-#include <stdlib.h>
+// It's odd that including this breaks in GCC 7 but the build doesn't break
+// if I remove it even under GCC 4.8.
+// #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
 #include <cxxabi.h>

table/block.cc

@@ -17,44 +17,14 @@
 #include <vector>
 
 #include "rocksdb/comparator.h"
+#include "table/format.h"
 #include "table/block_hash_index.h"
 #include "table/block_prefix_index.h"
-#include "table/format.h"
 #include "util/coding.h"
 #include "util/logging.h"
-#include "db/dbformat.h"
 
 namespace rocksdb {
 
-uint32_t Block::NumRestarts() const {
-  assert(size_ >= 2*sizeof(uint32_t));
-  return DecodeFixed32(data_ + size_ - sizeof(uint32_t));
-}
-
-Block::Block(const BlockContents& contents)
-    : data_(contents.data.data()),
-      size_(contents.data.size()),
-      owned_(contents.heap_allocated),
-      cachable_(contents.cachable),
-      compression_type_(contents.compression_type) {
-  if (size_ < sizeof(uint32_t)) {
-    size_ = 0;  // Error marker
-  } else {
-    restart_offset_ = size_ - (1 + NumRestarts()) * sizeof(uint32_t);
-    if (restart_offset_ > size_ - sizeof(uint32_t)) {
-      // The size is too small for NumRestarts() and therefore
-      // restart_offset_ wrapped around.
-      size_ = 0;
-    }
-  }
-}
-
-Block::~Block() {
-  if (owned_) {
-    delete[] data_;
-  }
-}
-
 // Helper routine: decode the next block entry starting at "p",
 // storing the number of shared key bytes, non_shared key bytes,
 // and the length of the value in "*shared", "*non_shared", and
@@ -85,142 +55,85 @@ static inline const char* DecodeEntry(const char* p, const char* limit,
   return p;
 }
 
-class Block::Iter : public Iterator {
- private:
-  const Comparator* const comparator_;
-  const char* const data_;      // underlying block contents
-  uint32_t const restarts_;     // Offset of restart array (list of fixed32)
-  uint32_t const num_restarts_; // Number of uint32_t entries in restart array
+void BlockIter::Next() {
+  assert(Valid());
+  ParseNextKey();
+}
 
-  // current_ is offset in data_ of current entry.  >= restarts_ if !Valid
-  uint32_t current_;
-  uint32_t restart_index_;  // Index of restart block in which current_ falls
-  IterKey key_;
-  Slice value_;
-  Status status_;
-  BlockHashIndex* hash_index_;
-  BlockPrefixIndex* prefix_index_;
+void BlockIter::Prev() {
+  assert(Valid());
 
-  inline int Compare(const Slice& a, const Slice& b) const {
-    return comparator_->Compare(a, b);
-  }
-
-  // Return the offset in data_ just past the end of the current entry.
-  inline uint32_t NextEntryOffset() const {
-    return (value_.data() + value_.size()) - data_;
-  }
-
-  uint32_t GetRestartPoint(uint32_t index) {
-    assert(index < num_restarts_);
-    return DecodeFixed32(data_ + restarts_ + index * sizeof(uint32_t));
-  }
-
-  void SeekToRestartPoint(uint32_t index) {
-    key_.Clear();
-    restart_index_ = index;
-    // current_ will be fixed by ParseNextKey();
-
-    // ParseNextKey() starts at the end of value_, so set value_ accordingly
-    uint32_t offset = GetRestartPoint(index);
-    value_ = Slice(data_ + offset, 0);
-  }
-
- public:
-  Iter(const Comparator* comparator, const char* data, uint32_t restarts,
-       uint32_t num_restarts, BlockHashIndex* hash_index,
-       BlockPrefixIndex* prefix_index)
-      : comparator_(comparator),
-        data_(data),
-        restarts_(restarts),
-        num_restarts_(num_restarts),
-        current_(restarts_),
-        restart_index_(num_restarts_),
-        hash_index_(hash_index),
-        prefix_index_(prefix_index) {
-    assert(num_restarts_ > 0);
-  }
-
-  virtual bool Valid() const { return current_ < restarts_; }
-  virtual Status status() const { return status_; }
-  virtual Slice key() const {
-    assert(Valid());
-    return key_.GetKey();
-  }
-  virtual Slice value() const {
-    assert(Valid());
-    return value_;
-  }
-
-  virtual void Next() {
-    assert(Valid());
-    ParseNextKey();
-  }
-
-  virtual void Prev() {
-    assert(Valid());
-
-    // Scan backwards to a restart point before current_
-    const uint32_t original = current_;
-    while (GetRestartPoint(restart_index_) >= original) {
-      if (restart_index_ == 0) {
-        // No more entries
-        current_ = restarts_;
-        restart_index_ = num_restarts_;
-        return;
-      }
-      restart_index_--;
-    }
-
-    SeekToRestartPoint(restart_index_);
-    do {
-      // Loop until end of current entry hits the start of original entry
-    } while (ParseNextKey() && NextEntryOffset() < original);
-  }
-
-  virtual void Seek(const Slice& target) {
-    uint32_t index = 0;
-    bool ok = false;
-    if (prefix_index_) {
-      ok = PrefixSeek(target, &index);
-    } else {
-      ok = hash_index_ ? HashSeek(target, &index)
-        : BinarySeek(target, 0, num_restarts_ - 1, &index);
-    }
-
-    if (!ok) {
+  // Scan backwards to a restart point before current_
+  const uint32_t original = current_;
+  while (GetRestartPoint(restart_index_) >= original) {
+    if (restart_index_ == 0) {
+      // No more entries
+      current_ = restarts_;
+      restart_index_ = num_restarts_;
       return;
     }
-    SeekToRestartPoint(index);
-    // Linear search (within restart block) for first key >= target
+    restart_index_--;
+  }
 
-    while (true) {
-      if (!ParseNextKey() || Compare(key_.GetKey(), target) >= 0) {
-        return;
-      }
+  SeekToRestartPoint(restart_index_);
+  do {
+    // Loop until end of current entry hits the start of original entry
+  } while (ParseNextKey() && NextEntryOffset() < original);
+}
+
+void BlockIter::Seek(const Slice& target) {
+  if (data_ == nullptr) {  // Not init yet
+    return;
+  }
+  uint32_t index = 0;
+  bool ok = false;
+  if (prefix_index_) {
+    ok = PrefixSeek(target, &index);
+  } else {
+    ok = hash_index_ ? HashSeek(target, &index)
+      : BinarySeek(target, 0, num_restarts_ - 1, &index);
+  }
+
+  if (!ok) {
+    return;
+  }
+  SeekToRestartPoint(index);
+  // Linear search (within restart block) for first key >= target
+
+  while (true) {
+    if (!ParseNextKey() || Compare(key_.GetKey(), target) >= 0) {
+      return;
     }
   }
-  virtual void SeekToFirst() {
-    SeekToRestartPoint(0);
-    ParseNextKey();
-  }
+}
 
-  virtual void SeekToLast() {
-    SeekToRestartPoint(num_restarts_ - 1);
-    while (ParseNextKey() && NextEntryOffset() < restarts_) {
-      // Keep skipping
-    }
+void BlockIter::SeekToFirst() {
+  if (data_ == nullptr) {  // Not init yet
+    return;
   }
+  SeekToRestartPoint(0);
+  ParseNextKey();
+}
 
- private:
-  void CorruptionError() {
-    current_ = restarts_;
-    restart_index_ = num_restarts_;
-    status_ = Status::Corruption("bad entry in block");
-    key_.Clear();
-    value_.clear();
+void BlockIter::SeekToLast() {
+  if (data_ == nullptr) {  // Not init yet
+    return;
   }
+  SeekToRestartPoint(num_restarts_ - 1);
+  while (ParseNextKey() && NextEntryOffset() < restarts_) {
+    // Keep skipping
+  }
+}
 
-  bool ParseNextKey() {
+void BlockIter::CorruptionError() {
+  current_ = restarts_;
+  restart_index_ = num_restarts_;
+  status_ = Status::Corruption("bad entry in block");
+  key_.Clear();
+  value_.clear();
+}
+
+bool BlockIter::ParseNextKey() {
     current_ = NextEntryOffset();
     const char* p = data_ + current_;
     const char* limit = data_ + restarts_;  // Restarts come right after data
@@ -248,150 +161,194 @@ class Block::Iter : public Iterator {
     }
   }
 
-  // Binary search in restart array to find the first restart point
-  // with a key >= target (TODO: this comment is inaccurate)
-  bool BinarySeek(const Slice& target, uint32_t left, uint32_t right,
+// Binary search in restart array to find the first restart point
+// with a key >= target (TODO: this comment is inaccurate)
+bool BlockIter::BinarySeek(const Slice& target, uint32_t left, uint32_t right,
                   uint32_t* index) {
-    assert(left <= right);
+  assert(left <= right);
 
-    while (left < right) {
-      uint32_t mid = (left + right + 1) / 2;
-      uint32_t region_offset = GetRestartPoint(mid);
-      uint32_t shared, non_shared, value_length;
-      const char* key_ptr =
-          DecodeEntry(data_ + region_offset, data_ + restarts_, &shared,
-                      &non_shared, &value_length);
-      if (key_ptr == nullptr || (shared != 0)) {
-        CorruptionError();
-        return false;
-      }
-      Slice mid_key(key_ptr, non_shared);
-      int cmp = Compare(mid_key, target);
-      if (cmp < 0) {
-        // Key at "mid" is smaller than "target". Therefore all
-        // blocks before "mid" are uninteresting.
-        left = mid;
-      } else if (cmp > 0) {
-        // Key at "mid" is >= "target". Therefore all blocks at or
-        // after "mid" are uninteresting.
-        right = mid - 1;
-      } else {
-        left = right = mid;
-      }
-    }
-
-    *index = left;
-    return true;
-  }
-
-  // Compare target key and the block key of the block of `block_index`.
-  // Return -1 if error.
-  int CompareBlockKey(uint32_t block_index, const Slice& target) {
-    uint32_t region_offset = GetRestartPoint(block_index);
+  while (left < right) {
+    uint32_t mid = (left + right + 1) / 2;
+    uint32_t region_offset = GetRestartPoint(mid);
     uint32_t shared, non_shared, value_length;
-    const char* key_ptr = DecodeEntry(data_ + region_offset, data_ + restarts_,
-                                      &shared, &non_shared, &value_length);
+    const char* key_ptr =
+        DecodeEntry(data_ + region_offset, data_ + restarts_, &shared,
+                    &non_shared, &value_length);
     if (key_ptr == nullptr || (shared != 0)) {
       CorruptionError();
-      return 1;  // Return target is smaller
+      return false;
     }
-    Slice block_key(key_ptr, non_shared);
-    return Compare(block_key, target);
-  }
-
-  // Binary search in block_ids to find the first block
-  // with a key >= target
-  bool BinaryBlockIndexSeek(const Slice& target, uint32_t* block_ids,
-                            uint32_t left, uint32_t right,
-                            uint32_t* index) {
-    assert(left <= right);
-    uint32_t left_bound = left;
-
-    while (left <= right) {
-      uint32_t mid = (left + right) / 2;
-
-      int cmp = CompareBlockKey(block_ids[mid], target);
-      if (!status_.ok()) {
-        return false;
-      }
-      if (cmp < 0) {
-        // Key at "target" is larger than "mid". Therefore all
-        // blocks before or at "mid" are uninteresting.
-        left = mid + 1;
-      } else {
-        // Key at "target" is <= "mid". Therefore all blocks
-        // after "mid" are uninteresting.
-        // If there is only one block left, we found it.
-        if (left == right) break;
-        right = mid;
-      }
-    }
-
-    if (left == right) {
-      // In one of the two following cases:
-      // (1) left is the first one of block_ids
-      // (2) there is a gap of blocks between block of `left` and `left-1`.
-      // we can further distinguish the case of key in the block or key not
-      // existing, by comparing the target key and the key of the previous
-      // block to the left of the block found.
-      if (block_ids[left] > 0 &&
-          (left == left_bound || block_ids[left - 1] != block_ids[left] - 1) &&
-          CompareBlockKey(block_ids[left] - 1, target) > 0) {
-        current_ = restarts_;
-        return false;
-      }
-
-      *index = block_ids[left];
-      return true;
+    Slice mid_key(key_ptr, non_shared);
+    int cmp = Compare(mid_key, target);
+    if (cmp < 0) {
+      // Key at "mid" is smaller than "target". Therefore all
+      // blocks before "mid" are uninteresting.
+      left = mid;
+    } else if (cmp > 0) {
+      // Key at "mid" is >= "target". Therefore all blocks at or
+      // after "mid" are uninteresting.
+      right = mid - 1;
     } else {
-      assert(left > right);
-      // Mark iterator invalid
-      current_ = restarts_;
-      return false;
+      left = right = mid;
     }
   }
 
-  bool HashSeek(const Slice& target, uint32_t* index) {
-    assert(hash_index_);
-    auto restart_index = hash_index_->GetRestartIndex(target);
-    if (restart_index == nullptr) {
+  *index = left;
+  return true;
+}
+
+// Compare target key and the block key of the block of `block_index`.
+// Return -1 if error.
+int BlockIter::CompareBlockKey(uint32_t block_index, const Slice& target) {
+  uint32_t region_offset = GetRestartPoint(block_index);
+  uint32_t shared, non_shared, value_length;
+  const char* key_ptr = DecodeEntry(data_ + region_offset, data_ + restarts_,
+                                    &shared, &non_shared, &value_length);
+  if (key_ptr == nullptr || (shared != 0)) {
+    CorruptionError();
+    return 1;  // Return target is smaller
+  }
+  Slice block_key(key_ptr, non_shared);
+  return Compare(block_key, target);
+}
+
+// Binary search in block_ids to find the first block
+// with a key >= target
+bool BlockIter::BinaryBlockIndexSeek(const Slice& target, uint32_t* block_ids,
+                          uint32_t left, uint32_t right,
+                          uint32_t* index) {
+  assert(left <= right);
+  uint32_t left_bound = left;
+
+  while (left <= right) {
+    uint32_t mid = (left + right) / 2;
+
+    int cmp = CompareBlockKey(block_ids[mid], target);
+    if (!status_.ok()) {
+      return false;
+    }
+    if (cmp < 0) {
+      // Key at "target" is larger than "mid". Therefore all
+      // blocks before or at "mid" are uninteresting.
+      left = mid + 1;
+    } else {
+      // Key at "target" is <= "mid". Therefore all blocks
+      // after "mid" are uninteresting.
+      // If there is only one block left, we found it.
+      if (left == right) break;
+      right = mid;
+    }
+  }
+
+  if (left == right) {
+    // In one of the two following cases:
+    // (1) left is the first one of block_ids
+    // (2) there is a gap of blocks between block of `left` and `left-1`.
+    // we can further distinguish the case of key in the block or key not
+    // existing, by comparing the target key and the key of the previous
+    // block to the left of the block found.
+    if (block_ids[left] > 0 &&
+        (left == left_bound || block_ids[left - 1] != block_ids[left] - 1) &&
+        CompareBlockKey(block_ids[left] - 1, target) > 0) {
       current_ = restarts_;
       return false;
     }
 
-    // the elements in restart_array[index : index + num_blocks]
-    // are all with same prefix. We'll do binary search in that small range.
-    auto left = restart_index->first_index;
-    auto right = restart_index->first_index + restart_index->num_blocks - 1;
-    return BinarySeek(target, left, right, index);
+    *index = block_ids[left];
+    return true;
+  } else {
+    assert(left > right);
+    // Mark iterator invalid
+    current_ = restarts_;
+    return false;
+  }
+}
+
+bool BlockIter::HashSeek(const Slice& target, uint32_t* index) {
+  assert(hash_index_);
+  auto restart_index = hash_index_->GetRestartIndex(target);
+  if (restart_index == nullptr) {
+    current_ = restarts_;
+    return false;
   }
 
-  bool PrefixSeek(const Slice& target, uint32_t* index) {
-    assert(prefix_index_);
-    uint32_t* block_ids = nullptr;
-    uint32_t num_blocks = prefix_index_->GetBlocks(target, &block_ids);
+  // the elements in restart_array[index : index + num_blocks]
+  // are all with same prefix. We'll do binary search in that small range.
+  auto left = restart_index->first_index;
+  auto right = restart_index->first_index + restart_index->num_blocks - 1;
+  return BinarySeek(target, left, right, index);
+}
 
+bool BlockIter::PrefixSeek(const Slice& target, uint32_t* index) {
+  assert(prefix_index_);
+  uint32_t* block_ids = nullptr;
+  uint32_t num_blocks = prefix_index_->GetBlocks(target, &block_ids);
 
-    if (num_blocks == 0) {
-      current_ = restarts_;
-      return false;
-    } else  {
-      return BinaryBlockIndexSeek(target, block_ids, 0, num_blocks - 1, index);
+  if (num_blocks == 0) {
+    current_ = restarts_;
+    return false;
+  } else  {
+    return BinaryBlockIndexSeek(target, block_ids, 0, num_blocks - 1, index);
+  }
+}
+
+uint32_t Block::NumRestarts() const {
+  assert(size_ >= 2*sizeof(uint32_t));
+  return DecodeFixed32(data_ + size_ - sizeof(uint32_t));
+}
+
+Block::Block(const BlockContents& contents)
+    : data_(contents.data.data()),
+      size_(contents.data.size()),
+      owned_(contents.heap_allocated),
+      cachable_(contents.cachable),
+      compression_type_(contents.compression_type) {
+  if (size_ < sizeof(uint32_t)) {
+    size_ = 0;  // Error marker
+  } else {
+    restart_offset_ = size_ - (1 + NumRestarts()) * sizeof(uint32_t);
+    if (restart_offset_ > size_ - sizeof(uint32_t)) {
+      // The size is too small for NumRestarts() and therefore
+      // restart_offset_ wrapped around.
+      size_ = 0;
     }
   }
-};
+}
 
-Iterator* Block::NewIterator(const Comparator* cmp) {
+Block::~Block() {
+  if (owned_) {
+    delete[] data_;
+  }
+}
+
+Iterator* Block::NewIterator(const Comparator* cmp, BlockIter* iter) {
   if (size_ < 2*sizeof(uint32_t)) {
-    return NewErrorIterator(Status::Corruption("bad block contents"));
+    if (iter != nullptr) {
+      iter->SetStatus(Status::Corruption("bad block contents"));
+      return iter;
+    } else {
+      return NewErrorIterator(Status::Corruption("bad block contents"));
+    }
   }
   const uint32_t num_restarts = NumRestarts();
   if (num_restarts == 0) {
-    return NewEmptyIterator();
+    if (iter != nullptr) {
+      iter->SetStatus(Status::OK());
+      return iter;
+    } else {
+      return NewEmptyIterator();
+    }
   } else {
-    return new Iter(cmp, data_, restart_offset_, num_restarts,
+    if (iter != nullptr) {
+      iter->Initialize(cmp, data_, restart_offset_, num_restarts,
                     hash_index_.get(), prefix_index_.get());
+    } else {
+      iter = new BlockIter(cmp, data_, restart_offset_, num_restarts,
+                    hash_index_.get(), prefix_index_.get());
+    }
   }
+
+  return iter;
 }
 
 void Block::SetBlockHashIndex(BlockHashIndex* hash_index) {
@@ -402,4 +359,15 @@ void Block::SetBlockPrefixIndex(BlockPrefixIndex* prefix_index) {
   prefix_index_.reset(prefix_index);
 }
 
+size_t Block::ApproximateMemoryUsage() const {
+  size_t usage = size();
+  if (hash_index_) {
+    usage += hash_index_->ApproximateMemoryUsage();
+  }
+  if (prefix_index_) {
+    usage += prefix_index_->ApproximateMemoryUsage();
+  }
+  return usage;
+}
+
 }  // namespace rocksdb

table/block.h

@@ -13,11 +13,13 @@
 
 #include "rocksdb/iterator.h"
 #include "rocksdb/options.h"
+#include "db/dbformat.h"
 
 namespace rocksdb {
 
 struct BlockContents;
 class Comparator;
+class BlockIter;
 class BlockHashIndex;
 class BlockPrefixIndex;
 
@@ -40,10 +42,17 @@ class Block {
   // NOTE: for the hash based lookup, if a key prefix doesn't match any key,
   // the iterator will simply be set as "invalid", rather than returning
   // the key that is just pass the target key.
-  Iterator* NewIterator(const Comparator* comparator);
+  //
+  // If iter is null, return new Iterator
+  // If iter is not null, update this one and return it as Iterator*
+  Iterator* NewIterator(const Comparator* comparator,
+      BlockIter* iter = nullptr);
   void SetBlockHashIndex(BlockHashIndex* hash_index);
   void SetBlockPrefixIndex(BlockPrefixIndex* prefix_index);
 
+  // Report an approximation of how much memory has been used.
+  size_t ApproximateMemoryUsage() const;
+
  private:
   const char* data_;
   size_t size_;
@@ -57,8 +66,126 @@ class Block {
   // No copying allowed
   Block(const Block&);
   void operator=(const Block&);
+};
+
+class BlockIter : public Iterator {
+ public:
+  BlockIter()
+      : comparator_(nullptr),
+        data_(nullptr),
+        restarts_(0),
+        num_restarts_(0),
+        current_(0),
+        restart_index_(0),
+        status_(Status::OK()),
+        hash_index_(nullptr),
+        prefix_index_(nullptr) {}
+
+  BlockIter(const Comparator* comparator, const char* data, uint32_t restarts,
+       uint32_t num_restarts, BlockHashIndex* hash_index,
+       BlockPrefixIndex* prefix_index)
+      : BlockIter() {
+    Initialize(comparator, data, restarts, num_restarts,
+        hash_index, prefix_index);
+  }
+
+  void Initialize(const Comparator* comparator, const char* data,
+      uint32_t restarts, uint32_t num_restarts, BlockHashIndex* hash_index,
+      BlockPrefixIndex* prefix_index) {
+    assert(data_ == nullptr);           // Ensure it is called only once
+    assert(num_restarts > 0);           // Ensure the param is valid
+
+    comparator_ = comparator;
+    data_ = data;
+    restarts_ = restarts;
+    num_restarts_ = num_restarts;
+    current_ = restarts_;
+    restart_index_ = num_restarts_;
+    hash_index_ = hash_index;
+    prefix_index_ = prefix_index;
+  }
+
+  void SetStatus(Status s) {
+    status_ = s;
+  }
+
+  virtual bool Valid() const override { return current_ < restarts_; }
+  virtual Status status() const override { return status_; }
+  virtual Slice key() const override {
+    assert(Valid());
+    return key_.GetKey();
+  }
+  virtual Slice value() const override {
+    assert(Valid());
+    return value_;
+  }
+
+  virtual void Next() override;
+
+  virtual void Prev() override;
+
+  virtual void Seek(const Slice& target) override;
+
+  virtual void SeekToFirst() override;
+
+  virtual void SeekToLast() override;
+
+ private:
+  const Comparator* comparator_;
+  const char* data_;       // underlying block contents
+  uint32_t restarts_;      // Offset of restart array (list of fixed32)
+  uint32_t num_restarts_;  // Number of uint32_t entries in restart array
+
+  // current_ is offset in data_ of current entry.  >= restarts_ if !Valid
+  uint32_t current_;
+  uint32_t restart_index_;  // Index of restart block in which current_ falls
+  IterKey key_;
+  Slice value_;
+  Status status_;
+  BlockHashIndex* hash_index_;
+  BlockPrefixIndex* prefix_index_;
+
+  inline int Compare(const Slice& a, const Slice& b) const {
+    return comparator_->Compare(a, b);
+  }
+
+  // Return the offset in data_ just past the end of the current entry.
+  inline uint32_t NextEntryOffset() const {
+    return (value_.data() + value_.size()) - data_;
+  }
+
+  uint32_t GetRestartPoint(uint32_t index) {
+    assert(index < num_restarts_);
+    return DecodeFixed32(data_ + restarts_ + index * sizeof(uint32_t));
+  }
+
+  void SeekToRestartPoint(uint32_t index) {
+    key_.Clear();
+    restart_index_ = index;
+    // current_ will be fixed by ParseNextKey();
+
+    // ParseNextKey() starts at the end of value_, so set value_ accordingly
+    uint32_t offset = GetRestartPoint(index);
+    value_ = Slice(data_ + offset, 0);
+  }
+
+  void CorruptionError();
+
+  bool ParseNextKey();
+
+  bool BinarySeek(const Slice& target, uint32_t left, uint32_t right,
+                  uint32_t* index);
+
+  int CompareBlockKey(uint32_t block_index, const Slice& target);
+
+  bool BinaryBlockIndexSeek(const Slice& target, uint32_t* block_ids,
+                            uint32_t left, uint32_t right,
+                            uint32_t* index);
+
+  bool HashSeek(const Slice& target, uint32_t* index);
+
+  bool PrefixSeek(const Slice& target, uint32_t* index);
 
-  class Iter;
 };
 
 }  // namespace rocksdb

table/block_based_table_builder.cc

@@ -45,7 +45,6 @@ namespace rocksdb {
 
 extern const std::string kHashIndexPrefixesBlock;
 extern const std::string kHashIndexPrefixesMetadataBlock;
-namespace {
 
 typedef BlockBasedTableOptions::IndexType IndexType;
 
@@ -335,8 +334,6 @@ Slice CompressBlock(const Slice& raw,
   return raw;
 }
 
-}  // anonymous namespace
-
 // kBlockBasedTableMagicNumber was picked by running
 //    echo rocksdb.table.block_based | sha1sum
 // and taking the leading 64 bits.

table/block_based_table_reader.cc

@@ -135,11 +135,17 @@ class BlockBasedTable::IndexReader {
   virtual ~IndexReader() {}
 
   // Create an iterator for index access.
-  virtual Iterator* NewIterator() = 0;
+  // An iter is passed in, if it is not null, update this one and return it
+  // If it is null, create a new Iterator
+  virtual Iterator* NewIterator(BlockIter* iter = nullptr) = 0;
 
   // The size of the index.
   virtual size_t size() const = 0;
 
+  // Report an approximation of how much memory has been used other than memory
+  // that was allocated in block cache.
+  virtual size_t ApproximateMemoryUsage() const = 0;
+
  protected:
   const Comparator* comparator_;
 };
@@ -168,12 +174,17 @@ class BinarySearchIndexReader : public IndexReader {
     return s;
   }
 
-  virtual Iterator* NewIterator() override {
-    return index_block_->NewIterator(comparator_);
+  virtual Iterator* NewIterator(BlockIter* iter = nullptr) override {
+    return index_block_->NewIterator(comparator_, iter);
   }
 
   virtual size_t size() const override { return index_block_->size(); }
 
+  virtual size_t ApproximateMemoryUsage() const override {
+    assert(index_block_);
+    return index_block_->ApproximateMemoryUsage();
+  }
+
  private:
   BinarySearchIndexReader(const Comparator* comparator, Block* index_block)
       : IndexReader(comparator), index_block_(index_block) {
@@ -284,12 +295,18 @@ class HashIndexReader : public IndexReader {
     return Status::OK();
   }
 
-  virtual Iterator* NewIterator() override {
-    return index_block_->NewIterator(comparator_);
+  virtual Iterator* NewIterator(BlockIter* iter = nullptr) override {
+    return index_block_->NewIterator(comparator_, iter);
   }
 
   virtual size_t size() const override { return index_block_->size(); }
 
+  virtual size_t ApproximateMemoryUsage() const override {
+    assert(index_block_);
+    return index_block_->ApproximateMemoryUsage() +
+           prefixes_contents_.data.size();
+  }
+
  private:
   HashIndexReader(const Comparator* comparator, Block* index_block)
       : IndexReader(comparator),
@@ -542,6 +559,17 @@ std::shared_ptr<const TableProperties> BlockBasedTable::GetTableProperties()
   return rep_->table_properties;
 }
 
+size_t BlockBasedTable::ApproximateMemoryUsage() const {
+  size_t usage = 0;
+  if (rep_->filter) {
+    usage += rep_->filter->ApproximateMemoryUsage();
+  }
+  if (rep_->index_reader) {
+    usage += rep_->index_reader->ApproximateMemoryUsage();
+  }
+  return usage;
+}
+
 // Load the meta-block from the file. On success, return the loaded meta block
 // and its iterator.
 Status BlockBasedTable::ReadMetaBlock(
@@ -779,10 +807,11 @@ BlockBasedTable::CachableEntry<FilterBlockReader> BlockBasedTable::GetFilter(
   return { filter, cache_handle };
 }
 
-Iterator* BlockBasedTable::NewIndexIterator(const ReadOptions& read_options) {
+Iterator* BlockBasedTable::NewIndexIterator(const ReadOptions& read_options,
+        BlockIter* input_iter) {
   // index reader has already been pre-populated.
   if (rep_->index_reader) {
-    return rep_->index_reader->NewIterator();
+    return rep_->index_reader->NewIterator(input_iter);
   }
 
   bool no_io = read_options.read_tier == kBlockCacheTier;
@@ -796,7 +825,12 @@ Iterator* BlockBasedTable::NewIndexIterator(const ReadOptions& read_options) {
                         BLOCK_CACHE_INDEX_HIT, statistics);
 
   if (cache_handle == nullptr && no_io) {
-    return NewErrorIterator(Status::Incomplete("no blocking io"));
+    if (input_iter != nullptr) {
+      input_iter->SetStatus(Status::Incomplete("no blocking io"));
+      return input_iter;
+    } else {
+      return NewErrorIterator(Status::Incomplete("no blocking io"));
+    }
   }
 
   IndexReader* index_reader = nullptr;
@@ -811,7 +845,12 @@ Iterator* BlockBasedTable::NewIndexIterator(const ReadOptions& read_options) {
     if (!s.ok()) {
       // make sure if something goes wrong, index_reader shall remain intact.
       assert(index_reader == nullptr);
-      return NewErrorIterator(s);
+      if (input_iter != nullptr) {
+        input_iter->SetStatus(s);
+        return input_iter;
+      } else {
+        return NewErrorIterator(s);
+      }
     }
 
     cache_handle = block_cache->Insert(key, index_reader, index_reader->size(),
@@ -820,7 +859,8 @@ Iterator* BlockBasedTable::NewIndexIterator(const ReadOptions& read_options) {
   }
 
   assert(cache_handle);
-  auto iter = index_reader->NewIterator();
+  Iterator* iter;
+  iter = index_reader->NewIterator(input_iter);
   iter->RegisterCleanup(&ReleaseCachedEntry, block_cache, cache_handle);
 
   return iter;
@@ -828,8 +868,11 @@ Iterator* BlockBasedTable::NewIndexIterator(const ReadOptions& read_options) {
 
 // Convert an index iterator value (i.e., an encoded BlockHandle)
 // into an iterator over the contents of the corresponding block.
+// If input_iter is null, new a iterator
+// If input_iter is not null, update this iter and return it
 Iterator* BlockBasedTable::NewDataBlockIterator(Rep* rep,
-    const ReadOptions& ro, const Slice& index_value) {
+    const ReadOptions& ro, const Slice& index_value,
+    BlockIter* input_iter) {
   const bool no_io = (ro.read_tier == kBlockCacheTier);
   Cache* block_cache = rep->options.block_cache.get();
   Cache* block_cache_compressed = rep->options.
@@ -843,7 +886,12 @@ Iterator* BlockBasedTable::NewDataBlockIterator(Rep* rep,
   Status s = handle.DecodeFrom(&input);
 
   if (!s.ok()) {
-    return NewErrorIterator(s);
+    if (input_iter != nullptr) {
+      input_iter->SetStatus(s);
+      return input_iter;
+    } else {
+      return NewErrorIterator(s);
+    }
   }
 
   // If either block cache is enabled, we'll try to read from it.
@@ -889,7 +937,12 @@ Iterator* BlockBasedTable::NewDataBlockIterator(Rep* rep,
   if (block.value == nullptr) {
     if (no_io) {
       // Could not read from block_cache and can't do IO
-      return NewErrorIterator(Status::Incomplete("no blocking io"));
+      if (input_iter != nullptr) {
+        input_iter->SetStatus(Status::Incomplete("no blocking io"));
+        return input_iter;
+      } else {
+        return NewErrorIterator(Status::Incomplete("no blocking io"));
+      }
     }
     s = ReadBlockFromFile(rep->file.get(), rep->footer, ro, handle,
                           &block.value, rep->options.env);
@@ -897,15 +950,20 @@ Iterator* BlockBasedTable::NewDataBlockIterator(Rep* rep,
 
   Iterator* iter;
   if (block.value != nullptr) {
-    iter = block.value->NewIterator(&rep->internal_comparator);
+    iter = block.value->NewIterator(&rep->internal_comparator, input_iter);
     if (block.cache_handle != nullptr) {
       iter->RegisterCleanup(&ReleaseCachedEntry, block_cache,
-                            block.cache_handle);
+          block.cache_handle);
     } else {
       iter->RegisterCleanup(&DeleteHeldResource<Block>, block.value, nullptr);
     }
   } else {
-    iter = NewErrorIterator(s);
+    if (input_iter != nullptr) {
+      input_iter->SetStatus(s);
+      iter = input_iter;
+    } else {
+      iter = NewErrorIterator(s);
+    }
   }
   return iter;
 }
@@ -1023,12 +1081,14 @@ Status BlockBasedTable::Get(
                            const Slice& v),
     void (*mark_key_may_exist_handler)(void* handle_context)) {
   Status s;
-  Iterator* iiter = NewIndexIterator(read_options);
+  BlockIter iiter;
+  NewIndexIterator(read_options, &iiter);
+
   auto filter_entry = GetFilter(read_options.read_tier == kBlockCacheTier);
   FilterBlockReader* filter = filter_entry.value;
   bool done = false;
-  for (iiter->Seek(key); iiter->Valid() && !done; iiter->Next()) {
-    Slice handle_value = iiter->value();
+  for (iiter.Seek(key); iiter.Valid() && !done; iiter.Next()) {
+    Slice handle_value = iiter.value();
 
     BlockHandle handle;
     bool may_not_exist_in_filter =
@@ -1043,39 +1103,43 @@ Status BlockBasedTable::Get(
       RecordTick(rep_->options.statistics.get(), BLOOM_FILTER_USEFUL);
       break;
     } else {
-      unique_ptr<Iterator> block_iter(
-          NewDataBlockIterator(rep_, read_options, iiter->value()));
+      BlockIter biter;
+      NewDataBlockIterator(rep_, read_options, iiter.value(), &biter);
 
-      if (read_options.read_tier && block_iter->status().IsIncomplete()) {
+      if (read_options.read_tier && biter.status().IsIncomplete()) {
         // couldn't get block from block_cache
         // Update Saver.state to Found because we are only looking for whether
         // we can guarantee the key is not there when "no_io" is set
         (*mark_key_may_exist_handler)(handle_context);
         break;
       }
+      if (!biter.status().ok()) {
+        s = biter.status();
+        break;
+      }
 
       // Call the *saver function on each entry/block until it returns false
-      for (block_iter->Seek(key); block_iter->Valid(); block_iter->Next()) {
+      for (biter.Seek(key); biter.Valid(); biter.Next()) {
         ParsedInternalKey parsed_key;
-        if (!ParseInternalKey(block_iter->key(), &parsed_key)) {
+        if (!ParseInternalKey(biter.key(), &parsed_key)) {
           s = Status::Corruption(Slice());
         }
 
         if (!(*result_handler)(handle_context, parsed_key,
-                               block_iter->value())) {
+                               biter.value())) {
           done = true;
           break;
         }
       }
-      s = block_iter->status();
+      s = biter.status();
     }
   }
 
   filter_entry.Release(rep_->options.block_cache.get());
   if (s.ok()) {
-    s = iiter->status();
+    s = iiter.status();
   }
-  delete iiter;
+
   return s;
 }
 

table/block_based_table_reader.h

@@ -23,6 +23,7 @@
 namespace rocksdb {
 
 class Block;
+class BlockIter;
 class BlockHandle;
 class Cache;
 class FilterBlockReader;
@@ -95,6 +96,8 @@ class BlockBasedTable : public TableReader {
 
   std::shared_ptr<const TableProperties> GetTableProperties() const override;
 
+  size_t ApproximateMemoryUsage() const override;
+
   ~BlockBasedTable();
 
   bool TEST_filter_block_preloaded() const;
@@ -111,8 +114,10 @@ class BlockBasedTable : public TableReader {
   bool compaction_optimized_;
 
   class BlockEntryIteratorState;
+  // input_iter: if it is not null, update this one and return it as Iterator
   static Iterator* NewDataBlockIterator(Rep* rep, const ReadOptions& ro,
-                                        const Slice& index_value);
+                                        const Slice& index_value,
+                                        BlockIter* input_iter = nullptr);
 
   // For the following two functions:
   // if `no_io == true`, we will not try to read filter/index from sst file
@@ -120,6 +125,8 @@ class BlockBasedTable : public TableReader {
   CachableEntry<FilterBlockReader> GetFilter(bool no_io = false) const;
 
   // Get the iterator from the index reader.
+  // If input_iter is not set, return new Iterator
+  // If input_iter is set, update it and return it as Iterator
   //
   // Note: ErrorIterator with Status::Incomplete shall be returned if all the
   // following conditions are met:
@@ -127,7 +134,8 @@ class BlockBasedTable : public TableReader {
   //  2. index is not present in block cache.
   //  3. We disallowed any io to be performed, that is, read_options ==
   //     kBlockCacheTier
-  Iterator* NewIndexIterator(const ReadOptions& read_options);
+  Iterator* NewIndexIterator(const ReadOptions& read_options,
+                             BlockIter* input_iter = nullptr);
 
   // Read block cache from block caches (if set): block_cache and
   // block_cache_compressed.

table/block_prefix_index.cc

@@ -16,8 +16,6 @@
 
 namespace rocksdb {
 
-namespace {
-
 inline uint32_t Hash(const Slice& s) {
   return rocksdb::Hash(s.data(), s.size(), 0);
 }
@@ -26,8 +24,6 @@ inline uint32_t PrefixToBucket(const Slice& prefix, uint32_t num_buckets) {
   return Hash(prefix) % num_buckets;
 }
 
-
-
 // The prefix block index is simply a bucket array, with each entry pointing to
 // the blocks that span the prefixes hashed to this bucket.
 //
@@ -64,7 +60,6 @@ inline uint32_t EncodeIndex(uint32_t index) {
   return index | kBlockArrayMask;
 }
 
-
 // temporary storage for prefix information during index building
 struct PrefixRecord {
   Slice prefix;
@@ -74,8 +69,6 @@ struct PrefixRecord {
   PrefixRecord* next;
 };
 
-}  // anonymous namespace
-
 class BlockPrefixIndex::Builder {
  public:
   explicit Builder(const SliceTransform* internal_prefix_extractor)

table/cuckoo_table_builder.cc

@@ -8,6 +8,7 @@
 
 #include <assert.h>
 #include <algorithm>
+#include <limits>
 #include <string>
 #include <vector>
 
@@ -36,25 +37,18 @@ const std::string CuckooTablePropertyNames::kIsLastLevel =
 extern const uint64_t kCuckooTableMagicNumber = 0x926789d0c5f17873ull;
 
 CuckooTableBuilder::CuckooTableBuilder(
-    WritableFile* file, uint32_t fixed_key_length,
-    uint32_t fixed_value_length, double hash_table_ratio,
-    uint64_t file_size, uint32_t max_num_hash_table,
-    uint32_t max_search_depth, bool is_last_level,
-    uint64_t (*GetSliceHashPtr)(const Slice&, uint32_t, uint64_t))
+    WritableFile* file, double hash_table_ratio,
+    uint32_t max_num_hash_table, uint32_t max_search_depth,
+    uint64_t (*get_slice_hash)(const Slice&, uint32_t, uint64_t))
     : num_hash_table_(2),
       file_(file),
-      value_length_(fixed_value_length),
-      // 8 is the difference between sizes of user key and InternalKey.
-      bucket_size_(fixed_key_length +
-          fixed_value_length - (is_last_level ? 8 : 0)),
       hash_table_ratio_(hash_table_ratio),
-      max_num_buckets_(file_size / bucket_size_),
       max_num_hash_table_(max_num_hash_table),
       max_search_depth_(max_search_depth),
-      is_last_level_file_(is_last_level),
-      buckets_(max_num_buckets_),
-      make_space_for_key_call_id_(0),
-      GetSliceHash(GetSliceHashPtr) {
+      is_last_level_file_(false),
+      has_seen_first_key_(false),
+      get_slice_hash_(get_slice_hash),
+      closed_(false) {
   properties_.num_entries = 0;
   // Data is in a huge block.
   properties_.num_data_blocks = 1;
@@ -62,105 +56,120 @@ CuckooTableBuilder::CuckooTableBuilder(
   properties_.filter_size = 0;
 }
 
-CuckooTableBuilder::~CuckooTableBuilder() {
-}
-
 void CuckooTableBuilder::Add(const Slice& key, const Slice& value) {
-  if (NumEntries() == max_num_buckets_) {
-    status_ = Status::Corruption("Hash Table is full.");
+  if (properties_.num_entries >= kMaxVectorIdx - 1) {
+    status_ = Status::NotSupported("Number of keys in a file must be < 2^32-1");
     return;
   }
-  uint64_t bucket_id;
-  bool bucket_found = false;
-  autovector<uint64_t> hash_vals;
   ParsedInternalKey ikey;
   if (!ParseInternalKey(key, &ikey)) {
     status_ = Status::Corruption("Unable to parse key into inernal key.");
     return;
   }
-  Slice user_key = ikey.user_key;
-  for (uint32_t hash_cnt = 0; hash_cnt < num_hash_table_; ++hash_cnt) {
-    uint64_t hash_val = GetSliceHash(user_key, hash_cnt, max_num_buckets_);
-    if (buckets_[hash_val].is_empty) {
-      bucket_id = hash_val;
-      bucket_found = true;
-      break;
-    } else {
-      if (user_key.compare(
-            is_last_level_file_ ? Slice(buckets_[hash_val].key)
-            : ExtractUserKey(Slice(buckets_[hash_val].key))) == 0) {
-        status_ = Status::Corruption("Same key is being inserted again.");
-        return;
-      }
-      hash_vals.push_back(hash_val);
-    }
-  }
-  while (!bucket_found && !MakeSpaceForKey(key, &bucket_id, hash_vals)) {
-    // Rehash by increashing number of hash tables.
-    if (num_hash_table_ >= max_num_hash_table_) {
-      status_ = Status::Corruption("Too many collissions. Unable to hash.");
-      return;
-    }
-    // We don't really need to rehash the entire table because old hashes are
-    // still valid and we only increased the number of hash functions.
-    uint64_t hash_val = GetSliceHash(user_key,
-        num_hash_table_, max_num_buckets_);
-    ++num_hash_table_;
-    if (buckets_[hash_val].is_empty) {
-      bucket_found = true;
-      bucket_id = hash_val;
-      break;
-    } else {
-      hash_vals.push_back(hash_val);
-    }
+  // Determine if we can ignore the sequence number and value type from
+  // internal keys by looking at sequence number from first key. We assume
+  // that if first key has a zero sequence number, then all the remaining
+  // keys will have zero seq. no.
+  if (!has_seen_first_key_) {
+    is_last_level_file_ = ikey.sequence == 0;
+    has_seen_first_key_ = true;
   }
+  // Even if one sequence number is non-zero, then it is not last level.
+  assert(!is_last_level_file_ || ikey.sequence == 0);
   if (is_last_level_file_) {
-    buckets_[bucket_id].key.assign(user_key.data(), user_key.size());
+    kvs_.emplace_back(std::make_pair(
+          ikey.user_key.ToString(), value.ToString()));
   } else {
-    buckets_[bucket_id].key.assign(key.data(), key.size());
+    kvs_.emplace_back(std::make_pair(key.ToString(), value.ToString()));
   }
-  buckets_[bucket_id].value.assign(value.data(), value.size());
-  buckets_[bucket_id].is_empty = false;
 
   properties_.num_entries++;
 
-  // We assume that the keys are inserted in sorted order. To identify an
-  // unused key, which will be used in filling empty buckets in the table,
-  // we try to find gaps between successive keys inserted. This is done by
-  // maintaining the previous key and comparing it with next key.
-  if (unused_user_key_.empty()) {
-    if (prev_key_.empty()) {
-      prev_key_ = user_key.ToString();
-      return;
-    }
-    std::string new_user_key = prev_key_;
+  // We assume that the keys are inserted in sorted order as determined by
+  // Byte-wise comparator. To identify an unused key, which will be used in
+  // filling empty buckets in the table, we try to find gaps between successive
+  // keys inserted (ie, latest key and previous in kvs_).
+  if (unused_user_key_.empty() && kvs_.size() > 1) {
+    std::string prev_key = is_last_level_file_ ? kvs_[kvs_.size()-1].first
+      : ExtractUserKey(kvs_[kvs_.size()-1].first).ToString();
+    std::string new_user_key = prev_key;
     new_user_key.back()++;
     // We ignore carry-overs and check that it is larger than previous key.
-    if ((new_user_key > prev_key_) &&
-        (new_user_key < user_key.ToString())) {
+    if (Slice(new_user_key).compare(Slice(prev_key)) > 0 &&
+        Slice(new_user_key).compare(ikey.user_key) < 0) {
       unused_user_key_ = new_user_key;
-    } else {
-      prev_key_ = user_key.ToString();
     }
   }
 }
 
-Status CuckooTableBuilder::status() const { return status_; }
+Status CuckooTableBuilder::MakeHashTable(std::vector<CuckooBucket>* buckets) {
+  uint64_t num_buckets = kvs_.size() / hash_table_ratio_;
+  buckets->resize(num_buckets);
+  uint64_t make_space_for_key_call_id = 0;
+  for (uint32_t vector_idx = 0; vector_idx < kvs_.size(); vector_idx++) {
+    uint64_t bucket_id;
+    bool bucket_found = false;
+    autovector<uint64_t> hash_vals;
+    Slice user_key = is_last_level_file_ ? kvs_[vector_idx].first :
+      ExtractUserKey(kvs_[vector_idx].first);
+    for (uint32_t hash_cnt = 0; hash_cnt < num_hash_table_; ++hash_cnt) {
+      uint64_t hash_val = get_slice_hash_(user_key, hash_cnt, num_buckets);
+      if ((*buckets)[hash_val].vector_idx == kMaxVectorIdx) {
+        bucket_id = hash_val;
+        bucket_found = true;
+        break;
+      } else {
+        if (user_key.compare(is_last_level_file_
+              ? Slice(kvs_[(*buckets)[hash_val].vector_idx].first)
+              : ExtractUserKey(
+                kvs_[(*buckets)[hash_val].vector_idx].first)) == 0) {
+          return Status::NotSupported("Same key is being inserted again.");
+        }
+        hash_vals.push_back(hash_val);
+      }
+    }
+    while (!bucket_found && !MakeSpaceForKey(hash_vals,
+          ++make_space_for_key_call_id, buckets, &bucket_id)) {
+      // Rehash by increashing number of hash tables.
+      if (num_hash_table_ >= max_num_hash_table_) {
+        return Status::NotSupported("Too many collissions. Unable to hash.");
+      }
+      // We don't really need to rehash the entire table because old hashes are
+      // still valid and we only increased the number of hash functions.
+      uint64_t hash_val = get_slice_hash_(user_key,
+          num_hash_table_, num_buckets);
+      ++num_hash_table_;
+      if ((*buckets)[hash_val].vector_idx == kMaxVectorIdx) {
+        bucket_found = true;
+        bucket_id = hash_val;
+        break;
+      } else {
+        hash_vals.push_back(hash_val);
+      }
+    }
+    (*buckets)[bucket_id].vector_idx = vector_idx;
+  }
+  return Status::OK();
+}
 
 Status CuckooTableBuilder::Finish() {
   assert(!closed_);
   closed_ = true;
-
-  if (unused_user_key_.empty()) {
-    if (prev_key_.empty()) {
-      return Status::Corruption("Unable to find unused key");
-    }
-    // Try to find the key next to prev_key_ by handling carryovers.
-    std::string new_user_key = prev_key_;
+  std::vector<CuckooBucket> buckets;
+  Status s = MakeHashTable(&buckets);
+  if (!s.ok()) {
+    return s;
+  }
+  if (unused_user_key_.empty() && !kvs_.empty()) {
+    // Try to find the key next to last key by handling carryovers.
+    std::string last_key =
+      is_last_level_file_ ? kvs_[kvs_.size()-1].first
+      : ExtractUserKey(kvs_[kvs_.size()-1].first).ToString();
+    std::string new_user_key = last_key;
     int curr_pos = new_user_key.size() - 1;
     while (curr_pos >= 0) {
       ++new_user_key[curr_pos];
-      if (new_user_key > prev_key_) {
+      if (new_user_key > last_key) {
         unused_user_key_ = new_user_key;
         break;
       }
@@ -171,29 +180,32 @@ Status CuckooTableBuilder::Finish() {
     }
   }
   std::string unused_bucket;
-  if (is_last_level_file_) {
-    unused_bucket = unused_user_key_;
-  } else {
-    ParsedInternalKey ikey(unused_user_key_, 0, kTypeValue);
-    AppendInternalKey(&unused_bucket, ikey);
+  if (!kvs_.empty()) {
+    if (is_last_level_file_) {
+      unused_bucket = unused_user_key_;
+    } else {
+      ParsedInternalKey ikey(unused_user_key_, 0, kTypeValue);
+      AppendInternalKey(&unused_bucket, ikey);
+    }
   }
   properties_.fixed_key_len = unused_bucket.size();
+  uint32_t value_length = kvs_.empty() ? 0 : kvs_[0].second.size();
+  uint32_t bucket_size = value_length + properties_.fixed_key_len;
   properties_.user_collected_properties[
         CuckooTablePropertyNames::kValueLength].assign(
-        reinterpret_cast<const char*>(&value_length_), sizeof(value_length_));
+        reinterpret_cast<const char*>(&value_length), sizeof(value_length));
 
-  unused_bucket.resize(bucket_size_, 'a');
+  unused_bucket.resize(bucket_size, 'a');
   // Write the table.
   uint32_t num_added = 0;
-  for (auto& bucket : buckets_) {
-    Status s;
-    if (bucket.is_empty) {
+  for (auto& bucket : buckets) {
+    if (bucket.vector_idx == kMaxVectorIdx) {
       s = file_->Append(Slice(unused_bucket));
     } else {
       ++num_added;
-      s = file_->Append(Slice(bucket.key));
+      s = file_->Append(kvs_[bucket.vector_idx].first);
       if (s.ok()) {
-        s = file_->Append(Slice(bucket.value));
+        s = file_->Append(kvs_[bucket.vector_idx].second);
       }
     }
     if (!s.ok()) {
@@ -202,17 +214,17 @@ Status CuckooTableBuilder::Finish() {
   }
   assert(num_added == NumEntries());
 
-  uint64_t offset = buckets_.size() * bucket_size_;
+  uint64_t offset = buckets.size() * bucket_size;
   unused_bucket.resize(properties_.fixed_key_len);
   properties_.user_collected_properties[
     CuckooTablePropertyNames::kEmptyKey] = unused_bucket;
   properties_.user_collected_properties[
     CuckooTablePropertyNames::kNumHashTable].assign(
         reinterpret_cast<char*>(&num_hash_table_), sizeof(num_hash_table_));
+  uint64_t num_buckets = buckets.size();
   properties_.user_collected_properties[
     CuckooTablePropertyNames::kMaxNumBuckets].assign(
-        reinterpret_cast<const char*>(&max_num_buckets_),
-        sizeof(max_num_buckets_));
+        reinterpret_cast<const char*>(&num_buckets), sizeof(num_buckets));
   properties_.user_collected_properties[
     CuckooTablePropertyNames::kIsLastLevel].assign(
         reinterpret_cast<const char*>(&is_last_level_file_),
@@ -228,7 +240,7 @@ Status CuckooTableBuilder::Finish() {
   BlockHandle property_block_handle;
   property_block_handle.set_offset(offset);
   property_block_handle.set_size(property_block.size());
-  Status s = file_->Append(property_block);
+  s = file_->Append(property_block);
   offset += property_block.size();
   if (!s.ok()) {
     return s;
@@ -266,16 +278,31 @@ uint64_t CuckooTableBuilder::NumEntries() const {
 uint64_t CuckooTableBuilder::FileSize() const {
   if (closed_) {
     return file_->GetFileSize();
-  } else {
-    // This is not the actual size of the file as we need to account for
-    // hash table ratio. This returns the size of filled buckets in the table
-    // scaled up by a factor of 1/hash table ratio.
-    return (properties_.num_entries * bucket_size_) / hash_table_ratio_;
+  } else if (properties_.num_entries == 0) {
+    return 0;
   }
+  // This is not the actual size of the file as we need to account for
+  // hash table ratio. This returns the size of filled buckets in the table
+  // scaled up by a factor of 1/hash_table_ratio.
+  return ((kvs_[0].first.size() + kvs_[0].second.size()) *
+      properties_.num_entries) / hash_table_ratio_;
 }
 
-bool CuckooTableBuilder::MakeSpaceForKey(const Slice& key,
-    uint64_t *bucket_id, autovector<uint64_t> hash_vals) {
+// This method is invoked when there is no place to insert the target key.
+// It searches for a set of elements that can be moved to accommodate target
+// key. The search is a BFS graph traversal with first level (hash_vals)
+// being all the buckets target key could go to.
+// Then, from each node (curr_node), we find all the buckets that curr_node
+// could go to. They form the children of curr_node in the tree.
+// We continue the traversal until we find an empty bucket, in which case, we
+// move all elements along the path from first level to this empty bucket, to
+// make space for target key which is inserted at first level (*bucket_id).
+// If tree depth exceedes max depth, we return false indicating failure.
+bool CuckooTableBuilder::MakeSpaceForKey(
+    const autovector<uint64_t>& hash_vals,
+    const uint64_t make_space_for_key_call_id,
+    std::vector<CuckooBucket>* buckets,
+    uint64_t* bucket_id) {
   struct CuckooNode {
     uint64_t bucket_id;
     uint32_t depth;
@@ -292,13 +319,11 @@ bool CuckooTableBuilder::MakeSpaceForKey(const Slice& key,
   // unique id for this invocation of the method. We store this number into
   // the nodes that we explore in current method call.
   // It is unlikely for the increment operation to overflow because the maximum
-  // number of times this will be called is <= max_num_hash_table_ +
-  // max_num_buckets_.
-  ++make_space_for_key_call_id_;
+  // no. of times this will be called is <= max_num_hash_table_ + kvs_.size().
   for (uint32_t hash_cnt = 0; hash_cnt < num_hash_table_; ++hash_cnt) {
     uint64_t bucket_id = hash_vals[hash_cnt];
-    buckets_[bucket_id].make_space_for_key_call_id =
-      make_space_for_key_call_id_;
+    (*buckets)[bucket_id].make_space_for_key_call_id =
+      make_space_for_key_call_id;
     tree.push_back(CuckooNode(bucket_id, 0, 0));
   }
   bool null_found = false;
@@ -308,21 +333,21 @@ bool CuckooTableBuilder::MakeSpaceForKey(const Slice& key,
     if (curr_node.depth >= max_search_depth_) {
       break;
     }
-    CuckooBucket& curr_bucket = buckets_[curr_node.bucket_id];
+    CuckooBucket& curr_bucket = (*buckets)[curr_node.bucket_id];
     for (uint32_t hash_cnt = 0; hash_cnt < num_hash_table_; ++hash_cnt) {
-      uint64_t child_bucket_id = GetSliceHash(
-          is_last_level_file_ ? curr_bucket.key
-          : ExtractUserKey(Slice(curr_bucket.key)),
-          hash_cnt, max_num_buckets_);
-      if (buckets_[child_bucket_id].make_space_for_key_call_id ==
-          make_space_for_key_call_id_) {
+      uint64_t child_bucket_id = get_slice_hash_(
+          is_last_level_file_ ? kvs_[curr_bucket.vector_idx].first
+          : ExtractUserKey(Slice(kvs_[curr_bucket.vector_idx].first)),
+          hash_cnt, buckets->size());
+      if ((*buckets)[child_bucket_id].make_space_for_key_call_id ==
+          make_space_for_key_call_id) {
         continue;
       }
-      buckets_[child_bucket_id].make_space_for_key_call_id =
-        make_space_for_key_call_id_;
+      (*buckets)[child_bucket_id].make_space_for_key_call_id =
+        make_space_for_key_call_id;
       tree.push_back(CuckooNode(child_bucket_id, curr_node.depth + 1,
             curr_pos));
-      if (buckets_[child_bucket_id].is_empty) {
+      if ((*buckets)[child_bucket_id].vector_idx == kMaxVectorIdx) {
         null_found = true;
         break;
       }
@@ -331,23 +356,21 @@ bool CuckooTableBuilder::MakeSpaceForKey(const Slice& key,
   }
 
   if (null_found) {
+    // There is an empty node in tree.back(). Now, traverse the path from this
+    // empty node to top of the tree and at every node in the path, replace
+    // child with the parent. Stop when first level is reached in the tree
+    // (happens when 0 <= bucket_to_replace_pos < num_hash_table_) and return
+    // this location in first level for target key to be inserted.
     uint32_t bucket_to_replace_pos = tree.size()-1;
-    while (bucket_to_replace_pos >= 0) {
+    while (bucket_to_replace_pos >= num_hash_table_) {
       CuckooNode& curr_node = tree[bucket_to_replace_pos];
-      if (bucket_to_replace_pos >= num_hash_table_) {
-        buckets_[curr_node.bucket_id] =
-          buckets_[tree[curr_node.parent_pos].bucket_id];
-        bucket_to_replace_pos = curr_node.parent_pos;
-      } else {
-        *bucket_id = curr_node.bucket_id;
-        return true;
-      }
+      (*buckets)[curr_node.bucket_id] =
+        (*buckets)[tree[curr_node.parent_pos].bucket_id];
+      bucket_to_replace_pos = curr_node.parent_pos;
     }
-    assert(false);
-    return true;
-  } else {
-    return false;
+    *bucket_id = tree[bucket_to_replace_pos].bucket_id;
   }
+  return null_found;
 }
 
 }  // namespace rocksdb

table/cuckoo_table_builder.h

@@ -6,7 +6,9 @@
 #pragma once
 #ifndef ROCKSDB_LITE
 #include <stdint.h>
+#include <limits>
 #include <string>
+#include <utility>
 #include <vector>
 #include "rocksdb/status.h"
 #include "table/table_builder.h"
@@ -19,14 +21,12 @@ namespace rocksdb {
 class CuckooTableBuilder: public TableBuilder {
  public:
   CuckooTableBuilder(
-      WritableFile* file, uint32_t fixed_key_length,
-      uint32_t fixed_value_length, double hash_table_ratio,
-      uint64_t file_size, uint32_t max_num_hash_table,
-      uint32_t max_search_depth, bool is_last_level,
-      uint64_t (*GetSliceHash)(const Slice&, uint32_t, uint64_t));
+      WritableFile* file, double hash_table_ratio, uint32_t max_num_hash_table,
+      uint32_t max_search_depth,
+      uint64_t (*get_slice_hash)(const Slice&, uint32_t, uint64_t));
 
   // REQUIRES: Either Finish() or Abandon() has been called.
-  ~CuckooTableBuilder();
+  ~CuckooTableBuilder() {}
 
   // Add key,value to the table being constructed.
   // REQUIRES: key is after any previously added key according to comparator.
@@ -34,7 +34,7 @@ class CuckooTableBuilder: public TableBuilder {
   void Add(const Slice& key, const Slice& value) override;
 
   // Return non-ok iff some error has been detected.
-  Status status() const override;
+  Status status() const override { return status_; }
 
   // Finish building the table.  Stops using the file passed to the
   // constructor after this function returns.
@@ -57,35 +57,37 @@ class CuckooTableBuilder: public TableBuilder {
 
  private:
   struct CuckooBucket {
-    CuckooBucket(): is_empty(true), make_space_for_key_call_id(0) {}
-    std::string key;
-    std::string value;
-    bool is_empty;
-    uint64_t make_space_for_key_call_id;
+    CuckooBucket()
+      : vector_idx(kMaxVectorIdx), make_space_for_key_call_id(0) {}
+    uint32_t vector_idx;
+    // This number will not exceed kvs_.size() + max_num_hash_table_.
+    // We assume number of items is <= 2^32.
+    uint32_t make_space_for_key_call_id;
   };
+  static const uint32_t kMaxVectorIdx = std::numeric_limits<int32_t>::max();
 
-  bool MakeSpaceForKey(const Slice& key, uint64_t* bucket_id,
-      autovector<uint64_t> hash_vals);
+  bool MakeSpaceForKey(
+      const autovector<uint64_t>& hash_vals,
+      const uint64_t call_id,
+      std::vector<CuckooBucket>* buckets,
+      uint64_t* bucket_id);
+  Status MakeHashTable(std::vector<CuckooBucket>* buckets);
 
   uint32_t num_hash_table_;
   WritableFile* file_;
-  const uint32_t value_length_;
-  const uint32_t bucket_size_;
   const double hash_table_ratio_;
-  const uint64_t max_num_buckets_;
   const uint32_t max_num_hash_table_;
   const uint32_t max_search_depth_;
-  const bool is_last_level_file_;
+  bool is_last_level_file_;
   Status status_;
-  std::vector<CuckooBucket> buckets_;
+  std::vector<std::pair<std::string, std::string>> kvs_;
   TableProperties properties_;
-  uint64_t make_space_for_key_call_id_;
-  uint64_t (*GetSliceHash)(const Slice& s, uint32_t index,
+  bool has_seen_first_key_;
+  uint64_t (*get_slice_hash_)(const Slice& s, uint32_t index,
     uint64_t max_num_buckets);
   std::string unused_user_key_ = "";
-  std::string prev_key_;
 
-  bool closed_ = false;  // Either Finish() or Abandon() has been called.
+  bool closed_;  // Either Finish() or Abandon() has been called.
 
   // No copying allowed
   CuckooTableBuilder(const CuckooTableBuilder&) = delete;

table/cuckoo_table_builder_test.cc

@@ -10,26 +10,15 @@
 
 #include "table/meta_blocks.h"
 #include "table/cuckoo_table_builder.h"
-#include "util/random.h"
 #include "util/testharness.h"
 #include "util/testutil.h"
 
 namespace rocksdb {
-
 extern const uint64_t kCuckooTableMagicNumber;
 
 namespace {
 std::unordered_map<std::string, std::vector<uint64_t>> hash_map;
 
-void AddHashLookups(const std::string& s, uint64_t bucket_id,
-    uint32_t num_hash_fun) {
-  std::vector<uint64_t> v;
-  for (uint32_t i = 0; i < num_hash_fun; i++) {
-    v.push_back(bucket_id + i);
-  }
-  hash_map[s] = v;
-}
-
 uint64_t GetSliceHash(const Slice& s, uint32_t index,
     uint64_t max_num_buckets) {
   return hash_map[s.ToString()][index];
@@ -40,9 +29,14 @@ class CuckooBuilderTest {
  public:
   CuckooBuilderTest() {
     env_ = Env::Default();
+    Options options;
+    options.allow_mmap_reads = true;
+    env_options_ = EnvOptions(options);
   }
 
-  void CheckFileContents(const std::string& expected_data,
+  void CheckFileContents(const std::vector<std::string>& keys,
+      const std::vector<std::string>& values,
+      const std::vector<uint64_t>& expected_locations,
       std::string expected_unused_bucket, uint64_t expected_max_buckets,
       uint32_t expected_num_hash_fun, bool expected_is_last_level) {
     // Read file
@@ -55,18 +49,19 @@ class CuckooBuilderTest {
     TableProperties* props = nullptr;
     ASSERT_OK(ReadTableProperties(read_file.get(), read_file_size,
           kCuckooTableMagicNumber, env_, nullptr, &props));
-    ASSERT_EQ(props->num_entries, num_items);
-    ASSERT_EQ(props->fixed_key_len, key_length);
+    ASSERT_EQ(props->num_entries, keys.size());
+    ASSERT_EQ(props->fixed_key_len, keys.empty() ? 0 : keys[0].size());
 
     // Check unused bucket.
     std::string unused_key = props->user_collected_properties[
       CuckooTablePropertyNames::kEmptyKey];
-    ASSERT_EQ(expected_unused_bucket.substr(0, key_length), unused_key);
+    ASSERT_EQ(expected_unused_bucket.substr(0,
+          props->fixed_key_len), unused_key);
 
     uint32_t value_len_found =
       *reinterpret_cast<const uint32_t*>(props->user_collected_properties[
                 CuckooTablePropertyNames::kValueLength].data());
-    ASSERT_EQ(value_length, value_len_found);
+    ASSERT_EQ(values.empty() ? 0 : values[0].size(), value_len_found);
     const uint64_t max_buckets =
       *reinterpret_cast<const uint64_t*>(props->user_collected_properties[
                 CuckooTablePropertyNames::kMaxNumBuckets].data());
@@ -80,411 +75,322 @@ class CuckooBuilderTest {
                 CuckooTablePropertyNames::kIsLastLevel].data());
     ASSERT_EQ(expected_is_last_level, is_last_level_found);
     delete props;
+
     // Check contents of the bucket.
-    std::string read_data;
-    read_data.resize(expected_data.size());
-    Slice read_slice;
-    ASSERT_OK(read_file->Read(0, expected_data.size(),
-          &read_slice, &read_data[0]));
-    ASSERT_EQ(expected_data, read_data);
+    std::vector<bool> keys_found(keys.size(), false);
+    uint32_t bucket_size = expected_unused_bucket.size();
+    for (uint32_t i = 0; i < max_buckets; ++i) {
+      Slice read_slice;
+      ASSERT_OK(read_file->Read(i*bucket_size, bucket_size,
+            &read_slice, nullptr));
+      uint32_t key_idx = std::find(expected_locations.begin(),
+          expected_locations.end(), i) - expected_locations.begin();
+      if (key_idx == keys.size()) {
+        // i is not one of the expected locaitons. Empty bucket.
+        ASSERT_EQ(read_slice.compare(expected_unused_bucket), 0);
+      } else {
+        keys_found[key_idx] = true;
+        ASSERT_EQ(read_slice.compare(keys[key_idx] + values[key_idx]), 0);
+      }
+    }
+    for (auto key_found : keys_found) {
+      // Check that all keys were found.
+      ASSERT_TRUE(key_found);
+    }
+  }
+
+  std::string GetInternalKey(Slice user_key, bool zero_seqno) {
+    IterKey ikey;
+    ikey.SetInternalKey(user_key, zero_seqno ? 0 : 1000, kTypeValue);
+    return ikey.GetKey().ToString();
   }
 
   Env* env_;
-  const EnvOptions env_options_;
+  EnvOptions env_options_;
   std::string fname;
-  uint64_t file_size = 100000;
-  uint32_t num_items = 20;
-  uint32_t num_hash_fun = 64;
-  double hash_table_ratio = 0.9;
-  uint32_t ikey_length;
-  uint32_t user_key_length;
-  uint32_t key_length;
-  uint32_t value_length;
-  uint32_t bucket_length;
+  const double kHashTableRatio = 0.9;
 };
 
-TEST(CuckooBuilderTest, NoCollision) {
-  hash_map.clear();
-  uint32_t expected_num_hash_fun = 2;
-  std::vector<std::string> user_keys(num_items);
-  std::vector<std::string> keys(num_items);
-  std::vector<std::string> values(num_items);
-  uint64_t bucket_ids = 0;
-  for (uint32_t i = 0; i < num_items; i++) {
-    user_keys[i] = "keys" + std::to_string(i+100);
-    ParsedInternalKey ikey(user_keys[i], i + 1000, kTypeValue);
-    AppendInternalKey(&keys[i], ikey);
-    values[i] = "value" + std::to_string(i+100);
-    AddHashLookups(user_keys[i], bucket_ids, num_hash_fun);
-    bucket_ids += num_hash_fun;
-  }
-
-  ikey_length = keys[0].size();
-  key_length = ikey_length;
-  value_length = values[0].size();
-  bucket_length = ikey_length + value_length;
-  uint64_t expected_max_buckets = file_size / bucket_length;
-  std::string expected_unused_user_key = "keys10:";
-  ParsedInternalKey ikey(expected_unused_user_key, 0, kTypeValue);
-  std::string expected_unused_bucket;
-  AppendInternalKey(&expected_unused_bucket, ikey);
-  expected_unused_bucket.resize(bucket_length, 'a');
+TEST(CuckooBuilderTest, SuccessWithEmptyFile) {
   unique_ptr<WritableFile> writable_file;
-  fname = test::TmpDir() + "/BasicTest_writable_file";
+  fname = test::TmpDir() + "/NoCollisionFullKey";
   ASSERT_OK(env_->NewWritableFile(fname, &writable_file, env_options_));
-  CuckooTableBuilder cuckoo_builder(
-      writable_file.get(), ikey_length,
-      value_length, hash_table_ratio,
-      file_size, num_hash_fun, 100, false, GetSliceHash);
-  ASSERT_OK(cuckoo_builder.status());
-  uint32_t key_idx = 0;
-  std::string expected_file_data = "";
-  for (uint32_t i = 0; i < expected_max_buckets; i++) {
-    if (key_idx * num_hash_fun == i && key_idx < num_items) {
-      cuckoo_builder.Add(Slice(keys[key_idx]), Slice(values[key_idx]));
-      ASSERT_EQ(cuckoo_builder.NumEntries(), key_idx + 1);
-      ASSERT_OK(cuckoo_builder.status());
-      expected_file_data.append(keys[key_idx] + values[key_idx]);
-      ++key_idx;
-    } else {
-      expected_file_data.append(expected_unused_bucket);
-    }
-  }
-  ASSERT_OK(cuckoo_builder.Finish());
-  writable_file->Close();
-  CheckFileContents(expected_file_data, expected_unused_bucket,
-      expected_max_buckets, expected_num_hash_fun, false);
+  CuckooTableBuilder builder(writable_file.get(), kHashTableRatio,
+      4, 100, GetSliceHash);
+  ASSERT_OK(builder.status());
+  ASSERT_OK(builder.Finish());
+  ASSERT_OK(writable_file->Close());
+  CheckFileContents({}, {}, {}, "", 0, 2, false);
 }
 
-TEST(CuckooBuilderTest, NoCollisionLastLevel) {
-  hash_map.clear();
-  uint32_t expected_num_hash_fun = 2;
-  std::vector<std::string> user_keys(num_items);
-  std::vector<std::string> keys(num_items);
-  std::vector<std::string> values(num_items);
-  uint64_t bucket_ids = 0;
-  for (uint32_t i = 0; i < num_items; i++) {
-    user_keys[i] = "keys" + std::to_string(i+100);
-    // Set zero sequence number in all keys.
-    ParsedInternalKey ikey(user_keys[i], 0, kTypeValue);
-    AppendInternalKey(&keys[i], ikey);
-    values[i] = "value" + std::to_string(i+100);
-    AddHashLookups(user_keys[i], bucket_ids, num_hash_fun);
-    bucket_ids += num_hash_fun;
+TEST(CuckooBuilderTest, WriteSuccessNoCollisionFullKey) {
+  uint32_t num_hash_fun = 4;
+  std::vector<std::string> user_keys = {"key01", "key02", "key03", "key04"};
+  std::vector<std::string> values = {"v01", "v02", "v03", "v04"};
+  hash_map = {
+    {user_keys[0], {0, 1, 2, 3}},
+    {user_keys[1], {1, 2, 3, 4}},
+    {user_keys[2], {2, 3, 4, 5}},
+    {user_keys[3], {3, 4, 5, 6}}
+  };
+  std::vector<uint64_t> expected_locations = {0, 1, 2, 3};
+  std::vector<std::string> keys;
+  for (auto& user_key : user_keys) {
+    keys.push_back(GetInternalKey(user_key, false));
   }
-  ikey_length = keys[0].size();
-  user_key_length = user_keys[0].size();
-  key_length = user_key_length;
-  value_length = values[0].size();
-  bucket_length = key_length + value_length;
-  uint64_t expected_max_buckets = file_size / bucket_length;
-  std::string expected_unused_bucket = "keys10:";
-  expected_unused_bucket.resize(bucket_length, 'a');
+
   unique_ptr<WritableFile> writable_file;
-  fname = test::TmpDir() + "/NoCollisionLastLevel_writable_file";
+  fname = test::TmpDir() + "/NoCollisionFullKey";
   ASSERT_OK(env_->NewWritableFile(fname, &writable_file, env_options_));
-  CuckooTableBuilder cuckoo_builder(
-      writable_file.get(), ikey_length,
-      value_length, hash_table_ratio,
-      file_size, num_hash_fun, 100, true, GetSliceHash);
-  ASSERT_OK(cuckoo_builder.status());
-  uint32_t key_idx = 0;
-  std::string expected_file_data = "";
-  for (uint32_t i = 0; i < expected_max_buckets; i++) {
-    if (key_idx * num_hash_fun == i && key_idx < num_items) {
-      cuckoo_builder.Add(Slice(keys[key_idx]), Slice(values[key_idx]));
-      ASSERT_EQ(cuckoo_builder.NumEntries(), key_idx + 1);
-      ASSERT_OK(cuckoo_builder.status());
-      expected_file_data.append(user_keys[key_idx] + values[key_idx]);
-      ++key_idx;
-    } else {
-      expected_file_data.append(expected_unused_bucket);
-    }
+  CuckooTableBuilder builder(writable_file.get(), kHashTableRatio,
+      num_hash_fun, 100, GetSliceHash);
+  ASSERT_OK(builder.status());
+  for (uint32_t i = 0; i < user_keys.size(); i++) {
+    builder.Add(Slice(keys[i]), Slice(values[i]));
+    ASSERT_EQ(builder.NumEntries(), i + 1);
+    ASSERT_OK(builder.status());
   }
-  ASSERT_OK(cuckoo_builder.Finish());
-  writable_file->Close();
-  CheckFileContents(expected_file_data, expected_unused_bucket,
-      expected_max_buckets, expected_num_hash_fun, true);
+  ASSERT_OK(builder.Finish());
+  ASSERT_OK(writable_file->Close());
+
+  uint32_t expected_max_buckets = keys.size() / kHashTableRatio;
+  std::string expected_unused_bucket = GetInternalKey("key05", true);
+  expected_unused_bucket += std::string(values[0].size(), 'a');
+  CheckFileContents(keys, values, expected_locations,
+      expected_unused_bucket, expected_max_buckets, 2, false);
 }
 
-TEST(CuckooBuilderTest, WithCollision) {
-  // Take keys with colliding hash function values.
-  hash_map.clear();
-  num_hash_fun = 20;
-  num_items = num_hash_fun;
-  uint32_t expected_num_hash_fun = num_hash_fun;
-  std::vector<std::string> user_keys(num_items);
-  std::vector<std::string> keys(num_items);
-  std::vector<std::string> values(num_items);
-  for (uint32_t i = 0; i < num_items; i++) {
-    user_keys[i] = "keys" + std::to_string(i+100);
-    ParsedInternalKey ikey(user_keys[i], i + 1000, kTypeValue);
-    AppendInternalKey(&keys[i], ikey);
-    values[i] = "value" + std::to_string(i+100);
-    // Make all hash values collide.
-    AddHashLookups(user_keys[i], 0, num_hash_fun);
+TEST(CuckooBuilderTest, WriteSuccessWithCollisionFullKey) {
+  uint32_t num_hash_fun = 4;
+  std::vector<std::string> user_keys = {"key01", "key02", "key03", "key04"};
+  std::vector<std::string> values = {"v01", "v02", "v03", "v04"};
+  hash_map = {
+    {user_keys[0], {0, 1, 2, 3}},
+    {user_keys[1], {0, 1, 2, 3}},
+    {user_keys[2], {0, 1, 2, 3}},
+    {user_keys[3], {0, 1, 2, 3}},
+  };
+  std::vector<uint64_t> expected_locations = {0, 1, 2, 3};
+  std::vector<std::string> keys;
+  for (auto& user_key : user_keys) {
+    keys.push_back(GetInternalKey(user_key, false));
   }
-  ikey_length = keys[0].size();
-  value_length = values[0].size();
-  key_length = ikey_length;
-  bucket_length = key_length + value_length;
-  uint64_t expected_max_buckets = file_size / bucket_length;
-  std::string expected_unused_user_key = "keys10:";
-  ParsedInternalKey ikey(expected_unused_user_key, 0, kTypeValue);
-  std::string expected_unused_bucket;
-  AppendInternalKey(&expected_unused_bucket, ikey);
-  expected_unused_bucket.resize(bucket_length, 'a');
+
   unique_ptr<WritableFile> writable_file;
-  fname = test::TmpDir() + "/WithCollision_writable_file";
+  fname = test::TmpDir() + "/WithCollisionFullKey";
   ASSERT_OK(env_->NewWritableFile(fname, &writable_file, env_options_));
-  CuckooTableBuilder cuckoo_builder(
-      writable_file.get(), key_length, value_length, hash_table_ratio,
-      file_size, num_hash_fun, 100, false, GetSliceHash);
-  ASSERT_OK(cuckoo_builder.status());
-  uint32_t key_idx = 0;
-  std::string expected_file_data = "";
-  for (uint32_t i = 0; i < expected_max_buckets; i++) {
-    if (key_idx  == i && key_idx < num_items) {
-      cuckoo_builder.Add(Slice(keys[key_idx]), Slice(values[key_idx]));
-      ASSERT_EQ(cuckoo_builder.NumEntries(), key_idx + 1);
-      ASSERT_OK(cuckoo_builder.status());
-      expected_file_data.append(keys[key_idx] + values[key_idx]);
-      ++key_idx;
-    } else {
-      expected_file_data.append(expected_unused_bucket);
-    }
+  CuckooTableBuilder builder(writable_file.get(), kHashTableRatio,
+      num_hash_fun, 100, GetSliceHash);
+  ASSERT_OK(builder.status());
+  for (uint32_t i = 0; i < user_keys.size(); i++) {
+    builder.Add(Slice(keys[i]), Slice(values[i]));
+    ASSERT_EQ(builder.NumEntries(), i + 1);
+    ASSERT_OK(builder.status());
   }
-  ASSERT_OK(cuckoo_builder.Finish());
-  writable_file->Close();
-  CheckFileContents(expected_file_data, expected_unused_bucket,
-      expected_max_buckets, expected_num_hash_fun, false);
+  ASSERT_OK(builder.Finish());
+  ASSERT_OK(writable_file->Close());
+
+  uint32_t expected_max_buckets = keys.size() / kHashTableRatio;
+  std::string expected_unused_bucket = GetInternalKey("key05", true);
+  expected_unused_bucket += std::string(values[0].size(), 'a');
+  CheckFileContents(keys, values, expected_locations,
+      expected_unused_bucket, expected_max_buckets, 4, false);
 }
 
-TEST(CuckooBuilderTest, FailWithTooManyCollisions) {
-  // Take keys with colliding hash function values.
-  // Take more keys than the number of hash functions.
-  hash_map.clear();
-  num_hash_fun = 20;
-  num_items = num_hash_fun + 1;
-  std::vector<std::string> user_keys(num_items);
-  std::vector<std::string> keys(num_items);
-  std::vector<std::string> values(num_items);
-  for (uint32_t i = 0; i < num_items; i++) {
-    user_keys[i] = "keys" + std::to_string(i+100);
-    ParsedInternalKey ikey(user_keys[i], i + 1000, kTypeValue);
-    AppendInternalKey(&keys[i], ikey);
-    values[i] = "value" + std::to_string(i+100);
-    // Make all hash values collide.
-    AddHashLookups(user_keys[i], 0, num_hash_fun);
-  }
-  ikey_length = keys[0].size();
-  value_length = values[0].size();
-  unique_ptr<WritableFile> writable_file;
-  fname = test::TmpDir() + "/FailWithTooManyCollisions_writable";
-  ASSERT_OK(env_->NewWritableFile(fname, &writable_file, env_options_));
-  CuckooTableBuilder cuckoo_builder(
-      writable_file.get(), ikey_length,
-      value_length, hash_table_ratio, file_size, num_hash_fun,
-      100, false, GetSliceHash);
-  ASSERT_OK(cuckoo_builder.status());
-  for (uint32_t key_idx = 0; key_idx < num_items-1; key_idx++) {
-    cuckoo_builder.Add(Slice(keys[key_idx]), Slice(values[key_idx]));
-    ASSERT_OK(cuckoo_builder.status());
-    ASSERT_EQ(cuckoo_builder.NumEntries(), key_idx + 1);
-  }
-  cuckoo_builder.Add(Slice(keys.back()), Slice(values.back()));
-  ASSERT_TRUE(cuckoo_builder.status().IsCorruption());
-  cuckoo_builder.Abandon();
-  writable_file->Close();
-}
-
-TEST(CuckooBuilderTest, FailWhenSameKeyInserted) {
-  hash_map.clear();
-  std::string user_key = "repeatedkey";
-  AddHashLookups(user_key, 0, num_hash_fun);
-  std::string key_to_reuse1, key_to_reuse2;
-  ParsedInternalKey ikey1(user_key, 1000, kTypeValue);
-  ParsedInternalKey ikey2(user_key, 1001, kTypeValue);
-  AppendInternalKey(&key_to_reuse1, ikey1);
-  AppendInternalKey(&key_to_reuse2, ikey2);
-  std::string value = "value";
-  ikey_length = key_to_reuse1.size();
-  value_length = value.size();
-  unique_ptr<WritableFile> writable_file;
-  fname = test::TmpDir() + "/FailWhenSameKeyInserted_writable_file";
-  ASSERT_OK(env_->NewWritableFile(fname, &writable_file, env_options_));
-  CuckooTableBuilder cuckoo_builder(
-      writable_file.get(), ikey_length,
-      value_length, hash_table_ratio, file_size, num_hash_fun,
-      100, false, GetSliceHash);
-  ASSERT_OK(cuckoo_builder.status());
-  cuckoo_builder.Add(Slice(key_to_reuse1), Slice(value));
-  ASSERT_OK(cuckoo_builder.status());
-  ASSERT_EQ(cuckoo_builder.NumEntries(), 1U);
-  cuckoo_builder.Add(Slice(key_to_reuse2), Slice(value));
-  ASSERT_TRUE(cuckoo_builder.status().IsCorruption());
-  cuckoo_builder.Abandon();
-  writable_file->Close();
-}
-
-TEST(CuckooBuilderTest, WithACollisionPath) {
-  hash_map.clear();
+TEST(CuckooBuilderTest, WithCollisionPathFullKey) {
   // Have two hash functions. Insert elements with overlapping hashes.
   // Finally insert an element with hash value somewhere in the middle
   // so that it displaces all the elements after that.
-  num_hash_fun = 2;
-  uint32_t expected_num_hash_fun = num_hash_fun;
-  uint32_t max_search_depth = 100;
-  num_items = 2*max_search_depth + 2;
-  std::vector<std::string> user_keys(num_items);
-  std::vector<std::string> keys(num_items);
-  std::vector<std::string> values(num_items);
-  std::vector<uint64_t> expected_bucket_id(num_items);
-  for (uint32_t i = 0; i < num_items - 1; i++) {
-    user_keys[i] = "keys" + std::to_string(i+100);
-    ParsedInternalKey ikey(user_keys[i], i + 1000, kTypeValue);
-    AppendInternalKey(&keys[i], ikey);
-    values[i] = "value" + std::to_string(i+100);
-    // Make all hash values collide with the next element.
-    AddHashLookups(user_keys[i], i, num_hash_fun);
-    if (i <= max_search_depth) {
-      expected_bucket_id[i] = i;
-    } else {
-      expected_bucket_id[i] = i+1;
-    }
-  }
-  user_keys.back() = "keys" + std::to_string(num_items + 99);
-  ParsedInternalKey ikey(user_keys.back(), num_items + 1000, kTypeValue);
-  AppendInternalKey(&keys.back(), ikey);
-  values.back() = "value" + std::to_string(num_items+100);
-  // Make hash values collide with first and middle elements.
-  // Inserting at 0 will fail after exceeding search depth limit.
-  hash_map[user_keys.back()] = {0, max_search_depth + 1};
-  expected_bucket_id.back() = max_search_depth + 1;
-
-  ikey_length = keys[0].size();
-  value_length = values[0].size();
-  key_length = ikey_length;
-  bucket_length = key_length + value_length;
-
-  uint64_t expected_max_buckets = file_size / bucket_length;
-  std::string expected_unused_user_key = "keys10:";
-  ikey = ParsedInternalKey(expected_unused_user_key, 0, kTypeValue);
-  std::string expected_unused_bucket;
-  AppendInternalKey(&expected_unused_bucket, ikey);
-  expected_unused_bucket.resize(bucket_length, 'a');
-  std::string expected_file_data = "";
-  for (uint32_t i = 0; i < expected_max_buckets; i++) {
-    expected_file_data += expected_unused_bucket;
+  uint32_t num_hash_fun = 2;
+  std::vector<std::string> user_keys = {"key01", "key02", "key03",
+    "key04", "key05"};
+  std::vector<std::string> values = {"v01", "v02", "v03", "v04", "v05"};
+  hash_map = {
+    {user_keys[0], {0, 1}},
+    {user_keys[1], {1, 2}},
+    {user_keys[2], {2, 3}},
+    {user_keys[3], {3, 4}},
+    {user_keys[4], {0, 2}},
+  };
+  std::vector<uint64_t> expected_locations = {0, 1, 3, 4, 2};
+  std::vector<std::string> keys;
+  for (auto& user_key : user_keys) {
+    keys.push_back(GetInternalKey(user_key, false));
   }
 
   unique_ptr<WritableFile> writable_file;
-  fname = test::TmpDir() + "/WithCollisionPath_writable_file";
+  fname = test::TmpDir() + "/WithCollisionPathFullKey";
   ASSERT_OK(env_->NewWritableFile(fname, &writable_file, env_options_));
-  CuckooTableBuilder cuckoo_builder(
-      writable_file.get(), key_length,
-      value_length, hash_table_ratio, file_size,
-      num_hash_fun, max_search_depth, false, GetSliceHash);
-  ASSERT_OK(cuckoo_builder.status());
-  for (uint32_t key_idx = 0; key_idx < num_items; key_idx++) {
-    cuckoo_builder.Add(Slice(keys[key_idx]), Slice(values[key_idx]));
-    ASSERT_OK(cuckoo_builder.status());
-    ASSERT_EQ(cuckoo_builder.NumEntries(), key_idx + 1);
-    expected_file_data.replace(expected_bucket_id[key_idx]*bucket_length,
-        bucket_length, keys[key_idx] + values[key_idx]);
+  CuckooTableBuilder builder(writable_file.get(), kHashTableRatio,
+      num_hash_fun, 100, GetSliceHash);
+  ASSERT_OK(builder.status());
+  for (uint32_t i = 0; i < user_keys.size(); i++) {
+    builder.Add(Slice(keys[i]), Slice(values[i]));
+    ASSERT_EQ(builder.NumEntries(), i + 1);
+    ASSERT_OK(builder.status());
   }
-  ASSERT_OK(cuckoo_builder.Finish());
-  writable_file->Close();
-  CheckFileContents(expected_file_data, expected_unused_bucket,
-      expected_max_buckets, expected_num_hash_fun, false);
+  ASSERT_OK(builder.Finish());
+  ASSERT_OK(writable_file->Close());
+
+  uint32_t expected_max_buckets = keys.size() / kHashTableRatio;
+  std::string expected_unused_bucket = GetInternalKey("key06", true);
+  expected_unused_bucket += std::string(values[0].size(), 'a');
+  CheckFileContents(keys, values, expected_locations,
+      expected_unused_bucket, expected_max_buckets, 2, false);
+}
+
+TEST(CuckooBuilderTest, WriteSuccessNoCollisionUserKey) {
+  uint32_t num_hash_fun = 4;
+  std::vector<std::string> user_keys = {"key01", "key02", "key03", "key04"};
+  std::vector<std::string> values = {"v01", "v02", "v03", "v04"};
+  hash_map = {
+    {user_keys[0], {0, 1, 2, 3}},
+    {user_keys[1], {1, 2, 3, 4}},
+    {user_keys[2], {2, 3, 4, 5}},
+    {user_keys[3], {3, 4, 5, 6}}
+  };
+  std::vector<uint64_t> expected_locations = {0, 1, 2, 3};
+
+  unique_ptr<WritableFile> writable_file;
+  fname = test::TmpDir() + "/NoCollisionUserKey";
+  ASSERT_OK(env_->NewWritableFile(fname, &writable_file, env_options_));
+  CuckooTableBuilder builder(writable_file.get(), kHashTableRatio,
+      num_hash_fun, 100, GetSliceHash);
+  ASSERT_OK(builder.status());
+  for (uint32_t i = 0; i < user_keys.size(); i++) {
+    builder.Add(Slice(GetInternalKey(user_keys[i], true)), Slice(values[i]));
+    ASSERT_EQ(builder.NumEntries(), i + 1);
+    ASSERT_OK(builder.status());
+  }
+  ASSERT_OK(builder.Finish());
+  ASSERT_OK(writable_file->Close());
+
+  uint32_t expected_max_buckets = user_keys.size() / kHashTableRatio;
+  std::string expected_unused_bucket = "key05";
+  expected_unused_bucket += std::string(values[0].size(), 'a');
+  CheckFileContents(user_keys, values, expected_locations,
+      expected_unused_bucket, expected_max_buckets, 2, true);
+}
+
+TEST(CuckooBuilderTest, WriteSuccessWithCollisionUserKey) {
+  uint32_t num_hash_fun = 4;
+  std::vector<std::string> user_keys = {"key01", "key02", "key03", "key04"};
+  std::vector<std::string> values = {"v01", "v02", "v03", "v04"};
+  hash_map = {
+    {user_keys[0], {0, 1, 2, 3}},
+    {user_keys[1], {0, 1, 2, 3}},
+    {user_keys[2], {0, 1, 2, 3}},
+    {user_keys[3], {0, 1, 2, 3}},
+  };
+  std::vector<uint64_t> expected_locations = {0, 1, 2, 3};
+
+  unique_ptr<WritableFile> writable_file;
+  fname = test::TmpDir() + "/WithCollisionUserKey";
+  ASSERT_OK(env_->NewWritableFile(fname, &writable_file, env_options_));
+  CuckooTableBuilder builder(writable_file.get(), kHashTableRatio,
+      num_hash_fun, 100, GetSliceHash);
+  ASSERT_OK(builder.status());
+  for (uint32_t i = 0; i < user_keys.size(); i++) {
+    builder.Add(Slice(GetInternalKey(user_keys[i], true)), Slice(values[i]));
+    ASSERT_EQ(builder.NumEntries(), i + 1);
+    ASSERT_OK(builder.status());
+  }
+  ASSERT_OK(builder.Finish());
+  ASSERT_OK(writable_file->Close());
+
+  uint32_t expected_max_buckets = user_keys.size() / kHashTableRatio;
+  std::string expected_unused_bucket = "key05";
+  expected_unused_bucket += std::string(values[0].size(), 'a');
+  CheckFileContents(user_keys, values, expected_locations,
+      expected_unused_bucket, expected_max_buckets, 4, true);
+}
+
+TEST(CuckooBuilderTest, WithCollisionPathUserKey) {
+  uint32_t num_hash_fun = 2;
+  std::vector<std::string> user_keys = {"key01", "key02", "key03",
+    "key04", "key05"};
+  std::vector<std::string> values = {"v01", "v02", "v03", "v04", "v05"};
+  hash_map = {
+    {user_keys[0], {0, 1}},
+    {user_keys[1], {1, 2}},
+    {user_keys[2], {2, 3}},
+    {user_keys[3], {3, 4}},
+    {user_keys[4], {0, 2}},
+  };
+  std::vector<uint64_t> expected_locations = {0, 1, 3, 4, 2};
+
+  unique_ptr<WritableFile> writable_file;
+  fname = test::TmpDir() + "/WithCollisionPathUserKey";
+  ASSERT_OK(env_->NewWritableFile(fname, &writable_file, env_options_));
+  CuckooTableBuilder builder(writable_file.get(), kHashTableRatio,
+      num_hash_fun, 2, GetSliceHash);
+  ASSERT_OK(builder.status());
+  for (uint32_t i = 0; i < user_keys.size(); i++) {
+    builder.Add(Slice(GetInternalKey(user_keys[i], true)), Slice(values[i]));
+    ASSERT_EQ(builder.NumEntries(), i + 1);
+    ASSERT_OK(builder.status());
+  }
+  ASSERT_OK(builder.Finish());
+  ASSERT_OK(writable_file->Close());
+
+  uint32_t expected_max_buckets = user_keys.size() / kHashTableRatio;
+  std::string expected_unused_bucket = "key06";
+  expected_unused_bucket += std::string(values[0].size(), 'a');
+  CheckFileContents(user_keys, values, expected_locations,
+      expected_unused_bucket, expected_max_buckets, 2, true);
 }
 
 TEST(CuckooBuilderTest, FailWhenCollisionPathTooLong) {
-  hash_map.clear();
   // Have two hash functions. Insert elements with overlapping hashes.
-  // Finally insert an element which will displace all the current elements.
-  num_hash_fun = 2;
+  // Finally try inserting an element with hash value somewhere in the middle
+  // and it should fail because the no. of elements to displace is too high.
+  uint32_t num_hash_fun = 2;
+  std::vector<std::string> user_keys = {"key01", "key02", "key03",
+    "key04", "key05"};
+  hash_map = {
+    {user_keys[0], {0, 1}},
+    {user_keys[1], {1, 2}},
+    {user_keys[2], {2, 3}},
+    {user_keys[3], {3, 4}},
+    {user_keys[4], {0, 1}},
+  };
 
-  uint32_t max_search_depth = 100;
-  num_items = 2*max_search_depth + 3;
-  std::vector<std::string> user_keys(num_items);
-  std::vector<std::string> keys(num_items);
-  std::vector<std::string> values(num_items);
-  for (uint32_t i = 0; i < num_items - 1; i++) {
-    user_keys[i] = "keys" + std::to_string(i+100);
-    ParsedInternalKey ikey(user_keys[i], i + 1000, kTypeValue);
-    AppendInternalKey(&keys[i], ikey);
-    values[i] = "value" + std::to_string(i+100);
-    // Make all hash values collide with the next element.
-    AddHashLookups(user_keys[i], i, num_hash_fun);
-  }
-  user_keys.back() = "keys" + std::to_string(num_items + 99);
-  ParsedInternalKey ikey(user_keys.back(), num_items + 1000, kTypeValue);
-  AppendInternalKey(&keys.back(), ikey);
-  values.back() = "value" + std::to_string(num_items+100);
-  // Make hash values collide with middle element.
-  hash_map[user_keys.back()] = {0, max_search_depth + 1};
-
-  ikey_length = keys[0].size();
-  value_length = values[0].size();
   unique_ptr<WritableFile> writable_file;
-  fname = test::TmpDir() + "/FailWhenCollisionPathTooLong_writable";
+  fname = test::TmpDir() + "/WithCollisionPathUserKey";
   ASSERT_OK(env_->NewWritableFile(fname, &writable_file, env_options_));
-  CuckooTableBuilder cuckoo_builder(
-      writable_file.get(), ikey_length,
-      value_length, hash_table_ratio, file_size, num_hash_fun,
-      max_search_depth, false, GetSliceHash);
-  ASSERT_OK(cuckoo_builder.status());
-  for (uint32_t key_idx = 0; key_idx < num_items-1; key_idx++) {
-    cuckoo_builder.Add(Slice(keys[key_idx]), Slice(values[key_idx]));
-    ASSERT_OK(cuckoo_builder.status());
-    ASSERT_EQ(cuckoo_builder.NumEntries(), key_idx + 1);
+  CuckooTableBuilder builder(writable_file.get(), kHashTableRatio,
+      num_hash_fun, 2, GetSliceHash);
+  ASSERT_OK(builder.status());
+  for (uint32_t i = 0; i < user_keys.size(); i++) {
+    builder.Add(Slice(GetInternalKey(user_keys[i], false)), Slice("value"));
+    ASSERT_EQ(builder.NumEntries(), i + 1);
+    ASSERT_OK(builder.status());
   }
-  cuckoo_builder.Add(Slice(keys.back()), Slice(values.back()));
-  ASSERT_TRUE(cuckoo_builder.status().IsCorruption());
-  cuckoo_builder.Abandon();
-  writable_file->Close();
+  ASSERT_TRUE(builder.Finish().IsNotSupported());
+  ASSERT_OK(writable_file->Close());
 }
 
-TEST(CuckooBuilderTest, FailWhenTableIsFull) {
-  hash_map.clear();
-  file_size = 160;
-
-  num_items = 7;
-  std::vector<std::string> user_keys(num_items);
-  std::vector<std::string> keys(num_items);
-  std::vector<std::string> values(num_items);
-  for (uint32_t i = 0; i < num_items; i++) {
-    user_keys[i] = "keys" + std::to_string(i+1000);
-    ParsedInternalKey ikey(user_keys[i], i + 1000, kTypeValue);
-    AppendInternalKey(&keys[i], ikey);
-    values[i] = "value" + std::to_string(i+100);
-    AddHashLookups(user_keys[i], i, num_hash_fun);
-  }
-  ikey_length = keys[0].size();
-  value_length = values[0].size();
-  bucket_length = ikey_length + value_length;
-  // Check that number of items is tight.
-  ASSERT_GT(bucket_length * num_items, file_size);
-  ASSERT_LE(bucket_length * (num_items-1), file_size);
+TEST(CuckooBuilderTest, FailWhenSameKeyInserted) {
+  hash_map = {{"repeatedkey", {0, 1, 2, 3}}};
+  uint32_t num_hash_fun = 4;
+  std::string user_key = "repeatedkey";
 
   unique_ptr<WritableFile> writable_file;
-  fname = test::TmpDir() + "/FailWhenTabelIsFull_writable";
+  fname = test::TmpDir() + "/FailWhenSameKeyInserted";
   ASSERT_OK(env_->NewWritableFile(fname, &writable_file, env_options_));
-  CuckooTableBuilder cuckoo_builder(
-      writable_file.get(), ikey_length,
-      value_length, hash_table_ratio, file_size, num_hash_fun,
-      100, false, GetSliceHash);
-  ASSERT_OK(cuckoo_builder.status());
-  for (uint32_t key_idx = 0; key_idx < num_items-1; key_idx++) {
-    cuckoo_builder.Add(Slice(keys[key_idx]), Slice(values[key_idx]));
-    ASSERT_OK(cuckoo_builder.status());
-    ASSERT_EQ(cuckoo_builder.NumEntries(), key_idx + 1);
-  }
-  cuckoo_builder.Add(Slice(keys.back()), Slice(values.back()));
-  ASSERT_TRUE(cuckoo_builder.status().IsCorruption());
-  cuckoo_builder.Abandon();
-  writable_file->Close();
+  CuckooTableBuilder builder(writable_file.get(), kHashTableRatio,
+      num_hash_fun, 100, GetSliceHash);
+  ASSERT_OK(builder.status());
+
+  builder.Add(Slice(GetInternalKey(user_key, false)), Slice("value1"));
+  ASSERT_EQ(builder.NumEntries(), 1u);
+  ASSERT_OK(builder.status());
+  builder.Add(Slice(GetInternalKey(user_key, true)), Slice("value2"));
+  ASSERT_EQ(builder.NumEntries(), 2u);
+  ASSERT_OK(builder.status());
+
+  ASSERT_TRUE(builder.Finish().IsNotSupported());
+  ASSERT_OK(writable_file->Close());
 }
 }  // namespace rocksdb
 

table/cuckoo_table_reader.cc

@@ -10,8 +10,14 @@
 #ifndef ROCKSDB_LITE
 #include "table/cuckoo_table_reader.h"
 
+#include <algorithm>
+#include <limits>
 #include <string>
+#include <utility>
+#include <vector>
+#include "rocksdb/iterator.h"
 #include "table/meta_blocks.h"
+#include "util/arena.h"
 #include "util/coding.h"
 
 namespace rocksdb {
@@ -22,10 +28,9 @@ CuckooTableReader::CuckooTableReader(
     const Options& options,
     std::unique_ptr<RandomAccessFile>&& file,
     uint64_t file_size,
-    uint64_t (*GetSliceHashPtr)(const Slice&, uint32_t, uint64_t))
+    uint64_t (*get_slice_hash)(const Slice&, uint32_t, uint64_t))
     : file_(std::move(file)),
-      file_size_(file_size),
-      GetSliceHash(GetSliceHashPtr) {
+      get_slice_hash_(get_slice_hash) {
   if (!options.allow_mmap_reads) {
     status_ = Status::InvalidArgument("File is not mmaped");
   }
@@ -85,7 +90,7 @@ Status CuckooTableReader::Get(
     return Status::Corruption("Unable to parse key into inernal key.");
   }
   for (uint32_t hash_cnt = 0; hash_cnt < num_hash_fun_; ++hash_cnt) {
-    uint64_t hash_val = GetSliceHash(ikey.user_key, hash_cnt, num_buckets_);
+    uint64_t hash_val = get_slice_hash_(ikey.user_key, hash_cnt, num_buckets_);
     assert(hash_val < num_buckets_);
     uint64_t offset = hash_val * bucket_length_;
     const char* bucket = &file_data_.data()[offset];
@@ -104,9 +109,167 @@ Status CuckooTableReader::Get(
   return Status::OK();
 }
 
-Iterator* CuckooTableReader::NewIterator(const ReadOptions&, Arena* arena) {
-  // TODO(rbs): Implement this as this will be used in compaction.
-  return nullptr;
+class CuckooTableIterator : public Iterator {
+ public:
+  explicit CuckooTableIterator(CuckooTableReader* reader);
+  ~CuckooTableIterator() {}
+  bool Valid() const override;
+  void SeekToFirst() override;
+  void SeekToLast() override;
+  void Seek(const Slice& target) override;
+  void Next() override;
+  void Prev() override;
+  Slice key() const override;
+  Slice value() const override;
+  Status status() const override { return status_; }
+  void LoadKeysFromReader();
+
+ private:
+  struct {
+    bool operator()(const std::pair<Slice, uint32_t>& first,
+        const std::pair<Slice, uint32_t>& second) const {
+      return first.first.compare(second.first) < 0;
+    }
+  } CompareKeys;
+  void PrepareKVAtCurrIdx();
+  CuckooTableReader* reader_;
+  Status status_;
+  // Contains a map of keys to bucket_id sorted in key order.
+  // We assume byte-wise comparison for key ordering.
+  std::vector<std::pair<Slice, uint32_t>> key_to_bucket_id_;
+  // We assume that the number of items can be stored in uint32 (4 Billion).
+  uint32_t curr_key_idx_;
+  Slice curr_value_;
+  IterKey curr_key_;
+  // No copying allowed
+  CuckooTableIterator(const CuckooTableIterator&) = delete;
+  void operator=(const Iterator&) = delete;
+};
+
+CuckooTableIterator::CuckooTableIterator(CuckooTableReader* reader)
+  : reader_(reader),
+    curr_key_idx_(std::numeric_limits<int32_t>::max()) {
+  key_to_bucket_id_.clear();
+  curr_value_.clear();
+  curr_key_.Clear();
 }
+
+void CuckooTableIterator::LoadKeysFromReader() {
+  key_to_bucket_id_.reserve(reader_->GetTableProperties()->num_entries);
+  for (uint32_t bucket_id = 0; bucket_id < reader_->num_buckets_; bucket_id++) {
+    Slice read_key;
+    status_ = reader_->file_->Read(bucket_id * reader_->bucket_length_,
+        reader_->key_length_, &read_key, nullptr);
+    if (read_key != Slice(reader_->unused_key_)) {
+      key_to_bucket_id_.push_back(std::make_pair(read_key, bucket_id));
+    }
+  }
+  assert(key_to_bucket_id_.size() ==
+      reader_->GetTableProperties()->num_entries);
+  std::sort(key_to_bucket_id_.begin(), key_to_bucket_id_.end(), CompareKeys);
+  curr_key_idx_ = key_to_bucket_id_.size();
+}
+
+void CuckooTableIterator::SeekToFirst() {
+  curr_key_idx_ = 0;
+  PrepareKVAtCurrIdx();
+}
+
+void CuckooTableIterator::SeekToLast() {
+  curr_key_idx_ = key_to_bucket_id_.size() - 1;
+  PrepareKVAtCurrIdx();
+}
+
+void CuckooTableIterator::Seek(const Slice& target) {
+  // We assume that the target is an internal key. If this is last level file,
+  // we need to take only the user key part to seek.
+  Slice target_to_search = reader_->is_last_level_ ?
+    ExtractUserKey(target) : target;
+  auto seek_it = std::lower_bound(key_to_bucket_id_.begin(),
+      key_to_bucket_id_.end(),
+      std::make_pair(target_to_search, 0),
+      CompareKeys);
+  curr_key_idx_ = std::distance(key_to_bucket_id_.begin(), seek_it);
+  PrepareKVAtCurrIdx();
+}
+
+bool CuckooTableIterator::Valid() const {
+  return curr_key_idx_ < key_to_bucket_id_.size();
+}
+
+void CuckooTableIterator::PrepareKVAtCurrIdx() {
+  if (!Valid()) {
+    curr_value_.clear();
+    curr_key_.Clear();
+    return;
+  }
+  uint64_t offset = ((uint64_t) key_to_bucket_id_[curr_key_idx_].second
+      * reader_->bucket_length_) + reader_->key_length_;
+  status_ = reader_->file_->Read(offset, reader_->value_length_,
+      &curr_value_, nullptr);
+  if (reader_->is_last_level_) {
+    // Always return internal key.
+    curr_key_.SetInternalKey(
+        key_to_bucket_id_[curr_key_idx_].first, 0, kTypeValue);
+  }
+}
+
+void CuckooTableIterator::Next() {
+  if (!Valid()) {
+    curr_value_.clear();
+    curr_key_.Clear();
+    return;
+  }
+  ++curr_key_idx_;
+  PrepareKVAtCurrIdx();
+}
+
+void CuckooTableIterator::Prev() {
+  if (curr_key_idx_ == 0) {
+    curr_key_idx_ = key_to_bucket_id_.size();
+  }
+  if (!Valid()) {
+    curr_value_.clear();
+    curr_key_.Clear();
+    return;
+  }
+  --curr_key_idx_;
+  PrepareKVAtCurrIdx();
+}
+
+Slice CuckooTableIterator::key() const {
+  assert(Valid());
+  if (reader_->is_last_level_) {
+    return curr_key_.GetKey();
+  } else {
+    return key_to_bucket_id_[curr_key_idx_].first;
+  }
+}
+
+Slice CuckooTableIterator::value() const {
+  assert(Valid());
+  return curr_value_;
+}
+
+Iterator* CuckooTableReader::NewIterator(const ReadOptions&, Arena* arena) {
+  if (!status().ok()) {
+    return NewErrorIterator(
+        Status::Corruption("CuckooTableReader status is not okay."));
+  }
+  CuckooTableIterator* iter;
+  if (arena == nullptr) {
+    iter = new CuckooTableIterator(this);
+  } else {
+    auto iter_mem = arena->AllocateAligned(sizeof(CuckooTableIterator));
+    iter = new (iter_mem) CuckooTableIterator(this);
+  }
+  if (iter->status().ok()) {
+    iter->LoadKeysFromReader();
+  }
+  return iter;
+}
+
+size_t CuckooTableReader::ApproximateMemoryUsage() const { return 0; }
+
 }  // namespace rocksdb
 #endif

table/cuckoo_table_reader.h

@@ -11,6 +11,8 @@
 #ifndef ROCKSDB_LITE
 #include <string>
 #include <memory>
+#include <utility>
+#include <vector>
 
 #include "db/dbformat.h"
 #include "rocksdb/env.h"
@@ -27,10 +29,10 @@ class CuckooTableReader: public TableReader {
       const Options& options,
       std::unique_ptr<RandomAccessFile>&& file,
       uint64_t file_size,
-      uint64_t (*GetSliceHash)(const Slice&, uint32_t, uint64_t));
+      uint64_t (*get_slice_hash)(const Slice&, uint32_t, uint64_t));
   ~CuckooTableReader() {}
 
-  std::shared_ptr<const TableProperties> GetTableProperties() const {
+  std::shared_ptr<const TableProperties> GetTableProperties() const override {
     return table_props_;
   }
 
@@ -40,20 +42,25 @@ class CuckooTableReader: public TableReader {
       const ReadOptions& readOptions, const Slice& key, void* handle_context,
       bool (*result_handler)(void* arg, const ParsedInternalKey& k,
                              const Slice& v),
-      void (*mark_key_may_exist_handler)(void* handle_context) = nullptr);
+      void (*mark_key_may_exist_handler)(void* handle_context) = nullptr)
+    override;
 
-  Iterator* NewIterator(const ReadOptions&, Arena* arena = nullptr);
+  Iterator* NewIterator(const ReadOptions&, Arena* arena = nullptr) override;
+
+  // Report an approximation of how much memory has been used.
+  size_t ApproximateMemoryUsage() const override;
 
   // Following methods are not implemented for Cuckoo Table Reader
-  uint64_t ApproximateOffsetOf(const Slice& key) { return 0; }
-  void SetupForCompaction() {}
-  void Prepare(const Slice& target) {}
+  uint64_t ApproximateOffsetOf(const Slice& key) override { return 0; }
+  void SetupForCompaction() override {}
+  void Prepare(const Slice& target) override {}
   // End of methods not implemented.
 
  private:
+  friend class CuckooTableIterator;
+  void LoadAllKeys(std::vector<std::pair<Slice, uint32_t>>* key_to_bucket_id);
   std::unique_ptr<RandomAccessFile> file_;
   Slice file_data_;
-  const uint64_t file_size_;
   bool is_last_level_;
   std::shared_ptr<const TableProperties> table_props_;
   Status status_;
@@ -63,7 +70,7 @@ class CuckooTableReader: public TableReader {
   uint32_t value_length_;
   uint32_t bucket_length_;
   uint64_t num_buckets_;
-  uint64_t (*GetSliceHash)(const Slice& s, uint32_t index,
+  uint64_t (*get_slice_hash_)(const Slice& s, uint32_t index,
       uint64_t max_num_buckets);
 };
 

table/cuckoo_table_reader_test.cc

@@ -11,6 +11,8 @@ int main() {
 }
 #else
 
+#define __STDC_FORMAT_MACROS
+#include <inttypes.h>
 #include <gflags/gflags.h>
 #include <vector>
 #include <string>
@@ -20,6 +22,7 @@ int main() {
 #include "table/cuckoo_table_builder.h"
 #include "table/cuckoo_table_reader.h"
 #include "table/cuckoo_table_factory.h"
+#include "util/arena.h"
 #include "util/random.h"
 #include "util/testharness.h"
 #include "util/testutil.h"
@@ -94,25 +97,27 @@ class CuckooReaderTest {
     values.resize(num_items);
   }
 
-  void CreateCuckooFile(bool is_last_level) {
+  std::string NumToStr(int64_t i) {
+    return std::string(reinterpret_cast<char*>(&i), sizeof(i));
+  }
+
+  void CreateCuckooFileAndCheckReader() {
     unique_ptr<WritableFile> writable_file;
     ASSERT_OK(env->NewWritableFile(fname, &writable_file, env_options));
     CuckooTableBuilder builder(
-        writable_file.get(), keys[0].size(), values[0].size(), 0.9,
-        10000, kNumHashFunc, 100, is_last_level, GetSliceHash);
+        writable_file.get(), 0.9, kNumHashFunc, 100, GetSliceHash);
     ASSERT_OK(builder.status());
     for (uint32_t key_idx = 0; key_idx < num_items; ++key_idx) {
       builder.Add(Slice(keys[key_idx]), Slice(values[key_idx]));
-      ASSERT_EQ(builder.NumEntries(), key_idx + 1);
       ASSERT_OK(builder.status());
+      ASSERT_EQ(builder.NumEntries(), key_idx + 1);
     }
     ASSERT_OK(builder.Finish());
     ASSERT_EQ(num_items, builder.NumEntries());
     file_size = builder.FileSize();
     ASSERT_OK(writable_file->Close());
-  }
 
-  void CheckReader() {
+    // Check reader now.
     unique_ptr<RandomAccessFile> read_file;
     ASSERT_OK(env->NewRandomAccessFile(fname, &read_file, env_options));
     CuckooTableReader reader(
@@ -121,7 +126,6 @@ class CuckooReaderTest {
         file_size,
         GetSliceHash);
     ASSERT_OK(reader.status());
-
     for (uint32_t i = 0; i < num_items; ++i) {
       ValuesToAssert v(user_keys[i], values[i]);
       ASSERT_OK(reader.Get(
@@ -129,11 +133,79 @@ class CuckooReaderTest {
       ASSERT_EQ(1, v.call_count);
     }
   }
+  void UpdateKeys(bool with_zero_seqno) {
+    for (uint32_t i = 0; i < num_items; i++) {
+      ParsedInternalKey ikey(user_keys[i],
+          with_zero_seqno ? 0 : i + 1000, kTypeValue);
+      keys[i].clear();
+      AppendInternalKey(&keys[i], ikey);
+    }
+  }
+
+  void CheckIterator() {
+    unique_ptr<RandomAccessFile> read_file;
+    ASSERT_OK(env->NewRandomAccessFile(fname, &read_file, env_options));
+    CuckooTableReader reader(
+        options,
+        std::move(read_file),
+        file_size,
+        GetSliceHash);
+    ASSERT_OK(reader.status());
+    Iterator* it = reader.NewIterator(ReadOptions(), nullptr);
+    ASSERT_OK(it->status());
+    ASSERT_TRUE(!it->Valid());
+    it->SeekToFirst();
+    int cnt = 0;
+    while (it->Valid()) {
+      ASSERT_OK(it->status());
+      ASSERT_TRUE(Slice(keys[cnt]) == it->key());
+      ASSERT_TRUE(Slice(values[cnt]) == it->value());
+      ++cnt;
+      it->Next();
+    }
+    ASSERT_EQ(static_cast<uint32_t>(cnt), num_items);
+
+    it->SeekToLast();
+    cnt = num_items - 1;
+    ASSERT_TRUE(it->Valid());
+    while (it->Valid()) {
+      ASSERT_OK(it->status());
+      ASSERT_TRUE(Slice(keys[cnt]) == it->key());
+      ASSERT_TRUE(Slice(values[cnt]) == it->value());
+      --cnt;
+      it->Prev();
+    }
+    ASSERT_EQ(cnt, -1);
+
+    cnt = num_items / 2;
+    it->Seek(keys[cnt]);
+    while (it->Valid()) {
+      ASSERT_OK(it->status());
+      ASSERT_TRUE(Slice(keys[cnt]) == it->key());
+      ASSERT_TRUE(Slice(values[cnt]) == it->value());
+      ++cnt;
+      it->Next();
+    }
+    ASSERT_EQ(static_cast<uint32_t>(cnt), num_items);
+    delete it;
+
+    Arena arena;
+    it = reader.NewIterator(ReadOptions(), &arena);
+    ASSERT_OK(it->status());
+    ASSERT_TRUE(!it->Valid());
+    it->Seek(keys[num_items/2]);
+    ASSERT_TRUE(it->Valid());
+    ASSERT_OK(it->status());
+    ASSERT_TRUE(keys[num_items/2] == it->key());
+    ASSERT_TRUE(values[num_items/2] == it->value());
+    ASSERT_OK(it->status());
+    it->~Iterator();
+  }
 
   std::vector<std::string> keys;
   std::vector<std::string> user_keys;
   std::vector<std::string> values;
-  uint32_t num_items;
+  uint64_t num_items;
   std::string fname;
   uint64_t file_size;
   Options options;
@@ -142,46 +214,64 @@ class CuckooReaderTest {
 };
 
 TEST(CuckooReaderTest, WhenKeyExists) {
-  SetUp(10);
+  SetUp(kNumHashFunc);
   fname = test::TmpDir() + "/CuckooReader_WhenKeyExists";
-  for (uint32_t i = 0; i < num_items; i++) {
-    user_keys[i] = "keys" + std::to_string(i+100);
+  for (uint64_t i = 0; i < num_items; i++) {
+    user_keys[i] = "key" + NumToStr(i);
     ParsedInternalKey ikey(user_keys[i], i + 1000, kTypeValue);
     AppendInternalKey(&keys[i], ikey);
-    values[i] = "value" + std::to_string(i+100);
-    AddHashLookups(user_keys[i], i * kNumHashFunc, kNumHashFunc);
+    values[i] = "value" + NumToStr(i);
+    // Give disjoint hash values.
+    AddHashLookups(user_keys[i], i, kNumHashFunc);
   }
-  CreateCuckooFile(false);
-  CheckReader();
+  CreateCuckooFileAndCheckReader();
   // Last level file.
-  CreateCuckooFile(true);
-  CheckReader();
+  UpdateKeys(true);
+  CreateCuckooFileAndCheckReader();
   // Test with collision. Make all hash values collide.
   hash_map.clear();
   for (uint32_t i = 0; i < num_items; i++) {
     AddHashLookups(user_keys[i], 0, kNumHashFunc);
   }
-  CreateCuckooFile(false);
-  CheckReader();
+  UpdateKeys(false);
+  CreateCuckooFileAndCheckReader();
   // Last level file.
-  CreateCuckooFile(true);
-  CheckReader();
+  UpdateKeys(true);
+  CreateCuckooFileAndCheckReader();
+}
+
+TEST(CuckooReaderTest, CheckIterator) {
+  SetUp(2*kNumHashFunc);
+  fname = test::TmpDir() + "/CuckooReader_CheckIterator";
+  for (uint64_t i = 0; i < num_items; i++) {
+    user_keys[i] = "key" + NumToStr(i);
+    ParsedInternalKey ikey(user_keys[i], 0, kTypeValue);
+    AppendInternalKey(&keys[i], ikey);
+    values[i] = "value" + NumToStr(i);
+    // Give disjoint hash values, in reverse order.
+    AddHashLookups(user_keys[i], num_items-i-1, kNumHashFunc);
+  }
+  CreateCuckooFileAndCheckReader();
+  CheckIterator();
+  // Last level file.
+  UpdateKeys(true);
+  CreateCuckooFileAndCheckReader();
+  CheckIterator();
 }
 
 TEST(CuckooReaderTest, WhenKeyNotFound) {
   // Add keys with colliding hash values.
-  SetUp(kNumHashFunc / 2);
+  SetUp(kNumHashFunc);
   fname = test::TmpDir() + "/CuckooReader_WhenKeyNotFound";
-  for (uint32_t i = 0; i < num_items; i++) {
-    user_keys[i] = "keys" + std::to_string(i+100);
+  for (uint64_t i = 0; i < num_items; i++) {
+    user_keys[i] = "key" + NumToStr(i);
     ParsedInternalKey ikey(user_keys[i], i + 1000, kTypeValue);
     AppendInternalKey(&keys[i], ikey);
-    values[i] = "value" + std::to_string(i+100);
+    values[i] = "value" + NumToStr(i);
     // Make all hash values collide.
     AddHashLookups(user_keys[i], 0, kNumHashFunc);
   }
-  CreateCuckooFile(false);
-  CheckReader();
+  CreateCuckooFileAndCheckReader();
   unique_ptr<RandomAccessFile> read_file;
   ASSERT_OK(env->NewRandomAccessFile(fname, &read_file, env_options));
   CuckooTableReader reader(
@@ -191,7 +281,7 @@ TEST(CuckooReaderTest, WhenKeyNotFound) {
       GetSliceHash);
   ASSERT_OK(reader.status());
   // Search for a key with colliding hash values.
-  std::string not_found_user_key = "keys" + std::to_string(num_items + 100);
+  std::string not_found_user_key = "key" + NumToStr(num_items);
   std::string not_found_key;
   AddHashLookups(not_found_user_key, 0, kNumHashFunc);
   ParsedInternalKey ikey(not_found_user_key, 1000, kTypeValue);
@@ -202,10 +292,10 @@ TEST(CuckooReaderTest, WhenKeyNotFound) {
   ASSERT_EQ(0, v.call_count);
   ASSERT_OK(reader.status());
   // Search for a key with an independent hash value.
-  std::string not_found_user_key2 = "keys" + std::to_string(num_items + 101);
-  std::string not_found_key2;
+  std::string not_found_user_key2 = "key" + NumToStr(num_items + 1);
   AddHashLookups(not_found_user_key2, kNumHashFunc, kNumHashFunc);
   ParsedInternalKey ikey2(not_found_user_key2, 1000, kTypeValue);
+  std::string not_found_key2;
   AppendInternalKey(&not_found_key2, ikey2);
   ASSERT_OK(reader.Get(
         ReadOptions(), Slice(not_found_key2), &v, AssertValues, nullptr));
@@ -213,21 +303,21 @@ TEST(CuckooReaderTest, WhenKeyNotFound) {
   ASSERT_OK(reader.status());
 
   // Test read with corrupted key.
-  not_found_key2.pop_back();
-  ASSERT_TRUE(!ParseInternalKey(not_found_key2, &ikey));
+  Slice corrupt_key("corrupt_ikey");
+  ASSERT_TRUE(!ParseInternalKey(corrupt_key, &ikey));
   ASSERT_TRUE(reader.Get(
-        ReadOptions(), Slice(not_found_key2), &v,
+        ReadOptions(), corrupt_key, &v,
         AssertValues, nullptr).IsCorruption());
   ASSERT_EQ(0, v.call_count);
   ASSERT_OK(reader.status());
 
   // Test read when key is unused key.
-  std::string unused_user_key = "keys10:";
+  std::string unused_key =
+    reader.GetTableProperties()->user_collected_properties.at(
+    CuckooTablePropertyNames::kEmptyKey);
   // Add hash values that map to empty buckets.
-  AddHashLookups(unused_user_key, kNumHashFunc, kNumHashFunc);
-  std::string unused_key;
-  ParsedInternalKey ikey3(unused_user_key, 1000, kTypeValue);
-  AppendInternalKey(&unused_key, ikey3);
+  AddHashLookups(ExtractUserKey(unused_key).ToString(),
+      kNumHashFunc, kNumHashFunc);
   ASSERT_OK(reader.Get(
         ReadOptions(), Slice(unused_key), &v, AssertValues, nullptr));
   ASSERT_EQ(0, v.call_count);
@@ -266,20 +356,17 @@ void BM_CuckooRead(uint64_t num, uint32_t key_length,
   }
   std::string fname = FLAGS_file_dir + "/cuckoo_read_benchmark";
 
-  uint64_t predicted_file_size =
-    num * (key_length + value_length) / hash_ratio + 1024;
-
   unique_ptr<WritableFile> writable_file;
   ASSERT_OK(env->NewWritableFile(fname, &writable_file, env_options));
   CuckooTableBuilder builder(
-      writable_file.get(), key_length + 8, value_length, hash_ratio,
-      predicted_file_size, kMaxNumHashTable, 1000, true, GetSliceMurmurHash);
+      writable_file.get(), hash_ratio,
+      kMaxNumHashTable, 1000, GetSliceMurmurHash);
   ASSERT_OK(builder.status());
   for (uint64_t key_idx = 0; key_idx < num; ++key_idx) {
     // Value is just a part of key.
     std::string new_key(reinterpret_cast<char*>(&key_idx), sizeof(key_idx));
     new_key = std::string(key_length - new_key.size(), 'k') + new_key;
-    ParsedInternalKey ikey(new_key, num, kTypeValue);
+    ParsedInternalKey ikey(new_key, 0, kTypeValue);
     std::string full_key;
     AppendInternalKey(&full_key, ikey);
     builder.Add(Slice(full_key), Slice(&full_key[0], value_length));
@@ -304,8 +391,8 @@ void BM_CuckooRead(uint64_t num, uint32_t key_length,
     reader.GetTableProperties()->user_collected_properties;
   const uint32_t num_hash_fun = *reinterpret_cast<const uint32_t*>(
       user_props.at(CuckooTablePropertyNames::kNumHashTable).data());
-  fprintf(stderr, "With %lu items and hash table ratio %f, number of hash"
-      " functions used: %u.\n", num, hash_ratio, num_hash_fun);
+  fprintf(stderr, "With %" PRIu64 " items and hash table ratio %f, number of"
+      " hash functions used: %u.\n", num, hash_ratio, num_hash_fun);
   ReadOptions r_options;
   for (auto& key : keys) {
     int cnt = 0;
@@ -316,6 +403,10 @@ void BM_CuckooRead(uint64_t num, uint32_t key_length,
   std::random_shuffle(keys.begin(), keys.end());
 
   uint64_t time_now = env->NowMicros();
+  reader.NewIterator(ReadOptions(), nullptr);
+  fprintf(stderr, "Time taken for preparing iterator for %" PRIu64 " items: %" PRIu64 " ms.\n",
+      num, (env->NowMicros() - time_now)/1000);
+  time_now = env->NowMicros();
   for (uint64_t i = 0; i < num_reads; ++i) {
     reader.Get(r_options, Slice(keys[i % num]), nullptr, DoNothing, nullptr);
   }

table/filter_block.cc

@@ -184,4 +184,7 @@ bool FilterBlockReader::MayMatch(uint64_t block_offset, const Slice& entry) {
   return true;  // Errors are treated as potential matches
 }
 
+size_t FilterBlockReader::ApproximateMemoryUsage() const {
+  return num_ * 4 + 5 + (offset_ - data_);
+}
 }

table/filter_block.h

@@ -74,6 +74,7 @@ class FilterBlockReader {
     bool delete_contents_after_use = false);
   bool KeyMayMatch(uint64_t block_offset, const Slice& key);
   bool PrefixMayMatch(uint64_t block_offset, const Slice& prefix);
+  size_t ApproximateMemoryUsage() const;
 
  private:
   const FilterPolicy* policy_;

table/format.cc

@@ -33,6 +33,7 @@ extern const uint64_t kPlainTableMagicNumber;
 const uint64_t kLegacyPlainTableMagicNumber = 0;
 const uint64_t kPlainTableMagicNumber = 0;
 #endif
+const uint32_t DefaultStackBufferSize = 5000;
 
 void BlockHandle::EncodeTo(std::string* dst) const {
   // Sanity check that all fields have been set
@@ -203,40 +204,29 @@ Status ReadFooterFromFile(RandomAccessFile* file,
   return footer->DecodeFrom(&footer_input);
 }
 
-Status ReadBlockContents(RandomAccessFile* file,
-                         const Footer& footer,
-                         const ReadOptions& options,
-                         const BlockHandle& handle,
-                         BlockContents* result,
-                         Env* env,
-                         bool do_uncompress) {
-  result->data = Slice();
-  result->cachable = false;
-  result->heap_allocated = false;
-
-  // Read the block contents as well as the type/crc footer.
-  // See table_builder.cc for the code that built this structure.
+// 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(RandomAccessFile* 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());
-  char* buf = new char[n + kBlockTrailerSize];
-  Slice contents;
 
   PERF_TIMER_AUTO(block_read_time);
-  Status s = file->Read(handle.offset(), n + kBlockTrailerSize, &contents, buf);
+  Status s = file->Read(handle.offset(), n + kBlockTrailerSize, contents, buf);
   PERF_TIMER_MEASURE(block_read_time);
   PERF_COUNTER_ADD(block_read_count, 1);
   PERF_COUNTER_ADD(block_read_byte, n + kBlockTrailerSize);
 
   if (!s.ok()) {
-    delete[] buf;
     return s;
   }
-  if (contents.size() != n + kBlockTrailerSize) {
-    delete[] buf;
+  if (contents->size() != n + kBlockTrailerSize) {
     return Status::Corruption("truncated block read");
   }
 
   // Check the crc of the type and the block contents
-  const char* data = contents.data();    // Pointer to where Read put the data
+  const char* data = contents->data();  // Pointer to where Read put the data
   if (options.verify_checksums) {
     uint32_t value = DecodeFixed32(data + n + 1);
     uint32_t actual = 0;
@@ -255,12 +245,27 @@ Status ReadBlockContents(RandomAccessFile* file,
       s = Status::Corruption("block checksum mismatch");
     }
     if (!s.ok()) {
-      delete[] buf;
       return s;
     }
-    PERF_TIMER_MEASURE(block_checksum_time);
+    PERF_TIMER_STOP(block_checksum_time);
   }
+  return s;
+}
 
+// Decompress a block according to params
+// May need to malloc a space for cache usage
+Status DecompressBlock(BlockContents* result, size_t block_size,
+                          bool do_uncompress, const char* buf,
+                          const Slice& contents, bool use_stack_buf) {
+  Status s;
+  size_t n = block_size;
+  const char* data = contents.data();
+
+  result->data = Slice();
+  result->cachable = false;
+  result->heap_allocated = false;
+
+  PERF_TIMER_AUTO(block_decompress_time);
   rocksdb::CompressionType compression_type =
       static_cast<rocksdb::CompressionType>(data[n]);
   // If the caller has requested that the block not be uncompressed
@@ -269,12 +274,19 @@ Status ReadBlockContents(RandomAccessFile* file,
       // File implementation gave us pointer to some other data.
       // Use it directly under the assumption that it will be live
       // while the file is open.
-      delete[] buf;
       result->data = Slice(data, n);
       result->heap_allocated = false;
       result->cachable = false;  // Do not double-cache
     } else {
-      result->data = Slice(buf, n);
+      if (use_stack_buf) {
+        // Need to allocate space in heap for cache usage
+        char* new_buf = new char[n];
+        memcpy(new_buf, buf, n);
+        result->data = Slice(new_buf, n);
+      } else {
+        result->data = Slice(buf, n);
+      }
+
       result->heap_allocated = true;
       result->cachable = true;
     }
@@ -282,12 +294,73 @@ Status ReadBlockContents(RandomAccessFile* file,
     s = Status::OK();
   } else {
     s = UncompressBlockContents(data, n, result);
-    delete[] buf;
   }
   PERF_TIMER_STOP(block_decompress_time);
   return s;
 }
 
+// Read and Decompress block
+// Use buf in stack as temp reading buffer
+Status ReadAndDecompressFast(RandomAccessFile* file, const Footer& footer,
+                             const ReadOptions& options,
+                             const BlockHandle& handle, BlockContents* result,
+                             Env* env, bool do_uncompress) {
+  Status s;
+  Slice contents;
+  size_t n = static_cast<size_t>(handle.size());
+  char buf[DefaultStackBufferSize];
+
+  s = ReadBlock(file, footer, options, handle, &contents, buf);
+  if (!s.ok()) {
+    return s;
+  }
+  s = DecompressBlock(result, n, do_uncompress, buf, contents, true);
+  if (!s.ok()) {
+    return s;
+  }
+  return s;
+}
+
+// Read and Decompress block
+// Use buf in heap as temp reading buffer
+Status ReadAndDecompress(RandomAccessFile* file, const Footer& footer,
+                         const ReadOptions& options, const BlockHandle& handle,
+                         BlockContents* result, Env* env, bool do_uncompress) {
+  Status s;
+  Slice contents;
+  size_t n = static_cast<size_t>(handle.size());
+  char* buf = new char[n + kBlockTrailerSize];
+
+  s = ReadBlock(file, footer, options, handle, &contents, buf);
+  if (!s.ok()) {
+    delete[] buf;
+    return s;
+  }
+  s = DecompressBlock(result, n, do_uncompress, buf, contents, false);
+  if (!s.ok()) {
+    delete[] buf;
+    return s;
+  }
+
+  if (result->data.data() != buf) {
+    delete[] buf;
+  }
+  return s;
+}
+
+Status ReadBlockContents(RandomAccessFile* file, const Footer& footer,
+                         const ReadOptions& options, const BlockHandle& handle,
+                         BlockContents* result, Env* env, bool do_uncompress) {
+  size_t n = static_cast<size_t>(handle.size());
+  if (do_uncompress && n + kBlockTrailerSize < DefaultStackBufferSize) {
+    return ReadAndDecompressFast(file, footer, options, handle, result, env,
+                                 do_uncompress);
+  } else {
+    return ReadAndDecompress(file, footer, options, handle, result, env,
+                             do_uncompress);
+  }
+}
+
 //
 // The 'data' points to the raw block contents that was read in from file.
 // This method allocates a new heap buffer and the raw block

table/merger.cc

@@ -23,7 +23,7 @@
 #include "util/autovector.h"
 
 namespace rocksdb {
-namespace {
+namespace merger {
 typedef std::priority_queue<
           IteratorWrapper*,
           std::vector<IteratorWrapper*>,
@@ -43,7 +43,7 @@ MaxIterHeap NewMaxIterHeap(const Comparator* comparator) {
 MinIterHeap NewMinIterHeap(const Comparator* comparator) {
   return MinIterHeap(MinIteratorComparator(comparator));
 }
-}  // namespace
+}  // namespace merger
 
 const size_t kNumIterReserve = 4;
 
@@ -56,8 +56,8 @@ class MergingIterator : public Iterator {
         current_(nullptr),
         use_heap_(true),
         direction_(kForward),
-        maxHeap_(NewMaxIterHeap(comparator_)),
-        minHeap_(NewMinIterHeap(comparator_)) {
+        maxHeap_(merger::NewMaxIterHeap(comparator_)),
+        minHeap_(merger::NewMinIterHeap(comparator_)) {
     children_.resize(n);
     for (int i = 0; i < n; i++) {
       children_[i].Set(children[i]);
@@ -274,8 +274,8 @@ class MergingIterator : public Iterator {
     kReverse
   };
   Direction direction_;
-  MaxIterHeap maxHeap_;
-  MinIterHeap minHeap_;
+  merger::MaxIterHeap maxHeap_;
+  merger::MinIterHeap minHeap_;
 };
 
 void MergingIterator::FindSmallest() {
@@ -302,8 +302,8 @@ void MergingIterator::FindLargest() {
 
 void MergingIterator::ClearHeaps() {
   use_heap_ = true;
-  maxHeap_ = NewMaxIterHeap(comparator_);
-  minHeap_ = NewMinIterHeap(comparator_);
+  maxHeap_ = merger::NewMaxIterHeap(comparator_);
+  minHeap_ = merger::NewMinIterHeap(comparator_);
 }
 
 Iterator* NewMergingIterator(const Comparator* cmp, Iterator** list, int n,

table/plain_table_reader.h

@@ -78,6 +78,10 @@ class PlainTableReader: public TableReader {
     return table_properties_;
   }
 
+  virtual size_t ApproximateMemoryUsage() const override {
+    return arena_.MemoryAllocatedBytes();
+  }
+
   PlainTableReader(const Options& options, unique_ptr<RandomAccessFile>&& file,
                    const EnvOptions& storage_options,
                    const InternalKeyComparator& internal_comparator,

table/table_reader.h

@@ -52,6 +52,9 @@ class TableReader {
   // Prepare work that can be done before the real Get()
   virtual void Prepare(const Slice& target) {}
 
+  // Report an approximation of how much memory has been used.
+  virtual size_t ApproximateMemoryUsage() const = 0;
+
   // Calls (*result_handler)(handle_context, ...) repeatedly, starting with
   // the entry found after a call to Seek(key), until result_handler returns
   // false, where k is the actual internal key for a row found and v as the

table/table_reader_bench.cc

@@ -144,14 +144,12 @@ void TableReaderBenchmark(Options& opts, EnvOptions& env_options,
           // Query one existing key;
           std::string key = MakeKey(r1, r2, through_db);
           uint64_t start_time = Now(env, measured_by_nanosecond);
-          port::MemoryBarrier();
           if (!through_db) {
             s = table_reader->Get(read_options, key, arg, DummySaveValue,
                                   nullptr);
           } else {
             s = db->Get(read_options, key, &result);
           }
-          port::MemoryBarrier();
           hist.Add(Now(env, measured_by_nanosecond) - start_time);
         } else {
           int r2_len;
@@ -167,7 +165,6 @@ void TableReaderBenchmark(Options& opts, EnvOptions& env_options,
           std::string end_key = MakeKey(r1, r2 + r2_len, through_db);
           uint64_t total_time = 0;
           uint64_t start_time = Now(env, measured_by_nanosecond);
-          port::MemoryBarrier();
           Iterator* iter;
           if (!through_db) {
             iter = table_reader->NewIterator(read_options);
@@ -180,7 +177,6 @@ void TableReaderBenchmark(Options& opts, EnvOptions& env_options,
               break;
             }
             // verify key;
-            port::MemoryBarrier();
             total_time += Now(env, measured_by_nanosecond) - start_time;
             assert(Slice(MakeKey(r1, r2 + count, through_db)) == iter->key());
             start_time = Now(env, measured_by_nanosecond);
@@ -195,7 +191,6 @@ void TableReaderBenchmark(Options& opts, EnvOptions& env_options,
             assert(false);
           }
           delete iter;
-          port::MemoryBarrier();
           total_time += Now(env, measured_by_nanosecond) - start_time;
           hist.Add(total_time);
         }

util/bloom.cc

@@ -15,9 +15,6 @@
 namespace rocksdb {
 
 namespace {
-static uint32_t BloomHash(const Slice& key) {
-  return Hash(key.data(), key.size(), 0xbc9f1d34);
-}
 
 class BloomFilterPolicy : public FilterPolicy {
  private:

util/dynamic_bloom.cc

@@ -14,9 +14,6 @@
 namespace rocksdb {
 
 namespace {
-static uint32_t BloomHash(const Slice& key) {
-  return Hash(key.data(), key.size(), 0xbc9f1d34);
-}
 
 uint32_t GetTotalBitsForLocality(uint32_t total_bits) {
   uint32_t num_blocks =

util/hash.h

@@ -17,6 +17,10 @@ namespace rocksdb {
 
 extern uint32_t Hash(const char* data, size_t n, uint32_t seed);
 
+inline uint32_t BloomHash(const Slice& key) {
+  return Hash(key.data(), key.size(), 0xbc9f1d34);
+}
+
 inline uint32_t GetSliceHash(const Slice& s) {
   return Hash(s.data(), s.size(), 397);
 }

util/ldb_cmd.cc

@@ -556,6 +556,10 @@ void ManifestDumpCommand::DoCommand() {
   std::shared_ptr<Cache> tc(NewLRUCache(
       options.max_open_files - 10, options.table_cache_numshardbits,
       options.table_cache_remove_scan_count_limit));
+  // Notice we are using the default options not through SanitizeOptions(),
+  // if VersionSet::DumpManifest() depends on any option done by
+  // SanitizeOptions(), we need to initialize it manually.
+  options.db_paths.emplace_back("dummy", 0);
   VersionSet* versions = new VersionSet(dbname, &options, sopt, tc.get());
   Status s = versions->DumpManifest(options, file, verbose_, is_key_hex_);
   if (!s.ok()) {

util/options_builder.cc

@@ -25,7 +25,7 @@ CompactionStyle PickCompactionStyle(size_t write_buffer_size,
   // Otherwise, calculate a score based on threshold and expected value of
   // two styles, weighing reads 4X important than writes.
   int expected_levels = static_cast<int>(ceil(
-      log(target_db_size / write_buffer_size) / log(kBytesForLevelMultiplier)));
+      ::log(target_db_size / write_buffer_size) / ::log(kBytesForLevelMultiplier)));
 
   int expected_max_files_universal =
       static_cast<int>(ceil(log2(target_db_size / write_buffer_size)));
@@ -111,8 +111,8 @@ void OptimizeForLevel(int read_amplification_threshold,
                       int write_amplification_threshold,
                       uint64_t target_db_size, Options* options) {
   int expected_levels_one_level0_file =
-      static_cast<int>(ceil(log(target_db_size / options->write_buffer_size) /
-                            log(kBytesForLevelMultiplier)));
+      static_cast<int>(ceil(::log(target_db_size / options->write_buffer_size) /
+                            ::log(kBytesForLevelMultiplier)));
 
   int level0_stop_writes_trigger =
       read_amplification_threshold - expected_levels_one_level0_file;

util/statistics.cc

@@ -4,6 +4,9 @@
 //  of patent rights can be found in the PATENTS file in the same directory.
 //
 #include "util/statistics.h"
+
+#define __STDC_FORMAT_MACROS
+#include <inttypes.h>
 #include "rocksdb/statistics.h"
 #include "port/likely.h"
 #include <algorithm>
@@ -96,7 +99,7 @@ std::string StatisticsImpl::ToString() const {
   for (const auto& t : TickersNameMap) {
     if (t.first < TICKER_ENUM_MAX || enable_internal_stats_) {
       char buffer[kBufferSize];
-      snprintf(buffer, kBufferSize, "%s COUNT : %ld\n",
+      snprintf(buffer, kBufferSize, "%s COUNT : %" PRIu64 "\n",
                t.second.c_str(), getTickerCount(t.first));
       res.append(buffer);
     }

util/thread_local.h

@@ -10,6 +10,7 @@
 #pragma once
 
 #include <atomic>
+#include <functional>
 #include <memory>
 #include <unordered_map>
 #include <vector>

utilities/geodb/geodb_impl.cc

@@ -307,7 +307,7 @@ Status GeoDBImpl::searchQuadIds(const GeoPosition& position,
 
   // how many level of details to look for
   int numberOfTilesAtMaxDepth = floor((bottomRight.x - topLeft.x) / 256);
-  int zoomLevelsToRise = floor(log(numberOfTilesAtMaxDepth) / log(2));
+  int zoomLevelsToRise = floor(::log(numberOfTilesAtMaxDepth) / ::log(2));
   zoomLevelsToRise++;
   int levels = std::max(0, Detail - zoomLevelsToRise);
 
@@ -344,7 +344,7 @@ GeoDBImpl::Pixel GeoDBImpl::PositionToPixel(const GeoPosition& pos,
   double latitude = clip(pos.latitude, MinLatitude, MaxLatitude);
   double x = (pos.longitude + 180) / 360;
   double sinLatitude = sin(latitude * PI / 180);
-  double y = 0.5 - log((1 + sinLatitude) / (1 - sinLatitude)) / (4 * PI);
+  double y = 0.5 - ::log((1 + sinLatitude) / (1 - sinLatitude)) / (4 * PI);
   double mapSize = MapSize(levelOfDetail);
   double X = floor(clip(x * mapSize + 0.5, 0, mapSize - 1));
   double Y = floor(clip(y * mapSize + 0.5, 0, mapSize - 1));

utilities/spatialdb/spatial_db.cc

@@ -5,15 +5,20 @@
 
 #ifndef ROCKSDB_LITE
 
+#include "rocksdb/utilities/spatial_db.h"
+
+#define __STDC_FORMAT_MACROS
+#include <inttypes.h>
 #include <string>
 #include <vector>
 #include <algorithm>
+#include <atomic>
 #include <set>
+#include <unordered_set>
 
 #include "rocksdb/cache.h"
 #include "rocksdb/db.h"
 #include "rocksdb/utilities/stackable_db.h"
-#include "rocksdb/utilities/spatial_db.h"
 #include "util/coding.h"
 #include "utilities/spatialdb/utils.h"
 
@@ -197,25 +202,106 @@ bool FeatureSet::Deserialize(const Slice& input) {
   return true;
 }
 
+std::string FeatureSet::DebugString() const {
+  std::string out = "{";
+  bool comma = false;
+  for (const auto& iter : map_) {
+    if (comma) {
+      out.append(", ");
+    } else {
+      comma = true;
+    }
+    out.append("\"" + iter.first + "\": ");
+    switch (iter.second.type()) {
+      case Variant::kNull:
+        out.append("null");
+      case Variant::kBool:
+        if (iter.second.get_bool()) {
+          out.append("true");
+        } else {
+          out.append("false");
+        }
+        break;
+      case Variant::kInt: {
+        char buf[32];
+        snprintf(buf, sizeof(buf), "%" PRIu64, iter.second.get_int());
+        out.append(buf);
+        break;
+      }
+      case Variant::kDouble: {
+        char buf[32];
+        snprintf(buf, sizeof(buf), "%lf", iter.second.get_double());
+        out.append(buf);
+        break;
+      }
+      case Variant::kString:
+        out.append("\"" + iter.second.get_string() + "\"");
+        break;
+      default:
+        assert(false);
+    }
+  }
+  return out + "}";
+}
+
 class SpatialIndexCursor : public Cursor {
  public:
+  // tile_box is inclusive
   SpatialIndexCursor(Iterator* spatial_iterator, Iterator* data_iterator,
                      const BoundingBox<uint64_t>& tile_bbox, uint32_t tile_bits)
-      : spatial_iterator_(spatial_iterator),
-        data_iterator_(data_iterator),
-        tile_bbox_(tile_bbox),
-        tile_bits_(tile_bits),
+      : data_iterator_(data_iterator),
         valid_(true) {
-    current_x_ = tile_bbox.min_x;
-    current_y_ = tile_bbox.min_y;
-    UpdateQuadKey();
-    ReSeek();
-    if (valid_) {
-      // this is the first ID returned, so I don't care about return value of
-      // Dedup
-      Dedup();
+    // calculate quad keys we'll need to query
+    std::vector<uint64_t> quad_keys;
+    quad_keys.reserve((tile_bbox.max_x - tile_bbox.min_x + 1) *
+                      (tile_bbox.max_y - tile_bbox.min_y + 1));
+    for (uint64_t x = tile_bbox.min_x; x <= tile_bbox.max_x; ++x) {
+      for (uint64_t y = tile_bbox.min_y; y <= tile_bbox.max_y; ++y) {
+        quad_keys.push_back(GetQuadKeyFromTile(x, y, tile_bits));
+      }
     }
+    std::sort(quad_keys.begin(), quad_keys.end());
+
+    // load primary key ids for all quad keys
+    for (auto quad_key : quad_keys) {
+      std::string encoded_quad_key;
+      PutFixed64BigEndian(&encoded_quad_key, quad_key);
+      Slice slice_quad_key(encoded_quad_key);
+
+      // If CheckQuadKey is true, there is no need to reseek, since
+      // spatial_iterator is already pointing at the correct quad key. This is
+      // an optimization.
+      if (!CheckQuadKey(spatial_iterator, slice_quad_key)) {
+        spatial_iterator->Seek(slice_quad_key);
+      }
+
+      while (CheckQuadKey(spatial_iterator, slice_quad_key)) {
+        // extract ID from spatial_iterator
+        uint64_t id;
+        bool ok = GetFixed64BigEndian(
+            Slice(spatial_iterator->key().data() + sizeof(uint64_t),
+                  sizeof(uint64_t)),
+            &id);
+        if (!ok) {
+          valid_ = false;
+          status_ = Status::Corruption("Spatial index corruption");
+          break;
+        }
+        primary_key_ids_.insert(id);
+        spatial_iterator->Next();
+      }
+    }
+
+    if (!spatial_iterator->status().ok()) {
+      status_ = spatial_iterator->status();
+      valid_ = false;
+    }
+    delete spatial_iterator;
+
+    valid_ = valid_ && primary_key_ids_.size() > 0;
+
     if (valid_) {
+      primary_keys_iterator_ = primary_key_ids_.begin();
       ExtractData();
     }
   }
@@ -225,28 +311,13 @@ class SpatialIndexCursor : public Cursor {
   virtual void Next() override {
     assert(valid_);
 
-    // this do-while loop deals only with deduplication
-    do {
-      spatial_iterator_->Next();
-      if (ExtractID()) {
-        // OK, found what we needed
-        continue;
-      }
-
-      // move to the next tile
-      Increment();
-
-      if (ExtractID()) {
-        // no need to reseek, found what we needed
-        continue;
-      }
-      // reseek, find next good tile
-      ReSeek();
-    } while (valid_ && !Dedup() && valid_);
-
-    if (valid_) {
-      ExtractData();
+    ++primary_keys_iterator_;
+    if (primary_keys_iterator_ == primary_key_ids_.end()) {
+      valid_ = false;
+      return;
     }
+
+    ExtractData();
   }
 
   virtual const Slice blob() override { return current_blob_; }
@@ -258,88 +329,44 @@ class SpatialIndexCursor : public Cursor {
     if (!status_.ok()) {
       return status_;
     }
-    if (!spatial_iterator_->status().ok()) {
-      return spatial_iterator_->status();
-    }
     return data_iterator_->status();
   }
 
  private:
-  // returns true if OK, false if already returned (duplicate)
-  bool Dedup() {
-    assert(valid_);
-    uint64_t id;
-    bool ok = GetFixed64BigEndian(current_id_, &id);
-    if (!ok) {
-      valid_ = false;
-      status_ = Status::Corruption("Spatial index corruption");
-      return false;
-    }
-    if (returned_ids_.find(id) != returned_ids_.end()) {
-      return false;
-    }
-    returned_ids_.insert(id);
-    return true;
-  }
-  void ReSeek() {
-    while (valid_) {
-      spatial_iterator_->Seek(current_quad_key_);
-      if (ExtractID()) {
-        // found what we're looking for!
-        break;
-      }
-      Increment();
-    }
-  }
-
-  void Increment() {
-    ++current_x_;
-    if (current_x_ > tile_bbox_.max_x) {
-      current_x_ = tile_bbox_.min_x;
-      ++current_y_;
-    }
-    if (current_y_ > tile_bbox_.max_y) {
-      valid_ = false;
-    } else {
-      UpdateQuadKey();
-    }
-  }
-  void UpdateQuadKey() {
-    current_quad_key_.clear();
-    PutFixed64BigEndian(&current_quad_key_,
-                        GetQuadKeyFromTile(current_x_, current_y_, tile_bits_));
-  }
   // * returns true if spatial iterator is on the current quad key and all is
-  // well. Caller will call Next() to get new data
-  // * returns false if spatial iterator is not on current, or invalid or status
-  // bad. Caller will need to reseek to get new data
-  bool ExtractID() {
-    if (!spatial_iterator_->Valid()) {
-      // caller needs to reseek
+  // well
+  // * returns false if spatial iterator is not on current, or iterator is
+  // invalid or corruption
+  bool CheckQuadKey(Iterator* spatial_iterator, const Slice& quad_key) {
+    if (!spatial_iterator->Valid()) {
       return false;
     }
-    if (spatial_iterator_->key().size() != 2 * sizeof(uint64_t)) {
+    if (spatial_iterator->key().size() != 2 * sizeof(uint64_t)) {
       status_ = Status::Corruption("Invalid spatial index key");
       valid_ = false;
       return false;
     }
-    Slice quad_key(spatial_iterator_->key().data(), sizeof(uint64_t));
-    if (quad_key != current_quad_key_) {
+    Slice spatial_iterator_quad_key(spatial_iterator->key().data(),
+                                    sizeof(uint64_t));
+    if (spatial_iterator_quad_key != quad_key) {
       // caller needs to reseek
       return false;
     }
     // if we come to here, we have found the quad key
-    current_id_ = Slice(spatial_iterator_->key().data() + sizeof(uint64_t),
-                        sizeof(uint64_t));
     return true;
   }
+
   // doesn't return anything, but sets valid_ and status_ on corruption
   void ExtractData() {
     assert(valid_);
-    data_iterator_->Seek(current_id_);
+    std::string encoded_id;
+    PutFixed64BigEndian(&encoded_id, *primary_keys_iterator_);
 
-    if (!data_iterator_->Valid() || data_iterator_->key() != current_id_) {
-      status_ = Status::Corruption("Inconsistency in data column family");
+    data_iterator_->Seek(encoded_id);
+
+    if (!data_iterator_->Valid() ||
+        data_iterator_->key() != Slice(encoded_id)) {
+      status_ = Status::Corruption("Index inconsistency");
       valid_ = false;
       return;
     }
@@ -348,28 +375,22 @@ class SpatialIndexCursor : public Cursor {
     current_feature_set_.Clear();
     if (!GetLengthPrefixedSlice(&data, &current_blob_) ||
         !current_feature_set_.Deserialize(data)) {
-      status_ = Status::Corruption("Data column family corruption");
+      status_ = Status::Corruption("Primary key column family corruption");
       valid_ = false;
       return;
     }
   }
 
-  unique_ptr<Iterator> spatial_iterator_;
   unique_ptr<Iterator> data_iterator_;
-  BoundingBox<uint64_t> tile_bbox_;
-  uint32_t tile_bits_;
-  uint64_t current_x_;
-  uint64_t current_y_;
-  std::string current_quad_key_;
-  Slice current_id_;
   bool valid_;
   Status status_;
 
   FeatureSet current_feature_set_;
   Slice current_blob_;
 
-  // used for deduplicating results
-  std::set<uint64_t> returned_ids_;
+  // This is loaded from spatial iterator.
+  std::unordered_set<uint64_t> primary_key_ids_;
+  std::unordered_set<uint64_t>::iterator primary_keys_iterator_;
 };
 
 class ErrorCursor : public Cursor {