kubo icon indicating copy to clipboard operation
kubo copied to clipboard

feat: DHT Reprovide Sweep

Open guillaumemichel opened this issue 7 months ago • 1 comments

Integrate the DHT SweepingReprovider as opt-in feature in Kubo.

Note that the SweepingReprovider cannot make use of the accelerated DHT client due to kubo interfaces restrictions.

  • Depends on https://github.com/libp2p/go-libp2p-kad-dht/pull/1082 :point_left: Remaining work is mostly documented there
  • Part of https://github.com/ipshipyard/roadmaps/issues/6 and https://github.com/ipshipyard/roadmaps/issues/7

guillaumemichel avatar Jun 10 '25 13:06 guillaumemichel

Gateway Conformance test failing even without any new code (see here). Either test or new dependencies broken.

guillaumemichel avatar Jun 11 '25 14:06 guillaumemichel

🎉

guillaumemichel avatar Jun 25 '25 11:06 guillaumemichel

Current status:

  • Based on https://github.com/libp2p/go-libp2p-kad-dht/pull/1135
  • I didn't write a changelog yet, if we want to tackle https://github.com/ipfs/kubo/issues/10909 here or in another PR, the changelog and config docs will change
  • SweepingProvider tests are currently limited to existing test/cli/provider_test.go, that I adapted to test both the old (BurstProvider) and the new (SweepingProvider) systems.
    • If we want to make it a default at some point, we may want to test all provide related tests against the SweepingProvider (including sharness, etc.)
    • We may want to write new tests specifically for the SweepingProvider
    • Most of the SweepingProvider behavior we may want to test is time based
      • harness tests may not be adapted if we cannot fake time.
      • Not sure whether we already have a test suite mocking time, and if not, whether it is worth it to set it up
      • Maybe we are fine with the simple smoke tests here, and more advanced behavior tests directly at kad-dht
  • This is the minimal integration of the SweepingProvider
    • Missing extra features, such as ipfs provide stats, ipfs provide status <cid>, etc. This part isn't implemented yet in kad-dht.
    • It should be fine to merge this large PR, and add extra features incrementally, in smaller PRs later on.

guillaumemichel avatar Aug 27 '25 15:08 guillaumemichel

I wonder if we should just drop the burstProvider altogether.

@guillaumemichel If we can configure the sweeping provider to behave as the burst provider by adjusting some parameter(s), then maybe that would simplify things.

gammazero avatar Aug 28 '25 21:08 gammazero

I wonder if we should just drop the burstProvider altogether.

Do you mean dropping the old provider altogether and replacing it with the new module? IIRC we agreed to make the SweepingProvider opt in for kubo 0.38.

If we can configure the sweeping provider to behave as the burst provider by adjusting some parameter(s), then maybe that would simplify things.

Both the BurstProvider and the SweepingProvider need to share a common interface used at many places in kubo and boxo.

I figured it would make more sense to build a wrapper around the old system that we are about to drop, to use the new system's interface, rather than retrofit the new system into the old one, and keep using the old interface.

Happy to change if anything seems wrong or you have a better suggestion.

guillaumemichel avatar Aug 28 '25 21:08 guillaumemichel

Do you mean dropping the old provider altogether and replacing it with the new module? IIRC we agreed to make the SweepingProvider opt in for kubo 0.38.

No, we should continue with what we planned. I just want to make sure we can drop the old provider after v0.38, which is the current plan.

I figured it would make more sense to build a wrapper around the old system that we are about to drop, to use the new system's interface, rather than retrofit the new system into the old one, and keep using the old interface. Happy to change if anything seems wrong or you have a better suggestion.

Also, after thinking about it more, I think this will make it easiest to drop the old system while having the advantage of exercising the new interfaces. Again, all good, nothing to change.

gammazero avatar Aug 29 '25 23:08 gammazero

Opened separate PR with config migration:

  • https://github.com/ipfs/kubo/pull/10951 Please take a look. The idea is to review that there, and ~~then merge that PR into this branch, and then merge everything together.~~ We may merge them separately to avoid noise in this one.

lidel avatar Sep 04 '25 00:09 lidel