Skip to content
This repository was archived by the owner on Jun 27, 2023. It is now read-only.

write blocks retrieved from the exchange to the blockstore#92

Merged
Jorropo merged 9 commits intoipfs:masterfrom
MichaelMure:bitswap-no-add-bs
Jul 28, 2022
Merged

write blocks retrieved from the exchange to the blockstore#92
Jorropo merged 9 commits intoipfs:masterfrom
MichaelMure:bitswap-no-add-bs

Conversation

@MichaelMure
Copy link
Copy Markdown
Contributor

This follows the change in bitswap where that responsibility was removed (ipfs/go-bitswap#571).

@MichaelMure
Copy link
Copy Markdown
Contributor Author

cc @Jorropo

blockservice.go Outdated
logger.Debugf("BlockService.BlockFetched %s", b.Cid())

// also write in the blockstore for caching
// TODO: for performance reason, it might make sense to batch those writes
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This probably needs to happen before we merge (it has significant performance implications). A simple version would be to read from rblocks with a default select statement (i.e., drain it), then write a batch. That's not perfect, but it'll help as long as rblocks is buffered (IIRC, it is).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's not perfect, but it'll help as long as rblocks is buffered (IIRC, it is).

Turns out, it's not buffered: https://github.com/ipfs/go-bitswap/blob/b18a91d6023b83821c72253bbe5e37190db64d63/internal/getter/getter.go#L93

This follows the change in bitswap where that responsibility was removed.
@MichaelMure
Copy link
Copy Markdown
Contributor Author

I added the Put batching, tests to make sure that blockservice does write blocks, and followed the interface changes.

Again, we need to release and bubble up the dependency. I think the last remaining point is possibly to buffer the output channel in bitswap: #92 (comment)

@Jorropo
Copy link
Copy Markdown
Contributor

Jorropo commented Jul 27, 2022

Blocking in MichaelMure#1

@Jorropo Jorropo force-pushed the bitswap-no-add-bs branch from 06f850f to 7374061 Compare July 28, 2022 03:14
@Jorropo
Copy link
Copy Markdown
Contributor

Jorropo commented Jul 28, 2022

The batch loop was too confusing, I rewrote it (hopefully) slightly better.

I would like a merge review on 30ca6aa pls.

Copy link
Copy Markdown
Contributor

@Jorropo Jorropo left a comment

Choose a reason for hiding this comment

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

LGTM, let's dog food this

@Jorropo Jorropo merged commit 275986e into ipfs:master Jul 28, 2022
@MichaelMure
Copy link
Copy Markdown
Contributor Author

I would like a merge review on 30ca6aa pls.

LGTM

@ajnavarro ajnavarro mentioned this pull request Aug 24, 2022
72 tasks
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