Cleanup

docs/metrics.md

@@ -44,7 +44,7 @@ bucketlistDB-<X>.bulk.loads                 | meter     | number of entries Buck
 bucketlistDB-live.bulk.inflationWinners     | timer     | time to load inflation winners
 bucketlistDB-live.bulk.poolshareTrustlines  | timer     | time to load poolshare trustlines by accountID and assetID
 bucketlistDB-live.bulk.prefetch             | timer     | time to prefetch
-bucketlistDB-<X>.point.<y>                | timer     | time to load single entry of type <Y> on BucketList <X> (live/hotArchive)
+bucketlistDB-<X>.point.<Y>                | timer     | time to load single entry of type <Y> on BucketList <X> (live/hotArchive)
 bucketlistDB-cache.hit                    | meter     | number of cache hits on Live BucketList Disk random eviction cache
 bucketlistDB-cache.miss                   | meter     | number of cache misses on Live BucketList Disk random eviction cache
 crypto.verify.hit                         | meter     | number of signature cache hits

docs/stellar-core_example.cfg

@@ -247,7 +247,7 @@ BUCKETLIST_DB_CACHED_PERCENT = 25
 # Size, in MB, determining whether a bucket should have an individual
 # key index or a key range index. If bucket size is below this value, range
 # based index will be used. If set to 0, all buckets are range indexed. If
-# BUCKETLIST_DB_INDEX_PAGE_SIZE_EXPONENT == 0, value ingnored and all
+# BUCKETLIST_DB_INDEX_PAGE_SIZE_EXPONENT == 0, value ignored and all
 # buckets have individual key index.
 BUCKETLIST_DB_INDEX_CUTOFF = 250
 

src/bucket/BucketIndexUtils.cpp

@@ -31,8 +31,7 @@ getPageSizeFromConfig(Config const& cfg)
 template <class BucketT>
 std::unique_ptr<typename BucketT::IndexT const>
 createIndex(BucketManager& bm, std::filesystem::path const& filename,
-            Hash const& hash, asio::io_context& ctx,
-            std::optional<SHA256>& hasher)
+            Hash const& hash, asio::io_context& ctx, SHA256* hasher)
 {
     BUCKET_TYPE_ASSERT(BucketT);
 
@@ -89,12 +88,12 @@ loadIndex(BucketManager const& bm, std::filesystem::path const& filename,
 template std::unique_ptr<typename LiveBucket::IndexT const>
 createIndex<LiveBucket>(BucketManager& bm,
                         std::filesystem::path const& filename, Hash const& hash,
-                        asio::io_context& ctx, std::optional<SHA256>& hasher);
+                        asio::io_context& ctx, SHA256* hasher);
 template std::unique_ptr<typename HotArchiveBucket::IndexT const>
 createIndex<HotArchiveBucket>(BucketManager& bm,
                               std::filesystem::path const& filename,
                               Hash const& hash, asio::io_context& ctx,
-                              std::optional<SHA256>& hasher);
+                              SHA256* hasher);
 
 template std::unique_ptr<typename LiveBucket::IndexT const>
 loadIndex<LiveBucket>(BucketManager const& bm,

src/bucket/BucketIndexUtils.h

@@ -101,8 +101,7 @@ std::streamoff getPageSizeFromConfig(Config const& cfg);
 template <class BucketT>
 std::unique_ptr<typename BucketT::IndexT const>
 createIndex(BucketManager& bm, std::filesystem::path const& filename,
-            Hash const& hash, asio::io_context& ctx,
-            std::optional<SHA256>& hasher);
+            Hash const& hash, asio::io_context& ctx, SHA256* hasher);
 
 // Loads index from given file. If file does not exist or if saved
 // index does not have expected version or pageSize, return null

src/bucket/BucketOutputIterator.cpp

@@ -198,9 +198,8 @@ BucketOutputIterator<BucketT>::getBucket(BucketManager& bucketManager,
     if (auto b = bucketManager.getBucketIfExists<BucketT>(hash);
         !b || !b->isIndexed())
     {
-        std::optional<SHA256> empty{};
         index =
-            createIndex<BucketT>(bucketManager, mFilename, hash, mCtx, empty);
+            createIndex<BucketT>(bucketManager, mFilename, hash, mCtx, nullptr);
     }
 
     return bucketManager.adoptFileAsBucket<BucketT>(mFilename.string(), hash,

src/bucket/BucketUtils.h

@@ -171,6 +171,9 @@ class EvictionStatistics
                                       EvictionCounters& counters);
 };
 
+// Enum for more granular LedgerEntry types for Bucket metric reporting.
+// Specifically, this enum differentiates between TEMPORARY and PERSISTENT
+// CONTRACT_DATA types, which the regular LedgerEntryType enum does not.
 enum class LedgerEntryTypeAndDurability : uint32_t
 {
     ACCOUNT = 0,
@@ -187,6 +190,7 @@ enum class LedgerEntryTypeAndDurability : uint32_t
     NUM_TYPES = 11,
 };
 
+// Metrics for BucketEntry state by LedgerEntryTypeAndDurability
 struct BucketEntryCounters
 {
     std::map<LedgerEntryTypeAndDurability, size_t> entryTypeCounts;

src/bucket/DiskIndex.cpp

@@ -65,7 +65,7 @@ DiskIndex<BucketT>::scan(IterT start, LedgerKey const& k) const
 
     // Search for the key in the index before checking the bloom filter so we
     // return the correct iterator to the caller. This may be slightly less
-    // effecient then checking the bloom filter first, but the filter's primary
+    // efficient then checking the bloom filter first, but the filter's primary
     // purpose is to avoid disk lookups, not to avoid in-memory index search.
     auto keyIter =
         std::lower_bound(start, mData.keysToOffset.end(), k, lower_bound_pred);
@@ -120,8 +120,7 @@ template <class BucketT>
 DiskIndex<BucketT>::DiskIndex(BucketManager& bm,
                               std::filesystem::path const& filename,
                               std::streamoff pageSize, Hash const& hash,
-                              asio::io_context& ctx,
-                              std::optional<SHA256>& hasher)
+                              asio::io_context& ctx, SHA256* hasher)
     : mBloomLookupMeter(bm.getBloomLookupMeter<BucketT>())
     , mBloomMissMeter(bm.getBloomMissMeter<BucketT>())
 {
@@ -144,15 +143,14 @@ DiskIndex<BucketT>::DiskIndex(BucketManager& bm,
     std::streamoff pageUpperBound = 0;
     typename BucketT::EntryT be;
     size_t iter = 0;
-    [[maybe_unused]] size_t count = 0;
+    size_t count = 0;
 
     std::vector<uint64_t> keyHashes;
     auto seed = shortHash::getShortHashInitKey();
-    SHA256* hasherPtr = hasher.has_value() ? &hasher.value() : nullptr;
 
-    while (in && in.readOne(be, hasherPtr))
+    while (in && in.readOne(be, hasher))
     {
-        // peridocially check if bucket manager is exiting to stop indexing
+        // periodically check if bucket manager is exiting to stop indexing
         // gracefully
         if (++iter >= 1000)
         {
@@ -240,6 +238,7 @@ DiskIndex<BucketT>::DiskIndex(BucketManager& bm,
             {
                 mData.filter =
                     std::make_unique<BinaryFuseFilter16>(keyHashes, seed);
+                break;
             }
             catch (std::out_of_range& e)
             {
@@ -259,6 +258,9 @@ DiskIndex<BucketT>::DiskIndex(BucketManager& bm,
                 seed[0]++;
             }
         }
+
+        // Population failure is probabilistic is very, very unlikely.
+        releaseAssertOrThrow(mData.filter);
     }
 
     CLOG_DEBUG(Bucket, "Indexed {} positions in {}", mData.keysToOffset.size(),

src/bucket/DiskIndex.h

@@ -120,7 +120,7 @@ template <class BucketT> class DiskIndex : public NonMovableOrCopyable
     // Constructor for creating a fresh index.
     DiskIndex(BucketManager& bm, std::filesystem::path const& filename,
               std::streamoff pageSize, Hash const& hash, asio::io_context& ctx,
-              std::optional<SHA256>& hasher);
+              SHA256* hasher);
 
     // Constructor for loading pre-existing index from disk. Must call preLoad
     // before calling this constructor to properly deserialize index.

src/bucket/HotArchiveBucketIndex.cpp

@@ -15,7 +15,7 @@ namespace stellar
 
 HotArchiveBucketIndex::HotArchiveBucketIndex(
     BucketManager& bm, std::filesystem::path const& filename, Hash const& hash,
-    asio::io_context& ctx, std::optional<SHA256>& hasher)
+    asio::io_context& ctx, SHA256* hasher)
     : mDiskIndex(bm, filename, getPageSize(bm.getConfig(), 0), hash, ctx,
                  hasher)
 {

src/bucket/HotArchiveBucketIndex.h

@@ -58,7 +58,7 @@ class HotArchiveBucketIndex : public NonMovableOrCopyable
     HotArchiveBucketIndex(BucketManager& bm,
                           std::filesystem::path const& filename,
                           Hash const& hash, asio::io_context& ctx,
-                          std::optional<SHA256>& hasher);
+                          SHA256* hasher);
 
     template <class Archive>
     HotArchiveBucketIndex(BucketManager const& bm, Archive& ar,

src/bucket/InMemoryIndex.cpp

@@ -51,7 +51,7 @@ InMemoryBucketState::scan(IterT start, LedgerKey const& searchKey) const
 
 InMemoryIndex::InMemoryIndex(BucketManager const& bm,
                              std::filesystem::path const& filename,
-                             std::optional<SHA256>& hasher)
+                             SHA256* hasher)
 {
     XDRInputFileStream in;
     in.open(filename.string());
@@ -60,9 +60,8 @@ InMemoryIndex::InMemoryIndex(BucketManager const& bm,
     std::streamoff lastOffset = 0;
     std::optional<std::streamoff> firstOffer;
     std::optional<std::streamoff> lastOffer;
-    SHA256* hasherPtr = hasher.has_value() ? &hasher.value() : nullptr;
 
-    while (in && in.readOne(be, hasherPtr))
+    while (in && in.readOne(be, hasher))
     {
         if (++iter >= 1000)
         {

src/bucket/InMemoryIndex.h

@@ -68,8 +68,7 @@ class InMemoryIndex
     using IterT = InMemoryBucketState::IterT;
 
     InMemoryIndex(BucketManager const& bm,
-                  std::filesystem::path const& filename,
-                  std::optional<SHA256>& hasher);
+                  std::filesystem::path const& filename, SHA256* hasher);
 
     IterT
     begin() const

src/bucket/LiveBucketIndex.cpp

@@ -40,7 +40,7 @@ LiveBucketIndex::getPageSize(Config const& cfg, size_t bucketSize)
 LiveBucketIndex::LiveBucketIndex(BucketManager& bm,
                                  std::filesystem::path const& filename,
                                  Hash const& hash, asio::io_context& ctx,
-                                 std::optional<SHA256>& hasher)
+                                 SHA256* hasher)
     : mCacheHitMeter(bm.getCacheHitMeter())
     , mCacheMissMeter(bm.getCacheMissMeter())
 {

src/bucket/LiveBucketIndex.h

@@ -98,8 +98,7 @@ class LiveBucketIndex : public NonMovableOrCopyable
 
     // Constructor for creating new index from Bucketfile
     LiveBucketIndex(BucketManager& bm, std::filesystem::path const& filename,
-                    Hash const& hash, asio::io_context& ctx,
-                    std::optional<SHA256>& hasher);
+                    Hash const& hash, asio::io_context& ctx, SHA256* hasher);
 
     // Constructor for loading pre-existing index from disk
     template <class Archive>

src/bucket/readme.md

@@ -89,7 +89,7 @@ lookup speed and memory overhead. The following configuration flags control thes
     Larger values slow down lookup speed but
     decrease memory usage.
 - `BUCKETLIST_DB_INDEX_CUTOFF`
-  - Bucket file size, in MB, that determines wether the Bucket is cached in memory or not.
+  - Bucket file size, in MB, that determines whether Bucket is cached in memory or not.
     Default value is 250 MB, which indexes the first ~5 levels with the `IndividualIndex`.
     Larger values speed up lookups but increase memory usage.
 - `BUCKETLIST_DB_PERSIST_INDEX`

src/catchup/IndexBucketsWork.cpp

@@ -87,10 +87,9 @@ IndexBucketsWork::IndexWork::postWork()
             if (!self->mIndex)
             {
                 // TODO: Fix this when archive BucketLists assume state
-                std::optional<SHA256> empty{};
                 self->mIndex = createIndex<LiveBucket>(
                     bm, self->mBucket->getFilename(), self->mBucket->getHash(),
-                    ctx, empty);
+                    ctx, nullptr);
             }
 
             app.postOnMainThread(

src/historywork/VerifyBucketWork.cpp

@@ -59,7 +59,7 @@ VerifyBucketWork::spawnVerifier()
         std::static_pointer_cast<VerifyBucketWork>(shared_from_this()));
     app.postOnBackgroundThread(
         [&app, filename, weak, hash, &index = mIndex]() {
-            auto hasher = std::make_optional<SHA256>();
+            SHA256 hasher;
             asio::error_code ec;
 
             // No point in verifying buckets if things are shutting down
@@ -77,10 +77,10 @@ VerifyBucketWork::spawnVerifier()
 
                 index = createIndex<LiveBucket>(
                     app.getBucketManager(), filename, hash,
-                    app.getWorkerIOContext(), hasher);
+                    app.getWorkerIOContext(), &hasher);
                 releaseAssertOrThrow(index);
 
-                uint256 vHash = hasher->finish();
+                uint256 vHash = hasher.finish();
                 if (vHash == hash)
                 {
                     CLOG_DEBUG(History, "Verified hash ({}) for {}",