Speed up get_unconfirmed_messages with denormalized first_confirmed_at#1132
Merged
Merged
Conversation
Replace the NOT EXISTS anti-join against message_confirmations + chain_txs with a direct check on messages.first_confirmed_at, which is already populated by the trg_update_first_confirmed trigger added in migration 0049. Query time on production-sized data drops from ~10s to ~50ms warm, using the existing ix_messages_first_confirmed index. The chain parameter is dropped: it filtered by which chain confirmed a message, but first_confirmed_at is set by any chain. In production only ETH confirmations are recorded, so the two are equivalent today.
foxpatch-aleph
left a comment
There was a problem hiding this comment.
Click Re-request review to retry.
foxpatch-aleph
approved these changes
May 13, 2026
foxpatch-aleph
left a comment
There was a problem hiding this comment.
Clean, well-motivated refactor replacing a slow anti-join with a direct IS NULL check on the trigger-maintained first_confirmed_at column. Removes the now-unnecessary chain parameter and handles both callers. The semantic change (any chain's confirmation now counts) is actually more correct than the old chain-filtered behavior.
src/aleph/db/accessors/messages.py (line 421): Excellent simplification. The old branching logic between chain=None and chain= was a source of complexity for a feature (per-chain unconfirmed queries) that had no real use case.
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.
Summary
NOT EXISTSanti-join againstmessage_confirmations+chain_txsinget_unconfirmed_messageswith a direct check onmessages.first_confirmed_at IS NULL. The column is already trigger-maintained (migration 0049), so no schema change is needed.chainparameter and update the two callers (chains/ethereum.py,chains/nuls2.py) accordingly.Performance
Measured against a production-sized dataset (~22k unconfirmed signed messages):
Before (anti-join): ~10s
After (denorm column, using existing
ix_messages_first_confirmed): ~300ms cold, ~50ms warm. No new index needed.Semantic note
The previous implementation filtered confirmations by chain (e.g.
chain=Chain.ETHfor the ETH packer).first_confirmed_atis set by a confirmation on any chain. In production only ETH confirmations are recorded, so the behavior is equivalent today. If multi-chain confirmations are reintroduced, the unconfirmed query will need to reconsider this.Test plan
hatch run testing:test tests/db/test_messages.py::test_get_unconfirmed_messages tests/db/test_messages.py::test_get_unconfirmed_messages_trusted_messages -vget_unconfirmed_messageswithoutchain=.