ipfs / boxo

A set of reference libraries for building IPFS applications and implementations in Go.
https://github.com/ipfs/boxo#readme
Other
205 stars 89 forks source link

fix(bitswap): wantlist overflow handling #629

Closed gammazero closed 2 months ago

gammazero commented 3 months ago

Handle incoming wants that can not be added to the peer ledger without exceeding the peer want limit. These are handled by trying to make room for them by canceling existing wants for which there is no block. If this does not make sufficient room, then any lower priority wants that have blocks are canceled. This fix prevents the wantlist from filling up with CIDs that the server does not have.

Priority is also considered when truncating a wantlist that exceeds the size limit. Considering priority in wantlist truncation and overflow handling ensures that higher priority tasks handled when wantlist size needs to be limited. This will be more important in the future if/when priority is determined by a block's dag path (higher priority when closer to root).

Fixes #527

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 96.29630% with 4 lines in your changes missing coverage. Please review.

Project coverage is 59.92%. Comparing base (96ceee6) to head (d24abc7).

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #629      +/-   ##
==========================================
+ Coverage   59.82%   59.92%   +0.10%     
==========================================
  Files         238      238              
  Lines       30004    30049      +45     
==========================================
+ Hits        17949    18008      +59     
+ Misses      10437    10425      -12     
+ Partials     1618     1616       -2     
Files Coverage Δ
bitswap/server/internal/decision/peer_ledger.go 92.79% <100.00%> (-1.39%) :arrow_down:
bitswap/server/internal/decision/engine.go 91.84% <95.83%> (+0.93%) :arrow_up:

... and 15 files with indirect coverage changes

Wondertan commented 3 months ago

This option will need a minor change