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

feat(messagequeue): rebroadcast wantlist #106

Merged
merged 3 commits into from
Apr 10, 2019
Merged

Conversation

hannahhoward
Copy link
Contributor

Goals

Provide a failsafe to losing wants on other end by rebroadcasting a wantlist every thirty seconds

Implementation

Every peer messagequeue has a rebroadcast timer, which, when it expires, resends all the wants. On the other hand, existing logic in the decision engine will dedup these wants if they are already queued up (see peer request queue)

For discussion

What's the right interval? I set 30 seconds as a start --- seems long enough to so that there won't be a ton of new network traffic, but short enough that a lost want doesn't mean a super extra long recovery

Do we need something smarter? Or is just rebroadcasting at a regular interval ok? I would be inclined to try this to start, and do something smarter (like rebroadcast if wants sit on the list a long time) later.

fix #99, fix #65

Provide a failsafe to losing wants on other end by rebroadcasting a wantlist every thirty seconds

fix #99, fix #65
@ghost ghost assigned hannahhoward Apr 4, 2019
@ghost ghost added the status/in-progress In progress label Apr 4, 2019
Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

This is going to have some overhead but it's probably fine. One nit but this otherwise looks good to me.

func (mq *MessageQueue) SetRebroadcastInterval(delay time.Duration) {
mq.rebroadcastIntervalLk.Lock()
mq.rebroadcastInterval = delay
mq.rebroadcastTimer.Reset(delay)
Copy link
Member

Choose a reason for hiding this comment

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

We should probably check if rebroadcastTimer is nil here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point.

Make sure rebroadcast timer doesn't get reset if it's nil
@Stebalien
Copy link
Member

--- FAIL: TestWantlistRebroadcast (0.02s)
    messagequeue_test.go:185: wrong number of messages were sent for initial wants

timing on test was failure prone, corrected
@Stebalien Stebalien merged commit 2c47a55 into master Apr 10, 2019
@Stebalien
Copy link
Member

Test failure is ipfs/boxo#99.

@ghost ghost removed the status/in-progress In progress label Apr 10, 2019
@Stebalien
Copy link
Member

Thanks!

@Stebalien Stebalien deleted the feat/rebroadcast-wantlist branch April 10, 2019 17:56
Jorropo pushed a commit to Jorropo/go-libipfs that referenced this pull request Jan 26, 2023
…tlist

feat(messagequeue): rebroadcast wantlist

This commit was moved from ipfs/go-bitswap@2c47a55
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.

Fail to get block from connected peer Should the client repeat the broadcast block request?
2 participants