Skip to content

fix: allow only one backend connection at a time #173

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

Closed
wants to merge 1 commit into from

Conversation

0x416e746f6e
Copy link

fixes #153

.. effectively if only 1 concurrent request is allowed, this will
eliminate the need to open multiple connections
Copy link

vercel bot commented Apr 9, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
rollup-boost ⬜️ Ignored (Inspect) Apr 9, 2025 0:24am

@0xForerunner
Copy link
Collaborator

This doesn't seem like the right solution to me? We have situations where we want to have multiple concurrent requests.

@0x416e746f6e
Copy link
Author

We have situations where we want to have multiple concurrent requests.

what are those @0xForerunner?

@0xForerunner
Copy link
Collaborator

Right here for example

Copy link
Collaborator

@0xForerunner 0xForerunner left a comment

Choose a reason for hiding this comment

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

Causes issues with concurrency.

@0x416e746f6e
Copy link
Author

0x416e746f6e commented Apr 10, 2025

Right here for example

I was more looking to hear about use-cases when concurrency is needed, rather than places where it's being used because it can be used.

op-node typically opens exactly 2 connections with it's EL authrpc:

  • no. 1 is used for FCU, getPayload, newPayload, and getBlockByHash calls
  • no. 2 is used for getBlockByNumber

there is no concurrency there. why suddenly we need it in rollup-boost?

@0xOsiris
Copy link
Collaborator

0xOsiris commented Apr 10, 2025

Right here for example

I was more looking to hear about use-cases when concurrency is needed, rather than places where it's being used because it can be used.

op-node typically opens exactly 2 connections with it's EL authrpc:

  • no. 1 is used for FCU, getPayload, newPayload, and getBlockByHash calls
  • no. 2 is used for getBlockByNumber

there is no concurrency there. why suddenly we need it in rollup-boost?

There are certainly cases where we are going to have an open connection, and we cannot drop the subsequent connection. Not all FCU's are guaranteed to come in at 2s intervals. For example when op-node is backfilling deposit blocks to catch up to the L1 block time. FCU/Get Payload requests are going to be coming in on millisecond time horizon's, but not dropping this connections is critically important.

In any case we never want to drop a new connection just because we currently have an open one even if we shouldn't be holding on to the connection in the first place. This would break liveness

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.

bug: race condition due to multiple backend connections being open
4 participants