Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Allow cluster formation with mixed protocols #1567

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

Merged
raboof merged 1 commit into apache:main from sadekmunawar:mixed-protocols
Nov 27, 2024

Conversation

@sadekmunawar
Copy link
Contributor

@sadekmunawar sadekmunawar commented Nov 27, 2024

Rolling upgrade from Akka to Pekko requires cluster formation with mixed protocol nodes. If the config accept-protocol-names contains both "akka" and "pekko", nodes using either protocol should be able join the same cluster.

Relates to #765

Copy link
Member

@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.

thanks @sadekmunawar - this looks like a necessary change

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.

Another great observation, and thanks for also adding a test!

@raboof raboof merged commit e4fa6f5 into apache:main Nov 27, 2024
Copy link
Member

raboof commented Nov 27, 2024

(I'll backport)

pjfanning reacted with thumbs up emoji

Copy link
Member

@sadekmunawar do you think doing a release with your changes is a good idea or are you in the middle of testing mixed clusters and feel like you might find more issues?

Copy link
Contributor Author

@sadekmunawar do you think doing a release with your changes is a good idea or are you in the middle of testing mixed clusters and feel like you might find more issues?

@pjfanning I had to make one more change for the migration. I'll create a PR by the end of the week. Once that's merged, I think a release can be done.

Copy link
Member

@sadekmunawar any news on progress? If you don't have time to create a PR for the remaining issue, could you give us some pointers so someone else could create the PR instead?

Copy link
Contributor Author

sadekmunawar commented Dec 10, 2024
edited
Loading

@pjfanning There are two missing pieces left:

  1. Deserialization of Akka cluster messages. I created the PR for this one Enable deserialization of old Akka cluster messages (mixed pekko/akka cluster) #1578
  2. Supporting Akka config for JoinConfigCompatChecker. An exception is thrown here, when a Pekko node tries to find the downing path in the Akka config. I solved it by simply checking if the path exists in the config before doing the comparison.
 val downingProviderResult = {
 if (toCheck.hasPath(DowningProviderPath) && actualConfig.hasPath(DowningProviderPath))
 JoinConfigCompatChecker.checkEquality(List(DowningProviderPath), toCheck, actualConfig)
 else Invalid(im.Seq("Downing path not found. Set configuration-compatibility-check.enforce-on-join to off to enable cluster joining."))
 }

While this all that's necessary for the rolling upgrade support, I tried to find a better approach for the second problem. See this commit. I haven't had the chance to integration test this approach, yet. It should fix the exception issue, but it likely sends configs with Pekko paths to Akka nodes here

Copy link
Member

pjfanning commented Dec 16, 2024
edited
Loading

I am going to revert this due to #1586

A build of #1587 with the nightly builds temporarily enabled shows that this PR is the cause.

We can try to fix this issue again but we need a working build first - #1590.

He-Pin reacted with heart emoji


import org.apache.pekko.testkit.{ LongRunningTest, PekkoSpec }

object MixedProtocolClusterSpec {
Copy link
Member

@pjfanning pjfanning Dec 16, 2024

Choose a reason for hiding this comment

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

this test doesn't work in the nightly build CI job and the change breaks many other cluster tests too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pjfanning pjfanning pjfanning approved these changes

@raboof raboof raboof approved these changes

Assignees

No one assigned

Labels

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /