Skip to content
This repository has been archived by the owner on Feb 1, 2023. It is now read-only.

Prune peers that send too many consecutive DONT_HAVEs #261

Merged
merged 1 commit into from Feb 14, 2020

Conversation

dirkmc
Copy link
Contributor

@dirkmc dirkmc commented Feb 14, 2020

Fixes #257

A potential issue I see with this simplistic approach is that if for example we are downloading a big file, the DAG will be split into many subbranches, so if for example

  • there are two peers
  • the DAG is split into two sub-branches from the root
  • peer A has all blocks
  • peer B has one sub-branch with half the leaf blocks

In this case a situation may arise where

  • peer B will repeatedly respond with DONT_HAVE at the beginning
  • peer B's consecutive DONT_HAVE limit will be exceeded
  • peer B will be removed from the session

If peer B's latency was lower than peer A, then it would have been faster to keep peer B in the session.

@Stebalien I'm not so familiar with the structure that is produced by chunking, is this likely to be a problem in practice?

@Stebalien
Copy link
Member

That could happen if both have the root, peer A has everything, peer B has bar, and we try to download foo first.

.
├── bar
│   ├── big1
│   ├── big2
│   ├── big3
│   ├── big4
│   ├── big5
│   └── big6
└── foo
    ├── small1
    ├── small2
    ├── small3
    ├── small4
    ├── small5
    ├── small6
    └── small7

We'll quickly download foo (small), then take longer downloading bar. However, we do also randomly broadcast a single want-have to all connected peers periodically. If we're downloading a large enough dataset for this to matter, I expect we'll hit that timeout and add peer B back into the session.

changesBufferSize = 128
// If the session receives this many DONT_HAVEs in a row from a peer,
// it prunes the peer from the session
peerDontHaveLimit = 16
Copy link
Member

Choose a reason for hiding this comment

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

I'd consider dropping this to something really small for now (4?).

@Stebalien Stebalien marked this pull request as ready for review February 14, 2020 22:57
@Stebalien Stebalien merged commit 9ddd13e into master Feb 14, 2020
@Stebalien Stebalien deleted the fix/prune-session-peers branch February 14, 2020 23:13
Jorropo pushed a commit to Jorropo/go-libipfs that referenced this pull request Jan 26, 2023
Prune peers that send too many consecutive DONT_HAVEs

This commit was moved from ipfs/go-bitswap@9ddd13e
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prune peers from Session if they stop sending blocks
2 participants