more fixes

Author: dranikpg

src/server/tiered_storage.cc

@@ -225,7 +225,7 @@ class TieredStorage::ShardOpManager : public tiering::OpManager {
   bool NotifyFetched(const OwnedEntryId& id, tiering::DiskSegment segment,
                      tiering::Decoder* decoder) override;
 
-  bool NotifyDelete(tiering::DiskSegment segment, bool was_read) override;
+  bool NotifyDelete(tiering::DiskSegment segment, bool in_memory) override;
 
   void EnqueueForDefrag(tiering::DiskSegment segment);
 
@@ -414,7 +414,7 @@ bool TieredStorage::ShardOpManager::NotifyFetched(const OwnedEntryId& id,
   return false;
 }
 
-bool TieredStorage::ShardOpManager::NotifyDelete(tiering::DiskSegment segment, bool was_read) {
+bool TieredStorage::ShardOpManager::NotifyDelete(tiering::DiskSegment segment, bool in_memory) {
   DVLOG(2) << "NotifyDelete [" << segment.offset << "," << segment.length << "]";
 
   if (OccupiesWholePages(segment.length))
@@ -428,8 +428,8 @@ bool TieredStorage::ShardOpManager::NotifyDelete(tiering::DiskSegment segment, b
   // If we have memory, upload the page for defrag. It will be reshuffled and offloaded more packed.
   // Otherwise background scans of fragmented bins will discover them
   if (bin.fragmented && ts_->UploadBudget() > 0) {
-    // Limit number of IO operations if we need to read from disk (was_read is false)
-    if (was_read || stats_.pending_defrags < kMaxPendingDefrags) {
+    // Limit number of IO operations if we need to read from disk (in_memory is false)
+    if (in_memory || stats_.pending_defrags < kMaxPendingDefrags) {
       EnqueueForDefrag(bin.segment);
     }
   }
@@ -668,7 +668,7 @@ void TieredStorage::RunOffloading(DbIndex dbid) {
 
   const auto start_cycles = base::CycleClock::Now();
 
-  // Takes up a small fixed amount of time and is best done before offloading (to be picked up)
+  // Takes up a small bounded amount of time and is best done before offloading (to be picked up)
   RunDefragScan();
 
   // Don't run offloading if there's only very little space left

src/server/tiering/op_manager.h

@@ -94,9 +94,9 @@ class OpManager {
   virtual bool NotifyFetched(const OwnedEntryId& id, DiskSegment segment, Decoder*) = 0;
 
   // Notify delete. Return true if the filled segment needs to be marked as free.
-  // Under was_read this function was called after a read completion as the follow-up delete
+  // Under in_memory this function was called after a read completion as the follow-up delete
   // operation - the full page is still in the op_managers pending operations cache
-  virtual bool NotifyDelete(DiskSegment segment, bool was_read) = 0;
+  virtual bool NotifyDelete(DiskSegment segment, bool in_memory) = 0;
 
   // Describes pending read futures for a single entry
   struct EntryOps {

src/server/tiering/op_manager_test.cc

@@ -78,7 +78,7 @@ struct OpManagerTest : PoolTestBase, OpManager {
     return false;
   }
 
-  bool NotifyDelete(DiskSegment segment, bool from_read) override {
+  bool NotifyDelete(DiskSegment segment, bool in_memory) override {
     return true;
   }
 

src/server/tiering/small_bins.cc

@@ -85,9 +85,9 @@ size_t SmallBins::SerializeBin(FilledBin* bin, io::MutableBytes dest) {
 }
 
 SmallBins::KeySegmentList SmallBins::ReportStashed(BinId id, DiskSegment segment) {
-  DVLOG(1) << "ReportStashed " << id;
+  DCHECK(pending_bins_.contains(id));                    // valid pending operation id
+  DCHECK(stashed_bins_.Find(segment.offset).is_done());  // new unoccupied destination
 
-  DCHECK(pending_bins_.contains(id));
   auto seg_map_node = pending_bins_.extract(id);
   const auto& seg_map = seg_map_node.mapped();
   DCHECK_GT(seg_map.size(), 0u) << id;

src/server/tiering/small_bins.h

@@ -122,16 +122,8 @@ class SmallBins {
     }
 
     static uint64_t HashFn(uint64_t v) {
-      // Keys are page-aligned disk offsets (multiples of kPageSize), so their low bits are always
-      // zero and their high bits are zero for small files. std::hash<uint64_t> is the identity on
-      // libstdc++, which would funnel every bin into a single segment and make DashTable split
-      // forever. Mix the bits (murmur3 fmix64) so offsets distribute evenly.
-      v ^= v >> 33;
-      v *= 0xff51afd7ed558ccdULL;
-      v ^= v >> 33;
-      v *= 0xc4ceb9fe1a85ec53ULL;
-      v ^= v >> 33;
-      return v;
+      // Keys are page alined (% 4096 = 0) which breaks hashing, so scale down to page index
+      return v / kPageSize;
     }
   };