You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Alberto Gomez <al...@est.tech> on 2020/05/19 12:13:57 UTC
Question about version checks inside fromData method in
GatewaySenderEventImpl
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.
RE: Question about version checks inside fromData method in
GatewaySenderEventImpl
Posted by Alberto Bustamante Reyes <al...@est.tech>.
Hi Juan Jose,
I think Alberto is asking about how the check is done, not about why its done. The method he is asking about is mixing the two ways we know for handling backward compatibility.
One is creating the "toDataPre_GEODE_X_X_X" and "fromDataPre_GEODE_X_X_X" methods. And the other one is using ifs to check the versions. But this method is mixing both of them.
BR/
Alberto B.
________________________________
De: Ju@N <ju...@gmail.com>
Enviado: martes, 19 de mayo de 2020 14:54
Para: dev@geode.apache.org <de...@geode.apache.org>
Asunto: 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 <al...@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
Re: Question about version checks inside fromData method in
GatewaySenderEventImpl
Posted by Bruce Schuchardt <bs...@pivotal.io>.
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" <ju...@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 <al...@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 <ju...@gmail.com>
> Sent: Tuesday, May 19, 2020 2:54 PM
> To: dev@geode.apache.org <de...@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 <al...@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
Re: Question about version checks inside fromData method in GatewaySenderEventImpl
Posted by "Ju@N" <ju...@gmail.com>.
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 <al...@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 <ju...@gmail.com>
> Sent: Tuesday, May 19, 2020 2:54 PM
> To: dev@geode.apache.org <de...@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 <al...@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
Re: Question about version checks inside fromData method in
GatewaySenderEventImpl
Posted by Alberto Gomez <al...@est.tech>.
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 <ju...@gmail.com>
Sent: Tuesday, May 19, 2020 2:54 PM
To: dev@geode.apache.org <de...@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 <al...@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
Re: Question about version checks inside fromData method in GatewaySenderEventImpl
Posted by "Ju@N" <ju...@gmail.com>.
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 <al...@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