He-Pin opened a new issue, #3160:
URL: https://github.com/apache/pekko/issues/3160
### Motivation
Classic remoting is deprecated, but some downstream projects still depend on
it. For example, Apache Flink has an open migration discussion in FLINK-33505
because it uses Pekko classic remoting and cannot immediately move to Artery.
Issue #3126 fixed one concrete stale ACK failure mode where a delayed ACK
with `cumulativeAck > resendBuffer.maxSeq` could make classic remoting
quarantine an otherwise recoverable association. That fix makes
future/out-of-window ACKs harmless.
There appears to be a remaining stale ACK class around EndpointReader
UID/source identity.
### Current behavior
Classic remoting decodes ACKs in `EndpointReader` and forwards the bare
`Ack` to `ReliableDeliverySupervisor`:
- `EndpointReader` is constructed with the remote UID from the associated
handle.
- `EndpointReader.receive` decodes `(Option[Ack], Option[Message])` and
sends `reliableDelivery ! ack`.
- `EndpointReader.notReading` also continues to decode inbound payloads and
forward ACKs.
- `ReliableDeliverySupervisor` only gates ACK processing on `uidConfirmed`;
the `Ack` message itself carries no reader UID/source identity.
The wire ACK only contains `cumulativeAck` and `nacks`; it does not contain
UID/incarnation information.
This means a delayed ACK from an older reader/connection can be processed
after the supervisor has confirmed a newer UID for the same remote address. If
the stale ACK has `cumulativeAck <= resendBuffer.maxSeq`, the #3126 guard will
not drop it. Depending on the ACK contents, it can either:
- acknowledge and remove entries from the current resend buffer, even though
the ACK belongs to an older UID/incarnation, or
- trigger `ResendUnfulfillableException` when stale NACKs cannot be matched,
which is still escalated to `HopelessAssociation`.
### Why this matters
Artery avoids this class of problem by carrying source identity and
incarnation state in the system message delivery path:
- `SystemMessageDelivery.Ack(seqNo, from: UniqueAddress)` and `Nack(seqNo,
from: UniqueAddress)` include the remote unique address.
- System message delivery also clears state when the association incarnation
changes via `ClearSystemMessageDelivery` / incarnation checks.
Classic remoting has partial protection through `uidConfirmed` and `GotUid`
reset, but the ACK actor message is not tied to the `EndpointReader` UID that
decoded it. That leaves a gap after UID confirmation and during
handoff/read-only reader transitions.
### Proposed change
Keep the classic wire protocol unchanged, but tag ACKs internally with the
UID of the `EndpointReader` that decoded them.
One possible approach:
1. Add an internal message such as `AckFromReader(uid: Int, ack: Ack)` in
`ReliableDeliverySupervisor` or a nearby internal scope.
2. Change `EndpointReader` to send `AckFromReader(uid, ack)` instead of a
bare `Ack` when `reliableDeliverySupervisor` is defined.
3. In `ReliableDeliverySupervisor`, process the ACK only if `uidConfirmed`
and `uid.contains(readerUid)`.
4. Drop ACKs from stale reader UIDs with a bounded/debug log.
5. Preserve the existing bare `Ack` handling only if needed for
tests/backward internal compatibility, or migrate tests to the new internal
message.
This would be wire-compatible and scoped to classic remoting internals.
### Suggested tests
- A supervisor-level regression test where:
- old UID is confirmed and system messages are buffered/sent,
- a new UID is confirmed, resetting the resend buffer,
- a stale `AckFromReader(oldUid, Ack(...))` arrives with a sequence number
that would otherwise fit the new buffer,
- the stale ACK is ignored and does not clear the new resend buffer.
- A matching valid `AckFromReader(currentUid, Ack(...))` should still
acknowledge the current buffer.
- A handoff/read-only reader test, if practical, covering that `notReading`
ACKs from stale UIDs do not affect the current writer/supervisor.
### Related work
- #3126: stale future ACKs should not quarantine classic remoting.
- FLINK-33505: downstream context where classic remoting remains relevant.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
