numinnex commented on code in PR #4553:
URL: https://github.com/apache/iggy/pull/4553#discussion_r4287380439
##########
foreign/swift/Sources/Iggy/Client/ClientCore.swift:
##########
@@ -736,22 +913,19 @@ actor ClientCore {
state = .disconnected
let old = connection
connection = nil
- // A Register still in flight is retried on the next connection and
- // must arrive under the identity and secret its body was built from.
- if session.isBound {
- session.reset()
- }
await old?.close()
Review Comment:
This closes the shared main connection without taking `exchangeLock`. When
`failOver` (which holds only `routingLock`) reaches it via `settle` or
`handleLeaderRedirection`, another caller's frame already written in
`exchangeLocked` is cut. That caller gets `.disconnected`, marks the outcome
uncertain, resumes, and replays on the new node. A NotPrimary there then
surfaces as `transientNotCommitted` for a send that most likely committed on
the old node, and a retry appends it twice.
This is routine with a shared client and partition primaries spread across
nodes. Rust avoids it because the attempt holds the link lock for the whole
exchange (tcp_client.rs:1596) and `disconnect_transport` waits on it (:1311).
Suggest taking `exchangeLock` around the close for a live connection,
released before `connect`/`resumeSession`, since `exchangeOnce` takes the same
lock and would deadlock.
--
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]
