diff --git a/crates/core/src/operations/mod.rs b/crates/core/src/operations/mod.rs index 89eb2852a..d3b686b07 100644 --- a/crates/core/src/operations/mod.rs +++ b/crates/core/src/operations/mod.rs @@ -308,9 +308,8 @@ impl From> for OpError { /// If the contract is not found, it will try to get it first if the `try_get` parameter is set. async fn start_subscription_request(op_manager: &OpManager, key: ContractKey) { - // Check if we are the optimal location for this contract - // If we are and no other peers are suitable, skip subscription for now - // TODO: In the future, still find next-best peers for redundancy (issue #1793) + // Phase 1 fix for issue #1848: Allow nodes at optimal location to subscribe to next-best peers + // This prevents isolation when a node is at the optimal network position let own_location = op_manager.ring.connection_manager.own_location(); let closest = op_manager .ring @@ -318,14 +317,12 @@ async fn start_subscription_request(op_manager: &OpManager, key: ContractKey) { if closest.is_none() { // No other peers available for caching - // Check if we should be caching this locally - if op_manager.ring.should_seed(&key) { - tracing::debug!( - %key, - "Skipping subscription - node is optimal location and no other peers available" - ); - return; - } + tracing::warn!( + %key, + "No remote peers available for subscription - node may become isolated" + ); + // Still attempt subscription in case new peers join later + // The subscribe operation will handle the no-peers case appropriately } let sub_op = subscribe::start_op(key); diff --git a/crates/core/src/operations/subscribe.rs b/crates/core/src/operations/subscribe.rs index 659c0d4a9..e195101f3 100644 --- a/crates/core/src/operations/subscribe.rs +++ b/crates/core/src/operations/subscribe.rs @@ -66,13 +66,18 @@ pub(crate) async fn request_subscribe( sub_op: SubscribeOp, ) -> Result<(), OpError> { if let Some(SubscribeState::PrepareRequest { id, key }) = &sub_op.state { - // Find a remote peer to handle the subscription - const EMPTY: &[PeerId] = &[]; - let target = match op_manager.ring.closest_potentially_caching(key, EMPTY) { + // Phase 1 fix for issue #1848: Properly find next-best peer for subscription + // Exclude ourselves to ensure we find a remote peer, even if we're at optimal location + let own_location = op_manager.ring.connection_manager.own_location(); + let skip_self = [own_location.peer.clone()]; + let target = match op_manager + .ring + .closest_potentially_caching(key, &skip_self[..]) + { Some(peer) => peer, None => { - // No remote peers available - tracing::debug!(%key, "No remote peers available for subscription"); + // No remote peers available - this may happen when node is isolated + tracing::warn!(%key, "No remote peers available for subscription - node may be isolated"); return Err(RingError::NoCachingPeers(*key).into()); } };