Add new migration strategy for Pekko Persistence snapshots

[pjfanning]

See #1421

The docs in https://cwiki.apache.org/confluence/display/PEKKO/Pekko+Akka+Compatibility can be updated when this or its equivalent are ready for release.

The issue is that the 'manifest' value saved with the snapshot includes the class name of the serializer which can be 'akka' or 'org.apache.pekko' prefixed.

The idea is that the new config pekko.persistence.snapshot-store.auto-migrate-manifest supports 3 values:

• "pekko" is the default and is the existing behaviour in Pekko 1.0.3 and 1.1.0-M0 - this supports reading Pekko and Akka snapshots and saving them with Pekko serializer name
• "akka" is a new option that still supports reading Pekko and Akka snapshots but saves them with an Akka serializer name
  • this is useful if you have a mixed cluster of Akka and Pekko nodes - the Akka nodes will not be able to handle if a Pekko serializer class name appears in the snapshot data
• "no-migration" is a new option to basically not support old snapshots saved with Akka - this is more performant because we get to skip some checks when reading/writing snapshot data

[pjfanning]

@SakulK you were closely involved in #837 and #841 - would you have an opinion on this PR?

[SakulK]

@SakulK you were closely involved in #837 and #841 - would you have an opinion on this PR?

I think this is a great idea, making akka to pekko migration even safer due to an easier rollback in case it's needed, so thumbs up from me 👍

[mdedetrich]

Does it make sense to have a fourh option which is only reading pekko snapshots, it definitely won't be the default now but later on jf we introduce some change that we know won't work with akka it may make sense to change it as a default to signal that "we don't support all with this release"

[pjfanning]

Does it make sense to have a fourh option which is only reading pekko snapshots, it definitely won't be the default now but later on jf we introduce some change that we know won't work with akka it may make sense to change it as a default to signal that "we don't support all with this release"

The 'ignore' option in this PR only supports Pekko snapshots and avoids the overhead of having to to try to repair the serializer class names that get saved in the persisted data.

[mdedetrich]

The 'ignore' option in this PR only supports Pekko snapshots and avoids the overhead of having to to try to repair the serializer class names that get saved in the persisted data.

Thanks, I was just skimming this on the phone so I couldn't look at it properly which brings me to the second point, maybe we can rename the options like this so its more explicit/clear?

• Ignore -> Pekko
• Pekko -> PekkoAndAkka
• Akka -> Akka

Wdyt?

[pjfanning]

The 'ignore' option in this PR only supports Pekko snapshots and avoids the overhead of having to to try to repair the serializer class names that get saved in the persisted data.

Thanks, I was just skimming this on the phone so I couldn't look at it properly which brings me to the second point, maybe we can rename the options like this so its more explicit/clear?

• Ignore -> Pekko
• Pekko -> PekkoAndAkka
• Akka -> Akka

Wdyt?

Thanks @mdedetrich - I agree the naming isn't right but I don't think your suggestion quite works either. Let me think about better naming and I'll come back with some changes for review.

[pjfanning]

The 'ignore' option in this PR only supports Pekko snapshots and avoids the overhead of having to to try to repair the serializer class names that get saved in the persisted data.

Thanks, I was just skimming this on the phone so I couldn't look at it properly which brings me to the second point, maybe we can rename the options like this so its more explicit/clear?

• Ignore -> Pekko
• Pekko -> PekkoAndAkka
• Akka -> Akka

Wdyt?

Thanks @mdedetrich - I agree the naming isn't right but I don't think your suggestion quite works either. Let me think about better naming and I'll come back with some changes for review.

I pushed some some changes. The config is about migrating data and is not about whether we support Pekko and Akka snapshots. I renamed 'Ignore' to 'NoMigration'. With 'NoMigration', we will fail if there is an attempt to update an Akka generated snapshot. With 'Akka' and 'Pekko', we support both Akka and Pekko generated snapshots but the config controls what serializer name value gets included in the persisted data.

[pjfanning]

@mdedetrich @kerr @raboof @Roiocam @nvollmar @samueleresca I'm hoping to get this into the 1.1.0 release and I was hoping to get that release process started maybe next week. Would any of you have time to review this?

[samueleresca]

I went through the #1421 discussion and reviewed the PR. The changes look reasonable to me.

[nvollmar]

lgtm