Cleaned up rebase

src/bucket/Bucket.cpp

@@ -1071,23 +1071,6 @@ HotArchiveBucket::isTombstoneEntry(HotArchiveBucketEntry const& e)
     return e.type() == HOT_ARCHIVE_LIVE;
 }
 
-template std::shared_ptr<LiveBucket> Bucket::merge<LiveBucket>(
-    BucketManager& bucketManager, uint32_t maxProtocolVersion,
-    std::shared_ptr<LiveBucket> const& oldBucket,
-    std::shared_ptr<LiveBucket> const& newBucket,
-    std::vector<std::shared_ptr<LiveBucket>> const& shadows,
-    bool keepTombstoneEntries, bool countMergeEvents, asio::io_context& ctx,
-    bool doFsync);
-
-template std::shared_ptr<HotArchiveBucket> Bucket::merge<HotArchiveBucket>(
-    BucketManager& bucketManager, uint32_t maxProtocolVersion,
-    std::shared_ptr<HotArchiveBucket> const& oldBucket,
-    std::shared_ptr<HotArchiveBucket> const& newBucket,
-    std::vector<std::shared_ptr<HotArchiveBucket>> const& shadows,
-    bool keepTombstoneEntries, bool countMergeEvents, asio::io_context& ctx,
-    bool doFsync);
-}
-
 BucketEntryCounters&
 BucketEntryCounters::operator+=(BucketEntryCounters const& other)
 {
@@ -1114,4 +1097,20 @@ BucketEntryCounters::operator!=(BucketEntryCounters const& other) const
 {
     return !(*this == other);
 }
+
+template std::shared_ptr<LiveBucket> Bucket::merge<LiveBucket>(
+    BucketManager& bucketManager, uint32_t maxProtocolVersion,
+    std::shared_ptr<LiveBucket> const& oldBucket,
+    std::shared_ptr<LiveBucket> const& newBucket,
+    std::vector<std::shared_ptr<LiveBucket>> const& shadows,
+    bool keepTombstoneEntries, bool countMergeEvents, asio::io_context& ctx,
+    bool doFsync);
+
+template std::shared_ptr<HotArchiveBucket> Bucket::merge<HotArchiveBucket>(
+    BucketManager& bucketManager, uint32_t maxProtocolVersion,
+    std::shared_ptr<HotArchiveBucket> const& oldBucket,
+    std::shared_ptr<HotArchiveBucket> const& newBucket,
+    std::vector<std::shared_ptr<HotArchiveBucket>> const& shadows,
+    bool keepTombstoneEntries, bool countMergeEvents, asio::io_context& ctx,
+    bool doFsync);
 }

src/bucket/Bucket.h

@@ -37,6 +37,14 @@ namespace stellar
  * Two buckets can be merged together efficiently (in a single pass): elements
  * from the newer bucket overwrite elements from the older bucket, the rest are
  * merged in sorted order, and all elements are hashed while being added.
+ *
+ * Different types of BucketList vary on the type of entries they contain and by
+ * extension the merge logic of those entries. Additionally, some types of
+ * BucketList may have special operations only relevant to that specific type.
+ * This pure virtual base class provides the core functionality of a BucketList
+ * container and must be extened for each specific BucketList type. In
+ * particular, the fresh and merge functions must be defined for the specific
+ * type, while other functionality can be shared.
  */
 
 class AbstractLedgerTxn;
@@ -45,6 +53,8 @@ class BucketManager;
 struct EvictionResultEntry;
 class EvictionStatistics;
 struct BucketEntryCounters;
+template <class BucketT> class SearchableBucketListSnapshot;
+enum class LedgerEntryTypeAndDurability : uint32_t;
 
 class Bucket : public NonMovableOrCopyable
 {
@@ -132,12 +142,18 @@ class Bucket : public NonMovableOrCopyable
     template <class BucketT> friend class BucketSnapshotBase;
 };
 
-template <class BucketT> class SearchableBucketListSnapshot;
+/*
+ * Live Buckets are used by the LiveBucketList to store the current canonical
+ * state of the ledger. They contain entries of type BucketEntry.
+ */
 class LiveBucket : public Bucket,
                    public std::enable_shared_from_this<LiveBucket>
 {
   public:
     LiveBucket();
+    virtual ~LiveBucket()
+    {
+    }
     LiveBucket(std::string const& filename, Hash const& hash,
                std::unique_ptr<BucketIndex const>&& index);
 
@@ -209,6 +225,10 @@ class LiveBucket : public Bucket,
     friend class LiveBucketSnapshot;
 };
 
+/*
+ * Hot Archive Buckets are used by the HotBucketList to store recently evicted
+ * entries. They contain entries of type HotArchiveBucketEntry.
+ */
 class HotArchiveBucket : public Bucket,
                          public std::enable_shared_from_this<HotArchiveBucket>
 {
@@ -219,6 +239,9 @@ class HotArchiveBucket : public Bucket,
 
   public:
     HotArchiveBucket();
+    virtual ~HotArchiveBucket()
+    {
+    }
     HotArchiveBucket(std::string const& filename, Hash const& hash,
                      std::unique_ptr<BucketIndex const>&& index);
     uint32_t getBucketVersion() const override;
@@ -237,7 +260,6 @@ class HotArchiveBucket : public Bucket,
     friend class HotArchiveBucketSnapshot;
 };
 
-enum class LedgerEntryTypeAndDurability : uint32_t;
 struct BucketEntryCounters
 {
     std::map<LedgerEntryTypeAndDurability, size_t> entryTypeCounts;

src/bucket/BucketIndexImpl.cpp

@@ -204,7 +204,11 @@ BucketIndexImpl<IndexT>::BucketIndexImpl(BucketManager& bm,
                 {
                     mData.keysToOffset.emplace_back(key, pos);
                 }
-                countEntry(be);
+
+                if constexpr (std::is_same<BucketEntryT, BucketEntry>::value)
+                {
+                    countEntry(be);
+                }
             }
 
             pos = in.pos();

src/bucket/BucketList.cpp

@@ -27,8 +27,8 @@ namespace stellar
 
 template <> BucketListDepth BucketListBase<LiveBucket>::kNumLevels = 11;
 
-// TODO: I made this number up, do some analysis and pick a better value or
-// make this a configurable network config.
+// TODO: This is an arbitrary number. Do some analysis and pick a better value
+// or make this a configurable network config.
 template <> BucketListDepth BucketListBase<HotArchiveBucket>::kNumLevels = 9;
 
 template <typename BucketT>
@@ -577,12 +577,6 @@ HotArchiveBucketList::addBatch(Application& app, uint32_t currLedger,
 
     for (uint32_t i = static_cast<uint32>(mLevels.size()) - 1; i != 0; --i)
     {
-        /*
-        CLOG_DEBUG(Bucket, "curr={}, half(i-1)={}, size(i-1)={},
-        roundDown(curr,half)={}, roundDown(curr,size)={}", currLedger,
-        levelHalf(i-1), levelSize(i-1), roundDown(currLedger, levelHalf(i-1)),
-        roundDown(currLedger, levelSize(i-1)));
-        */
         if (levelShouldSpill(currLedger, i - 1))
         {
             /**
@@ -771,7 +765,7 @@ LiveBucketList::addBatch(Application& app, uint32_t currLedger,
 }
 
 BucketEntryCounters
-BucketList::sumBucketEntryCounters() const
+LiveBucketList::sumBucketEntryCounters() const
 {
     BucketEntryCounters counters;
     for (auto const& lev : mLevels)

src/bucket/BucketList.h

@@ -401,6 +401,12 @@ class BucketListDepth
     template <class BucketT> friend class testutil::BucketListDepthModifier;
 };
 
+// While every BucketList shares the same high level structure wrt to spill
+// schedules, merges at the bucket level, etc, each BucketList type hold
+// different types of entries and has different merge logic at the individual
+// entry level. This pure virtual base class defines the shared structure of all
+// BucketLists. It must be extended for each specific BucketList type, where the
+// template parameter BucketT refers to the underlying Bucket type.
 template <class BucketT> class BucketListBase
 {
     static_assert(std::is_same_v<BucketT, LiveBucket> ||
@@ -503,6 +509,11 @@ template <class BucketT> class BucketListBase
     uint64_t getSize() const;
 };
 
+// The LiveBucketList stores the current canonical state of the ledger. It is
+// made up of LiveBucket buckets, which in turn store individual entries of type
+// BucketEntry. When an entry is "evicted" from the ledger, it is removed from
+// the LiveBucketList. Depending on the evicted entry type, it may then be added
+// to the HotArchiveBucketList.
 class LiveBucketList : public BucketListBase<LiveBucket>
 {
   public:
@@ -545,6 +556,9 @@ class LiveBucketList : public BucketListBase<LiveBucket>
     BucketEntryCounters sumBucketEntryCounters() const;
 };
 
+// The HotArchiveBucketList stores recently evicted entries. It contains Buckets
+// of type HotArchiveBucket, which store individual entries of type
+// HotArchiveBucketEntry.
 class HotArchiveBucketList : public BucketListBase<HotArchiveBucket>
 {
   private: