Skip to content

Conversation

@2color
Copy link
Member

@2color 2color commented Aug 28, 2025

  • delegated routing cached router
  • update permalink
  • add diagram

@github-actions
Copy link
Contributor

github-actions bot commented Aug 28, 2025

🚀 Build Preview on IPFS ready

Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

Thank you @2color, overall lgtm, most of my feedback is cosmetic.

Some things that need more work:

  • I was unable to build locally nor there is a preview – any idea whats going on?
  • Rephrase thee sections to make a stronger points (details inline, feel free to adjust my suggestions)


Another hypothesis is that the active probing in the background accelerates DHT lookups, especially for duplicate requests, thereby reducing the latency of DHT lookup. This is an area for further investigation

Either way, these results indicate that the cached address book and active probing have no negative impact on latency or success rates, and may even improve them.
Copy link
Member

Choose a reason for hiding this comment

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

We could teframe this to project more confidence. Too easy to poke holes in it:

  1. The P95 latency improvement (1.91s → 1.35s) is mentioned but then immediately undermined with "It's not entirely clear why this is
    the case"
  2. The speculation about provider churn seems out of place (if providers went offline between experiments, that would make the cache look.. worse not better (since cached addresses might be stale))
  3. The IPNI mention feels like a distraction - it's about a completely different system and doesn't help explain the results
  4. The "hypothesis about background probing" is interesting but vague

Perhaps we just capitalize on the things we know and clear latency improvement?

Suggested change
Either way, these results indicate that the cached address book and active probing have no negative impact on latency or success rates, and may even improve them.
### Key insights
With peer address caching enabled, we observed unexpected improvements beyond just address availability:
- P95 latency for successful responses improved from 1.91s to 1.35s (29% faster)
- Success rate increased from 52.0% to 57.2%
These improvements likely stem from the active background probing, which pre-validates peer connectivity. When duplicate CIDs are requested, Someguy can immediately return known-good peers from cache, accelerating the routing and avoiding DHT traversal for subsequent lookups of the same content.
The results demonstrate that the cached address book and active probing have no negative impact on latency or success rates, and actually improve both metrics.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree with the feedback.

My reading of the code suggest the following is not accurate:

When duplicate CIDs are requested, Someguy can immediately return known-good peers from cache, accelerating the routing and avoiding DHT traversal for subsequent lookups of the same content.

Someguy doesn't actually do this. Someguy only uses the peer cache after the kad-dht providers lookup yields a value (https://github.com/ipfs/someguy/blob/main/server_cached_router.go#L61-L69) and even then the iterator only adds cached peer addresses if there are none: https://github.com/ipfs/someguy/blob/main/server_cached_router.go#L93-L101

Copy link
Member Author

Choose a reason for hiding this comment

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

I looked in the go-libp2p-kad-dht and and there fallback to address book: The code doesn't implement any fallback mechanism to consult the address book when RPC responses contain no addresses. So if a provider comes back with an empty address list (prov.Addrs), no addresses are added.

Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

Lgtm, but we need to remove below duplicated sections.

@2color 2color merged commit cd43d8b into main Sep 5, 2025
3 checks passed
@2color 2color deleted the someguy-caching branch September 5, 2025 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants