-
Notifications
You must be signed in to change notification settings - Fork 192
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
Conversation
@pjfanning
pjfanning
left a comment
There was a problem hiding this 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
@raboof
raboof
left a comment
There was a problem hiding this 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
commented
Nov 27, 2024
(I'll backport)
pjfanning
commented
Nov 27, 2024
@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?
sadekmunawar
commented
Nov 27, 2024
@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.
pjfanning
commented
Dec 9, 2024
@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?
@pjfanning There are two missing pieces left:
- Deserialization of Akka cluster messages. I created the PR for this one Enable deserialization of old Akka cluster messages (mixed pekko/akka cluster) #1578
- 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
There was a problem hiding this comment.
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
Rolling upgrade from Akka to Pekko requires cluster formation with mixed protocol nodes. If the config
accept-protocol-namescontains both "akka" and "pekko", nodes using either protocol should be able join the same cluster.Relates to #765