Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Address ordering bug (mixed pekko/akka cluster) #1562

Merged
merged 1 commit into from
Nov 25, 2024

Conversation

sadekmunawar
Copy link
Contributor

This bug causes major issues when a cluster contains nodes using both "akka" and "pekko". It can be a significant obstacle when migrating from Akka to Pekko through a rolling upgrade, especially if the migration requires the cluster to run with mixed protocols for an extended period.

Due to this bug, the DistributedPubSubMediator fails to send messages to some registered nodes. In the DistributedPubSubMediator, node addresses are stored in a set. However, when set membership relies on the faulty addressOrdering method, set.contains returns false for some addresses that are actually in the set.

Copy link
Member

@He-Pin He-Pin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks ,and this should be backport to older version too.

@pjfanning
Copy link
Contributor

A test failed in Scala 3 run. Could be an intermittent issue that affects the test already.

[11-25 02:13:26.569] [info] - must deliver first message *** FAILED *** (8 seconds, 144 milliseconds)
[11-25 02:13:26.569] [info]   java.lang.AssertionError: assertion failed: timeout (6 seconds) during expectMsg while waiting for OnNext(d)
[11-25 02:13:26.569] [info]   at scala.runtime.Scala3RunTime$.assertFailed(Scala3RunTime.scala:8)
[11-25 02:13:26.569] [info]   at org.apache.pekko.testkit.TestKitBase.expectMsg_internal(TestKit.scala:472)
[11-25 02:13:26.569] [info]   at org.apache.pekko.testkit.TestKitBase.expectMsg(TestKit.scala:449)
[11-25 02:13:26.569] [info]   at org.apache.pekko.testkit.TestKitBase.expectMsg$(TestKit.scala:169)
[11-25 02:13:26.569] [info]   at org.apache.pekko.testkit.TestKit.expectMsg(TestKit.scala:982)
[11-25 02:13:26.569] [info]   at org.apache.pekko.stream.testkit.TestSubscriber$ManualProbe.expectNext(StreamTestKit.scala:523)
[11-25 02:13:26.569] [info]   at org.apache.pekko.remote.artery.SendQueueSpec.test$1$$anonfun$1(SendQueueSpec.scala:188)
[11-25 02:13:26.569] [info]   at org.apache.pekko.remote.artery.SendQueueSpec.test$1$$anonfun$adapted$1(SendQueueSpec.scala:175)
[11-25 02:13:26.569] [info]   at scala.collection.immutable.Range.foreach(Range.scala:190)
[11-25 02:13:26.569] [info]   at org.apache.pekko.remote.artery.SendQueueSpec.test$1(SendQueueSpec.scala:175)
[11-25 02:13:26.569] [info]   at org.apache.pekko.remote.artery.SendQueueSpec.f$proxy5$1(SendQueueSpec.scala:204)

@pjfanning pjfanning changed the title Fix Address ordering bug Fix Address ordering bug (mixed pekko/akka cluster) Nov 25, 2024
Copy link
Contributor

@pjfanning pjfanning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm - thanks

@pjfanning pjfanning added this to the 1.1.3 milestone Nov 25, 2024
Copy link
Member

@raboof raboof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great find, thank you!

@pjfanning pjfanning merged commit e004814 into apache:main Nov 25, 2024
8 of 9 checks passed
pjfanning pushed a commit to pjfanning/incubator-pekko that referenced this pull request Nov 25, 2024
pjfanning added a commit that referenced this pull request Nov 25, 2024
Co-authored-by: sadekmunawar <[email protected]>
Co-authored-by: Sadek Munawar <[email protected]>
@sadekmunawar sadekmunawar deleted the address-fix branch November 26, 2024 03:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants