TomPlanche commented on PR #4388: URL: https://github.com/apache/iggy/pull/4388#issuecomment-5998815040
Thanks @numinnex for the great review. I checked each point against the code, and these five hold: - `tcp_client.rs:659`: the send guard turns a disconnect during an auto-login sweep into an endless redial, because `fail_sign_in` reads `NotConnected` as a lost connection. - `tcp_client.rs:176`: the leader redirect in `login_user` goes through the trait `connect()`, so it clears the flag and undoes the disconnect. - `client.rs:1006`: the aborted heartbeat can leave the client in `Connecting`, and the TCP `disconnect()` then returns early with no teardown. - `tcp_client.rs:235`: the send guard has no unit test, and after `shutdown()` it hides `ClientShutdown` behind `NotConnected`. - `quic_client.rs:707`: the QUIC and WebSocket ladders can still install a connection after the disconnect. The doc and test nits are valid too. So a flag checked at the entry points is not enough: it must be checked at every step of the reconnect sweep, on all three transports. That is the same code #4345 changes, and close to problems 5 to 8 of #4287 and to #3651. I asked on #4287 how you want it scoped before I change more of the state machine. -- 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]
