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

Routing.Type=auto: announcements over HTTP PUT cid.contact always fail #9504

Closed
lidel opened this issue Dec 14, 2022 · 4 comments · Fixed by #9524
Closed

Routing.Type=auto: announcements over HTTP PUT cid.contact always fail #9504

lidel opened this issue Dec 14, 2022 · 4 comments · Fixed by #9524
Assignees
Labels
effort/days Estimated to take multiple days, but less than a week exp/expert Having worked on the specific codebase is important kind/bug A bug in existing code (including security flaws) P1 High: Likely tackled by core team if no one steps up topic/IPNI Issues related to InterPlanetary Network Indexer

Comments

@lidel
Copy link
Member

lidel commented Dec 14, 2022

Version

0.18.0-rc1

Config

default produced by 'ipfs init' (Routing.Type=auto)

Problem

Only GET /routing/v1/providers/{CID} in IPIP-337 is currently supported by https://cid.contact/, and they have no short term plans to enable it.

At the same time, Kubo 0.18.0-rc1 running with Routing.Type=auto (default) sends HTTP PUTs that will always fail.

Solution

TBD, two obvious options come to mind:

  • (A) disable announcing over HTTP in auto mode for now, we will enable it in the future release after cid.contact supports it
  • (B) make it future-proof: detect IPNI being read-only (failing HTTP PUT multiple times) and temporarily block announcements on it (e.g. for 22h, to match default Reprovide.Interval)

I don't know how "stable" PUT /routing/v1/providers API is. If it is ready, then my vote would be to implement (B) in 0.18.0-rc2, as it is safer behavior that accounts for the slow rate of nodes updating to new versions of Kubo. (B) allows 0.18 nodes to start publishing as soon cid.contact supports it.

cc @ajnavarro @guseggert @willscott @masih

@lidel lidel added kind/bug A bug in existing code (including security flaws) exp/expert Having worked on the specific codebase is important P1 High: Likely tackled by core team if no one steps up effort/days Estimated to take multiple days, but less than a week topic/IPNI Issues related to InterPlanetary Network Indexer labels Dec 14, 2022
@ajnavarro
Copy link
Member

ajnavarro commented Dec 14, 2022

I agree with B. We could use https://github.com/hashicorp/go-retryablehttp . Integration should be easy. WDYT?

@lidel
Copy link
Member Author

lidel commented Dec 14, 2022

Yes, that would mitigate temporary issues with HTTP, which feels useful, but may not be enough for (B).
We need to make it smarter than a retry-per-request-with-expotential-backoff: need additional global "cooldown" flag per HTTP endpoint that NEVER responded with success to a PUT. Such flag would disable PUTs for some time (22h) to avoid wasting resources on a useless endpoint.

@masih
Copy link
Member

masih commented Dec 15, 2022

For reference IPNI will return 501 Not Implemented on PUT. That status code is not a retry-able error really, so maybe upon observing that status code the logic can aggressively increase retry intervals between consecutive PUTs, e.g. in order of days.

@guseggert
Copy link
Contributor

I'm strongly favoring A) since B) makes a bunch of assumptions about how providing will work but we haven't really implemented and exercised it yet (to my knowledge?). I think we should treat the upgrade problem as a separate issue and favor incremental delivery of features instead of trying to front-load them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/days Estimated to take multiple days, but less than a week exp/expert Having worked on the specific codebase is important kind/bug A bug in existing code (including security flaws) P1 High: Likely tackled by core team if no one steps up topic/IPNI Issues related to InterPlanetary Network Indexer
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants