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

refactor: simplify messageQueue onSent #349

Merged
merged 4 commits into from
Apr 13, 2020
Merged

Conversation

dirkmc
Copy link
Contributor

@dirkmc dirkmc commented Apr 13, 2020

No description provided.

@dirkmc dirkmc requested a review from Stebalien April 13, 2020 15:22
mq.bcstWants.MarkSent(bcstEntries[i])
}
for i := 0; i < peerSentCount; i++ {
mq.peerWants.MarkSent(peerEntries[i])
Copy link
Member

Choose a reason for hiding this comment

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

This won't quite work as we will have removed some of the peer wants to peers that don't support HAVE.

Copy link
Contributor Author

@dirkmc dirkmc Apr 13, 2020

Choose a reason for hiding this comment

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

We remove them when we realize the peer doesn't support HAVE, so in MarkSent() we could just check if they've already been removed from pending before adding them to sent.
But actually I think it's clearer to put them in a list. I pushed a commit with this change.

@@ -566,6 +566,7 @@ func (mq *MessageQueue) extractOutgoingMessage(supportsHave bool) (bsmsg.BitSwap
}

// Add each regular want-have / want-block to the message
peerSent := make([]wantlist.Entry, 0, len(peerEntries))
Copy link
Member

Choose a reason for hiding this comment

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

nit: we can actually reuse the original slice if we do this correctly. But this is a low priority as I'm pretty sure we're going to delete this code when we land #347.

@Stebalien Stebalien merged commit 6099047 into master Apr 13, 2020
Jorropo pushed a commit to Jorropo/go-libipfs that referenced this pull request Jan 26, 2023
…-onSent

refactor: simplify messageQueue onSent

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

2 participants