Repository navigation
Fix classic remoting seed addresses in BasicClusterConfigSpec - #3742
Merged
Merged
Conversation
…3741 Motivation: The cluster configuration regression uses Artery seed addresses even when the nightly build selects classic remoting, preventing either node from joining. Modification: Derive the seed protocol from the effective transport configuration, bind classic remoting to loopback, and assert member protocols and hosts. Result: Both nodes form the expected two-member Up cluster with either transport. Tests: - JDK 17: BasicClusterConfigSpec reproduced the timeout before the fix with -Dpekko.remote.artery.enabled=off. - JDK 17: BasicClusterConfigSpec and BasicClusterManualSpec passed with classic remoting and default Artery, 3 tests per run. - Native scalafmt, headerCreateAll, checkCodeStyle (including +headerCheckAll), and git diff --check passed. - validatePullRequest encountered an ARM macOS LevelDB JNI architecture mismatch in persistence-typed; remaining validation is in progress. References: Fixes apache#3741
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
There are no active review issues for the test-only cluster setup and assertion changes.
No open issues 🎉
What changed in this PR
Updates BasicClusterConfigSpec to address the classic-remoting cluster formation failure in #3741, exposed by the membership assertions added in #3657. Changes remain confined to test setup and assertions; published documentation snippets are unchanged.
Changes:
- Derives seed-address protocols from the transport configuration, including the nightly JVM override.
- Binds classic remoting to loopback to match the seed addresses.
- Adds member protocol and host assertions alongside existing membership and port checks.
| File | Description |
|---|---|
cluster-typed/src/test/scala/docs/org/apache/pekko/cluster/typed/BasicClusterExampleSpec.scala |
Aligns seed addresses with the active transport and verifies member protocols and hosts. |
Review effort: Balanced
weiwen99
approved these changes
Nov 2, 2026
weiwen99
left a comment
weiwen99
left a comment
There was a problem hiding this comment.
LGTM — test-only fix, no must-fix findings.
I verified the affected specs on Zulu JDK 25.0.3 (forked test JVM):
- Artery (default), using the JDK-25 nightly settings (
pekko.test.timefactor=4,
fork-join-executor.minimum-runnable=8, virtualized dispatchers):
BasicClusterConfigSpec+BasicClusterManualSpec— 3/3 passed. - Classic (
-Dpekko.remote.artery.enabled=off), with the nightly classic-remoting
flags (cluster.assert=on, Netty leak detection PARANOID, etc.):
the same two specs — 3/3 passed. - On current
mainwith the same JDK 25 classic setup,BasicClusterConfigSpec
still fails with the reportedeventually ... 0 was not equal to 2timeout,
so the fix is effective.
Notes (non-blocking):
ConfigFactory.defaultReference()mergesreference.confand then applies
system-property overrides (documented behavior), soarteryEnabledsees the
nightly-Dpekko.remote.artery.enabled=offand matches the ActorSystem's own
config precedence. Good.- PR CI's classic-remoting job covers only cluster, distributed-data,
cluster-tools, and cluster-metrics;cluster-typedis covered only by the
nightly classic job. This changed spec is therefore not exercised by PR
checks. Consider adding at least this spec to the PR classic matrix in a
follow-up; not a blocker for this PR. - The
if (arteryEnabled) "pekko" else "pekko.tcp"mapping intentionally covers
only the two transports this fixture configures; fine as-is.
No docs changes needed; the published snippets are unchanged.
pjfanning
added a commit
to pjfanning/incubator-pekko
that referenced
this pull request
Nov 2, 2026
Motivation: BasicClusterConfigSpec broke under classic remoting (apache#3741) and was only caught by the nightly build, because PR validation does not run cluster-typed/test with Artery disabled. Modification: Add cluster-typed/test as a separate matrix entry in the Pekko Classic Remoting Tests job. Result: Classic remoting regressions in cluster-typed are caught on PRs. Tests: - Not run locally - CI workflow change; this PR's checks exercise it References: Refs apache#3741, Refs apache#3742
pjfanning
added a commit
that referenced
this pull request
Nov 2, 2026
* Run cluster-typed tests with classic remoting in PR validation Motivation: BasicClusterConfigSpec broke under classic remoting (#3741) and was only caught by the nightly build, because PR validation does not run cluster-typed/test with Artery disabled. Modification: Add cluster-typed/test as a separate matrix entry in the Pekko Classic Remoting Tests job. Result: Classic remoting regressions in cluster-typed are caught on PRs. Tests: - Not run locally - CI workflow change; this PR's checks exercise it References: Refs #3741, Refs #3742 * Also run cluster-sharding-typed tests with classic remoting in PR validation Motivation: The nightly classic remoting job runs cluster-sharding-typed/test alongside cluster-typed/test; PR validation should match. Modification: Add cluster-sharding-typed/test to the cluster-typed matrix entry. Result: PR validation covers the same typed cluster modules as the nightly job. Tests: - Not run locally - CI workflow change; this PR's checks exercise it References: Refs #3741 * Shorten seed-node-timeout in BasicClusterConfigSpec Motivation: With classic remoting, BasicClusterConfigSpec "init cluster" was flaky. The first seed waits the default 5s seed-node-timeout before forming the cluster, and the other node retries InitJoin every 5s. A retry that lands just before the first seed forms the cluster pushes the join to ~10s, which races the 10s eventually timeout. Modification: Set pekko.cluster.seed-node-timeout = 1s in the per-port test config, which is not part of the docs snippet. Result: The two nodes form a cluster within a few seconds instead of up to ~10s. Tests: - Not run locally - CI runs cluster-typed tests with classic remoting References: Refs #3745
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
BasicClusterConfigSpecfails in the nightly classic-remoting job because its seed addresses usepekko://and loopback while classic remoting usespekko.tcp://and the host IP. The membership assertions added in #3657 exposed this existing configuration mismatch; the earlier test only started the systems.Modification
127.0.0.1to match the seed addresses.Upassertions.Result
Both nodes form the expected cluster with Artery or classic remoting. The change is confined to test setup and assertions; all published documentation snippets remain unchanged.
Tests
Validation used Temurin JDK 17.0.21.
sbt -Dpekko.remote.artery.enabled=off "cluster-typed / Test / testOnly docs.org.apache.pekko.cluster.typed.BasicClusterConfigSpec"reproduced the timeout with 0 members instead of 2.BasicClusterConfigSpecandBasicClusterManualSpecpassed with default Artery and with-Dpekko.remote.artery.enabled=off: 3 tests per run.headerCreateAll,checkCodeStyle(including+headerCheckAllfor Scala 2.13 and 3), andgit diff --check: passed.validatePullRequestwith multi-node tests disabled: cluster-typed (77 tests), cluster-sharding-typed (104 tests), and docs (372 tests) passed. The persistence-typed test JVM failed because LevelDB JNI contains x86 binaries incompatible with this ARM Mac.persistence-typed / pr-validation:testwith-Dpekko.persistence.journal.leveldb.native=off -Dpekko.persistence.journal.leveldb-shared.store.native=off: all 47 tests passed using the supported Java LevelDB implementation.--print --output-format stream-jsonmode: no must-fix findings.References
Fixes #3741