Evict persistent entries to Hot Archive #4585
Conversation
SirTyson
force-pushed
the
persitent-eviction
branch
from
48998cb to
afe423d
Compare
src/bucket/BucketIndexImpl.cpp
Outdated
| releaseAssert(mData.filter); | ||
| releaseAssert(in.mData.filter); | ||
| if (!(*(mData.filter) == *(in.mData.filter))) | ||
| if (!mData.filter) |
There was a problem hiding this comment.
This got me scratching my head a bit, could it be simplified to
if (!mData.filter || !in.mData.filter)
{
return mData.filter == in.mData.filter;
}
return *mData.filter == *in.mData.filter;(at least that's how I understand this)
There was a problem hiding this comment.
This is checking that each index either:
- Doesn't have a filter pointer
- Has the same filter as the other index
What you provided won't work, as there are more checks later in the function for other index fields, so we can't return true after checking the filter. I've update this function though and added some comments to hopefully make it more clear.
| releaseAssertOrThrow(isPersistentEntry(entry.data)); | ||
| // Unfortunately, for legacy purposes, evictedTemporaryLedgerKeys is | ||
| // misnamed and stores all evicted keys, both temp and persistent. | ||
| mLedgerCloseMeta.v1().evictedTemporaryLedgerKeys.emplace_back( |
There was a problem hiding this comment.
Are we still planning to rename this field?
There was a problem hiding this comment.
Yes, but I haven't pushed the XDR changes yet.
src/bucket/BucketManager.h
Outdated
|
|
||
| // Returns a pair of vectors representing entries evicted this ledger, where | ||
| // the first vector constains all deleted keys (TTL and temporary), and | ||
| // the second vector contains all archived keys (persistent and |
src/bucket/BucketManager.h
Outdated
| // the second vector contains all archived keys (persistent and | ||
| // ContractCode). Note that when an entry is archived, its TTL key will be | ||
| // included in the deleted keys vector. | ||
| std::pair<std::vector<LedgerKey>, std::vector<LedgerEntry>> |
There was a problem hiding this comment.
nit: I may be overly cautious here, but I feel like it would be safer to return a struct with named fields here. Even though the types are different, there is still some risk of confusion. At least it definitely would make the caller code easier to read.
Description
Resolves #4395
This PR evicts persitent entries to the Hot Archive in p23. Specifically, the background eviction scan will delete expired persistent entries from the live bucketlist and add them to the hot archive. This change also includes eviction meta for these types.
Operations have not yet been modified to work with the evicted entries. This means that
InvokeHostFunctionOpdoes not check the hot archive for the existence of archived entries, andRestoreFootprintOpwill not be able to restore evicted entries. These changes will be added in a follow up PR.Checklist
clang-formatv8.0.0 (viamake formator the Visual Studio extension)