Yeah, that code is confusing. I think the ordinal constants should all
be changed to shorts and the constructor for Version should take a short
as well. I can see why someone creating a new ordinal would be hesitant
to do that - it will need to be tested.
The on-wire representation of a Version isn't limited to a byte. We use
writeOrdinal() to serialize version ordinals.
I'll open a JIRA to address this.
On 8/23/19 9:16 AM, Kirk Lund wrote:
I think they did that because the datatypes in the Version class are too
small. I'm not sure why bytes and shorts were chosen.
The version constants are defined as bytes:
private static final *byte* GEODE_1_11_0_ORDINAL = 107;
While the ordinal is defined as a short:
private final *short* ordinal;
I'm assuming this was done because Version ends up on the wire. Does it go
into every message or just handshake/join messages? I suppose we can't
change these datatypes to integers until Geode 2.0 because they would end
up being a breaking change.
Maybe we should scan through all serialized datatypes for bytes/shorts that
are going to run out...
On Fri, Aug 23, 2019 at 9:02 AM Bruce Schuchardt <bschucha...@pivotal.io>
wrote:
We've been incrementing the serialization version by 5 for each x.x.0
release but I see that we went from 105 in 1.10.0 to 107 in develop for
1.11.0. That gives us only one opportunity to make a serialization
change if we need to release patches for 1.10. I don't think that's
safe & we need to bump the ordinal for 1.11.0 to 110.
