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

Deprecate TTFB, RESP_TIMEOUT, introduce MAX_CONCURRENT_REQUESTS #8839

Open
wants to merge 21 commits into
base: master
Choose a base branch
from

Conversation

StefanBratanov
Copy link
Contributor

@StefanBratanov StefanBratanov commented Nov 20, 2024

PR Description

Main changes are in Eth2IncomingRequestHandler, Eth2OutgoingRequestHandler and RpcHandler

Reintroduced the timeouts as constants to handle slow servers. Furthermore, they could be should probably be configured per different methods. (as per the consensus-specs PR describes)

Fixed Issue(s)

related to #8803

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.

Changelog

  • I thought about adding a changelog entry, and added one if I deemed necessary.

this.asyncRunner = asyncRunner;
this.rpcMethod = rpcMethod;
concurrentRequestsSemaphore = new Semaphore(maxConcurrentRequests);
Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 ... this is a blocking approach, with a tight timeout... should we have an actual async queue here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I realised that, actually I am working on a different idea right now. Should be done soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed the approach to use ThrottlingTaskQueue which makes much more sense

@StefanBratanov StefanBratanov marked this pull request as ready for review November 23, 2024 19:03
@StefanBratanov StefanBratanov changed the title Replace TTFB, RESP_TIMEOUT with MAX_CONCURRENT_REQUESTS Deprecate TTFB, RESP_TIMEOUT, introduce MAX_CONCURRENT_REQUESTS Nov 25, 2024
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.

2 participants