He-Pin opened a new issue, #3689:
URL: https://github.com/apache/pekko/issues/3689
## Summary
`Replicator.isKnownNode` is built from the sets that the member-event
handlers populate. A member whose *first* observed status is `Leaving`,
`Exiting`, `Down`, `PreparingForShutdown` or `ReadyForShutdown` is not recorded
in any of the sets that `isKnownNode` consults, so its `Read`, `Write` and
`DeltaPropagation` messages are silently discarded.
## Evidence
`distributed-data/src/main/scala/org/apache/pekko/cluster/ddata/Replicator.scala:1406-1407`:
```scala
private def isKnownNode(node: UniqueAddress): Boolean =
nodes(node) || weaklyUpNodes(node) || joiningNodes(node) ||
selfUniqueAddress == node
```
Note that `exitingNodes` — declared at `:1401` and populated at `:2466` — is
not part of the check.
The single call site is `:1590-1604`:
```scala
case msg: SendingSystemUid =>
msg.fromNode match {
case Some(fromNode) if !isKnownNode(fromNode) =>
// When restarting a node with same host:port it is possible that
a Replicator on another node
// is sending messages to the restarted node even if it hasn't
joined the same cluster.
// Therefore we check that the message was from a known cluster
member
log.info("Ignoring message [{}] from [{}] unknown node [{}]",
Logging.simpleName(msg), replyTo, fromNode)
case _ =>
msg match {
case Read(key, _) => receiveRead(key)
case Write(key, envelope, _) => receiveWrite(key,
envelope)
case DeltaPropagation(from, reply, deltas) =>
receiveDeltaPropagation(from, reply, deltas)
}
}
```
The sets are populated by the dispatch at `:1615-1621`:
```scala
case MemberJoined(m) => receiveMemberJoining(m) // joiningNodes
+= (:2444)
case MemberWeaklyUp(m) => receiveMemberWeaklyUp(m) // weaklyUpNodes
+= (:2448)
case MemberUp(m) => receiveMemberUp(m) // nodes +=
(:2456)
case MemberExited(m) => receiveMemberExiting(m) // exitingNodes
+= (:2466)
case MemberRemoved(m, _) => receiveMemberRemoved(m)
case ClusterShuttingDown => receiveMemberRemoved(cluster.selfMember)
case evt: MemberEvent => receiveOtherMemberEvent(evt.member)
```
and `receiveOtherMemberEvent` (`:2487-2492`) only maintains `leader`:
```scala
def receiveOtherMemberEvent(m: Member): Unit =
if (matchingRole(m)) {
// replace, it's possible that the ordering is changed since it based
on MemberStatus
leader = leader.filterNot(_.uniqueAddress == m.uniqueAddress)
leader += m
}
```
`MemberLeft`, `MemberDowned`, `MemberPreparingForShutdown` and
`MemberReadyForShutdown` all fall into that catch-all.
## When this is reachable
The Replicator subscribes with `InitialStateAsEvents` (`:58`).
`ClusterEvent.diffMemberEvents`
(`cluster/src/main/scala/org/apache/pekko/cluster/ClusterEvent.scala:552-561`)
emits exactly **one** event per member, chosen from that member's *current*
status:
```scala
case m if m.status == Joining => MemberJoined(m)
case m if m.status == WeaklyUp => MemberWeaklyUp(m)
case m if m.status == Up => MemberUp(m)
case m if m.status == Leaving => MemberLeft(m)
case m if m.status == Exiting => MemberExited(m)
case m if m.status == Down => MemberDowned(m)
case m if m.status == PreparingForShutdown =>
MemberPreparingForShutdown(m)
case m if m.status == ReadyForShutdown =>
MemberReadyForShutdown(m)
```
So a Replicator that starts while a peer is already past `Up` never receives
a `MemberUp` for that peer:
| peer's current status | event delivered | set updated | `isKnownNode` |
|---|---|---|---|
| `Joining` | `MemberJoined` | `joiningNodes` | true |
| `WeaklyUp` | `MemberWeaklyUp` | `weaklyUpNodes` | true |
| `Up` | `MemberUp` | `nodes` | true |
| `Leaving` | `MemberLeft` | *(only `leader`)* | **false** |
| `Exiting` | `MemberExited` | `exitingNodes` | **false** — the set is not
consulted |
| `Down` | `MemberDowned` | *(only `leader`)* | **false** |
| `PreparingForShutdown` / `ReadyForShutdown` | respective event | *(only
`leader`)* | **false** |
The ordinary live path is unaffected: a member that transitions `Up →
Leaving → Exiting` is already in `nodes` from its earlier `MemberUp`, and
`nodes` is only pruned at `MemberRemoved` (`:2475`).
Concrete scenario: node C joins a cluster in which node B has already begun
leaving. C's Replicator replays B as `MemberLeft`, records nothing but
`leader`, and then discards B's `Write` / `DeltaPropagation` with only an
`log.info`. B's `Read` of C likewise goes unanswered.
## Impact
Silent data loss and non-convergence for the affected peer, plus unanswered
reads (the sender times out). A `Leaving` or `Exiting` member is still a full
cluster member and still participates in replication until it is `Removed`, so
dropping its writes is not the intended behaviour. The only signal is an
INFO-level log line.
Note the discarded set is `Read`/`Write`/`DeltaPropagation` only — `Status`
and `Gossip` take the separate branch at `:1584-1588` and are not gated by
`isKnownNode`.
## Suggested direction
Two gaps need closing, and they are separable:
1. `Exiting` — add `exitingNodes` to `isKnownNode`. Small and targeted.
2. `Leaving` / `Down` / `PreparingForShutdown` / `ReadyForShutdown` —
nothing records these members at all when they are first seen in that status.
Either `receiveOtherMemberEvent` should track them in a set that `isKnownNode`
consults, or the initial-state handling needs to record every current member
regardless of status.
Worth preserving the original intent of the check, which per the comment at
`:1593-1595` is to reject messages from a *restarted* peer that reuses the same
host:port but has not rejoined — that is what the `UniqueAddress` (address +
uid) comparison is for, and it should keep working after the fix.
## Test
A multi-jvm case in `distributed-data/src/multi-jvm/` where one node begins
leaving (or is downed) before a second node's Replicator starts, then asserts
the second node applies a `Write` / `DeltaPropagation` originating from the
first.
--
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]
