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

check settings_pack::max_out_request_queue before alert outstanding_r… #7714

Merged

Conversation

tkittich
Copy link
Contributor

…equest_limit_reached

This should fix performance warning: max outstanding piece requests reached, outstanding_request_limit_reached

…equest_limit_reached

This should fix performance warning: max outstanding piece requests reached, outstanding_request_limit_reached
@arvidn
Copy link
Owner

arvidn commented Jul 25, 2024

The existing check compares against m_max_out_request_queue which is set to this setting, here:

m_max_out_request_queue(aux::clamp_assign<std::uint16_t(m_settings.get_int(settings_pack::max_out_request_queue)))

This limit can be adjusted if the peer indicates a reqq limit in the extended handshake, or if the peer is snubbed (or if the peer is identified as bitcomet).

So, I think you're right to check against the setting instead. I think the existing check can be removed however.

@arvidn
Copy link
Owner

arvidn commented Jul 25, 2024

If you take my suggestion, please add a note to the Changelog as well, that this issue was addressed.

@arvidn arvidn added this to the 2.0.11 milestone Jul 25, 2024
tkittich and others added 2 commits July 27, 2024 14:28
check settings_pack::max_out_request_queue before alerting performance warning: max outstanding piece requests reached

Co-authored-by: Arvid Norberg <[email protected]>
@tkittich
Copy link
Contributor Author

If you take my suggestion, please add a note to the Changelog as well, that this issue was addressed.

Thank you for the review & suggestions.

@arvidn arvidn merged commit 5ba42b6 into arvidn:RC_2_0 Jul 27, 2024
42 checks passed
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