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

feat: remote backend #117

Merged
merged 9 commits into from
May 24, 2024
Merged

feat: remote backend #117

merged 9 commits into from
May 24, 2024

Conversation

hacdias
Copy link
Member

@hacdias hacdias commented Apr 17, 2024

On top of #111. Closes #88. This PR adds support for remote backends (as described in #88). I made it such that it is the most flexible as I could. It should in scenarios where Libp2p is completely disabled (no bitswap, no dht), but also if libp2p needs to be enabled for things such as peer routing for peering and such.

@hacdias hacdias self-assigned this Apr 17, 2024
@hacdias hacdias force-pushed the remote-backend branch 2 times, most recently from f7a2098 to 8d7348a Compare April 17, 2024 14:13
@hacdias hacdias requested review from lidel and aschmahmann and removed request for lidel April 17, 2024 14:14
@hacdias hacdias marked this pull request as ready for review April 17, 2024 14:14
main.go Outdated Show resolved Hide resolved
@lidel lidel added this to the v1.2 milestone Apr 19, 2024
@hacdias hacdias force-pushed the remote-backend branch 2 times, most recently from f5a8b9c to f1ea37f Compare April 22, 2024 08:34
@hacdias hacdias requested a review from lidel April 22, 2024 08:34
@hacdias hacdias closed this Apr 22, 2024
@hacdias hacdias reopened this Apr 22, 2024
@hacdias hacdias force-pushed the peering-cache branch 2 times, most recently from 17cb3e8 to a5aa71e Compare April 22, 2024 12:28
@lidel lidel modified the milestones: v1.2, v1.3 Apr 24, 2024
Base automatically changed from peering-cache to main April 24, 2024 08:31
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

Thanks, nearly there :)

We need to streamline configuration a bit – fetch my changes, I've tweaked some docs, but need your help with comments inline.

Second ask is to extend gateway-conformance.yml Github Actions workflow in this repo to ensure block/car backendns behave the same way as running libp2p and bitswap.

In addition to current test (with lib2p), it should run separate backend tests similar to ones from https://github.com/ipfs/bifrost-gateway/blob/0bb1a904cc4af6c0b5e3398a67f320c1eec383a1/.github/workflows/gateway-conformance.yml#L14

setup_routing.go Outdated Show resolved Hide resolved
main.go Show resolved Hide resolved
main.go Outdated
bitswap := cctx.Bool("bitswap")
dhtRouting := DHTRouting(cctx.String("dht-routing"))
seedPeering := cctx.Bool("seed-peering")
noLibp2p := !bitswap && dhtRouting == DHTOff && !seedPeering
Copy link
Member

@lidel lidel May 21, 2024

Choose a reason for hiding this comment

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

💭 noLibp2p setup feels overly complicated, and baloons the surface or permutations we need to test.

In practice either user will run their own libp2p node for dht+bitswap, or they will run no libp2p at all, and delegate both retrieval and routing to remote HTTP endpoints.

@hacdias Could you tweak it to have an explicit, global RAINBOW_LIBP2P=true|false (with true being default) that controls when SetupNoLibp2p is used instead of Setup[WithLibp2p]?

Then, inside SetupNoLibp2p just check if bitswap, dht-routing, seed-peering, libp2p flags are set to [expected values], and error otherwise.

We want end user's configuration to be simple:
RAINBOW_LIBP2P=false RAINBOW_REMOTE_BACKENDS=http://trustless-gateway.link ./rainbow

Copy link
Member Author

Choose a reason for hiding this comment

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

Then, inside SetupNoLibp2p just check if bitswap, dht-routing, seed-peering, libp2p flags are set to true, and error otherwise.

Shouldn't they be false and dht-routing=off? If we want to check if they haven't been set, we need to do that before calling SetupNoLibp2p.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have pushed a commit with a global flag

@hacdias hacdias requested a review from lidel May 21, 2024 09:09
@hacdias hacdias force-pushed the remote-backend branch 4 times, most recently from ad48392 to 5271857 Compare May 21, 2024 09:38
@lidel lidel force-pushed the remote-backend branch 6 times, most recently from f4d6632 to e525f8b Compare May 22, 2024 22:11
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

Thank you once again for salvaging this work @hacdias ❤️

I've pushed some test/docs, and simplified UX a bit, but kept both LIBP2P and BITSWAP flags, as they will be useful for testing when we start experimenting with HTTP retrieval.

Please take a look, and if no concerns, feel free to merge.

(we want to ship this as v1.3.0, but probably need to land #138 first)

.github/workflows/gateway-conformance.yml Outdated Show resolved Hide resolved
Comment on lines +340 to 348
// as a convenience to the end user, and to reduce confusion
// libp2p is disabled when remote backends are defined
remoteBackends := cctx.StringSlice("remote-backends")
if len(remoteBackends) > 0 {
fmt.Printf("RAINBOW_REMOTE_BACKENDS set, forcing RAINBOW_LIBP2P=false\n")
libp2p = false
bitswap = false
dhtRouting = DHTOff
}
Copy link
Member

Choose a reason for hiding this comment

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

ℹ️ ended up with this, to ensure we are not running into unexpected edge cases that we missed in test coverage, or may have in future after someone refactors things.

TLDR: for end user, logic is simple: setting a remote backend effectively disables libp2p and related features, and only HTTP backend and routers remain.

End user UX is:

$ RAINBOW_REMOTE_BACKENDS=http://127.0.0.1:8080 ./rainbow
Starting rainbow dev-build
RAINBOW_REMOTE_BACKENDS set, forcing RAINBOW_LIBP2P=false

hacdias and others added 9 commits May 23, 2024 09:19
These are only applied to libp2p nodes, not the entire rainbow.
Block and CAR backends have significate difference in code paths and
abstractions. We need to run conformance against them separately
to ensure both code paths behave the same way.
Setting RAINBOW_REMOTE_BACKENDS should be enough to
turn off libp2p and delegate everything to HTTP endpoints.
@lidel
Copy link
Member

lidel commented May 23, 2024

Merged ipfs/github-mgmt#205 and removed old conformance as required (we can add new one after this PR is mergeD)

@hacdias you should be able to merge now. If so, mind shipping as v1.3?

@hacdias hacdias merged commit ed0d3d3 into main May 24, 2024
13 checks passed
@hacdias hacdias deleted the remote-backend branch May 24, 2024 06:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: 🎉 Done
Development

Successfully merging this pull request may close these issues.

Add remote backend mode from bifrost-gateway
2 participants