Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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(routing): Delegated Routing #8997
feat(routing): Delegated Routing #8997
Changes from 15 commits
0638d25
104ae3e
b742db9
420c6fd
2f93364
25b9683
0f549a2
c6fb6e5
0b5c9d4
1ed2be0
32c6bba
4bff8ff
dc128d8
bc00d2d
43bc61e
6cb3a11
2f57adf
e3e2c6b
72e9546
4feb2ac
1e42022
04014f5
8020ba8
70d50fe
45a716e
79ed919
1cf68ff
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lidel idk if you have thoughts here, but I'm not sure how much we want to expose here.
Might be useful info to have to understand some notion of user preference, but also we may want to just do more in parallel depending on our concerns about resource usage.
AFAICT the only areas where priority kicks in are for the GetValue, GetPublicKey and FindPeer functions (https://github.com/libp2p/go-libp2p-routing-helpers/blob/506670d53a31503f257980dbf354b3ea1b990fb7/tiered.go#L30).
GetPublicKey
is basically unused,FindPeer
could probably be in parallel without issue, andGetValue
sort of makes sense to have prioritization (there's some weird edge-cases around IPNS records since they're mutable and you're trying to find the latest ones but don't want to hang around forever checking every system) but IIRC SearchValue is generally used anyway which has parallelism.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aschmahmann If we talk about future where we have DHT routers here as well, I think we need two optional settings:
priority
– allows setting the order (when it matters)mandatory
– allows for marking some routers as mandatory and others as best-effortWith these, we will be able to model custom rules around parallelism and best-effort/mandatory routers in the future, but for now we can skip them in config (thus optional with implicit default).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we add the
mandatory
flag we must stop usingroutinghelpers.TieredRouter
and use some new implementation that supports that.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack, this needs a design first.
We can "mandatory" decision/discussion to kubo 0.15 or later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add an issue to track it: #9083