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

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

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

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

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

Test Plan: CI

Reviewed By: hx235

Differential Revision: D50616172

Pulled By: pdillinger

fbshipit-source-id: fa7f778bc281e881a140522e774f480c6d1e5f48

HISTORY.md

@@ -1,12 +1,47 @@
 # Rocksdb Change Log
 > NOTE: Entries for next release do not go here. Follow instructions in `unreleased_history/README.txt`
 
+## 8.5.4 (09/26/2023)
+### Bug Fixes
+* Fixed a bug where compaction read under non direct IO still falls back to RocksDB internal prefetching after file system's prefetching returns non-OK status other than `Status::NotSupported()`
+
+### Behavior Changes
+* For non direct IO, eliminate the file system prefetching attempt for compaction read when `Options::compaction_readahead_size` is 0
+
+## 8.5.3 (09/01/2023)
+### Bug Fixes
+* Fixed a race condition in `GenericRateLimiter` that could cause it to stop granting requests
+
+## 8.5.2 (08/31/2023)
+### Bug fixes
+* Fix a bug where iterator may return incorrect result for DeleteRange() users if there was an error reading from a file.
+
+## 8.5.1 (08/31/2023)
+### Bug fixes
+* Fix a bug where if there is an error reading from offset 0 of a file from L1+ and that the file is not the first file in the sorted run, data can be lost in compaction and read/scan can return incorrect results.
+
+## 8.5.0 (07/21/2023)
+### Public API Changes
+* Removed recently added APIs `GeneralCache` and `MakeSharedGeneralCache()` as our plan changed to stop exposing a general-purpose cache interface. The old forms of these APIs, `Cache` and `NewLRUCache()`, are still available, although general-purpose caching support will be dropped eventually.
+
+### Behavior Changes
+* Option `periodic_compaction_seconds` no longer supports FIFO compaction: setting it has no effect on FIFO compactions. FIFO compaction users should only set option `ttl` instead.
+* Move prefetching responsibility to page cache for compaction read for non directIO use case
+
+### Performance Improvements
+* In case of direct_io, if buffer passed by callee is already aligned, RandomAccessFileRead::Read will avoid realloacting a new buffer, reducing memcpy and use already passed aligned buffer.
+* Small efficiency improvement to HyperClockCache by reducing chance of compiler-generated heap allocations
+
+### Bug Fixes
+* Fix use_after_free bug in async_io MultiReads when underlying FS enabled kFSBuffer. kFSBuffer is when underlying FS pass their own buffer instead of using RocksDB scratch in FSReadRequest. Right now it's an experimental feature.
+* Fix a bug in FileTTLBooster that can cause users with a large number of levels (more than 65) to see errors like "runtime error: shift exponent .. is too large.." (#11673).
+
 ## 8.4.0 (06/26/2023)
 ### New Features
 * Add FSReadRequest::fs_scratch which is a data buffer allocated and provided by underlying FileSystem to RocksDB during reads, when FS wants to provide its own buffer with data instead of using RocksDB provided FSReadRequest::scratch. This can help in cpu optimization by avoiding copy from file system's buffer to RocksDB buffer. More details on how to use/enable it in file_system.h. Right now its supported only for MultiReads(async + sync) with non direct io.
 * Start logging non-zero user-defined timestamp sizes in WAL to signal user key format in subsequent records and use it during recovery. This change will break recovery from WAL files written by early versions that contain user-defined timestamps. The workaround is to ensure there are no WAL files to recover (i.e. by flushing before close) before upgrade.
 * Added new property "rocksdb.obsolete-sst-files-size-property" that reports the size of SST files that have become obsolete but have not yet been deleted or scheduled for deletion
-* Start to record the value of the flag `AdvancedColumnFamilyOptions.persist_user_defined_timestamps` in the Manifest and table properties for a SST file when it is created. And use the recorded flag when creating a table reader for the SST file. This flag is only explicitly record if it's false. 
+* Start to record the value of the flag `AdvancedColumnFamilyOptions.persist_user_defined_timestamps` in the Manifest and table properties for a SST file when it is created. And use the recorded flag when creating a table reader for the SST file. This flag is only explicitly record if it's false.
 * Add a new option OptimisticTransactionDBOptions::shared_lock_buckets that enables sharing mutexes for validating transactions between DB instances, for better balancing memory efficiency and validation contention across DB instances. Different column families and DBs also now use different hash seeds in this validation, so that the same set of key names will not contend across DBs or column families.
 * Add a new ticker `rocksdb.files.marked.trash.deleted` to track the number of trash files deleted by background thread from the trash queue.
 * Add an API NewTieredVolatileCache() in include/rocksdb/cache.h to allocate an instance of a block cache with a primary block cache tier and a compressed secondary cache tier. A cache of this type distributes memory reservations against the block cache, such as WriteBufferManager, table reader memory etc., proportionally across both the primary and compressed secondary cache.
@@ -30,7 +65,7 @@ For Leveled Compaction users, `CompactRange()` with `bottommost_level_compaction
 ### Bug Fixes
 * Reduced cases of illegally using Env::Default() during static destruction by never destroying the internal PosixEnv itself (except for builds checking for memory leaks). (#11538)
 * Fix extra prefetching during seek in async_io when BlockBasedTableOptions.num_file_reads_for_auto_readahead is 1 leading to extra reads than required.
-* Fix a bug where compactions that are qualified to be run as 2 subcompactions were only run as one subcompaction. 
+* Fix a bug where compactions that are qualified to be run as 2 subcompactions were only run as one subcompaction.
 * Fix a use-after-move bug in block.cc.
 
 ## 8.3.0 (05/19/2023)

db/compaction/compaction_picker_test.cc

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

db/compaction/file_pri.h

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

env/unique_id_gen.cc

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

file/prefetch_test.cc

@@ -151,6 +151,7 @@ TEST_P(PrefetchTest, Basic) {
   Options options;
   SetGenericOptions(env.get(), use_direct_io, options);
   options.statistics = CreateDBStatistics();
+  options.compaction_readahead_size = 2 * 1024 * 1024;
 
   const int kNumKeys = 1100;
   int buff_prefetch_count = 0;
@@ -238,16 +239,9 @@ TEST_P(PrefetchTest, Basic) {
     fs->ClearPrefetchCount();
   } else {
     ASSERT_FALSE(fs->IsPrefetchCalled());
-    if (use_direct_io) {
-      // To rule out false positive by the SST file tail prefetch during
-      // compaction output verification
-      ASSERT_GT(buff_prefetch_count, 1);
-    } else {
-      // In buffered IO, compaction readahead size is 0, leading to no prefetch
-      // during compaction input read
-      ASSERT_EQ(buff_prefetch_count, 1);
-    }
-
+    // To rule out false positive by the SST file tail prefetch during
+    // compaction output verification
+    ASSERT_GT(buff_prefetch_count, 1);
     buff_prefetch_count = 0;
 
     ASSERT_GT(cur_table_open_prefetch_tail_read.count,

include/rocksdb/version.h

@@ -13,7 +13,7 @@
 // minor or major version number planned for release.
 #define ROCKSDB_MAJOR 8
 #define ROCKSDB_MINOR 5
-#define ROCKSDB_PATCH 0
+#define ROCKSDB_PATCH 4
 
 // Do not use these. We made the mistake of declaring macros starting with
 // double underscore. Now we have to live with our choice. We'll deprecate these

table/block_based/block_prefetcher.cc

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

table/compaction_merging_iterator.cc

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

table/merging_iterator.cc

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

unreleased_history/behavior_changes/fifo_ttl_periodic_compaction.md

@@ -1 +0,0 @@
-Option `periodic_compaction_seconds` no longer supports FIFO compaction: setting it has no effect on FIFO compactions. FIFO compaction users should only set option `ttl` instead.

unreleased_history/behavior_changes/fs_prefetch_compaction_read.md

@@ -1 +0,0 @@
-Move prefetching responsibility to page cache for compaction read for non directIO use case

unreleased_history/performance_improvements/avoid_memcpy_directio.md

@@ -1 +0,0 @@
-In case of direct_io, if buffer passed by callee is already aligned, RandomAccessFileRead::Read will avoid realloacting a new buffer, reducing memcpy and use already passed aligned buffer.

unreleased_history/performance_improvements/hcc_perf

@@ -1 +0,0 @@
-Small efficiency improvement to HyperClockCache by reducing chance of compiler-generated heap allocations

unreleased_history/public_api_changes/rename_migration_caches.md

@@ -1 +0,0 @@
-Removed recently added APIs `GeneralCache` and `MakeSharedGeneralCache()` as our plan changed to stop exposing a general-purpose cache interface. The old forms of these APIs, `Cache` and `NewLRUCache()`, are still available, although general-purpose caching support will be dropped eventually.

unreleased_history/release.sh

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

util/async_file_reader.cc

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

util/crc32c.cc

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

util/rate_limiter.cc

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