numinnex commented on code in PR #4553:
URL: https://github.com/apache/iggy/pull/4553#discussion_r4286437729
##########
foreign/swift/Sources/Iggy/Client/ClientCore.swift:
##########
@@ -192,12 +210,76 @@ actor ClientCore {
guard reconnected != nil else {
throw IggyError(failure.code, context: "the request deadline
passed while reconnecting after: \(error)")
Review Comment:
An error thrown by `reconnect` itself in the `withDeadline` above
(`cannotEstablishConnection`, `unauthenticated`, ...) escapes `exchange()` raw,
and the `transientNotCommitted` already earned for a written request is lost.
Rust maps it with `reconnect.map_err(|e| retry_outcome.observe(e))`
(tcp_client.rs:340).
On the coordinator path `gradedLoss` then reports it as
`transientNotAccepted` and `PollRouter.send` restamps, so a store/delete offset
can apply twice. Typed `sendMessages` callers get `cannotEstablishConnection`
for a send that may have committed.
Catch the reconnect error and throw `failure` (or `outcome.observe` it) when
`!replay`.
##########
foreign/swift/Sources/Iggy/Client/ClientCore.swift:
##########
@@ -192,12 +210,76 @@ actor ClientCore {
guard reconnected != nil else {
throw IggyError(failure.code, context: "the request deadline
passed while reconnecting after: \(error)")
}
- guard replay else {
- logger.warning("Reconnected, but the outcome of command
\(code) is unknown: replaying it could apply it twice")
+ if let retained = request, retained.clientID == session.clientID,
Review Comment:
The resumed replay reuses `outcome` without marking it uncertain, though the
first attempt was written and lost. Rust seeds it with
`RetryOutcome::for_reconnect(code, &error)` (tcp_client.rs:259).
If the replay is then refused as `transientNotAccepted` (node still loading,
primary moved) past the short window, `sendRaw` drops the frame at :402
(`request = nil`) and restamps it under a new id. If the first attempt
committed, the batch is applied twice. A `historyUnavailable` reply escapes raw
the same way and `sendWithPartitionContext` resends it.
Suggest seeding `outcome` as uncertain whenever `!replay` before this
`sendRaw`, plus a test where the resumed replay is refused.
--
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]