Conversation
Evanev7
force-pushed
the
zenoh2
branch
2 times, most recently
from
5ed8701 to
eb581ec
Compare
team-wcv
pushed a commit
to team-wcv/exo
that referenced
this pull request
May 11, 2026
Maintainer feedback on exo-explore#2063 was that the `_fallback_interface_ips` path was papering over a discovery race instead of solving a concrete bug: "is our discovery service not finding a real connection that exists? i plan to replace discovery somewhat soon (exo-explore#2076 and beyond)." Agreed on review. Changes: - `find_ip_prioritised`: restore `if not ips: return None`. The 20s socket-discovery window is short, placement decisions can wait, and the JACCL retry loop added in this same PR already absorbs the startup race at the next layer down. - Drop `_fallback_interface_ips` and `_is_candidate_host_ip` helpers. - Reorder the `min(...)` tuple so interface type is the primary key and `_address_priority` is only a tiebreaker. Without this, a `ring=True` ring placement could prefer a LAN-class ethernet IP over a Thunderbolt IP that happened to be link-local (169.254/16), which inverts the ring-prefers-TB intent. - Simplify `_address_priority` to a two-class RFC1918/everything-else split. The previous CGNAT vs link-local vs catch-all ranking only mattered together with the dropped fallback; with type as primary key the simpler split is enough to keep LAN ahead of Tailscale at parity. Test impact: - Drop `test_ring_placement_uses_advertised_lan_ips_for_rdma_only_topology` and `test_jaccl_placement_uses_advertised_lan_ip_for_rdma_coordinator`. Both depended on the fallback by constructing RDMA-only topologies. - Replace them with `test_ring_placement_prefers_lan_ip_over_tailscale_ip` and `test_jaccl_placement_prefers_lan_ip_over_tailscale_ip`, which exercise the address-priority tiebreaker against realistic dual-homed (LAN + Tailscale) socket-reachable topologies. - `test_placement_prefers_socket_reachable_rank_zero` now seeds an explicit listener->peer socket edge (so placement can resolve without the fallback) and a second peer->listener edge (so the rank-zero rotation has a strict winner).
Evanev7
force-pushed
the
zenoh2
branch
23 times, most recently
from
9c5a12e to
59e6db6
Compare
Evanev7
force-pushed
the
zenoh2
branch
7 times, most recently
from
b727183 to
5f33b32
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
same impl as #2073 but purely the zenoh to libp2p changes.
offers no benefit over libp2p but should make the other PR more comprehensible.