While GatewaySenderEventImpl is structured correctly to fit into the 
backward-compatibility serialization framework it does have some odd code.  It 
looks like mistakes were made in the past in the serialization code for this 
class and the odd code is trying to compensate.

The "version" variable used in fromData is not from the serialization framework 
- it's initialized in the fromDataPre_GEODE_1_9_0_0 method.   When serialized 
It writes Version.GEODE_1_9_0.ordinal(), the value of its VERSION constant.  
That constant has a comment from Gester that maybe he can explain.

" // It should use current version. But it was hard-coded to be 0x11, i.e. 
GEODE_120_ORDINAL,
  // by mistake since 120 to pre-190
  protected static final short VERSION = Version.GEODE_1_9_0.ordinal();"

The fromDataPre_GEODE_1_9_0_0 method still has this hex 0x11 magic number and 
sets up a deserialization stream based on a pre-geode version.

On 5/19/20, 6:49 AM, "Ju@N" <jujora...@gmail.com> wrote:

    I misunderstood the question, sorry about that.
    I'm not entirely familiar with the serialization versioning mechanism, so
    I'll leave somebody else with deeper knowledge to answer.
    Cheers.
    
    
    
    On Tue, 19 May 2020 at 14:30, Alberto Gomez <alberto.go...@est.tech> wrote:
    
    > Hi Juan,
    >
    > Thanks for your answer.
    >
    > According to
    > 
https://cwiki.apache.org/confluence/display/GEODE/Managing+Backward+Compatibility
    > there are two ways to manage backward compatibility for classes that
    > implement SerializationVersions.
    >
    > Either implementing `toDataPre/fromDataPre` methods that Data
    > Serialization will invoke based on the version of the sender/receiver
    > (preferred way), or using `fromData/toData` methods using
    > `InternalDataSerializer.getVersionForDataStream`.
    >
    > In the case of this class, we have `toDataPre/fromDataPre` methods
    > implemented so, according to what is described in the wiki, it should not
    > be necessary to add any extra check to the `fromData/toData`methods. But
    > there is this check I mentioned which is necessary according to some
    > backward compatibility tests in Geode. So my question is why is it
    > necessary?
    >
    > Thanks,
    >
    > -Alberto
    >
    >
    > ________________________________
    > From: Ju@N <jujora...@gmail.com>
    > Sent: Tuesday, May 19, 2020 2:54 PM
    > To: dev@geode.apache.org <dev@geode.apache.org>
    > Subject: Re: Question about version checks inside fromData method in
    > GatewaySenderEventImpl
    >
    > Hello Alberto,
    >
    > It looks like the property *isConcurrencyConflict* was added as part of
    > *GEODE-3967* [1] and this was released as part of Geode 1.9.0; that seems
    > to the reason why the check is in place: if we get an instance of
    > *GatewaySenderEventImpl* from a member running a version higher than 1.9.0
    > then we are 100% sure that the serialized form will contain the new field
    > so we can parse it, if the serialized *GatewaySenderEventImpl *comes from
    > an older member the filed won't be there so we don't even try to parse it.
    > Hope I didn't miss anything.
    > Cheers.
    >
    > [1]: https://issues.apache.org/jira/browse/GEODE-3967
    >
    > On Tue, 19 May 2020 at 13:14, Alberto Gomez <alberto.go...@est.tech>
    > wrote:
    >
    > > Hi,
    > >
    > > Looking at the fromData method of GatewaySenderEventImpl I see that it
    > > contains a conditional reading of the isConcurrencyConflict when version
    > is
    > > greater than Geode 1.9.0 one. See below:
    > >
    > >   @Override
    > >   public void fromData(DataInput in,
    > >       DeserializationContext context) throws IOException,
    > > ClassNotFoundException {
    > >     fromDataPre_GEODE_1_9_0_0(in, context);
    > >     if (version >= Version.GEODE_1_9_0.ordinal()) {
    > >       this.isConcurrencyConflict = DataSerializer.readBoolean(in);
    > >     }
    > >   }
    > >
    > > I have looked at the implementation of this method in other classes and
    > > have not seen this checking of version pattern. I have also observed 
that
    > > if the "if" is removed some backward compatibility tests fail.
    > >
    > > Could anybody tell me why this check (the if) is necessary given that
    > > there is already a fromDataPre_GEODE_1_9_0 method in the class?
    > >
    > > Thanks in advance,
    > >
    > > -Alberto G.
    > >
    >
    >
    > --
    > Ju@N
    >
    
    
    -- 
    Ju@N
    


Reply via email to