pjfanning commented on code in PR #3652:
URL: https://github.com/apache/pekko/pull/3652#discussion_r4358127390


##########
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 {

Review Comment:
   Fixed in 5f0f469b65. Each collision step now subscribes a probe to `Warning` 
events on the receiving system and expects a `UID collision old [...]` warning. 
With the `log.warning` removed, both collision tests fail. I couldn't make the 
count exact, though: the control and ordinary streams each send a 
`HandshakeReq`, and the retries repeat every `handshake-retry-interval`, so the 
number of collision warnings within the window depends on timing. The tests 
therefore expect at least one.



##########
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")

Review Comment:
   Applied in 5f0f469b65: the eight round trips in the two collision tests now 
use the default probe timeout, with their order and the collision checks 
unchanged. The two retry-polling tests keep `500.millis`.



##########
remote/src/main/scala/org/apache/pekko/remote/artery/ArteryTransport.scala:
##########
@@ -1103,4 +1119,22 @@ private[remote] object ArteryTransport {
       case _               => "message"
     }
 
+  /**
+   * Generates the system uid from the bind hostname and port in combination 
with timestamps,
+   * which makes collisions between systems less likely than with a plain 
random number.
+   */
+  def generateSystemUid(hostname: String, port: Int, timestamp1: Long): Long = 
{

Review Comment:
   Done in 5f0f469b65, updating each notice in place instead of adding a second 
line. All nine files changed by the ported Akka 2.9.0 PRs now carry the 2023 
source notice: your five, plus `Association.scala`, `ActorRefProvider.scala`, 
`RemoteActorRefProvider.scala` and `RemoteTransport.scala`. `Remoting.scala` 
(classic) and the new `SystemUidSpec` are Pekko-only and keep their headers. 
The header checks pass.



-- 
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]

Reply via email to