diff --git a/src/bucket/BucketManager.h b/src/bucket/BucketManager.h index a64bd9181f..1247649d1e 100644 --- a/src/bucket/BucketManager.h +++ b/src/bucket/BucketManager.h @@ -74,6 +74,7 @@ struct MergeCounters uint64_t mDeadEntryShadowElisions{0}; uint64_t mOutputIteratorTombstoneElisions{0}; + uint64_t mOutputIteratorLiveToInitConversions{0}; uint64_t mOutputIteratorBufferUpdates{0}; uint64_t mOutputIteratorActualWrites{0}; MergeCounters& operator+=(MergeCounters const& delta); diff --git a/src/bucket/BucketManagerImpl.cpp b/src/bucket/BucketManagerImpl.cpp index 3fbe228b64..231e2703a9 100644 --- a/src/bucket/BucketManagerImpl.cpp +++ b/src/bucket/BucketManagerImpl.cpp @@ -419,49 +419,54 @@ MergeCounters::operator+=(MergeCounters const& delta) mOutputIteratorTombstoneElisions += delta.mOutputIteratorTombstoneElisions; mOutputIteratorBufferUpdates += delta.mOutputIteratorBufferUpdates; mOutputIteratorActualWrites += delta.mOutputIteratorActualWrites; + mOutputIteratorLiveToInitConversions += + delta.mOutputIteratorLiveToInitConversions; return *this; } bool MergeCounters::operator==(MergeCounters const& other) const { - return ( - mPreInitEntryProtocolMerges == other.mPreInitEntryProtocolMerges && - mPostInitEntryProtocolMerges == other.mPostInitEntryProtocolMerges && - - mRunningMergeReattachments == other.mRunningMergeReattachments && - mFinishedMergeReattachments == other.mFinishedMergeReattachments && - - mNewMetaEntries == other.mNewMetaEntries && - mNewInitEntries == other.mNewInitEntries && - mNewLiveEntries == other.mNewLiveEntries && - mNewDeadEntries == other.mNewDeadEntries && - mOldMetaEntries == other.mOldMetaEntries && - mOldInitEntries == other.mOldInitEntries && - mOldLiveEntries == other.mOldLiveEntries && - mOldDeadEntries == other.mOldDeadEntries && - - mOldEntriesDefaultAccepted == other.mOldEntriesDefaultAccepted && - mNewEntriesDefaultAccepted == other.mNewEntriesDefaultAccepted && - mNewInitEntriesMergedWithOldDead == - other.mNewInitEntriesMergedWithOldDead && - mOldInitEntriesMergedWithNewLive == - other.mOldInitEntriesMergedWithNewLive && - mOldInitEntriesMergedWithNewDead == - other.mOldInitEntriesMergedWithNewDead && - mNewEntriesMergedWithOldNeitherInit == - other.mNewEntriesMergedWithOldNeitherInit && - - mShadowScanSteps == other.mShadowScanSteps && - mMetaEntryShadowElisions == other.mMetaEntryShadowElisions && - mLiveEntryShadowElisions == other.mLiveEntryShadowElisions && - mInitEntryShadowElisions == other.mInitEntryShadowElisions && - mDeadEntryShadowElisions == other.mDeadEntryShadowElisions && - - mOutputIteratorTombstoneElisions == - other.mOutputIteratorTombstoneElisions && - mOutputIteratorBufferUpdates == other.mOutputIteratorBufferUpdates && - mOutputIteratorActualWrites == other.mOutputIteratorActualWrites); + return (mPreInitEntryProtocolMerges == other.mPreInitEntryProtocolMerges && + mPostInitEntryProtocolMerges == + other.mPostInitEntryProtocolMerges && + + mRunningMergeReattachments == other.mRunningMergeReattachments && + mFinishedMergeReattachments == other.mFinishedMergeReattachments && + + mNewMetaEntries == other.mNewMetaEntries && + mNewInitEntries == other.mNewInitEntries && + mNewLiveEntries == other.mNewLiveEntries && + mNewDeadEntries == other.mNewDeadEntries && + mOldMetaEntries == other.mOldMetaEntries && + mOldInitEntries == other.mOldInitEntries && + mOldLiveEntries == other.mOldLiveEntries && + mOldDeadEntries == other.mOldDeadEntries && + + mOldEntriesDefaultAccepted == other.mOldEntriesDefaultAccepted && + mNewEntriesDefaultAccepted == other.mNewEntriesDefaultAccepted && + mNewInitEntriesMergedWithOldDead == + other.mNewInitEntriesMergedWithOldDead && + mOldInitEntriesMergedWithNewLive == + other.mOldInitEntriesMergedWithNewLive && + mOldInitEntriesMergedWithNewDead == + other.mOldInitEntriesMergedWithNewDead && + mNewEntriesMergedWithOldNeitherInit == + other.mNewEntriesMergedWithOldNeitherInit && + + mShadowScanSteps == other.mShadowScanSteps && + mMetaEntryShadowElisions == other.mMetaEntryShadowElisions && + mLiveEntryShadowElisions == other.mLiveEntryShadowElisions && + mInitEntryShadowElisions == other.mInitEntryShadowElisions && + mDeadEntryShadowElisions == other.mDeadEntryShadowElisions && + + mOutputIteratorTombstoneElisions == + other.mOutputIteratorTombstoneElisions && + mOutputIteratorBufferUpdates == + other.mOutputIteratorBufferUpdates && + mOutputIteratorActualWrites == other.mOutputIteratorActualWrites) && + mOutputIteratorLiveToInitConversions == + other.mOutputIteratorLiveToInitConversions; } void diff --git a/src/bucket/BucketOutputIterator.cpp b/src/bucket/BucketOutputIterator.cpp index aeb5dac49b..d1feaf697b 100644 --- a/src/bucket/BucketOutputIterator.cpp +++ b/src/bucket/BucketOutputIterator.cpp @@ -67,17 +67,36 @@ BucketOutputIterator::put(BucketEntry const& e) ++mMergeCounters.mOutputIteratorTombstoneElisions; return; } + std::optional maybeInitEntry; + if (!mKeepDeadEntries && e.type() == LIVEENTRY && + protocolVersionStartsFrom( + mMeta.ledgerVersion, + Bucket::FIRST_PROTOCOL_SUPPORTING_INITENTRY_AND_METAENTRY)) + { + // If mKeepDeadEntries is false (lowest level), + // we also want to convert the LIVEENTRY to an INITENTRY. + // This is because each level of the bucket list contains + // only one entry per key, and per CAP-0020, INIT ENTRY + // implies that either no entry with the same ledger key + // exists in an older bucket. Therefore, all entries of type + // LIVEENTRY in the lowest level are also of type INITENTRY. + ++mMergeCounters.mOutputIteratorLiveToInitConversions; + // Make a copy of e and set the type of the new entry to INITENTRY. + maybeInitEntry.emplace(e); + maybeInitEntry->type(INITENTRY); + } // Check to see if there's an existing buffered entry. if (mBuf) { // mCmp(e, *mBuf) means e < *mBuf; this should never be true since // it would mean that we're getting entries out of order. - releaseAssert(!mCmp(e, *mBuf)); + releaseAssert( + !mCmp(maybeInitEntry.has_value() ? *maybeInitEntry : e, *mBuf)); // Check to see if the new entry should flush (greater identity), or // merely replace (same identity), the buffered entry. - if (mCmp(*mBuf, e)) + if (mCmp(*mBuf, maybeInitEntry.has_value() ? *maybeInitEntry : e)) { ++mMergeCounters.mOutputIteratorActualWrites; mOut.writeOne(*mBuf, &mHasher, &mBytesPut); @@ -91,7 +110,7 @@ BucketOutputIterator::put(BucketEntry const& e) // In any case, replace *mBuf with e. ++mMergeCounters.mOutputIteratorBufferUpdates; - *mBuf = e; + *mBuf = maybeInitEntry.has_value() ? *maybeInitEntry : e; } std::shared_ptr diff --git a/src/bucket/test/BucketManagerTests.cpp b/src/bucket/test/BucketManagerTests.cpp index fc256c4cc2..fdef2c9212 100644 --- a/src/bucket/test/BucketManagerTests.cpp +++ b/src/bucket/test/BucketManagerTests.cpp @@ -759,6 +759,8 @@ class StopAndRestartBucketMergesTest mMergeCounters.mDeadEntryShadowElisions); CLOG_INFO(Bucket, "OutputIteratorTombstoneElisions: {}", mMergeCounters.mOutputIteratorTombstoneElisions); + CLOG_INFO(Bucket, "OutputIteratorLiveToInitConversions: {}", + mMergeCounters.mOutputIteratorLiveToInitConversions); CLOG_INFO(Bucket, "OutputIteratorBufferUpdates: {}", mMergeCounters.mOutputIteratorBufferUpdates); CLOG_INFO(Bucket, "OutputIteratorActualWrites: {}", @@ -914,6 +916,8 @@ class StopAndRestartBucketMergesTest CHECK(mMergeCounters.mOutputIteratorTombstoneElisions == other.mMergeCounters.mOutputIteratorTombstoneElisions); + CHECK(mMergeCounters.mOutputIteratorLiveToInitConversions == + other.mMergeCounters.mOutputIteratorLiveToInitConversions); CHECK(mMergeCounters.mOutputIteratorBufferUpdates == other.mMergeCounters.mOutputIteratorBufferUpdates); CHECK(mMergeCounters.mOutputIteratorActualWrites == diff --git a/src/bucket/test/BucketTests.cpp b/src/bucket/test/BucketTests.cpp index 4da46e26a7..3272227d89 100644 --- a/src/bucket/test/BucketTests.cpp +++ b/src/bucket/test/BucketTests.cpp @@ -399,6 +399,37 @@ TEST_CASE("merges proceed old-style despite newer shadows", } } +TEST_CASE("lowest level merge converts live to init", "[bucket][bucketinit]") +{ + VirtualClock clock; + Config const& cfg = getTestConfig(); + Application::pointer app = createTestApplication(clock, cfg); + auto& bm = app->getBucketManager(); + auto vers = getAppLedgerVersion(app); + + LedgerEntry entry = generateAccount(); + auto b1 = Bucket::fresh(bm, vers, {}, {entry}, {}, + /*countMergeEvents=*/true, clock.getIOContext(), + /*doFsync=*/true); + EntryCounts e1(b1); + CHECK(e1.nInit == 0); + CHECK(e1.nLive == 1); + auto b2 = Bucket::fresh(bm, vers, {}, {}, {}, + /*countMergeEvents=*/true, clock.getIOContext(), + /*doFsync=*/true); + + // Merge b1 and b2 into a new, lowest-level bucket. + // keepDeadEntries = false signfies the lowest level of the bucket list + auto bMerged = + Bucket::merge(bm, vers, b1, b2, /*shadows=*/{}, + /*keepDeadEntries=*/false, + /*countMergeEvents=*/true, clock.getIOContext(), + /*doFsync=*/true); + EntryCounts e2(bMerged); + CHECK(e2.nInit == 1); + CHECK(e2.nLive == 0); +} + TEST_CASE("merges refuse to exceed max protocol version", "[bucket][bucketmaxprotocol]") {