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

fix sending cancels when excluding peer #805

Merged
merged 1 commit into from
Jan 22, 2025
Merged

Conversation

gammazero
Copy link
Contributor

@gammazero gammazero commented Jan 22, 2025

When sending cancels for want-block and want-have requests, a peer may be excluded because it was the sender of a block and that peer will cancel the want itself without having to receive a cancel message. The would-be sender of the cancel message still needs to do everything, such as adjusting want counts, as if the cancel were being sent and then not send out the cancel message.

Closes #694

When sending cancels for want-block and want-have requests, a peer may be sxcluded because it was the sender of a block and that peer will cancel the want itself without having to receive a cancel message. The would-be sender of tha cancel message still needs to do everything, such as adjusting want counts, as if the cancel were being sent and then not send out the cancel message.
@gammazero gammazero requested a review from a team as a code owner January 22, 2025 18:51
Copy link

codecov bot commented Jan 22, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.47%. Comparing base (9d04741) to head (2d4df29).
Report is 2 commits behind head on main.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #805      +/-   ##
==========================================
- Coverage   60.47%   60.47%   -0.01%     
==========================================
  Files         244      244              
  Lines       31102    31106       +4     
==========================================
+ Hits        18809    18811       +2     
- Misses      10616    10618       +2     
  Partials     1677     1677              
Files with missing lines Coverage Δ
...wap/client/internal/peermanager/peerwantmanager.go 88.69% <100.00%> (+0.15%) ⬆️

... and 9 files with indirect coverage changes

@gammazero gammazero requested review from lidel and hsanjuan January 22, 2025 18:57
lidel added a commit to ipfs/kubo that referenced this pull request Jan 22, 2025
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

Not familiar with wantlist internals, but staging test suggests metric is fixed:

staging 01 – 0.32.1
staging 02 - kubo master / this PR

Before this PR

We had metric go negative on box 02:

image

With this PR

Metric seems to be fixed:

image

@gammazero gammazero merged commit 7dbbb4f into main Jan 22, 2025
18 checks passed
@gammazero gammazero deleted the fix-cancel-exclude-peer branch January 22, 2025 22:52
@hsanjuan

This comment was marked as outdated.

@hsanjuan

This comment was marked as outdated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bitswap: Don't send CANCEL to peer we got block from
3 participants