[GEODE-8330] Structural Improvements to Serialization Versioning

• Type: Improvement
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Fix Version/s: 1.14.0
• Component/s: serialization
• Labels: membership
• Assignee: Bill Burcham
• Reporter: Bill Burcham
• Created: 06/Jul/20 17:43
• Updated: 03/Sep/21 02:21
• Resolved: 07/Jul/20 19:25

Description

As a follow-on to GEODE-8240 we want to do another ticket to improve, outside the context of a big release-blocking bug, the versioning framework. The starting point for this work will be the work completed and merged for GEODE-8240.

Our goals are to:

• Make the version type hierarchy easily understood by Geode developers
• Make the framework foolproof so we prevent bugs like GEODE-8240

We’ll rely on a handful of techniques to accomplish this:

• Clear naming
• Orthogonality—separation of concerns, one way to do each thing
• No null—it’s not needed in this framework at all
• No exceptions (no throws)—prefer total functions instead
• Separate “model” code from I/O code

When fixing the rolling upgrade bug in GEODE-8240 we introduced a base type: VersionOrdinal for the existing "enum" Version. During the work for that ticket we saw lots of little things that needed cleaning up, but we didn't want to do them in that other PR because we had a couple support branches waiting for that ticket to complete. Also we wanted the GEODE-8240 changes to be minimal so as to minimize the complexity of our back-porting work.

Now that that ticket is complete and back-ported, this ticket will:

• get rid of confusing and wrong inheritance relationship between VersionOrdinalImpl and Version: now Version (i.e. known version) and UnknownVersion both extend AbstractVersion which implements VersionOrdinal—improved naming of this hierarchy will come, probably in a follow-on PR since it would touch lots of files
• flesh-out Versioning factory:
  • add Version getKnownVersion(final VersionOrdinal anyVersion, Version returnWhenUnknown) method that simply downcasts anyVersion if it can
  • ensure there's exactly one way to construct a version (VersionOrdinal) from a short, and there is exactly one way to get a known version (Version) from a VersionOrdinal
• version acquisition no longer throws exceptions ever
• eliminate InternalDistributedMember.getVersionObject() in favor of Versioning.getKnownVersion(Versioning.getVersionOrdinal(ver), Version.CURRENT): the latter makes it clear to maintainers that Version.CURRENT will be used as a stand-in for unknown versions
• move I/O logic to a separate class, VersioningIO
• eliminate tons of redundant and unused methods in Version

The type hierarchy after this story is complete will be:

    VersionOrdinal
    <<interface>>

          ^
          |

    AbstractVersion
    <<abstract>>

          ^
          |
   -----------------
   |               |

Version      UnknownVersion
<<enum>> 

In a separate ticket/story we will tackle the final improvements to the type names:

Version will become KnownVersion
VersionOrdinal will become Version yippee!!

Changing Version to KnownVersion will touch hundreds of files. We'll tackle that in a separate ticket/story/PR that is focused just on that renaming.

Issue Links

• is related to: GEODE-8240 View has old locator version number after rolling upgrade
• relates to: GEODE-8337 Rename Version enum to KnownVersion; VersionOrdinal to Version
• links to: GitHub Pull Request #5344