pjfanning commented on code in PR #3652:
URL: https://github.com/apache/pekko/pull/3652#discussion_r4358141147
##########
remote/src/test/scala/org/apache/pekko/remote/artery/RemoteConnectionSpec.scala:
##########
@@ -31,14 +36,181 @@ class RemoteConnectionSpec extends
ArteryMultiNodeSpec("pekko.remote.retry-gate-
"Remoting between systems" should {
+ "handle uid collision when connection TO two systems with same uid" in {
+ val localProbe = new TestProbe(localSystem)
+ val localPort =
RARP(localSystem).provider.getDefaultAddress.getPort().get
+
+ val conflictUid = localSystem.asInstanceOf[ExtendedActorSystem].uid + 1
+ val echoName = "echoA"
+
+ val remotePort1 = freePort()
+ val remoteSystem1 =
+ newRemoteSystem(extraConfig = Some(s"""
+ pekko.test-only-uid = $conflictUid
+ pekko.remote.artery.canonical.port=$remotePort1
+ """))
+ val remote1Probe = new TestProbe(remoteSystem1)
+
+ val remotePort2 = freePort()
+ val remoteSystem2 =
+ newRemoteSystem(extraConfig = Some(s"""
+ // same uid as remoteSystem1
+ pekko.test-only-uid = $conflictUid
+ pekko.remote.artery.canonical.port=$remotePort2
+ """))
+ val remote2Probe = new TestProbe(remoteSystem2)
+
+ localProbe.expectNoMessage(2.seconds)
+ localSystem.actorOf(TestActors.echoActorProps, echoName)
+ remoteSystem1.actorOf(TestActors.echoActorProps, echoName)
+ remoteSystem2.actorOf(TestActors.echoActorProps, echoName)
+
+ val selectionFromLocalToRemote1 =
+
localSystem.actorSelection(s"pekko://${remoteSystem1.name}@localhost:$remotePort1/user/$echoName")
+ val selectionFromLocalToRemote2 =
+
localSystem.actorSelection(s"pekko://${remoteSystem2.name}@localhost:$remotePort2/user/$echoName")
+ val selectionFromRemote1ToLocal =
+
remoteSystem1.actorSelection(s"pekko://${localSystem.name}@localhost:$localPort/user/$echoName")
+ val selectionFromRemote2ToLocal =
+
remoteSystem2.actorSelection(s"pekko://${localSystem.name}@localhost:$localPort/user/$echoName")
+
+ selectionFromLocalToRemote1.tell("ping1a", localProbe.ref)
+ localProbe.expectMsg(500.millis, "ping1a")
+
+ selectionFromRemote1ToLocal.tell("ping1b", remote1Probe.ref)
+ remote1Probe.expectMsg(500.millis, "ping1b")
+
+ EventFilter[UidCollisionException]().intercept {
+ selectionFromLocalToRemote2.tell("ping2a", localProbe.ref)
+ // doesn't get through
+ localProbe.expectNoMessage()
+ }(localSystem)
+ RARP(localSystem).provider.transport
+ .asInstanceOf[ArteryTransport]
+ .association(RARP(remoteSystem2).provider.getDefaultAddress)
+ .associationState
+ .uniqueRemoteAddressState() shouldBe UidUnknown // handshake not
completed
+
+ EventFilter[UidCollisionException]().intercept {
+ selectionFromRemote2ToLocal.tell("ping2b", remote2Probe.ref)
+ // doesn't get through in other direction
+ remote2Probe.expectNoMessage()
Review Comment:
Thanks, I reproduced this: a message from the colliding system reaches the
receiver's actor, and the reply never returns. I don't think the receiver can
fix it without either losing the healthy association or changing the wire
format:
- The envelope header identifies the sender system only by its uid. The
decoder resolves that uid to the healthy association (`Codecs.scala`), and
`InboundHandshake.isKnownOrigin` then accepts the message.
- Inbound connections are merged (TCP `MergeHub`, or the shared Aeron
stream) before decoding and `InboundHandshake`, so the receiver can't tell
which connection a message came from.
- Quarantining by uid would also cut off the healthy association.
- The receiver can't tell the colliding peer to stop either: its outbound
handshake to that peer never completes, so control messages to it are held back.
As you noted, this isn't a regression: before this PR the same collision
threw an `IllegalArgumentException` at the same point. In this PR, the
mitigation is the second part, which generates much more unique uids, plus
rejecting the colliding handshake cleanly. I've opened #3672 for a
protocol-level fix (e.g. identifying the peer independently of the uid), which
needs a rolling-upgrade plan. I haven't added a receiver-side non-delivery
assertion, since it would fail until #3672 is resolved.
--
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]
