[
https://issues.apache.org/jira/browse/GEODE-8963?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17290562#comment-17290562
]
ASF GitHub Bot commented on GEODE-8963:
---------------------------------------
Bill commented on a change in pull request #6050:
URL: https://github.com/apache/geode/pull/6050#discussion_r582368339
##########
File path:
geode-serialization/src/main/java/org/apache/geode/internal/serialization/KnownVersion.java
##########
@@ -41,6 +41,11 @@
/** The suffix to use in toDataPre / fromDataPre method names */
private final transient String methodSuffix;
+ /**
+ * The version used for client/server communications.
+ */
+ private final transient KnownVersion clientServerProtocolVersion;
Review comment:
I feel like this should be a `Version` not a `KnownVersion`. That would
be sufficient to support ordering comparisons. To say that this is a
`KnownVersion` implies that this expression should be meaningful:
```java
(knownVersion.getClientServerProtocolVersion()).getClientServerProtocolVersion()
```
But that expression is not meaningful. The first parenthesized
sub-expression is the client-server protocol version. But that thing shouldn't
have a client-server protocol version (the whole expression.)
On the other hand, I can see the value of a client version knowing other
stuff that a `KnownVersion` knows, such as `major`, `minor`, and `patch`. If
those things are important to you then I think there are two options:
1. add some new class to hold two `KnownVersion` instances: one for the
server and another for clients
2. change this field type to `Version` (and change the corresponding
constructor argument type) but when you need extra `KnownVersion` info for that
(ordinal) version, use `Versioning.getKnownVersionOrDefault()` to get at it.
OTOH it doesn't even make a lot of sense to talk about the major+minor+patch
components of a _client-server protocol_ version since the latter does not
change independently of our product versions. My option (2) might be useful
for: "looking up the most recent `KnownVersion` (a product version), in which
the client-server protocol identified by a certain `Version`, changed". But I
don't foresee that being useful.
----------------------------------------------------------------
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
> separate client/server compatibility from server/server version compatibility
> -----------------------------------------------------------------------------
>
> Key: GEODE-8963
> URL: https://issues.apache.org/jira/browse/GEODE-8963
> Project: Geode
> Issue Type: Improvement
> Components: serialization
> Reporter: Bruce J Schuchardt
> Assignee: Bruce J Schuchardt
> Priority: Minor
> Labels: pull-request-available
>
> A client's version is used for deserializing data received from the client
> and for serializing data sent to the client. It is also used to locate the
> map of Commands used to process client requests. Every time we cut a new
> release we bump this version in KnownVersions and create a new map of
> Commands, even though client/server communications protocols rarely change.
> We should have each KnownVersion hold a client/server compatibility number
> that is used to identify clients rather than the KnownVersion's ordinal.
> For instance,
> {code:java}
> public static final KnownVersion GEODE_1_15_0 =
> new KnownVersion("GEODE", "1.15.0", (byte) 1, (byte) 15, (byte) 0,
> (byte) 0,
> /*server/server version*/GEODE_1_15_0_ORDINAL,
> /*client/server version*/GEODE_1_15_0_ORDINAL);
>
> public static final KnownVersion GEODE_1_16_0 =
> new KnownVersion("GEODE", "1.16.0", (byte) 1, (byte) 16, (byte) 0,
> (byte) 0,
> /*server/server version*/GEODE_1_16_0_ORDINAL,
> /*client/server version*/GEODE_1_15_0_ORDINAL);
> public static final KnownVersion GEODE_1_17_0 =
> new KnownVersion("GEODE", "1.17.0", (byte) 1, (byte) 17, (byte) 0,
> (byte) 0,
> /*server/server version*/GEODE_1_17_0_ORDINAL,
> /*client/server version*/GEODE_1_15_0_ORDINAL);
> {code}
> In the above KnownVersions the client/server serialization is known to have
> not changed since v1.15.0 and so there is no need to use a newer KnownVersion
> for clients.
> Client handshake code will need to be changed to use the client/server
> ordinal when identifying clients and servers.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
