Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: log indexing #4167

Closed
wants to merge 7 commits into from
Closed

feat: log indexing #4167

wants to merge 7 commits into from

Conversation

rkrasiuk
Copy link
Member

@rkrasiuk rkrasiuk commented Aug 12, 2023

#3082 re-opened.

TBD

@rkrasiuk rkrasiuk marked this pull request as draft August 14, 2023 07:53
@rkrasiuk rkrasiuk added C-enhancement New feature or request A-staged-sync Related to staged sync (pipelines and stages) M-prevent-stale Prevents old inactive issues/PRs from being closed due to inactivity labels Aug 14, 2023
@gakonst
Copy link
Member

gakonst commented Aug 24, 2023

Should we take a serious crack at getting this over the line? i.e. update description, get ppl to review, make sure CI passes, remove from draft status? As discussed, this should be in alpha.8, so assuming in 2 Mondays, so prob we should start soon?

@codecov
Copy link

codecov bot commented Oct 19, 2023

Codecov Report

Merging #4167 (4e00541) into main (4dc15c3) will decrease coverage by 9.08%.
Report is 401 commits behind head on main.
The diff coverage is 7.67%.

❗ Current head 4e00541 differs from pull request most recent head 05f6e6a. Consider uploading reports for the commit 05f6e6a to get more accurate results

Additional details and impacted files

Impacted file tree graph

Files Coverage Δ
bin/reth/src/args/stage_args.rs 0.00% <ø> (ø)
bin/reth/src/cli/components.rs 0.00% <ø> (-90.48%) ⬇️
crates/primitives/src/log.rs 0.00% <ø> (-53.34%) ⬇️
crates/stages/src/stages/mod.rs 0.00% <ø> (ø)
crates/storage/db/src/tables/mod.rs 0.00% <ø> (-29.27%) ⬇️
...torage/db/src/tables/models/storage_sharded_key.rs 0.00% <ø> (ø)
crates/storage/provider/src/traits/history.rs 0.00% <ø> (ø)
crates/storage/provider/src/traits/receipts.rs 43.75% <ø> (ø)
crates/stages/src/sets.rs 0.00% <0.00%> (-71.58%) ⬇️
crates/stages/src/stages/index_account_history.rs 0.00% <0.00%> (-2.18%) ⬇️
... and 17 more

... and 250 files with indirect coverage changes

Flag Coverage Δ
integration-tests 17.00% <7.67%> (-9.08%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
reth binary 6.23% <0.00%> (-19.55%) ⬇️
blockchain tree 0.00% <ø> (-28.47%) ⬇️
pipeline 0.00% <0.00%> (-5.05%) ⬇️
storage (db) 16.49% <0.00%> (-13.48%) ⬇️
trie 0.00% <ø> (-22.54%) ⬇️
txpool 32.20% <ø> (-9.18%) ⬇️
networking 28.81% <ø> (-2.09%) ⬇️
rpc 24.01% <30.37%> (-2.47%) ⬇️
consensus 0.88% <ø> (-24.19%) ⬇️
revm 1.51% <ø> (-8.34%) ⬇️
payload builder 7.95% <ø> (-6.21%) ⬇️
primitives 21.58% <0.00%> (-7.59%) ⬇️

@rkrasiuk rkrasiuk force-pushed the rkrasiuk/log-indexing branch 2 times, most recently from ee765ba to 3637ab2 Compare October 20, 2023 15:02
@rkrasiuk rkrasiuk force-pushed the rkrasiuk/log-indexing branch from 3637ab2 to 4e52ec0 Compare December 6, 2023 15:07
@rkrasiuk
Copy link
Member Author

rkrasiuk commented Dec 18, 2023

The primary motivation for this PR was to improve the performance of log-related RPC handlers. Upon taking a closer look at the current implementation, we found and fixed an existing bottleneck which had severely degraded the log filter query performance (#5805). As of now our log query response time is satisfactory, so we are closing this PR to avoid incurring extra storage overhead by maintaining addition log indexes. Even then, such an approach only improves the lookup of rarely occurring logs and would arguably be worse for frequent log filter parameters e.g. Transfer & Deposit topics, common ERC20 or DEX log addresses

@rkrasiuk rkrasiuk closed this Dec 18, 2023
@rkrasiuk rkrasiuk deleted the rkrasiuk/log-indexing branch February 27, 2024 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-staged-sync Related to staged sync (pipelines and stages) C-enhancement New feature or request M-prevent-stale Prevents old inactive issues/PRs from being closed due to inactivity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants