[
https://issues.apache.org/jira/browse/GEODE-8330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17152349#comment-17152349
]
ASF GitHub Bot commented on GEODE-8330:
---------------------------------------
Bill commented on a change in pull request #5344:
URL: https://github.com/apache/geode/pull/5344#discussion_r450505543
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java
##########
@@ -2038,7 +2039,8 @@ protected boolean chunkEntries(DistributedRegion rgn, int
chunkSizeInBytes,
entry.key = key;
entry.setVersionTag(stamp.asVersionTag());
fillRes = mapEntry.fillInValue(rgn, entry, in,
rgn.getDistributionManager(),
- sender.getVersionObject());
+ Versioning
Review comment:
pulled ✓
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java
##########
@@ -2050,7 +2052,8 @@ protected boolean chunkEntries(DistributedRegion rgn, int
chunkSizeInBytes,
entry = new InitialImageOperation.Entry();
entry.key = key;
fillRes = mapEntry.fillInValue(rgn, entry, in,
rgn.getDistributionManager(),
- sender.getVersionObject());
+ Versioning
Review comment:
pulled ✓
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Structural Improvements to Serialization Versioning
> ---------------------------------------------------
>
> Key: GEODE-8330
> URL: https://issues.apache.org/jira/browse/GEODE-8330
> Project: Geode
> Issue Type: Improvement
> Components: serialization
> Reporter: Bill Burcham
> Assignee: Bill Burcham
> Priority: Major
> Labels: membership
>
> 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:
> {noformat}
> VersionOrdinal
> <<interface>>
> ^
> |
> AbstractVersion
> <<abstract>>
> ^
> |
> -----------------
> | |
> Version UnknownVersion
> <<enum>>
> {noformat}
> 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.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
