sadekmunawar commented on code in PR #1578:
URL: https://github.com/apache/pekko/pull/1578#discussion_r1879256878
##########
cluster/src/test/scala/org/apache/pekko/cluster/protobuf/ClusterMessageSerializerSpec.scala:
##########
@@ -177,6 +177,14 @@ class ClusterMessageSerializerSpec extends
PekkoSpec("pekko.actor.provider = clu
ClusterMessageSerializer.OldWelcomeManifest)
}
+ "be de-serializable with class manifests from Akka nodes" in {
+ val oldAkkaJoinAckManifest =
s"org.apache.pekko.cluster.InternalClusterAction$$InitJoinAck"
Review Comment:
I upgraded from Akka-2.6.2, so this change was necessary. But updating the
documentation to advise users to perform the rolling upgrade from a version
higher than 2.6.4 may be sufficient.
An alternative approach would be to revert the old manifest back to Akka
(e.g. ` val OldJoinManifest = s"akka.cluster.InternalClusterAction$$Join"`).
No version of Pekko is expected to use the old manifest in the cluster
messages, right?
##########
cluster/src/main/scala/org/apache/pekko/cluster/protobuf/ClusterMessageSerializer.scala:
##########
@@ -125,35 +125,43 @@ final class ClusterMessageSerializer(val system:
ExtendedActorSystem)
throw new IllegalArgumentException(s"Can't serialize object of type
${obj.getClass} in [${getClass.getName}]")
}
- def fromBinary(bytes: Array[Byte], manifest: String): AnyRef = manifest
match {
- case HeartbeatManifest => deserializeHeartBeat(bytes)
- case HeartbeatRspManifest => deserializeHeartBeatResponse(bytes)
- case GossipStatusManifest => deserializeGossipStatus(bytes)
- case GossipEnvelopeManifest => deserializeGossipEnvelope(bytes)
- case InitJoinManifest => deserializeInitJoin(bytes)
- case InitJoinAckManifest => deserializeInitJoinAck(bytes)
- case InitJoinNackManifest => deserializeInitJoinNack(bytes)
- case JoinManifest => deserializeJoin(bytes)
- case WelcomeManifest => deserializeWelcome(bytes)
- case LeaveManifest => deserializeLeave(bytes)
- case DownManifest => deserializeDown(bytes)
- case ExitingConfirmedManifest => deserializeExitingConfirmed(bytes)
- case ClusterRouterPoolManifest => deserializeClusterRouterPool(bytes)
- // needs to stay in Akka 2.6.5 to be able to talk to an Akka 2.5.{3,4}
node during rolling upgrade
- case HeartBeatManifestPre2523 => deserializeHeartBeatAsAddress(bytes)
- case HeartBeatRspManifest2523 =>
deserializeHeartBeatRspAsUniqueAddress(bytes)
- case OldGossipStatusManifest => deserializeGossipStatus(bytes)
- case OldGossipEnvelopeManifest => deserializeGossipEnvelope(bytes)
- case OldInitJoinManifest => deserializeInitJoin(bytes)
- case OldInitJoinAckManifest => deserializeInitJoinAck(bytes)
- case OldInitJoinNackManifest => deserializeInitJoinNack(bytes)
- case OldJoinManifest => deserializeJoin(bytes)
- case OldWelcomeManifest => deserializeWelcome(bytes)
- case OldLeaveManifest => deserializeLeave(bytes)
- case OldDownManifest => deserializeDown(bytes)
- case OldExitingConfirmedManifest => deserializeExitingConfirmed(bytes)
- case OldClusterRouterPoolManifest => deserializeClusterRouterPool(bytes)
- case _ => throw new
IllegalArgumentException(s"Unknown manifest [$manifest]")
+ def fromBinary(bytes: Array[Byte], manifest: String): AnyRef = {
+ val updatedManifest = {
+ if (manifest.startsWith("akka"))
Review Comment:
I agree. If we decide to keep the changes in this PR, then having boolean
guard would be better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@pekko.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@pekko.apache.org
For additional commands, e-mail: notifications-h...@pekko.apache.org
