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

[Merged by Bors] - collect latency and failure from every request and adjust latency based on response size #5601

Closed

Conversation

dshulyak
Copy link
Contributor

@dshulyak dshulyak commented Feb 25, 2024

closes: #5265

previous implementation was incomplete and had two flaws. it will not account for the difference in response size, larger payloads will naturally take more time to complete. it would skew peer selection logic towards peers that are slower to respond.

the other flaw was to use only hs/1 protocol for measuing latency, often peers are blocked on ax/1 (where we get a collection of ids, which is large and slow) and lack of prioritization there may cause inefficient retries.

Copy link

codecov bot commented Feb 26, 2024

Codecov Report

Attention: Patch coverage is 90.90909% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 79.8%. Comparing base (0c4c55d) to head (477c49c).
Report is 9 commits behind head on develop.

Files Patch % Lines
fetch/peers/peers.go 33.3% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##           develop   #5601     +/-   ##
=========================================
+ Coverage     79.6%   79.8%   +0.1%     
=========================================
  Files          270     270             
  Lines        27186   27232     +46     
=========================================
+ Hits         21667   21733     +66     
+ Misses        3973    3959     -14     
+ Partials      1546    1540      -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dshulyak dshulyak changed the title adjust peers selection based on atx sync latency collect latency and failure from every request and adjust latency based on response size Feb 27, 2024
@dshulyak dshulyak marked this pull request as ready for review February 27, 2024 06:34
@dshulyak
Copy link
Contributor Author

@ivan4th could you please review the change, i think it may somewhat conflict with your streaming patch. if you have suggestion how to make it less conflicting, i can adjust. i will take a look later at streaming pr

CHANGELOG.md Outdated Show resolved Hide resolved
fetch/peers/peers.go Outdated Show resolved Hide resolved
@dshulyak
Copy link
Contributor Author

bors merge

spacemesh-bors bot pushed a commit that referenced this pull request Feb 27, 2024
…ed on response size (#5601)

closes: #5265

previous implementation was incomplete and had two flaws. it will not account for the difference in response size, larger payloads will naturally take more time to complete. it would skew peer selection logic towards peers that are slower to respond.

the other flaw was to use only hs/1 protocol for measuing latency, often peers are blocked on ax/1 (where we get a collection of ids, which is large and slow) and lack of prioritization there may cause inefficient retries.
@ivan4th
Copy link
Contributor

ivan4th commented Feb 27, 2024

@dshulyak in #5562, an additional set of streaming fetcher methods are added which use new StreamRequest method of Server. The old methods are kept for now (streaming is feature-gated). The only way to combine it with this patch is merge one of the PRs and then update another, perhaps we could merge this one first

@spacemesh-bors
Copy link

Build failed:

@dshulyak
Copy link
Contributor Author

bors merge

spacemesh-bors bot pushed a commit that referenced this pull request Feb 27, 2024
…ed on response size (#5601)

closes: #5265

previous implementation was incomplete and had two flaws. it will not account for the difference in response size, larger payloads will naturally take more time to complete. it would skew peer selection logic towards peers that are slower to respond.

the other flaw was to use only hs/1 protocol for measuing latency, often peers are blocked on ax/1 (where we get a collection of ids, which is large and slow) and lack of prioritization there may cause inefficient retries.
@spacemesh-bors
Copy link

Pull request successfully merged into develop.

Build succeeded:

@spacemesh-bors spacemesh-bors bot changed the title collect latency and failure from every request and adjust latency based on response size [Merged by Bors] - collect latency and failure from every request and adjust latency based on response size Feb 27, 2024
@spacemesh-bors spacemesh-bors bot closed this Feb 27, 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.

collect latency and failure from every request and adjust latency based on response size
3 participants