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

interop: parallelized receipt fetching #13044

Merged
merged 7 commits into from
Dec 5, 2024

Conversation

axelKingsley
Copy link
Contributor

@axelKingsley axelKingsley commented Nov 22, 2024

What

Allows for ChainProcessor to initiate up to 10 blocks worth of receipts at a time by creating a worker pool during fetching.

Why

#12903
I have tried solving previously by adding BatchFetchReceipt to the eth client, which was pretty heavy. @protolambda suggests that parallelism on the client is roughly as effective as batch calls when operating over a web-socket, so I am proposing this solution too.
The receipt-focused PR can be found here: #12992

It's not clear if one of these solutions is more effective in production than another, but this PR is more to-the-point and it appears to help.

How

ChainProcessor now features the following:

  • maxFetcherThreads represents a maximum number of threads that can be used. On creation, set to 10.
  • update is replaced with rangeUpdate. Rather than taking next and only processing that block-height, rangeUpdate takes no arguments and instead generates its own range of blocks to fetch, up to maxFetcherThread of them.
  • The individual returns are sorted and processed, and if any result is an error, the processing stops

Testing

Tested this using the local-devnet debug messages which are now part of this PR:

  • "Fetching block" chain=900200 next=20 end=20 count=1 during normal operation
  • "Fetching block" chain=900201 next=27 end=45 count=10 after pausing the supervisor
  • "Fetching block" chain=900201 next=37 end=45 count=9 when there's less work than threads once caught up

@axelKingsley axelKingsley requested review from a team as code owners November 22, 2024 21:23
…-receipt-fetching
@mslipper
Copy link
Collaborator

mslipper commented Dec 5, 2024

Merged in develop so that the CI job this is waiting on can pass.

@protolambda protolambda added this pull request to the merge queue Dec 5, 2024
Copy link

codecov bot commented Dec 5, 2024

Codecov Report

Attention: Patch coverage is 0% with 98 lines in your changes missing coverage. Please review.

Project coverage is 43.08%. Comparing base (ee5c794) to head (fde053e).
Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
...r/supervisor/backend/processors/chain_processor.go 0.00% 98 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #13044      +/-   ##
===========================================
- Coverage    47.68%   43.08%   -4.61%     
===========================================
  Files          923      754     -169     
  Lines        77580    67886    -9694     
  Branches       849        0     -849     
===========================================
- Hits         36996    29246    -7750     
+ Misses       37849    36177    -1672     
+ Partials      2735     2463     -272     
Flag Coverage Δ
cannon-go-tests-32 ?
cannon-go-tests-64 ?
contracts-bedrock-tests ?

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

Files with missing lines Coverage Δ
...r/supervisor/backend/processors/chain_processor.go 0.00% <0.00%> (ø)

... and 180 files with indirect coverage changes

Merged via the queue into develop with commit 1eb223d Dec 5, 2024
43 checks passed
@protolambda protolambda deleted the interop-parallel-receipt-fetching branch December 5, 2024 20:22
sigma pushed a commit that referenced this pull request Dec 19, 2024
* interop: parallelized receipt fetching

* fix test

* remove elastic thread count

* Add Debug Message for Range Fetching

* rename end to last

* Remove Println

---------

Co-authored-by: Matthew Slipper <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants