Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

booster bitswap MVP executable #707

Merged
merged 8 commits into from
Sep 12, 2022
Merged

Conversation

hannahhoward
Copy link
Collaborator

@hannahhoward hannahhoward commented Aug 17, 2022

Goals

This is basically a "mashup" of booster-http, #377, and filecoin-project/dagstore#116 to kick the tires and see how to wire everything up for an independent boost executable.

Implementation

  • Copy booster-http files
  • Replace HTTPServer with bitswap server
  • Migrate over the indexed blockstore from DAGStore PR and modify it to only use the available APIs
  • Migrate over the piece selection function from [WIP] Bitswap integration in Boost #377, modify to not use go-fil-markets

TODO

  • Tests, comments
  • Read the libp2p key from the boostd repo
  • Return ipld ErrNotFound from GetSize() if the block is not found, because Bitswap checks for this error to decide if a block is present
    Note that the RemoteBlockstore calls across an RPC boundary, so the code will need to do string matching to check if the error message contains "not found"
  • Optimize DAG store PR
    • Make sure that blockstore.Has and blockstore.GetSize are efficient
    • Make sure that checking whether a block is in a piece that is unsealed a zero-cost is efficient

Merge Note

This PR is using the base branch of release/lotus1.17.2, so that we can use that for landing all upcoming changes that require the a libp2p update that is landing in Lotus 1.17.2 (early October). This is a workaround until we get better separation of the dependency chain with Lotus.

For discussion

Key issues identified:

  • dagstore.AcquireShard is not available on the API (and I assume it won't be) so that means unless a CarV2 was stored, we're going to be indexing every time we pull a shard. Perhaps this moves up the priority of PieceDirectory initial implementation #573 or associated efforts to make indexes more easily accessible
  • I couldn't figure out pricing without moving over a lot of go-fil-markets machinery around asks. We probably need to consider the API for figuring out of if a deal is free.

@hannahhoward
Copy link
Collaborator Author

Note: the build is probably broken until the libp2p 0.21 updates filter through the Lotus stack
-- filecoin-project/lotus#8970

@dirkmc
Copy link
Contributor

dirkmc commented Aug 17, 2022

Thanks for putting this together 🙏

  1. With respect to the blockstore interface, I'd suggest we expose the blockstore methods that bitswap needs on the boostd API. For now we can implement it in boostd using the DAG store, and then we can switch over to the piece meta service when it's ready.
  2. With regards to figuring out if a deal is free, we could just treat blocks in non-free deals as not found. eg Has(cid) would return false if the block is in a non-free deal.

@willscott
Copy link
Collaborator

What's the plan for index update to indicate bitswap is available?

is this a separate libp2p identity (or at least multiaddr host / port endpoint) the provider will expose?

@ribasushi
Copy link
Contributor

is this a separate libp2p identity (or at least multiaddr host / port endpoint) the provider will expose?

I'd say it should be configurable on the SP side: "announce using X and Y"

@jacobheun
Copy link
Contributor

I added an issue for tracking the index announcement work, #711, we should clarify what needs to happen in there. I added the address registration as a consideration we need to account for.

@dirkmc
Copy link
Contributor

dirkmc commented Aug 24, 2022

I did a bit of refactoring to clean up some unused variables etc, and rebased on top of main

@dirkmc
Copy link
Contributor

dirkmc commented Aug 24, 2022

I was able to retrieve a block using IPFS:

ipfs swarm connect <multiaddr>
ipfs get <cid>

@dirkmc
Copy link
Contributor

dirkmc commented Aug 24, 2022

Note: As part of productionizing this PR we need to look at optimizations in the DAG store PR: at the moment it's not very efficient in the way that it checks if a block is in a piece that is unsealed and free (cost is zero) to download.

@hannahhoward
Copy link
Collaborator Author

👍 on refactoring changes @dirkmc

@dirkmc
Copy link
Contributor

dirkmc commented Aug 31, 2022

I'm not sure we need the IsFreeAndUnsealed API on the retrieval market Provider.

  • pieceInUnsealedSector is simple enough we can just implement it on the boost side.
  • maybe we just expose GetDynamicAsk on the retrieval market Provider interface?

I'd suggest that we pass the piece CID but not the payload CID to GetDynamicAsk. Potentially it makes a call out to a third party system so it's likely too expensive to call for individual block cids.

We should also add caching per piece cid for pieceInUnsealedSector and GetDynamicAsk (but this can be done in a follow-up PR).

@hannahhoward
Copy link
Collaborator Author

@dirkmc I've made the requested changes in go-fil-markets and move the rest of the is-free-and-unsealed code within this repo.

@jacobheun jacobheun changed the base branch from main to release/lotus1.17.2 September 5, 2022 13:51
@jacobheun
Copy link
Contributor

I've updated the base branch on this to release/lotus1.17.2, we can use that for landing all upcoming changes that require the libp2p update that will land with 1.17.2

@jacobheun
Copy link
Contributor

For the rest of the outstanding TODO's we want to complete after merging this, can we get issues for those created and tracked in #709 so we can move forward with the merge here?

Is there anything else outstanding to land this PR other than creating those issues? I'd like to land this by eod Tuesday to unblock the tracing integration.

@nonsense nonsense marked this pull request as ready for review September 12, 2022 16:24
@nonsense nonsense merged commit 83ad7dd into release/lotus1.17.2 Sep 12, 2022
@nonsense nonsense mentioned this pull request Sep 12, 2022
8 tasks
@dirkmc dirkmc deleted the feat/bitswap-server branch September 13, 2022 09:33
nonsense added a commit that referenced this pull request Sep 13, 2022
* feat(booster-bitswap): booster bitswap MVP untested

* refactor(booster-bitswap): use API for fetching blocks

* fix(deps): update deps to compile

* feat(booster-bitswap): makefile & fixes

add commands to build booster-bitswap, and very a round tripped successful fetch from
booster-bitswap

* refactor: clean up unused vars etc

* fix: booster-bitsawp - check error when creating libp2p key

* refactor(node): avoid FreeAndUnsealed method

Co-authored-by: Dirk McCormick <[email protected]>
Co-authored-by: Anton Evangelatov <[email protected]>
LexLuthr pushed a commit that referenced this pull request Oct 4, 2022
* booster bitswap MVP executable (#707)

* feat(booster-bitswap): booster bitswap MVP untested

* refactor(booster-bitswap): use API for fetching blocks

* fix(deps): update deps to compile

* feat(booster-bitswap): makefile & fixes

add commands to build booster-bitswap, and very a round tripped successful fetch from
booster-bitswap

* refactor: clean up unused vars etc

* fix: booster-bitsawp - check error when creating libp2p key

* refactor(node): avoid FreeAndUnsealed method

Co-authored-by: Dirk McCormick <[email protected]>
Co-authored-by: Anton Evangelatov <[email protected]>

* booster-bitswap devnet and tracing (#796)

* return ipld ErrNotFound from remote blockstore interface (#798)

* fix: return ipld ErrNotFound from remote blockstore interface

* test: add more tests for ipld ErrNotFound

* test: comment out part of TestDummydealOnline that is flaky due to a bug in latest lotus (#802)

* fix normaliseError nil ptr dereference (#803)

* feat: shard selector (#807)

* LoadBalancer for bitswap (and later, more of libp2p) (#786)

* feat(loadbalancer): add message types

* feat(messages): add utility functions

* feat(loadbalancer): initial load balancer impl

implementation of the load balancer node itself

* feat(loadbalancer): add service node

implements code for running a service node

* feat(loadbalancer): integrate into boost and booster-bitswap

* Update loadbalancer/loadbalancer.go

Co-authored-by: Rod Vagg <[email protected]>

* Update loadbalancer/servicenode.go

Co-authored-by: Rod Vagg <[email protected]>

* Update loadbalancer/servicenode.go

Co-authored-by: Rod Vagg <[email protected]>

* Update loadbalancer/messages/messages.ipldsch

Co-authored-by: Rod Vagg <[email protected]>

* Update loadbalancer/messages/messages.ipldsch

Co-authored-by: Rod Vagg <[email protected]>

* refactor(loadbalancer): remove routing protocol

remove the routing protocol, instead relying on a set config. also remove forwarding response for
inbound requests

* fix(loadbalancer): update tests

* refactor(loadbalancer): integrate simplified load balancer

removed pub keys to minimize network traffic, added api's to configure and update bitswap peer id,
added auto config of bitswap peer id in booster-bitswap

* docs(gen): regenerate api docs

* chore(lint): fix lint errors

* fix(loadbalancer): minor bridgestream fix

* Update loadbalancer/servicenode.go

Co-authored-by: dirkmc <[email protected]>

* refactor(protocolproxy): address PR comments

renames, reconfigured architecture, etc

* refactor(make init print out peer id): remove apis and transparent peer id setting. have init print

Co-authored-by: Rod Vagg <[email protected]>
Co-authored-by: dirkmc <[email protected]>

* Add block filter via BadBits (#825)

* feat(booster-bitswap): add block filter via BadBits

* refactor(booster-bitswap): use bitswap blockfilter for filtering

* feat(blockfilter): only update when list is modified

* feat(blockFilter): add on disk caching

* Update cmd/booster-bitswap/blockfilter/blockfilter.go

Co-authored-by: dirkmc <[email protected]>

* fix(blockfilter): minor PR fixups

Co-authored-by: dirkmc <[email protected]>

* Libp2p 0.22 upgrade (#837)

* chore(deps): upgrade to Lotus RC & libp2p v0.22

* chore(deps): update go to 1.18

* ci(circle): update circle to go 1.18

* style(imports): fix imports

* fix(build): update ffi

* fix(lint): fix deprecated strings.Title method

* fix(mod): mod tidy

* Protocol Proxy cleanup (#836)

* refactor(booster-bitswap): minor UI fixes for booster-bitswap UI

* Update cmd/booster-bitswap/init.go

Co-authored-by: dirkmc <[email protected]>

Co-authored-by: dirkmc <[email protected]>

* feat: update to dagstore v0.5.5 (#849)

* add booster-bitswap to devnet (#866)

* bump lotus-test version

* add docker/booster-bitswap target in Makefile

Co-authored-by: Hannah Howard <[email protected]>
Co-authored-by: Dirk McCormick <[email protected]>
Co-authored-by: Rod Vagg <[email protected]>
dirkmc added a commit that referenced this pull request Oct 4, 2022
* booster bitswap MVP executable (#707)

* feat(booster-bitswap): booster bitswap MVP untested

* refactor(booster-bitswap): use API for fetching blocks

* fix(deps): update deps to compile

* feat(booster-bitswap): makefile & fixes

add commands to build booster-bitswap, and very a round tripped successful fetch from
booster-bitswap

* refactor: clean up unused vars etc

* fix: booster-bitsawp - check error when creating libp2p key

* refactor(node): avoid FreeAndUnsealed method

Co-authored-by: Dirk McCormick <[email protected]>
Co-authored-by: Anton Evangelatov <[email protected]>

* booster-bitswap devnet and tracing (#796)

* return ipld ErrNotFound from remote blockstore interface (#798)

* fix: return ipld ErrNotFound from remote blockstore interface

* test: add more tests for ipld ErrNotFound

* test: comment out part of TestDummydealOnline that is flaky due to a bug in latest lotus (#802)

* fix normaliseError nil ptr dereference (#803)

* feat: shard selector (#807)

* LoadBalancer for bitswap (and later, more of libp2p) (#786)

* feat(loadbalancer): add message types

* feat(messages): add utility functions

* feat(loadbalancer): initial load balancer impl

implementation of the load balancer node itself

* feat(loadbalancer): add service node

implements code for running a service node

* feat(loadbalancer): integrate into boost and booster-bitswap

* Update loadbalancer/loadbalancer.go

Co-authored-by: Rod Vagg <[email protected]>

* Update loadbalancer/servicenode.go

Co-authored-by: Rod Vagg <[email protected]>

* Update loadbalancer/servicenode.go

Co-authored-by: Rod Vagg <[email protected]>

* Update loadbalancer/messages/messages.ipldsch

Co-authored-by: Rod Vagg <[email protected]>

* Update loadbalancer/messages/messages.ipldsch

Co-authored-by: Rod Vagg <[email protected]>

* refactor(loadbalancer): remove routing protocol

remove the routing protocol, instead relying on a set config. also remove forwarding response for
inbound requests

* fix(loadbalancer): update tests

* refactor(loadbalancer): integrate simplified load balancer

removed pub keys to minimize network traffic, added api's to configure and update bitswap peer id,
added auto config of bitswap peer id in booster-bitswap

* docs(gen): regenerate api docs

* chore(lint): fix lint errors

* fix(loadbalancer): minor bridgestream fix

* Update loadbalancer/servicenode.go

Co-authored-by: dirkmc <[email protected]>

* refactor(protocolproxy): address PR comments

renames, reconfigured architecture, etc

* refactor(make init print out peer id): remove apis and transparent peer id setting. have init print

Co-authored-by: Rod Vagg <[email protected]>
Co-authored-by: dirkmc <[email protected]>

* Add block filter via BadBits (#825)

* feat(booster-bitswap): add block filter via BadBits

* refactor(booster-bitswap): use bitswap blockfilter for filtering

* feat(blockfilter): only update when list is modified

* feat(blockFilter): add on disk caching

* Update cmd/booster-bitswap/blockfilter/blockfilter.go

Co-authored-by: dirkmc <[email protected]>

* fix(blockfilter): minor PR fixups

Co-authored-by: dirkmc <[email protected]>

* Libp2p 0.22 upgrade (#837)

* chore(deps): upgrade to Lotus RC & libp2p v0.22

* chore(deps): update go to 1.18

* ci(circle): update circle to go 1.18

* style(imports): fix imports

* fix(build): update ffi

* fix(lint): fix deprecated strings.Title method

* fix(mod): mod tidy

* Protocol Proxy cleanup (#836)

* refactor(booster-bitswap): minor UI fixes for booster-bitswap UI

* Update cmd/booster-bitswap/init.go

Co-authored-by: dirkmc <[email protected]>

Co-authored-by: dirkmc <[email protected]>

* feat: update to dagstore v0.5.5 (#849)

* feat: bitswap client

* feat: bitswap client - output car file

* refactor: bitswap client - remove tracing

* feat: debug logs

* fix: write blocks to blockstore

* fix: duration output

* fix: duration output for block received

* feat: add pprof to bitswap client

* feat: protocol proxy logging

* feat: bitswap client - check host supports bitswap protocol

* feat: listen for bitswap requests locally as well as through forwarding protocol

Co-authored-by: Hannah Howard <[email protected]>
Co-authored-by: Anton Evangelatov <[email protected]>
Co-authored-by: Rod Vagg <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants