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

feat: refactor message receiving code #407

Closed
wants to merge 1 commit into from

Conversation

Stebalien
Copy link
Member

The primary motivation for this was to only account for wanted blocks in the engine.

This change also untangles some code, fixes some todos, and avoids reusing receiveBlocksFrom for processing local blocks.

bitswap.go Show resolved Hide resolved
bitswap.go Show resolved Hide resolved
@Stebalien Stebalien marked this pull request as draft June 6, 2020 00:08
@Stebalien Stebalien requested a review from dirkmc June 6, 2020 00:08
Copy link
Contributor

@dirkmc dirkmc left a comment

Choose a reason for hiding this comment

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

I always found the way we try to process locally received blocks the same as incoming messages a bit awkward, I like this better 👍

bitswap.go Show resolved Hide resolved
@dirkmc
Copy link
Contributor

dirkmc commented Jun 8, 2020

Any reason not to merge? (I see it's a draft PR)

Stebalien added a commit that referenced this pull request Jun 8, 2020
Quick alternative to #407 to fix the main issue.
dirkmc pushed a commit that referenced this pull request Jun 8, 2020
Quick alternative to #407 to fix the main issue.
@Stebalien
Copy link
Member Author

Any reason not to merge? (I see it's a draft PR)

I wanted to get in a quick fix first for the release. I left it as a draft due to that last question.

The primary motivation for this was to only account for _wanted_ blocks in the
engine.

This change also untangles some code, fixes some todos, and avoids reusing
`receiveBlocksFrom` for processing _local_ blocks.
@Stebalien Stebalien force-pushed the refactor/receive-message branch from 814ce68 to 30b442e Compare June 10, 2020 16:52
// Record how many bytes were received in the ledger
for _, block := range blks {
log.Debugw("Bitswap engine <- block", "local", e.self, "from", p, "cid", block.Cid(), "size", len(block.RawData()))
l.ReceivedBytes(len(block.RawData()))
Copy link
Member Author

Choose a reason for hiding this comment

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

We should probably just replace this with some form of l.ReceivedBlocks(...) and l.SentBlocks(...). But we can do that later.

@Stebalien Stebalien marked this pull request as ready for review June 10, 2020 16:55
@Stebalien Stebalien requested a review from dirkmc June 10, 2020 16:56
@Stebalien
Copy link
Member Author

Let's wait until we get #413 fixed before merging this (so we don't have to fork a patch release branch).

Copy link
Contributor

@dirkmc dirkmc left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@Jorropo
Copy link
Contributor

Jorropo commented Jan 27, 2023

This repository has been moved to https://github.com/ipfs/go-libipfs. There is not an easy way to transfer PRs, so if you would like to continue with this PR then please re-open it in the new repository and link to this PR.

@Jorropo Jorropo closed this Jan 27, 2023
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.

3 participants