Skip to content

feat: DHT Reprovide Sweep #10834

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 57 commits into
base: master
Choose a base branch
from
Open

feat: DHT Reprovide Sweep #10834

wants to merge 57 commits into from
+879 −353

Conversation

guillaumemichel
Copy link
Contributor

@guillaumemichel guillaumemichel commented Jun 10, 2025 •
edited
Loading

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.

Initial integration checklist:

@guillaumemichel guillaumemichel force-pushed the reprovide-sweep branch 2 times, most recently from c97cba6 to 323e72d Compare June 11, 2025 09:34
@guillaumemichel
Copy link
Contributor Author

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

@lidel lidel mentioned this pull request Jun 12, 2025
Closed
46 tasks
This was referenced Jun 12, 2025
Open
Open
This was referenced Jun 13, 2025
Closed
Closed
@lidel lidel force-pushed the reprovide-sweep branch from 7923957 to db54688 Compare June 24, 2025 21:47
@guillaumemichel
Copy link
Contributor Author

🎉

@lidel lidel mentioned this pull request Jun 29, 2025
Open
@guillaumemichel guillaumemichel mentioned this pull request Jul 7, 2025
Closed
@lidel lidel mentioned this pull request Jul 14, 2025
Open
51 tasks
@guillaumemichel guillaumemichel mentioned this pull request Jul 24, 2025
Open
42 tasks
gammazero
gammazero approved these changes Aug 15, 2025
View reviewed changes
Copy link
Contributor

@gammazero gammazero left a comment

Choose a reason for hiding this comment

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

  • Question about Purge function.
  • Maybe some missed errors.

Otherwise looks good.

@guillaumemichel guillaumemichel force-pushed the reprovide-sweep branch 2 times, most recently from a9bbe01 to 7c400c2 Compare August 20, 2025 10:15
@guillaumemichel guillaumemichel mentioned this pull request Aug 26, 2025
Open
1 task
@guillaumemichel guillaumemichel marked this pull request as ready for review August 27, 2025 14:40
@guillaumemichel guillaumemichel requested a review from a team as a code owner August 27, 2025 14:40
@guillaumemichel
Copy link
Contributor Author

Current status:

  • Based on provider: connectivity state machine libp2p/go-libp2p-kad-dht#1135 (not reviewed yet)
  • I didn't write a changelog yet, if we want to tackle Move Reprovider config to Provider #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
guillaumemichel commented Aug 28, 2025
View reviewed changes
test/cli/provider_test.go
Comment on lines +545 to +558
{
name: "BurstProvider",
reprovide: true,
apply: func(n *harness.Node) {
n.SetIPFSConfig("Reprovider.Sweep.Enabled", false)
},
},
{
name: "SweepingProvider",
reprovide: false,
apply: func(n *harness.Node) {
n.SetIPFSConfig("Reprovider.Sweep.Enabled", true)
},
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: I tried adding tests with Routing.AcceleratedDHTClient set to true, but they are timing out. I didn't investigate why since it seems out of the scope of this PR to add/debug tests related to the Accelerated DHT client.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gammazero gammazero gammazero approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@guillaumemichel @gammazero @lidel