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.

Sign up for GitHub

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

Jump to bottom

fix: limit number of connections #546

Merged
merged 5 commits into from
Aug 27, 2024
Merged

fix: limit number of connections #546

merged 5 commits into from
Aug 27, 2024

Conversation

atanmarko
Copy link
Member

@atanmarko atanmarko commented Aug 26, 2024 •
edited
Loading

Resolves #516

@atanmarko atanmarko self-assigned this Aug 26, 2024
@github-actions github-actions bot added the crate: zero_bin Anything related to the zero-bin subcrates. label Aug 26, 2024
@Nashtare Nashtare added this to the System strengthening milestone Aug 26, 2024
@atanmarko atanmarko force-pushed the fix/limit-connection-number branch from cf4dc82 to bb833e6 Compare August 27, 2024 09:42
@atanmarko atanmarko force-pushed the fix/limit-connection-number branch from bb833e6 to a778eba Compare August 27, 2024 09:43
@atanmarko
Copy link
Member Author

Sprayed some traces and did various tests with semaphores behavior and request duration. With current settings and semaphores, the Jerigon node and native tracer (so much more rpc calls), test_only proving 4096 blocks in parallel, ulimit 8096, runs without issues.

@atanmarko atanmarko marked this pull request as ready for review August 27, 2024 12:15
Nashtare
Nashtare approved these changes Aug 27, 2024
View reviewed changes
zero_bin/rpc/src/retry.rs Outdated Show resolved Hide resolved
0xaatif
0xaatif reviewed Aug 27, 2024
View reviewed changes
Copy link
Contributor

@0xaatif 0xaatif left a comment

Choose a reason for hiding this comment

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

Surprised that we want a semaphore and use reqwest, but lgtm

@atanmarko
Copy link
Member Author

atanmarko commented Aug 27, 2024 •
edited
Loading

Surprised that we want a semaphore and use reqwest, but lgtm

@0xaatif Any idea how to limit connection pool with existing reqwest API? I haven't found one. Seems old problem, also here

@atanmarko atanmarko merged commit 32a6ba7 into develop Aug 27, 2024
15 checks passed
@atanmarko atanmarko deleted the fix/limit-connection-number branch August 27, 2024 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xaatif 0xaatif 0xaatif left review comments

@Nashtare Nashtare Nashtare approved these changes

@muursh muursh muursh approved these changes

@praetoriansentry praetoriansentry Awaiting requested review from praetoriansentry

Assignees

@atanmarko atanmarko

Labels
crate: zero_bin Anything related to the zero-bin subcrates.
Projects
Zero EVM
Status: Done
Milestone
System strengthening
Development

Successfully merging this pull request may close these issues.

Limit number of parallel connections zero_bin can open
4 participants
@atanmarko @muursh @Nashtare @0xaatif