perf(consensus): batch block transaction UTXO lookups - #11186
Draft
upbqdn wants to merge 2 commits into
Draft
Conversation
Request::AwaitUtxo takes one outpoint and registers a pending request for it, so a caller with many outpoints needs one round trip through the state service per outpoint. Add Request::AnyChainUtxos, which takes a list and returns only the outpoints already present in the queued blocks, any non-finalized chain, or the finalized state. It never waits and never registers a pending request, so callers keep using AwaitUtxo for the outpoints it did not find. ReadRequest::AnyChainUtxo becomes AnyChainUtxos: its only caller was AwaitUtxo, which now passes a single-element list.
block_spent_utxos issued one Request::AwaitUtxo per transparent input and awaited each before starting the next, so a transaction with many inputs serialized that many round trips through the state service, and the block verifier could not finish until every transaction had done so. Look the outpoints up with Request::AnyChainUtxos in chunks of UTXO_LOOKUP_CHUNK_SIZE instead, falling back to AwaitUtxo for outpoints that have not reached the state yet. The lookups stop at the first chunk that is not fully present, so batching cannot make a transaction cost the state more lookups than the sequential version did: at most one chunk is speculative. This is an additive bound, not independence from the input count, since a transaction referencing outputs that really are in the state is still looked up in full. Closes #11185.
|
Note Complete: Audit complete. V12 did not find any issues that need review. Open the full results here. Analyzed six files, diff |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Closes #11185.
Solution
block_spent_utxosissued onezebra_state::Request::AwaitUtxoper transparent input and awaitedeach before starting the next, so a transaction with many inputs serialized that many round trips
through the state service.
Adds
zebra_state::Request::AnyChainUtxos, which takes a list of outpoints and returns only the onesalready in the queued blocks, any non-finalized chain, or the finalized state. Unlike
AwaitUtxoitnever waits and never registers a pending UTXO request, so the block verifier uses it for the bulk of
its lookups and falls back to
AwaitUtxoonly for outpoints that have not reached the state yet.Lookups go out in chunks of 64 and stop at the first chunk that is not fully present. That bound is
the reason for chunking rather than sending one request: without it, a transaction whose inputs are
all unknown would make the state look up every input before the sequential fallback failed on the
first, which is more work than the one lookup the old loop did before its timeout. With it, at most
one chunk is speculative.
To be precise about what that bound is and is not: it is additive. A transaction that references
outputs which really are in the state is still looked up in full, exactly as before. The worst case
is old cost plus one chunk, never a multiple of the input count.
Tests
Three new tests in
zebra-consensus/src/transaction/tests.rs, each built with a multi-inputtransaction and a
MockServicestate, so they assert the request sequence rather than just theoutcome:
block_utxo_lookups_use_one_bulk_request— oneAnyChainUtxoscarrying every outpoint in inputorder, then no further state requests.
block_utxo_lookups_await_only_the_missing_outpoints— one outpoint withheld from the bulkresponse is awaited with
AwaitUtxo, and the found ones are not.block_utxo_lookups_stop_at_the_first_chunk_with_misses— with 72 inputs and an empty bulkresponse, only the first 64 are ever requested.
Each was checked against the mutation it is meant to catch, and fails without the fix: reversing the
expected outpoint order, withholding a second outpoint, and removing the early
breakrespectively.Specifications & References
mempool_spent_utxos(UnspentBestChainUtxoper input). This PR deliberately leaves that pathuntouched.
Follow-up Work
how much of block verification's wall clock they account for. task: run the RPC-latency and Regtest mining benchmarks #11041 is the benchmark task that
should answer it, and its baseline should decide whether the chunk size of 64 is right.
read::any_utxosloops over the outpoints. There is no multi-get in
ReadDisk, andZebraDb::utxois two dependentlookups (
tx_loc_by_hashthenutxo_by_out_loc), so a genuinely batched DB read is a separatechange and should also wait for task: run the RPC-latency and Regtest mining benchmarks #11041's measurement.
AnyChainUtxosonce someone measures whether it needs thesame treatment.
PR Checklist
type(scope): description