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

Reduce number of ETH RPC calls during block polling #2775

Merged
merged 15 commits into from
Mar 23, 2023
Merged

Conversation

leszko
Copy link
Contributor

@leszko leszko commented Mar 21, 2023

What does this pull request do? Explain your changes. (required)

Reduce the number of ETH RPC calls during the block polling operations.

From my measurements, this change (together with changing blockPollingInterval from 10 to 20) should bring the RPC req reduction by the factor of 7-8x.

The main improvement is that I removed the pollNextBlock() and buildCanonicalChain() parts, which I believe are not important after the migration to Arbitrum. The rationale is that Ethereum had a different approach to the block finality than Arbitrum:

  • Ethereum - blocks could be reorganized, so at some point in time we could have blocks: 123, 234, 345, but after some time they could be reorganized to 123, 234, 456.
  • Arbitrum - it uses sequencer, so the blocks produced by each validator are always the same, therefore there is no reorganization happening, so if we have blocks 123, 234, 345, they will always stay this way

I believe this whole code part with pollNextBlock() was so complex because the block reorg needed to be supported.

Specific updates (required)

  • Remove pollNextBlock() and buidCanonicalChain()
  • Increase maxBlocksInGetLogsQuery from 60 to 1000 (this limit was too low which resulted in multiple eth_getLogs calls instead of one call
  • Remove redundant calls to get the most recent blocks (in one operation we made the same ETH RPC call multiple times)

How did you test each of these updates (required)

Tested with local runs (running for a long time).

Further improvements
The code could be refactored even further, but I don't want to do it in this PR, since I'd like to limit the number of changes done here to the minimum that brings good results. Additional possible improvements:

  • Remove all the stack and its operations (stack.go)
  • Remove block backfilling at the startup (Consider removing block backfilling #2235)
  • Reduce the number of req after a new round is initialized (we're currently making around 700 req, which seems too many)

fix #2298

Checklist:

@leszko leszko changed the title Rafal/reduce rpc req Reduce number of ETH RPC calls during block polling Mar 21, 2023
@leszko leszko marked this pull request as ready for review March 21, 2023 15:34
@leszko leszko requested review from thomshutt and mjh1 March 21, 2023 15:34
Copy link
Contributor

@thomshutt thomshutt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, thanks for the detailed explanation of why we've done each piece. If this works then it's also a big win in terms of reducing our code complexity!

@codecov
Copy link

codecov bot commented Mar 22, 2023

Codecov Report

Merging #2775 (109a6f3) into master (c65fed9) will increase coverage by 0.32450%.
The diff coverage is 60.60606%.

Impacted file tree graph

@@                 Coverage Diff                 @@
##              master       #2775         +/-   ##
===================================================
+ Coverage   56.28521%   56.60971%   +0.32450%     
===================================================
  Files             88          88                 
  Lines          19172       19078         -94     
===================================================
+ Hits           10791       10800          +9     
+ Misses          7789        7684        -105     
- Partials         592         594          +2     
Impacted Files Coverage Δ
cmd/livepeer/starter/starter.go 4.28954% <0.00000%> (ø)
eth/blockwatch/block_watcher.go 68.99441% <62.50000%> (+17.44574%) ⬆️

... and 2 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c65fed9...109a6f3. Read the comment docs.

Impacted Files Coverage Δ
cmd/livepeer/starter/starter.go 4.28954% <0.00000%> (ø)
eth/blockwatch/block_watcher.go 68.99441% <62.50000%> (+17.44574%) ⬆️

... and 2 files with indirect coverage changes

@leszko leszko merged commit 8c7ec01 into master Mar 23, 2023
@leszko leszko deleted the rafal/reduce_rpc_req branch March 23, 2023 06:35
This was referenced Aug 8, 2023
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.

Decrease number of ETH RPC requests
3 participants