This repository was archived by the owner on Feb 1, 2023. It is now read-only.
feat(sessions): add rebroadcasting, search backoff#133
Merged
hannahhoward merged 5 commits intomasterfrom Jun 4, 2019
Merged
Conversation
Contributor
Author
|
AAAND there's a data race, so, um, better go ahead and do that refactor after all :) |
Contributor
Author
|
JK not gonna refactor, case that turns out to be complicated and filters all the way up to bitswap and go-ipfs-exchange-interface :( |
hannahhoward
force-pushed
the
feat/improve-provider-requests
branch
from
58b4db1 to
9b04d41
Compare
Don't count consecutive ticks if there are no active wants
hannahhoward
force-pushed
the
feat/improve-provider-requests
branch
from
9b04d41 to
e2e3343
Compare
Re-setup provider search delay and rebroadcast delay on a per bitswap instance basis
Stebalien
reviewed
Jun 12, 2019
| func (s *Session) run(ctx context.Context) { | ||
| s.tick = time.NewTimer(provSearchDelay) | ||
| s.tick = time.NewTimer(s.provSearchDelay) | ||
| s.rebroadcast = time.NewTimer(s.rebroadcastDelay.Get()) |
Member
There was a problem hiding this comment.
Nit: I'm not sure using delay.D is all that useful here as we always just reuse the same initial delay. delay.D is designed for random delays (we'd need to use a ticker and reset it with rebroadcast.Reset(s.rebroadcastDelay.NextWaitTime())).
| } | ||
| } | ||
|
|
||
| func (s *Session) handleRebroadcast(ctx context.Context) { |
Member
There was a problem hiding this comment.
This doesn't actually rebroadcast anything, does it? We should probably just call this something like handlePeriodicSearch.
Stebalien
pushed a commit
that referenced
this pull request
Jul 4, 2019
feat(sessions): add rebroadcasting, search backoff
Jorropo
pushed a commit
to Jorropo/go-libipfs
that referenced
this pull request
Jan 26, 2023
…r-requests feat(sessions): add rebroadcasting, search backoff This commit was moved from ipfs/go-bitswap@86089ee
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Goals
Attempt to kill two birds with one stone:
Implemenation
Start tracking whether a tick is "consecutive" -- i.e no new unique blocks have been received since the last tick. On consecutive ticks do two things different:
It might be a problem to never go looking for providers for the same block again but...
By adding a very periodic search for more providers no matter what (i.e. 1 minute -- mirroring the rebroadcast time in pre-sessions bitswap), there is a last ditch insurance policy that you'll keep looking for providers just in case they appear.
And, as a bonus, this means if you are receiving blocks but have slow providers, eventually you'll go looking for faster ones.
Ideally:
fix #95, fix #107
For discussion
At this point, the global options SetProviderSearchDelay & SetRebroadcastDelay are starting to get to be a smell (need options on sessions) and need a refactor but this is a subtle enough change I wanted it evaluated on its own.