BrandonArp commented on issue #4327:
URL: https://github.com/apache/iggy/issues/4327#issuecomment-6071748771

   We hit this in a staging environment, and the trigger is broader than 
connect/disconnect cycles. A client whose server is down for a while wedges 
permanently, even if the application never calls `connect()` or `disconnect()` 
itself.
   
   Every failed reconnect sweep ends in `fail_connect()`, which publishes 
`DiagnosticEvent::Disconnected`. A client that keeps trying to reconnect (from 
the heartbeat, or from any request on a disconnected client, which goes through 
`send_raw_with_response` -> `connect()`) adds one event per failed sweep. After 
the 1000th, the next `fail_connect()` blocks in `publish_event`.
   
   This made it hard to diagnose. The blocked connect never releases the 
client's `ConnectCoordinator` (or its `routing_lock`, when a request triggered 
the reconnect). So every later call on that client, `get_stats()` for example, 
waits silently behind it. No socket is open, nothing is logged, and the client 
stays wedged after the server comes back. Only a new client recovers.
   
   What happened to us: the server (0.9.0) was down for about 2.5 hours (full 
disk, crash loop). Two replicas of a service each held a long-lived 
`IggyClient` (SDK 0.11.0, `reconnection_retries=3`, 
`reconnection_interval=1s`). They wedged within 30s of each other, about 2.5 
hours into the failed reconnects. Their health loop probes with `get_stats()` 
and hung there, so both stayed not-ready after the server recovered, until we 
restarted them. Fresh processes connected in about 3 s.
   
   Here is a minimal repro that needs no server, because it dials a closed port 
(SDK 0.11.0, async-broadcast 0.7.2):
   
   ```rust
   use iggy::prelude::*;
   use std::time::Duration;
   
   #[tokio::main]
   async fn main() {
       let cs = 
"iggy://iggy:[email protected]:1?reconnection_retries=0&reconnection_interval=1ms";
       let client = 
IggyClientBuilder::from_connection_string(cs).unwrap().build().unwrap();
       for i in 1..=1100u32 {
           if tokio::time::timeout(Duration::from_secs(5), 
client.connect()).await.is_err() {
               println!("connect #{i} hung");
               let stats = tokio::time::timeout(Duration::from_secs(5), 
client.get_stats()).await;
               println!("get_stats after that: {}", if stats.is_err() { "hung" 
} else { "returned" });
               return;
           }
       }
   }
   ```
   
   Output: `connect #1001 hung`, then `get_stats after that: hung`. Connects #1 
to #1000 each return `CannotEstablishConnection`. As the issue says, draining a 
`subscribe_events()` receiver in a spawned task doesn't change the result.
   
   +1 to `set_overflow(true)`. Until a release includes it, we're running 
0.11.0 with that one-line change in the TCP, QUIC and WebSocket clients. Is a 
fix planned for the next release?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to