reimplement fix for akka/pekko cluster #1594

sbt \
 -Dpekko.cluster.assert=on \
 -Dpekko.log.timestamps=true \
 -Dpekko.test.timefactor=2 \
 -Dpekko.actor.testkit.typed.timefactor=2 \
 -Dpekko.test.tags.exclude=gh-exclude,timing \
 -Dpekko.test.multi-in-test=false \
"cluster/testOnly *JoinConfigCompatCheckerRollingUpdateSpec"

@pjfanning pjfanning marked this pull request as draft

December 16, 2024 20:47

@pjfanning

Copy link

Member Author

pjfanning commented Dec 16, 2024

@raboof @kerr @sadekmunawar

My changes have fixed most tests but the new ones in this PR are still broken. It looks like we need more changes in org.apache.pekko.remote.Remoting$.localAddressForRemote.

Error: [12/16/2024 22:04:04.724] [MixedProtocolClusterSpec-pekko.actor.internal-dispatcher-4] [org.apache.pekko.cluster.ClusterActorRefProvider] No root guardian at [pekko.tcp://MixedProtocolClusterSpec@10.1.0.6:40819]
org.apache.pekko.remote.RemoteTransportException: No transport is loaded for protocol: [pekko.tcp], available protocols: [akka.tcp]
	at org.apache.pekko.remote.Remoting$.localAddressForRemote(Remoting.scala:125)
	at org.apache.pekko.remote.Remoting.localAddressForRemote(Remoting.scala:168)
	at org.apache.pekko.remote.RemoteActorRefProvider.rootGuardianAt(RemoteActorRefProvider.scala:488)

@pjfanning pjfanning marked this pull request as ready for review

December 17, 2024 12:15

@pjfanning pjfanning marked this pull request as draft

December 17, 2024 12:39

@pjfanning

Copy link

Member Author

pjfanning commented Dec 17, 2024 •

edited

Loading

@raboof @He-Pin @sadekmunawar I think I have all the tests passing now. I will check again later. If these changes are approved, I can remove the temp change to run the nightly tests during CI build of this PR.

#1610 raised for a flaky test

@He-Pin

Copy link

Member

He-Pin commented Dec 17, 2024

I have not look into the details, but will it replay the akka node with akka protocol ?otherwise, it will not work. can a akka node sending ping-pong after joined?

And there are akka.tcp and akka protocols, which should be tested too.

@pjfanning

Copy link

Member Author

pjfanning commented Dec 17, 2024

@He-Pin this PR is based on #1567 but makes extra changes because the nightly tests failed with the 1567 changes. Specifically, the some nightly tests use pekko.tcp/akka.tcp transports. 1567 did not properly support the .tcp transports. These changes fix that.

@pjfanning pjfanning marked this pull request as ready for review

December 21, 2024 15:43

@pjfanning pjfanning changed the title ~~(削除) try to find issue with akka/pekko cluster change (削除ここまで)~~ (追記) reimplement fix for akka/pekko cluster (追記ここまで)

Dec 21, 2024

@pjfanning pjfanning added this to the 1.2.0 milestone

Dec 28, 2024

@pjfanning pjfanning added the backport label

Dec 28, 2024

He-Pin

He-Pin reviewed

Dec 28, 2024

View reviewed changes

cluster/src/test/scala/org/apache/pekko/cluster/MixedProtocolClusterSpec.scala

pekko.coordinated-shutdown.terminate-actor-system = on

pekko.remote.classic.netty.tcp.port = 0

pekko.remote.artery.canonical.port = 0

Copy link

Member

@He-Pin He-Pin Dec 28, 2024

Choose a reason for hiding this comment

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

we need test both pekko.tcp and pekko, I mean both artery and classical transports, seems this only test one.

@pjfanning pjfanning marked this pull request as draft

December 28, 2024 16:32

@pjfanning pjfanning force-pushed the cluster-akka branch from 52a1769 to 1303acd Compare

December 28, 2024 16:51

@pjfanning pjfanning mentioned this pull request

Dec 28, 2024

No transport is loaded for protocol: [pekko], available protocols: [pekko.tcp] #1640

Open

@pjfanning

Copy link

Member Author

pjfanning commented Dec 29, 2024

@He-Pin @raboof does this look ok? When it is ok, I can remove the CI change that makes the nightly tests run as part of the PR CI build.

@raboof

Copy link

Member

raboof commented Dec 30, 2024

Looks reasonable to me.

@pjfanning pjfanning marked this pull request as ready for review

December 30, 2024 21:16

@pjfanning

Copy link

Member Author

pjfanning commented Dec 30, 2024

I changed the PR tests to include some cluster tests as per #1591

He-Pin

He-Pin reviewed

Dec 30, 2024

View reviewed changes

.github/workflows/build-test-prValidation.yml

fail-fast: false

matrix:

command:

- cluster/test distributed-data/test cluster-tools/test cluster-metrics/test

Copy link

Member

@He-Pin He-Pin Dec 30, 2024

Choose a reason for hiding this comment

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

need sbt cluster/MultiJvm/test too

Copy link

Member Author

@pjfanning pjfanning Dec 31, 2024

Choose a reason for hiding this comment

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

#1592 covers that. It looks like running the multiJvm tests will be more complicated. The aim of this small change is that the test modified in this PR runs in the PR CI - because at the momemt, it does not.

pjfanning added 10 commits

December 31, 2024 00:07

@pjfanning


 Revert "revert apache#1568 due to test failures ( apache#1587 )"

7d2356d

This reverts commit 7af03e5.

@pjfanning


 temp run nightly test in this PR

2dbe5b1

@pjfanning


 no need for square brackets because the set print adds them

a2b8cc9

@pjfanning


 logging to find issue

17655ee

@pjfanning


 support tcp protocols

d890888

@pjfanning


 Update ClusterDaemon.scala

2783e3c

@pjfanning


 remove temp logging

e2894b5

@pjfanning


 try to fix issue in Remoting

a767289

@pjfanning


 extra tests

fe9d1d3

@pjfanning


 more tests

77c9df3

pjfanning added 5 commits

December 31, 2024 00:07

@pjfanning


 ignore udp tests

516095e

@pjfanning


 try to make tests tidy up after failures

3fe0646

@pjfanning


 Update MixedProtocolClusterSpec.scala

a5d7ab6

@pjfanning


 Update MixedProtocolClusterSpec.scala

bb117cf

@pjfanning


 run main cluster tests for PR

638ecf5

@pjfanning pjfanning force-pushed the cluster-akka branch from 91f1024 to 638ecf5 Compare

December 30, 2024 23:07

He-Pin

He-Pin approved these changes

Dec 31, 2024

View reviewed changes

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.

Great!
lgtm, it covers pekko join akka and akka join pekko.

@pjfanning pjfanning merged commit ef212e9 into apache:main

Jan 1, 2025

9 checks passed

@pjfanning pjfanning deleted the cluster-akka branch

January 1, 2025 11:13

pjfanning added a commit to pjfanning/incubator-pekko that referenced this pull request

Jan 2, 2025

@pjfanning


 reimplement fix for akka/pekko cluster ( apache#1594 )

b17140a

* Revert "revert apache#1568 due to test failures (apache#1587)"
This reverts commit 7af03e5.
* temp run nightly test in this PR
* no need for square brackets because the set print adds them
* logging to find issue
* support tcp protocols
* Update ClusterDaemon.scala
* remove temp logging
* try to fix issue in Remoting
* extra tests
* more tests
* ignore udp tests
* try to make tests tidy up after failures
* Update MixedProtocolClusterSpec.scala
* Update MixedProtocolClusterSpec.scala
* run main cluster tests for PR

@pjfanning pjfanning mentioned this pull request

Jan 2, 2025

reimplement fix for akka/pekko cluster (#1594) #1664

Merged

pjfanning added a commit that referenced this pull request

Jan 2, 2025

@pjfanning


 reimplement fix for akka/pekko cluster ( #1594 ) ( #1664 )

38facf7

* Revert "revert #1568 due to test failures (#1587)"
This reverts commit 7af03e5.
* temp run nightly test in this PR
* no need for square brackets because the set print adds them
* logging to find issue
* support tcp protocols
* Update ClusterDaemon.scala
* remove temp logging
* try to fix issue in Remoting
* extra tests
* more tests
* ignore udp tests
* try to make tests tidy up after failures
* Update MixedProtocolClusterSpec.scala
* Update MixedProtocolClusterSpec.scala
* run main cluster tests for PR

Labels

backport

3 participants

@pjfanning @He-Pin @raboof

reimplement fix for akka/pekko cluster #1594

reimplement fix for akka/pekko cluster #1594

Uh oh!

Conversation

@pjfanning pjfanning commented Dec 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pjfanning commented Dec 16, 2024

Uh oh!

pjfanning commented Dec 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

He-Pin commented Dec 17, 2024

Uh oh!

pjfanning commented Dec 17, 2024

Uh oh!

@He-Pin He-Pin Dec 28, 2024

Choose a reason for hiding this comment

Uh oh!

pjfanning commented Dec 29, 2024

Uh oh!

raboof commented Dec 30, 2024

Uh oh!

pjfanning commented Dec 30, 2024

Uh oh!

@He-Pin He-Pin Dec 30, 2024

Choose a reason for hiding this comment

Uh oh!

@pjfanning pjfanning Dec 31, 2024

Choose a reason for hiding this comment

Uh oh!

@He-Pin He-Pin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

@pjfanning pjfanning commented Dec 16, 2024 •

edited

Loading

pjfanning commented Dec 17, 2024 •

edited

Loading