Skip to content

Commit cafb222

Browse files
SirTysonSirTyson
committed
Small fixes
1 parent 321d367 commit cafb222

7 files changed

+49
-61
lines changed

‎src/bucket/BucketManager.cpp

+21-34
Original file line numberDiff line numberDiff line change
@@ -1120,9 +1120,8 @@ BucketManager::startBackgroundEvictionScan(uint32_t ledgerSeq,
11201120
[bl = std::move(searchableBL), iter = cfg.evictionIterator(), ledgerSeq,
11211121
ledgerVers, sas, &counters = mBucketListEvictionCounters,
11221122
stats = mEvictionStatistics] {
1123-
return std::make_unique<EvictionResultCandidates>(
1124-
bl->scanForEviction(ledgerSeq, counters, iter, stats, sas,
1125-
ledgerVers));
1123+
return bl->scanForEviction(ledgerSeq, counters, iter, stats, sas,
1124+
ledgerVers);
11261125
});
11271126

11281127
mEvictionFuture = task->get_future();
@@ -1270,8 +1269,6 @@ BucketManager::assumeState(HistoryArchiveState const& has,
12701269
releaseAssert(threadIsMain());
12711270
releaseAssertOrThrow(mConfig.MODE_ENABLES_BUCKETLIST);
12721271

1273-
// Dependency: HAS supports Hot Archive BucketList
1274-
12751272
auto processBucketList = [&](auto& bl, auto const& hasBuckets) {
12761273
auto kNumLevels = std::remove_reference<decltype(bl)>::type::kNumLevels;
12771274
using BucketT =
@@ -1652,46 +1649,36 @@ BucketManager::scheduleVerifyReferencedBucketsWork(
16521649
continue;
16531650
}
16541651

1655-
// Returns filename, hash, and whether it's a hot archive bucket
1656-
auto loadFilenameAndHash =
1657-
[&]() -> std::tuple<std::string, Hash, bool> {
1658-
auto live = getBucketByHashInternal(h, mSharedLiveBuckets);
1659-
if (!live)
1660-
{
1661-
auto hot = getBucketByHashInternal(h, mSharedHotArchiveBuckets);
1662-
1663-
// Check both live and hot archive buckets for hash. If we don't
1664-
// find it in either, we're missing a bucket. Note that live and
1665-
// hot archive buckets are guaranteed to have no hash collisions
1666-
// due to type field in MetaEntry.
1667-
if (!hot)
1668-
{
1669-
throw std::runtime_error(
1670-
fmt::format(FMT_STRING("Missing referenced bucket {}"),
1671-
binToHex(h)));
1672-
}
1673-
return std::make_tuple(hot->getFilename().string(),
1674-
hot->getHash(), true);
1675-
}
1676-
else
1652+
auto maybeLiveBucket = getBucketByHashInternal(h, mSharedLiveBuckets);
1653+
if (!maybeLiveBucket)
1654+
{
1655+
auto hotBucket =
1656+
getBucketByHashInternal(h, mSharedHotArchiveBuckets);
1657+
1658+
// Check both live and hot archive buckets for hash. If we don't
1659+
// find it in either, we're missing a bucket. Note that live and
1660+
// hot archive buckets are guaranteed to have no hash collisions
1661+
// due to type field in MetaEntry.
1662+
if (!hotBucket)
16771663
{
1678-
return std::make_tuple(live->getFilename().string(),
1679-
live->getHash(), false);
1664+
throw std::runtime_error(fmt::format(
1665+
FMT_STRING("Missing referenced bucket {}"), binToHex(h)));
16801666
}
1681-
};
1682-
1683-
auto [filename, hash, isHot] = loadFilenameAndHash();
16841667

1685-
if (isHot)
1686-
{
1668+
auto filename = hotBucket->getFilename().string();
1669+
auto hash = hotBucket->getHash();
16871670
auto [indexIter, _] = hotIndexMap.emplace(i++, nullptr);
1671+
16881672
seq.emplace_back(
16891673
std::make_shared<VerifyBucketWork<HotArchiveBucket>>(
16901674
mApp, filename, hash, indexIter->second, nullptr));
16911675
}
16921676
else
16931677
{
1678+
auto filename = maybeLiveBucket->getFilename().string();
1679+
auto hash = maybeLiveBucket->getHash();
16941680
auto [indexIter, _] = liveIndexMap.emplace(i++, nullptr);
1681+
16951682
seq.emplace_back(std::make_shared<VerifyBucketWork<LiveBucket>>(
16961683
mApp, filename, hash, indexIter->second, nullptr));
16971684
}

‎src/bucket/BucketUtils.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
// under the Apache License, Version 2.0. See the COPYING file at the root
55
// of this distribution or at http://www.apache.org/licenses/LICENSE-2.0
66

7+
#include "util/NonCopyable.h"
78
#include "xdr/Stellar-ledger-entries.h"
89
#include <cstdint>
910
#include <list>
@@ -100,7 +101,7 @@ struct EvictionResultEntry
100101
// Hold the list of entries eligible for eviction on the given ledger. Note that
101102
// if these entries are updated during the given ledger, they may not actually
102103
// be evicted.
103-
struct EvictionResultCandidates
104+
struct EvictionResultCandidates : public NonMovableOrCopyable
104105
{
105106
// List of entries eligible for eviction in the order in which they occur in
106107
// the bucket

‎src/bucket/SearchableBucketList.cpp

+5-4
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313

1414
namespace stellar
1515
{
16-
EvictionResultCandidates
16+
std::unique_ptr<EvictionResultCandidates>
1717
SearchableLiveBucketListSnapshot::scanForEviction(
1818
uint32_t ledgerSeq, EvictionCounters& counters,
1919
EvictionIterator evictionIter, std::shared_ptr<EvictionStatistics> stats,
@@ -32,7 +32,8 @@ SearchableLiveBucketListSnapshot::scanForEviction(
3232
LiveBucketList::updateStartingEvictionIterator(
3333
evictionIter, sas.startingEvictionScanLevel, ledgerSeq);
3434

35-
EvictionResultCandidates result(sas, ledgerSeq, ledgerVers);
35+
std::unique_ptr<EvictionResultCandidates> result =
36+
std::make_unique<EvictionResultCandidates>(sas, ledgerSeq, ledgerVers);
3637
auto startIter = evictionIter;
3738
auto scanSize = sas.evictionScanSize;
3839

@@ -44,7 +45,7 @@ SearchableLiveBucketListSnapshot::scanForEviction(
4445

4546
// If we scan scanSize before hitting bucket EOF, exit early
4647
if (b.scanForEviction(evictionIter, scanSize, ledgerSeq,
47-
result.eligibleEntries, *this,
48+
result->eligibleEntries, *this,
4849
ledgerVers) == Loop::COMPLETE)
4950
{
5051
break;
@@ -59,7 +60,7 @@ SearchableLiveBucketListSnapshot::scanForEviction(
5960
}
6061
}
6162

62-
result.endOfRegionIterator = evictionIter;
63+
result->endOfRegionIterator = evictionIter;
6364
return result;
6465
}
6566

‎src/bucket/SearchableBucketList.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ class SearchableLiveBucketListSnapshot
3030
loadKeysWithLimits(std::set<LedgerKey, LedgerEntryIdCmp> const& inKeys,
3131
LedgerKeyMeter* lkMeter) const;
3232

33-
EvictionResultCandidates scanForEviction(
33+
std::unique_ptr<EvictionResultCandidates> scanForEviction(
3434
uint32_t ledgerSeq, EvictionCounters& counters, EvictionIterator iter,
3535
std::shared_ptr<EvictionStatistics> stats,
3636
StateArchivalSettings const& sas, uint32_t ledgerVers) const;

‎src/history/HistoryArchive.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -171,8 +171,8 @@ HistoryArchiveState::load(std::string const& inFile)
171171
in.exceptions(std::ios::badbit);
172172
cereal::JSONInputArchive ar(in);
173173
serialize(ar);
174-
if (version != HISTORY_ARCHIVE_STATE_VERSION_PRE_PROTOCOL_22 &&
175-
version != HISTORY_ARCHIVE_STATE_VERSION_POST_PROTOCOL_22)
174+
if (version != HISTORY_ARCHIVE_STATE_VERSION_BEFORE_HOT_ARCHIVE &&
175+
version != HISTORY_ARCHIVE_STATE_VERSION_WITH_HOT_ARCHIVE)
176176
{
177177
CLOG_ERROR(History, "Unexpected history archive state version: {}",
178178
version);
@@ -571,7 +571,7 @@ HistoryArchiveState::HistoryArchiveState(uint32_t ledgerSeq,
571571
std::string const& passphrase)
572572
: HistoryArchiveState(ledgerSeq, liveBuckets, passphrase)
573573
{
574-
version = HISTORY_ARCHIVE_STATE_VERSION_POST_PROTOCOL_22;
574+
version = HISTORY_ARCHIVE_STATE_VERSION_WITH_HOT_ARCHIVE;
575575
for (uint32_t i = 0; i < HotArchiveBucketList::kNumLevels; ++i)
576576
{
577577
HistoryStateBucket<HotArchiveBucket> b;

‎src/history/HistoryArchive.h

+9-9
Original file line numberDiff line numberDiff line change
@@ -82,10 +82,10 @@ struct HistoryArchiveState
8282
static constexpr size_t MAX_HISTORY_ARCHIVE_BUCKET_SIZE =
8383
1024ull * 1024ull * 1024ull * 100ull; // 100 GB
8484

85-
static inline unsigned const HISTORY_ARCHIVE_STATE_VERSION_PRE_PROTOCOL_22 =
86-
1;
8785
static inline unsigned const
88-
HISTORY_ARCHIVE_STATE_VERSION_POST_PROTOCOL_22 = 2;
86+
HISTORY_ARCHIVE_STATE_VERSION_BEFORE_HOT_ARCHIVE = 1;
87+
static inline unsigned const
88+
HISTORY_ARCHIVE_STATE_VERSION_WITH_HOT_ARCHIVE = 2;
8989

9090
struct BucketHashReturnT
9191
{
@@ -99,7 +99,7 @@ struct HistoryArchiveState
9999
}
100100
};
101101

102-
unsigned version{HISTORY_ARCHIVE_STATE_VERSION_PRE_PROTOCOL_22};
102+
unsigned version{HISTORY_ARCHIVE_STATE_VERSION_BEFORE_HOT_ARCHIVE};
103103
std::string server;
104104
std::string networkPassphrase;
105105
uint32_t currentLedger{0};
@@ -150,13 +150,13 @@ struct HistoryArchiveState
150150
// This is expected when the input file does not contain it, but
151151
// should only ever happen for older versions of History Archive
152152
// State.
153-
if (version >= HISTORY_ARCHIVE_STATE_VERSION_POST_PROTOCOL_22)
153+
if (version >= HISTORY_ARCHIVE_STATE_VERSION_WITH_HOT_ARCHIVE)
154154
{
155155
throw e;
156156
}
157157
}
158158
ar(CEREAL_NVP(currentBuckets));
159-
if (version >= HISTORY_ARCHIVE_STATE_VERSION_POST_PROTOCOL_22)
159+
if (version >= HISTORY_ARCHIVE_STATE_VERSION_WITH_HOT_ARCHIVE)
160160
{
161161
ar(CEREAL_NVP(hotArchiveBuckets));
162162
}
@@ -176,10 +176,10 @@ struct HistoryArchiveState
176176
// New versions of HistoryArchiveState should always have a
177177
// networkPassphrase.
178178
releaseAssertOrThrow(
179-
version < HISTORY_ARCHIVE_STATE_VERSION_POST_PROTOCOL_22);
179+
version < HISTORY_ARCHIVE_STATE_VERSION_WITH_HOT_ARCHIVE);
180180
}
181181
ar(CEREAL_NVP(currentBuckets));
182-
if (version >= HISTORY_ARCHIVE_STATE_VERSION_POST_PROTOCOL_22)
182+
if (version >= HISTORY_ARCHIVE_STATE_VERSION_WITH_HOT_ARCHIVE)
183183
{
184184
ar(CEREAL_NVP(hotArchiveBuckets));
185185
}
@@ -213,7 +213,7 @@ struct HistoryArchiveState
213213
bool
214214
hasHotArchiveBuckets() const
215215
{
216-
return version >= HISTORY_ARCHIVE_STATE_VERSION_POST_PROTOCOL_22;
216+
return version >= HISTORY_ARCHIVE_STATE_VERSION_WITH_HOT_ARCHIVE;
217217
}
218218
};
219219

‎src/historywork/DownloadBucketsWork.cpp

+8-9
Original file line numberDiff line numberDiff line change
@@ -90,13 +90,14 @@ DownloadBucketsWork::onSuccessCb(
9090
auto indexIter = indexMap.find(currId);
9191
releaseAssertOrThrow(indexIter != indexMap.end());
9292
releaseAssertOrThrow(indexIter->second);
93-
indexMap.erase(currId);
93+
auto index = std::move(indexIter->second);
94+
indexMap.erase(indexIter);
9495
indexMutex.unlock();
9596

9697
auto b = app.getBucketManager().adoptFileAsBucket<BucketT>(
9798
bucketPath, hexToBin256(hash),
9899
/*mergeKey=*/nullptr,
99-
/*index=*/std::move(indexIter->second));
100+
/*index=*/std::move(index));
100101
buckets[hash] = b;
101102
}
102103

@@ -157,10 +158,9 @@ DownloadBucketsWork::yieldMoreWork()
157158
auto self = weakSelf.lock();
158159
if (self)
159160
{
160-
onSuccessCb<HotArchiveBucket>(app, ft, hash, currId,
161-
self->mHotBuckets,
162-
self->mHotIndexMap,
163-
self->mHotIndexMapMutex);
161+
onSuccessCb<HotArchiveBucket>(
162+
app, ft, hash, currId, self->mHotBuckets,
163+
self->mHotIndexMap, self->mHotIndexMapMutex);
164164
}
165165
return true;
166166
};
@@ -182,9 +182,8 @@ DownloadBucketsWork::yieldMoreWork()
182182
if (self)
183183
{
184184
onSuccessCb<LiveBucket>(app, ft, hash, currId,
185-
self->mLiveBuckets,
186-
self->mLiveIndexMap,
187-
self->mLiveIndexMapMutex);
185+
self->mLiveBuckets, self->mLiveIndexMap,
186+
self->mLiveIndexMapMutex);
188187
}
189188
return true;
190189
};

0 commit comments

Comments
 (0)