You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Michael Oleske <mo...@pivotal.io> on 2019/09/12 17:11:03 UTC

Propose including GEODE-7178 in 1.10

Hi Geode Devs!

I'd like to propose including the fix for GEODE-7178.  This resolves an
issue that Ivan (https://markmail.org/message/dwwac42xmpo4xb2e) ran into in
1.10 RC1.

SHA: 91176d61df64bf1390cdba7b1cdc2b40cdfaba3a
Link to GitHub:
https://github.com/apache/geode/commit/91176d61df64bf1390cdba7b1cdc2b40cdfaba3a

Thanks!
-michael

Re: Propose including GEODE-7178 in 1.10

Posted by Dick Cavender <dc...@pivotal.io>.
This has been merged to release/1.10.0.



On Fri, Sep 13, 2019 at 11:14 AM Dick Cavender <dc...@pivotal.io> wrote:

> We have 3 plus one votes so we'll merge this as soon as the PR checks
> complete.
>
> -Dick
>
>
> On Fri, Sep 13, 2019 at 10:25 AM Blake Bender <bb...@pivotal.io> wrote:
>
>> +1, IMO this really needs to go in.
>>
>> Thanks,
>>
>> Blake
>>
>>
>> On Thu, Sep 12, 2019 at 3:30 PM Anthony Baker <ab...@pivotal.io> wrote:
>>
>> > My understanding is that this portion of the protocol is determined by
>> > instanceof checks, not the ordinal version.  The messages from the java
>> > client went through a different code path than messages from the native
>> > client.  So java clients using ordinal 45 still work (that’s why our
>> > backwards compatibility tests don’t fail).
>> >
>> > Anthony
>> >
>> >
>> > > On Sep 12, 2019, at 2:34 PM, Dan Smith <ds...@pivotal.io> wrote:
>> > >
>> > > +1 for getting this in 1.10.
>> > >
>> > > I am curious though - is the native client behaving like an older
>> > versions
>> > > of the java client, or is this totally unique behavior for the native
>> > > client? Is there some integration test that we are missing here?
>> > >
>> > > -Dan
>> > >
>> > > On Thu, Sep 12, 2019 at 11:52 AM Michael Oleske <mo...@pivotal.io>
>> > wrote:
>> > >
>> > >> Here is the Pull Request for the cherry pick as requested
>> > >> https://github.com/apache/geode/pull/4049
>> > >>
>> > >> -michael
>> > >>
>> > >> On Thu, Sep 12, 2019 at 10:28 AM Dick Cavender <dcavender@pivotal.io
>> >
>> > >> wrote:
>> > >>
>> > >>> Hi Michael, thank you for bringing your concern and fixing this
>> issue.
>> > >>>
>> > >>> Geode's release process dictates a time-based schedule <
>> > >>> https://cwiki.apache.org/confluence/display/GEODE/Release+Schedule>
>> to
>> > >> cut
>> > >>> release branches.  The “critical fixes” rule does allow critical
>> fixes
>> > to
>> > >>> be brought to the release branch by proposal on the dev list, as you
>> > have
>> > >>> done here.
>> > >>>
>> > >>> If there is consensus from the Geode community that your proposed
>> > change
>> > >>> satisfies the “critical fixes” rule, I will be happy to bring it to
>> the
>> > >>> 1.10.0 release branch.
>> > >>>
>> > >>> Due to the complexity of this change, could please open a PR against
>> > >>> release/1.10.0 containing the exact changes you want to merge?
>> > >>>
>> > >>> Regards
>> > >>>
>> > >>> -Dick
>> > >>>
>> > >>>
>> > >>> On Thu, Sep 12, 2019 at 10:23 AM Anthony Baker <ab...@pivotal.io>
>> > >> wrote:
>> > >>>
>> > >>>> +1 yes please!
>> > >>>>
>> > >>>>> On Sep 12, 2019, at 10:11 AM, Michael Oleske <mo...@pivotal.io>
>> > >>> wrote:
>> > >>>>>
>> > >>>>> Hi Geode Devs!
>> > >>>>>
>> > >>>>> I'd like to propose including the fix for GEODE-7178.  This
>> resolves
>> > >> an
>> > >>>>> issue that Ivan (https://markmail.org/message/dwwac42xmpo4xb2e)
>> ran
>> > >>>> into in
>> > >>>>> 1.10 RC1.
>> > >>>>>
>> > >>>>> SHA: 91176d61df64bf1390cdba7b1cdc2b40cdfaba3a
>> > >>>>> Link to GitHub:
>> > >>>>>
>> > >>>>
>> > >>>
>> > >>
>> >
>> https://github.com/apache/geode/commit/91176d61df64bf1390cdba7b1cdc2b40cdfaba3a
>> > >>>>>
>> > >>>>> Thanks!
>> > >>>>> -michael
>> > >>>>
>> > >>>>
>> > >>>
>> > >>
>> >
>> >
>>
>

Re: Propose including GEODE-7178 in 1.10

Posted by Dick Cavender <dc...@pivotal.io>.
We have 3 plus one votes so we'll merge this as soon as the PR checks
complete.

-Dick


On Fri, Sep 13, 2019 at 10:25 AM Blake Bender <bb...@pivotal.io> wrote:

> +1, IMO this really needs to go in.
>
> Thanks,
>
> Blake
>
>
> On Thu, Sep 12, 2019 at 3:30 PM Anthony Baker <ab...@pivotal.io> wrote:
>
> > My understanding is that this portion of the protocol is determined by
> > instanceof checks, not the ordinal version.  The messages from the java
> > client went through a different code path than messages from the native
> > client.  So java clients using ordinal 45 still work (that’s why our
> > backwards compatibility tests don’t fail).
> >
> > Anthony
> >
> >
> > > On Sep 12, 2019, at 2:34 PM, Dan Smith <ds...@pivotal.io> wrote:
> > >
> > > +1 for getting this in 1.10.
> > >
> > > I am curious though - is the native client behaving like an older
> > versions
> > > of the java client, or is this totally unique behavior for the native
> > > client? Is there some integration test that we are missing here?
> > >
> > > -Dan
> > >
> > > On Thu, Sep 12, 2019 at 11:52 AM Michael Oleske <mo...@pivotal.io>
> > wrote:
> > >
> > >> Here is the Pull Request for the cherry pick as requested
> > >> https://github.com/apache/geode/pull/4049
> > >>
> > >> -michael
> > >>
> > >> On Thu, Sep 12, 2019 at 10:28 AM Dick Cavender <dc...@pivotal.io>
> > >> wrote:
> > >>
> > >>> Hi Michael, thank you for bringing your concern and fixing this
> issue.
> > >>>
> > >>> Geode's release process dictates a time-based schedule <
> > >>> https://cwiki.apache.org/confluence/display/GEODE/Release+Schedule>
> to
> > >> cut
> > >>> release branches.  The “critical fixes” rule does allow critical
> fixes
> > to
> > >>> be brought to the release branch by proposal on the dev list, as you
> > have
> > >>> done here.
> > >>>
> > >>> If there is consensus from the Geode community that your proposed
> > change
> > >>> satisfies the “critical fixes” rule, I will be happy to bring it to
> the
> > >>> 1.10.0 release branch.
> > >>>
> > >>> Due to the complexity of this change, could please open a PR against
> > >>> release/1.10.0 containing the exact changes you want to merge?
> > >>>
> > >>> Regards
> > >>>
> > >>> -Dick
> > >>>
> > >>>
> > >>> On Thu, Sep 12, 2019 at 10:23 AM Anthony Baker <ab...@pivotal.io>
> > >> wrote:
> > >>>
> > >>>> +1 yes please!
> > >>>>
> > >>>>> On Sep 12, 2019, at 10:11 AM, Michael Oleske <mo...@pivotal.io>
> > >>> wrote:
> > >>>>>
> > >>>>> Hi Geode Devs!
> > >>>>>
> > >>>>> I'd like to propose including the fix for GEODE-7178.  This
> resolves
> > >> an
> > >>>>> issue that Ivan (https://markmail.org/message/dwwac42xmpo4xb2e)
> ran
> > >>>> into in
> > >>>>> 1.10 RC1.
> > >>>>>
> > >>>>> SHA: 91176d61df64bf1390cdba7b1cdc2b40cdfaba3a
> > >>>>> Link to GitHub:
> > >>>>>
> > >>>>
> > >>>
> > >>
> >
> https://github.com/apache/geode/commit/91176d61df64bf1390cdba7b1cdc2b40cdfaba3a
> > >>>>>
> > >>>>> Thanks!
> > >>>>> -michael
> > >>>>
> > >>>>
> > >>>
> > >>
> >
> >
>

Re: Propose including GEODE-7178 in 1.10

Posted by Blake Bender <bb...@pivotal.io>.
+1, IMO this really needs to go in.

Thanks,

Blake


On Thu, Sep 12, 2019 at 3:30 PM Anthony Baker <ab...@pivotal.io> wrote:

> My understanding is that this portion of the protocol is determined by
> instanceof checks, not the ordinal version.  The messages from the java
> client went through a different code path than messages from the native
> client.  So java clients using ordinal 45 still work (that’s why our
> backwards compatibility tests don’t fail).
>
> Anthony
>
>
> > On Sep 12, 2019, at 2:34 PM, Dan Smith <ds...@pivotal.io> wrote:
> >
> > +1 for getting this in 1.10.
> >
> > I am curious though - is the native client behaving like an older
> versions
> > of the java client, or is this totally unique behavior for the native
> > client? Is there some integration test that we are missing here?
> >
> > -Dan
> >
> > On Thu, Sep 12, 2019 at 11:52 AM Michael Oleske <mo...@pivotal.io>
> wrote:
> >
> >> Here is the Pull Request for the cherry pick as requested
> >> https://github.com/apache/geode/pull/4049
> >>
> >> -michael
> >>
> >> On Thu, Sep 12, 2019 at 10:28 AM Dick Cavender <dc...@pivotal.io>
> >> wrote:
> >>
> >>> Hi Michael, thank you for bringing your concern and fixing this issue.
> >>>
> >>> Geode's release process dictates a time-based schedule <
> >>> https://cwiki.apache.org/confluence/display/GEODE/Release+Schedule> to
> >> cut
> >>> release branches.  The “critical fixes” rule does allow critical fixes
> to
> >>> be brought to the release branch by proposal on the dev list, as you
> have
> >>> done here.
> >>>
> >>> If there is consensus from the Geode community that your proposed
> change
> >>> satisfies the “critical fixes” rule, I will be happy to bring it to the
> >>> 1.10.0 release branch.
> >>>
> >>> Due to the complexity of this change, could please open a PR against
> >>> release/1.10.0 containing the exact changes you want to merge?
> >>>
> >>> Regards
> >>>
> >>> -Dick
> >>>
> >>>
> >>> On Thu, Sep 12, 2019 at 10:23 AM Anthony Baker <ab...@pivotal.io>
> >> wrote:
> >>>
> >>>> +1 yes please!
> >>>>
> >>>>> On Sep 12, 2019, at 10:11 AM, Michael Oleske <mo...@pivotal.io>
> >>> wrote:
> >>>>>
> >>>>> Hi Geode Devs!
> >>>>>
> >>>>> I'd like to propose including the fix for GEODE-7178.  This resolves
> >> an
> >>>>> issue that Ivan (https://markmail.org/message/dwwac42xmpo4xb2e) ran
> >>>> into in
> >>>>> 1.10 RC1.
> >>>>>
> >>>>> SHA: 91176d61df64bf1390cdba7b1cdc2b40cdfaba3a
> >>>>> Link to GitHub:
> >>>>>
> >>>>
> >>>
> >>
> https://github.com/apache/geode/commit/91176d61df64bf1390cdba7b1cdc2b40cdfaba3a
> >>>>>
> >>>>> Thanks!
> >>>>> -michael
> >>>>
> >>>>
> >>>
> >>
>
>

Re: Propose including GEODE-7178 in 1.10

Posted by Anthony Baker <ab...@pivotal.io>.
My understanding is that this portion of the protocol is determined by instanceof checks, not the ordinal version.  The messages from the java client went through a different code path than messages from the native client.  So java clients using ordinal 45 still work (that’s why our backwards compatibility tests don’t fail).

Anthony


> On Sep 12, 2019, at 2:34 PM, Dan Smith <ds...@pivotal.io> wrote:
> 
> +1 for getting this in 1.10.
> 
> I am curious though - is the native client behaving like an older versions
> of the java client, or is this totally unique behavior for the native
> client? Is there some integration test that we are missing here?
> 
> -Dan
> 
> On Thu, Sep 12, 2019 at 11:52 AM Michael Oleske <mo...@pivotal.io> wrote:
> 
>> Here is the Pull Request for the cherry pick as requested
>> https://github.com/apache/geode/pull/4049
>> 
>> -michael
>> 
>> On Thu, Sep 12, 2019 at 10:28 AM Dick Cavender <dc...@pivotal.io>
>> wrote:
>> 
>>> Hi Michael, thank you for bringing your concern and fixing this issue.
>>> 
>>> Geode's release process dictates a time-based schedule <
>>> https://cwiki.apache.org/confluence/display/GEODE/Release+Schedule> to
>> cut
>>> release branches.  The “critical fixes” rule does allow critical fixes to
>>> be brought to the release branch by proposal on the dev list, as you have
>>> done here.
>>> 
>>> If there is consensus from the Geode community that your proposed change
>>> satisfies the “critical fixes” rule, I will be happy to bring it to the
>>> 1.10.0 release branch.
>>> 
>>> Due to the complexity of this change, could please open a PR against
>>> release/1.10.0 containing the exact changes you want to merge?
>>> 
>>> Regards
>>> 
>>> -Dick
>>> 
>>> 
>>> On Thu, Sep 12, 2019 at 10:23 AM Anthony Baker <ab...@pivotal.io>
>> wrote:
>>> 
>>>> +1 yes please!
>>>> 
>>>>> On Sep 12, 2019, at 10:11 AM, Michael Oleske <mo...@pivotal.io>
>>> wrote:
>>>>> 
>>>>> Hi Geode Devs!
>>>>> 
>>>>> I'd like to propose including the fix for GEODE-7178.  This resolves
>> an
>>>>> issue that Ivan (https://markmail.org/message/dwwac42xmpo4xb2e) ran
>>>> into in
>>>>> 1.10 RC1.
>>>>> 
>>>>> SHA: 91176d61df64bf1390cdba7b1cdc2b40cdfaba3a
>>>>> Link to GitHub:
>>>>> 
>>>> 
>>> 
>> https://github.com/apache/geode/commit/91176d61df64bf1390cdba7b1cdc2b40cdfaba3a
>>>>> 
>>>>> Thanks!
>>>>> -michael
>>>> 
>>>> 
>>> 
>> 


Re: Propose including GEODE-7178 in 1.10

Posted by Michael Oleske <mo...@pivotal.io>.
The native client does behave as an old Java client (ordinal 45).  I have
written a story (https://issues.apache.org/jira/browse/GEODE-7190) to have
Native Client updated.

-michael

On Thu, Sep 12, 2019 at 2:35 PM Dan Smith <ds...@pivotal.io> wrote:

> +1 for getting this in 1.10.
>
> I am curious though - is the native client behaving like an older versions
> of the java client, or is this totally unique behavior for the native
> client? Is there some integration test that we are missing here?
>
> -Dan
>
> On Thu, Sep 12, 2019 at 11:52 AM Michael Oleske <mo...@pivotal.io>
> wrote:
>
> > Here is the Pull Request for the cherry pick as requested
> > https://github.com/apache/geode/pull/4049
> >
> > -michael
> >
> > On Thu, Sep 12, 2019 at 10:28 AM Dick Cavender <dc...@pivotal.io>
> > wrote:
> >
> > > Hi Michael, thank you for bringing your concern and fixing this issue.
> > >
> > > Geode's release process dictates a time-based schedule <
> > > https://cwiki.apache.org/confluence/display/GEODE/Release+Schedule> to
> > cut
> > > release branches.  The “critical fixes” rule does allow critical fixes
> to
> > > be brought to the release branch by proposal on the dev list, as you
> have
> > > done here.
> > >
> > > If there is consensus from the Geode community that your proposed
> change
> > > satisfies the “critical fixes” rule, I will be happy to bring it to the
> > > 1.10.0 release branch.
> > >
> > > Due to the complexity of this change, could please open a PR against
> > > release/1.10.0 containing the exact changes you want to merge?
> > >
> > > Regards
> > >
> > > -Dick
> > >
> > >
> > > On Thu, Sep 12, 2019 at 10:23 AM Anthony Baker <ab...@pivotal.io>
> > wrote:
> > >
> > > > +1 yes please!
> > > >
> > > > > On Sep 12, 2019, at 10:11 AM, Michael Oleske <mo...@pivotal.io>
> > > wrote:
> > > > >
> > > > > Hi Geode Devs!
> > > > >
> > > > > I'd like to propose including the fix for GEODE-7178.  This
> resolves
> > an
> > > > > issue that Ivan (https://markmail.org/message/dwwac42xmpo4xb2e)
> ran
> > > > into in
> > > > > 1.10 RC1.
> > > > >
> > > > > SHA: 91176d61df64bf1390cdba7b1cdc2b40cdfaba3a
> > > > > Link to GitHub:
> > > > >
> > > >
> > >
> >
> https://github.com/apache/geode/commit/91176d61df64bf1390cdba7b1cdc2b40cdfaba3a
> > > > >
> > > > > Thanks!
> > > > > -michael
> > > >
> > > >
> > >
> >
>

Re: Propose including GEODE-7178 in 1.10

Posted by Dan Smith <ds...@pivotal.io>.
+1 for getting this in 1.10.

I am curious though - is the native client behaving like an older versions
of the java client, or is this totally unique behavior for the native
client? Is there some integration test that we are missing here?

-Dan

On Thu, Sep 12, 2019 at 11:52 AM Michael Oleske <mo...@pivotal.io> wrote:

> Here is the Pull Request for the cherry pick as requested
> https://github.com/apache/geode/pull/4049
>
> -michael
>
> On Thu, Sep 12, 2019 at 10:28 AM Dick Cavender <dc...@pivotal.io>
> wrote:
>
> > Hi Michael, thank you for bringing your concern and fixing this issue.
> >
> > Geode's release process dictates a time-based schedule <
> > https://cwiki.apache.org/confluence/display/GEODE/Release+Schedule> to
> cut
> > release branches.  The “critical fixes” rule does allow critical fixes to
> > be brought to the release branch by proposal on the dev list, as you have
> > done here.
> >
> > If there is consensus from the Geode community that your proposed change
> > satisfies the “critical fixes” rule, I will be happy to bring it to the
> > 1.10.0 release branch.
> >
> > Due to the complexity of this change, could please open a PR against
> > release/1.10.0 containing the exact changes you want to merge?
> >
> > Regards
> >
> > -Dick
> >
> >
> > On Thu, Sep 12, 2019 at 10:23 AM Anthony Baker <ab...@pivotal.io>
> wrote:
> >
> > > +1 yes please!
> > >
> > > > On Sep 12, 2019, at 10:11 AM, Michael Oleske <mo...@pivotal.io>
> > wrote:
> > > >
> > > > Hi Geode Devs!
> > > >
> > > > I'd like to propose including the fix for GEODE-7178.  This resolves
> an
> > > > issue that Ivan (https://markmail.org/message/dwwac42xmpo4xb2e) ran
> > > into in
> > > > 1.10 RC1.
> > > >
> > > > SHA: 91176d61df64bf1390cdba7b1cdc2b40cdfaba3a
> > > > Link to GitHub:
> > > >
> > >
> >
> https://github.com/apache/geode/commit/91176d61df64bf1390cdba7b1cdc2b40cdfaba3a
> > > >
> > > > Thanks!
> > > > -michael
> > >
> > >
> >
>

Re: Propose including GEODE-7178 in 1.10

Posted by Michael Oleske <mo...@pivotal.io>.
Here is the Pull Request for the cherry pick as requested
https://github.com/apache/geode/pull/4049

-michael

On Thu, Sep 12, 2019 at 10:28 AM Dick Cavender <dc...@pivotal.io> wrote:

> Hi Michael, thank you for bringing your concern and fixing this issue.
>
> Geode's release process dictates a time-based schedule <
> https://cwiki.apache.org/confluence/display/GEODE/Release+Schedule> to cut
> release branches.  The “critical fixes” rule does allow critical fixes to
> be brought to the release branch by proposal on the dev list, as you have
> done here.
>
> If there is consensus from the Geode community that your proposed change
> satisfies the “critical fixes” rule, I will be happy to bring it to the
> 1.10.0 release branch.
>
> Due to the complexity of this change, could please open a PR against
> release/1.10.0 containing the exact changes you want to merge?
>
> Regards
>
> -Dick
>
>
> On Thu, Sep 12, 2019 at 10:23 AM Anthony Baker <ab...@pivotal.io> wrote:
>
> > +1 yes please!
> >
> > > On Sep 12, 2019, at 10:11 AM, Michael Oleske <mo...@pivotal.io>
> wrote:
> > >
> > > Hi Geode Devs!
> > >
> > > I'd like to propose including the fix for GEODE-7178.  This resolves an
> > > issue that Ivan (https://markmail.org/message/dwwac42xmpo4xb2e) ran
> > into in
> > > 1.10 RC1.
> > >
> > > SHA: 91176d61df64bf1390cdba7b1cdc2b40cdfaba3a
> > > Link to GitHub:
> > >
> >
> https://github.com/apache/geode/commit/91176d61df64bf1390cdba7b1cdc2b40cdfaba3a
> > >
> > > Thanks!
> > > -michael
> >
> >
>

Re: Propose including GEODE-7178 in 1.10

Posted by Dick Cavender <dc...@pivotal.io>.
Hi Michael, thank you for bringing your concern and fixing this issue.

Geode's release process dictates a time-based schedule <
https://cwiki.apache.org/confluence/display/GEODE/Release+Schedule> to cut
release branches.  The “critical fixes” rule does allow critical fixes to
be brought to the release branch by proposal on the dev list, as you have
done here.

If there is consensus from the Geode community that your proposed change
satisfies the “critical fixes” rule, I will be happy to bring it to the
1.10.0 release branch.

Due to the complexity of this change, could please open a PR against
release/1.10.0 containing the exact changes you want to merge?

Regards

-Dick


On Thu, Sep 12, 2019 at 10:23 AM Anthony Baker <ab...@pivotal.io> wrote:

> +1 yes please!
>
> > On Sep 12, 2019, at 10:11 AM, Michael Oleske <mo...@pivotal.io> wrote:
> >
> > Hi Geode Devs!
> >
> > I'd like to propose including the fix for GEODE-7178.  This resolves an
> > issue that Ivan (https://markmail.org/message/dwwac42xmpo4xb2e) ran
> into in
> > 1.10 RC1.
> >
> > SHA: 91176d61df64bf1390cdba7b1cdc2b40cdfaba3a
> > Link to GitHub:
> >
> https://github.com/apache/geode/commit/91176d61df64bf1390cdba7b1cdc2b40cdfaba3a
> >
> > Thanks!
> > -michael
>
>

Re: Propose including GEODE-7178 in 1.10

Posted by Anthony Baker <ab...@pivotal.io>.
+1 yes please!

> On Sep 12, 2019, at 10:11 AM, Michael Oleske <mo...@pivotal.io> wrote:
> 
> Hi Geode Devs!
> 
> I'd like to propose including the fix for GEODE-7178.  This resolves an
> issue that Ivan (https://markmail.org/message/dwwac42xmpo4xb2e) ran into in
> 1.10 RC1.
> 
> SHA: 91176d61df64bf1390cdba7b1cdc2b40cdfaba3a
> Link to GitHub:
> https://github.com/apache/geode/commit/91176d61df64bf1390cdba7b1cdc2b40cdfaba3a
> 
> Thanks!
> -michael