fix(provider): wait for fullrt crawl to complete before providing #1214
Conversation
| type readyKadClosestPeersRouter interface { | ||
| KadClosestPeersRouter | ||
| Ready() bool | ||
| } |
There was a problem hiding this comment.
Seems like all closest peers routers should have a Ready method, since there must be some time to crawl the network. If that is true, then maybe better to make the Ready method part of the KadClosestPeersRouter interface.
There was a problem hiding this comment.
Hm.. regular router does not crawl network upfront, only accelerated DHT client does this.
But it could be simpler code if we dont have separate interface, and make regular router have Ready() that just returns true?
iiuc:
KadClosestPeersRouteris exported but relatively new(?), so if we change it, better to do it early- External implementations that dont have any startup delay would need to add
Ready() bool { return true }, probably ok
@guillaumemichel thoughts?
There was a problem hiding this comment.
I would rather keep the FullRT complexity out of the provider module, and not depend on accelerated DHT interfaces at all.
IMO ipfs/kubo#11137 is a better fix.
log when provider is waiting for fullrt crawl to complete and when it becomes ready, helping operators understand startup delays when using Accelerated DHT Client with sweep provider
| } | ||
| } | ||
|
|
||
| var loggedWaiting, loggedReady bool |
There was a problem hiding this comment.
This is UX improvement: I strongly believe we should log this to INFO level, so user is not surprised provide system does nothing.
Pushed small improvement in 1a2a13c that will print this (only state transition, we dont spam logs):
2026-01-08T15:57:20.808+0100 DEBUG dht/provider provider/provider.go:492 Starting SweepingProvider
2026-01-08T15:57:20.808+0100 INFO dht/provider provider/provider.go:285 waiting for router to be Ready() before providing
...
2026-01-08T16:02:03.124+0100 INFO dht/provider provider/provider.go:291 router is Ready(), starting to provide
2026-01-08T16:02:03.125+0100 DEBUG dht/provider provider/provider.go:744 prefix len approximation is 7
(if the PR is refactored, just make sure we keep this level of verbosity about Ready state)
There was a problem hiding this comment.
Users can also check the connectivity status in ipfs provide stat. Provide operations only happen when the status is online, not when disconnected or offline. The provider starts in disconnected state until it is ready to provide.
| // crawl to complete before providing. Otherwise, GetClosestPeers | ||
| // returns no peers and prefix length estimation is incorrect. | ||
| if readyRouter, ok := cfg.router.(readyKadClosestPeersRouter); ok { | ||
| if !readyRouter.Ready() { |
There was a problem hiding this comment.
If it is possible for the router to go from ready to not-ready state, then reset flags, and consider a single state variable.
| if !readyRouter.Ready() { | |
| if !readyRouter.Ready() { | |
| if loggedReady { | |
| // If router has gone from ready to not-ready state, reset flags. | |
| loggedReady = false | |
| loggedWaiting = false | |
| } |
If it is not possible for a router to go from ready to not-ready, then have a flag the avoids checks after reaching ready state.
|
IMO ipfs/kubo#11137 is a better fix and we can close this PR. It moves fullrt dependency to kubo instead of having dependency on fullrt interfaces (such as |
|
I'm fine either way. ipfs/kubo#11137 is simple and already merged, if you feel this is overly verbose, we can close. When it comes to UX, i think its unlikely people will debug logs. |
Fixes ipfs/kubo#11085
Previously, when using the
SweepingProviderwith the Accelerated DHT Client, the initial prefix length estimation and provide process would start before the initial fullrt crawl is complete. Before the crawl is complete, fullrt'sGetClosestPeersdoesn't return any peers, which makes the initial prefix length estimation and first provide operations incorrect.This patch updates the connectivity checker's probe function, so that the node is considered online by the connectivity checker only after the accelerated DHT client is
Ready(which means the initial crawl is complete).