stellar-core icon indicating copy to clipboard operation
stellar-core copied to clipboard

3651 prefetch entries in soroban ops

Open ThomasBrady opened this issue 1 year ago • 1 comments

Description

Adds LedgerKeyMeter to track the allowance of read bytes for each transaction when prefetching soroban ops.

Resolves #3651

Checklist

  • [ ] Reviewed the contributing document
  • [ ] Rebased on top of master (no merge commits)
  • [ ] Ran clang-format v8.0.0 (via make format or the Visual Studio extension)
  • [ ] Compiles
  • [ ] Ran all tests
  • [ ] If change impacts performance, include supporting evidence per the performance document

ThomasBrady avatar Jan 31 '24 23:01 ThomasBrady

While discussing pre-fetching work with Marta and Nico, a suggestion came up to have separation of phases (classic phase and Soroban phase) when pre-fetching entries. This will help us have better metrics on IO and also help in future parallelization work. (Already discussed with Tom)

anupsdf avatar Mar 21 '24 17:03 anupsdf

I'll squash this later, but for ease of reviewing, I changed the semantics of LedgerKeyMeter slightly:

  • If a key is not in the footprint of any transaction added to the meter, it has unlimited quota (i.e. the following always hold maxReadQuotaForKey(untrackedKey) == std::numeric_limits<uint32_t>::max and canLoad(untrackedKey) == true)
  • calling updateReadQuotaForKey(untrackedKey) is a no-op (deleted releaseAssert which asserted that the key belonged to a transaction which was previously added to the meter)

This covers the cases of keys which are prefetched but are not metered, namely TTL keys for keys in the footprint, and account keys. cc @dmkozh @SirTyson

ThomasBrady avatar Jun 11 '24 20:06 ThomasBrady

I'll squash this later, but for ease of reviewing, I changed the semantics of LedgerKeyMeter slightly:

  • If a key is not in the footprint of any transaction added to the meter, it has unlimited quota (i.e. the following always hold maxReadQuotaForKey(untrackedKey) == std::numeric_limits<uint32_t>::max and canLoad(untrackedKey) == true)
  • calling updateReadQuotaForKey(untrackedKey) is a no-op (deleted releaseAssert which asserted that the key belonged to a transaction which was previously added to the meter)

This covers the cases of keys which are prefetched but are not metered, namely TTL keys for keys in the footprint, and account keys. cc @dmkozh @SirTyson

New semantics look good! Should be able to merge in the next release after running the watcher node.

SirTyson avatar Jun 17 '24 18:06 SirTyson