go-bitswap icon indicating copy to clipboard operation
go-bitswap copied to clipboard

Remove DONT_HAVE when we send a cancel.

Open Stebalien opened this issue 6 years ago • 7 comments

If we send a cancel to a peer (because, e.g., the request for that block was canceled), we should remove any "don't have" information with respect to that peer. Otherwise, if the peer obtains the block and we suddenly decide that we care about it again, we'll fail to ask them for it.

This can actually happen when, e.g., seeking through a video that multiple peers are downloading at the same time.

Stebalien avatar Feb 08 '20 00:02 Stebalien

This is actually a more general issue. We should only remember "DONT_HAVEs" for a peer when those CIDs are in the peer's wantlist. This issue can also arise when we disconnect and reconnect to a peer.

It would be best if we could store this information inside our copy of the wantlists we send to peers to ensure it stays up-to-date.

Stebalien avatar Feb 08 '20 00:02 Stebalien

Actually, do we even need to record "don't have"s? IIRC, we don't remove things from wantlists until we stop wanting the block.

Stebalien avatar Feb 08 '20 00:02 Stebalien

For each CID that is wanted by any session, we keep a "block presence" table of which peers have sent us a HAVE or DONT_HAVE

  • CID1
    • peer A: DONT_HAVE
    • peer B: HAVE
  • CID2
    • peer A: HAVE
    • peer C: DONT_HAVE

We use this information when deciding which peer to send a want to. Note that this table is shared across all active sessions. Separately (in the PeerManager) we keep track of each want-have and want-block we've sent to each peer.

When the session shuts down, we remove all CIDs that were wanted by that session (and not wanted by any other session) from the "block presence" table, and send cancels to peers for those CIDs.

When we receive a block we send a cancel to all peers. We don't need the HAVE / DONT_HAVE information any more (although as currently implemented we don't remove it from the "block presence" table).

Is there another scenario in which we send a cancel to a peer?

dirkmc avatar Feb 10 '20 15:02 dirkmc

Is there another scenario in which we send a cancel to a peer?

When we cancel a want for a block (without canceling the rest of the session). That is, we can cancel a mySession.GetBlock.

I don't think we usually (or ever?) do this from go-ipfs at the moment but we will if users start using it as a library (and, e.g., start seeking around files).

Stebalien avatar Feb 10 '20 16:02 Stebalien

When we cancel a want for a block (without canceling the rest of the session). That is, we can cancel a mySession.GetBlock

It looks like in that case we don't actually send out a cancel to the peer (and didn't in the old Bitswap either).

We should send a cancel when the user cancels a call to GetBlock(). It's a little tricky because we need to make sure that no other session wants the same block before we send a cancel, but we can check that with the SessionInterestManager.

As you point out we should also make sure to sync the BlockPresenceManager when there is a disconnect.

dirkmc avatar Feb 10 '20 16:02 dirkmc

Ah... could you file a bug for that as well?

Stebalien avatar Feb 10 '20 17:02 Stebalien

Filed a bug at https://github.com/ipfs/go-bitswap/issues/253

dirkmc avatar Feb 10 '20 23:02 dirkmc