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

Design desired comprehensive routing config #9188

Closed
Tracked by #9150
BigLep opened this issue Aug 12, 2022 · 13 comments
Closed
Tracked by #9150

Design desired comprehensive routing config #9188

BigLep opened this issue Aug 12, 2022 · 13 comments
Assignees

Comments

@BigLep
Copy link
Contributor

BigLep commented Aug 12, 2022

Done Criteria

We have a design document that covers:

  1. Problems we need to solve and usescases we need to meet within Kubo around supporting multiple routers.
  2. Proposed solution(s) with justification and pros/cons. This proposal should show what the Kubo config will look like for a user attempting to satisfy their usecases.

Why Important

In #9150, we have an immediate usecase of getting Kubo to query Reframe.FindProviders whenever it would normally query the DHT so that the ipfs.io Gateway can use storetheindex as an extra router to the DHT. A config migration will be needed to go from our previous world without reframe to one with reframe. Repo migrations aren't free to implement and we'd like to make sure that the work we do for #9150 is moving us in a direction that doesn't require immediate additional config migrations as we support additional routers and options.

Notes

  1. The design document should be in a place that is public and easy/quick to leave contextual comments. This likely means Notion or Hackmd. If no preference, Notion is recommended so it fits within the Kubo document browse tree.
  2. Routing: conform DHT to common Router interface #9079 as defined in Routing: conform DHT to common Router interface #9079 (comment) doesn't account for areas like:
  • Other routers (peer routing, local DHT)
  • How multiple routers will be combined for each routing "method" in terms of execution ordering and result merging.
  1. https://github.com/libp2p/go-libp2p-routing-helpers contains some helpers for combining routers
  2. Not authoritative but Routing: conform DHT to common Router interface #9079 (comment) is one proposal sketch for what the config can look like.
  3. There may be other patterns in the codebase we can draw on (like datastore) that allow for nesting/wrapping.
@BigLep BigLep moved this to 🥞 Todo in IPFS Shipyard Team Aug 12, 2022
@BigLep BigLep changed the title Design desired router config Design desired comprehensive routing config Aug 12, 2022
@ajnavarro
Copy link
Member

Design document work in progress: https://hackmd.io/@ajnavarro/HJr059wC9

@ajnavarro
Copy link
Member

@BigLep document is ready for review!

@BigLep
Copy link
Contributor Author

BigLep commented Aug 18, 2022

@ajnavarro : Thanks a lot Antonio for putting this together. There’s a decent amount of subtlety here. For this week we need to settle on the design ideally and estimate the work. I would be great to get another set of eyes like @guseggert’s since I see @lidel already reviwed. I’m particularly interested to see if there’s a way to simplify.

Note to self: comments on code snippets or blocks in hackmd get put to the bottom. As a result, I have commented in the whitespace around code blocks so the comments can be seen inline.

@ajnavarro
Copy link
Member

ajnavarro commented Aug 18, 2022

I addressed all the comments. I will need some help for:

  • How to make bitswap WANT broadcasting fit here (I'll need some advice)
  • How far do we want to go with DHT configuration params. I'm using a flag to configure public or private DHTs because it is what it was defined in almost all previous documents.
  • I didn't understand the problem @lidel mentioned: ("should hardcode the implicit default routers in user configuration") could you develop a little more where the problem is?

Thanks!

@ajnavarro
Copy link
Member

Changed the JSON structure to make it more versatile. Also, after talking with @guseggert and @lidel, we are NOT going to need a migration step. Everything is described in the design document:

https://hackmd.io/@ajnavarro/HJr059wC9

@guseggert FYI, things I changed from your proposal:

  • Moved global timeout to helper routers. If we have a timeout per router, It makes more sense to me that the helper router (the parallel or sequential one) should own the global Timeout.
  • Removed MaxResults option.

@guseggert
Copy link
Contributor

How far do we want to go with DHT configuration params.

As long as we have a clear place to add them later, we don't have to figure all of this out right now.

@BigLep
Copy link
Contributor Author

BigLep commented Aug 19, 2022

@ajnavarro : I've lost access to https://hackmd.io/@ajnavarro/HJr059wC9 . Can you please reshare?

@BigLep BigLep moved this from 🥞 Todo to 🔎 In Review in IPFS Shipyard Team Aug 19, 2022
@BigLep
Copy link
Contributor Author

BigLep commented Aug 19, 2022

2022-08-19 conversation:

  1. Deferring "How to make bitswap WANT broadcasting" into the current design.
  2. Other items in Design desired comprehensive routing config #9188 (comment) have been covered.

At this point we think "we're done", but we should tie it off.

@ajnavarro
Copy link
Member

ajnavarro commented Aug 19, 2022

  • Routers (3)
  • FX estimation (5)
  • Sharness smoke test (1-2)
  • Testing on gateways (1-2)
  • Documentation (1)
  • Metrics ()
    • Edelweiss (1) (Define metrics we want, RPC metrics already there)

@ajnavarro
Copy link
Member

@BigLep Could you clarify how this is related to boost? Thanks!

@BigLep
Copy link
Contributor Author

BigLep commented Aug 19, 2022

@BigLep Could you clarify how this is related to boost? Thanks!

I'm confused. I don't think it is related to Bitswap/Boost integration and I don't have that mentioned anywhere.

Also, thanks for the estimates. I'll move those to the tracking issue for getting this into production: #9150

@BigLep
Copy link
Contributor Author

BigLep commented Aug 20, 2022

@ajnavarro : I think we can mark this done once:

  1. Any other open review comments have been responded to or closed
  2. We make clear that the design is intentionally deferring "how to make bitswap WANT broadcasting" from the current design and why we're deferring.

@ajnavarro
Copy link
Member

@BigLep I don't know why but Boost appeared in our conversation and I wrote the question down so I didn't forget.

  • All comments have been responded.
  • Adding into the document that bitswap WANT broadcasting is deferred to future iterations.

https://hackmd.io/@ajnavarro/HJr059wC9

Repository owner moved this from 🔎 In Review to 🎉 Done in IPFS Shipyard Team Aug 22, 2022
@BigLep BigLep moved this from 🎉 Done to ☑️ Done (Archive) in IPFS Shipyard Team Aug 30, 2022
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
Archived in project
Development

No branches or pull requests

3 participants