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

Idle tick behaviour #290

Closed
dirkmc opened this issue Mar 11, 2020 · 2 comments · Fixed by #291
Closed

Idle tick behaviour #290

dirkmc opened this issue Mar 11, 2020 · 2 comments · Fixed by #291

Comments

@dirkmc
Copy link
Contributor

dirkmc commented Mar 11, 2020

Current Behaviour

When a Session gets an idle tick (it hasn't received any blocks for a while) it

  • broadcasts want-have for all wants that were ever requested
  • queries the DHT for the first requested CID

However the wants are stored in a map, so the "first" CID is actually random (because golang randomizes the order of iteration over a map)

Proposal

When there is an idle tick:

  • broadcast want-have for all wants for which we haven't received a block (rather than all wants ever requested)
  • query the DHT for the first want that has not yet been received (rather than a random want / the first want ever requested)
@Stebalien
Copy link
Member

broadcasts want-have for all wants that were ever requested

You sure? I think we broadcast live wants (which shouldn't include wants for which we have already received blocks, as far as I know).

query the DHT for the first want that has not yet been received (rather than a random want / the first want ever requested)

Asking for the first want may work better in some cases, but asking for a random one may work better when we can't find a specific block. Ideally, we'd try them sequentially if that's not too hard.

@dirkmc
Copy link
Contributor Author

dirkmc commented Mar 11, 2020

I think we broadcast live wants (which shouldn't include wants for which we have already received blocks, as far as I know)

That's what I would have assumed, but I think there's actually a bug (probably introduced by the recent refactor) whereby the current behaviour is that the Session broadcasts wants for all blocks that have ever been requested.
Nevermind I think it's actually not including wants for which we've already received blocks, but the list is not size-limited.

It should instead have a dynamic, limited size list where

  • wants are added to the end of the list as the client requests blocks and wants are sent out
  • wants are dropped from the broadcast list as the corresponding blocks are received,
    and the list is refilled with live wants

Asking for the first want may work better in some cases

I'm not sure if that will work better. For example many peers may have the root block of a DAG, but the Session removes those peers if they send many DONT_HAVEs (because they don't have lower branches of the DAG). So in that case we wouldn't want to broadcast a request for the root block, only for the parts of the DAG that we haven't been able to retrieve.

Note that we do also periodically broadcast and query the DHT for a random live want.

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 a pull request may close this issue.

2 participants