accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Josh Elser <josh.el...@gmail.com>
Subject Re: [DISCUSS] Thrift 0.9.1 in Accumulo 1.6.1
Date Wed, 25 Jun 2014 19:30:28 GMT
Thank you for digging into this, Christopher!

On 6/25/14, 3:26 PM, Christopher wrote:
> For completeness here, and to wrap up this thread, the issue was identified
> as caused by THRIFT-1805, which regressed (a second time) an issue which
> suppressed TApplicationExceptions and simply caused the connection to be
> closed, resulting in what looked like a network issue (TTransportException)
> in the client.
>
> ACCUMULO-2950 was opened to deal with this, and the patch is being tested,
> and is expected to restore the original behavior in 0.9.0. It does fix the
> issue identified in ACCUMULO-2935, but I made it a separate issue, so it
> was clearly documenting the underlying problem, separately from the symptom.
>
> Further, I reviewed the entire changeset from 0.9.0 to 0.9.1 in
> libthrift.jar and I didn't see any other changes that should cause any
> trouble for us.
>
>
> --
> Christopher L Tubbs II
> http://gravatar.com/ctubbsii
>
>
> On Sun, Jun 22, 2014 at 5:41 PM, Josh Elser <josh.elser@gmail.com> wrote:
>
>> Since there was a discussion thread on this, I wanted to bring this up on
>> the list instead of just leaving a comment on the issue.
>>
>> I just bisect'ed a failing MetaSplitIT[1] and found that the changes
>> introduced by bumping to 0.9.1 were what supposedly introduced them. The
>> thing that worries me is that putting this in 1.6.1 came with a "it's
>> compatible" guarantee, when, to be perfectly honest, it's obviously not at
>> the level of compatibility that we want it to be for a bug-fix release (it
>> broke a test).
>>
>> Now, I haven't traced it back far enough to see what has exactly changed
>> with the exceptions (causing us to poll indefinitely instead of fail back
>> to the client), but that makes me worry that assumptions we have in the
>> implementation of our API, WRT exception handling, are suddenly invalid.
>>
>> I'll try to poke around at this some more to figure it out exactly.
>>
>> [1] https://issues.apache.org/jira/browse/ACCUMULO-2935
>>
>>
>> On 5/23/14, 3:09 PM, Christopher wrote:
>>
>>> Given this conversation, and because I can't really think of a good
>>> reason not to, I'm going to proceed with applying this to 1.6.1, after
>>> addressing the issues in RB. That satisfies my desire to make a 1.7.0
>>> minor release down the road, and satisfies busbey's concerns about
>>> mixing versioning semantics in 1.x.x (by avoiding 1.x.0 minor
>>> releases)
>>>
>>> --
>>> Christopher L Tubbs II
>>> http://gravatar.com/ctubbsii
>>>
>>>
>>> On Fri, May 16, 2014 at 4:36 PM, Christopher <ctubbsii@apache.org> wrote:
>>>
>>>> Yes. They are 100% forward/backwards compatible on the wire.
>>>>
>>>> However, it is my understanding that there were some minor additions
>>>> (new API) to 0.9.1 which won't work in 0.9.0... but that won't affect
>>>> us since we are not using those features (and wouldn't be adding
>>>> anything that leverages those features in any bugfixes on 1.6.x), and
>>>> because we provide the jar.
>>>>
>>>> --
>>>> Christopher L Tubbs II
>>>> http://gravatar.com/ctubbsii
>>>>
>>>>
>>>> On Fri, May 16, 2014 at 2:26 PM, Sean Busbey <busbey@cloudera.com>
>>>> wrote:
>>>>
>>>>> Do we know if thirft 0.9.0 and 0.9.1 are forward compatible?
>>>>>
>>>>>
>>>>> On Fri, May 16, 2014 at 12:59 PM, Christopher <ctubbsii@apache.org>
>>>>> wrote:
>>>>>
>>>>>   Correction: the current patch does *NOT* bump the wire version... I
>>>>>> thought I did that, but I did not.
>>>>>>
>>>>>> --
>>>>>> Christopher L Tubbs II
>>>>>> http://gravatar.com/ctubbsii
>>>>>>
>>>>>>
>>>>>> On Fri, May 16, 2014 at 12:28 PM, Christopher <ctubbsii@apache.org>
>>>>>> wrote:
>>>>>>
>>>>>>> Devs,
>>>>>>>
>>>>>>> I'm considering whether or not it'd be appropriate to push in
>>>>>>> ACCUMULO-1691 into the 1.6.1-SNAPSHOT branch.
>>>>>>> This would effectively bump our dependency on libthrift to 0.9.1.
>>>>>>> However, thrift 0.9.1 and 0.9.0 are 100% wire-compatible (I've
been
>>>>>>> assured by jfarrell and codesf in the #thrift IRC channel).
>>>>>>>
>>>>>>> Given that this we provide this dependency, and the bump would
fix
>>>>>>> some thrift bugs, and that Thrift's own API is backwards-compatible
in
>>>>>>> this version, I don't think this would impact our community except
in
>>>>>>> the positive.
>>>>>>>
>>>>>>> (Note: currently, my patch for ACCUMULO-1691 bumps up the wire
>>>>>>> version, but I plan on changing that so it doesn't, now that
I've been
>>>>>>> assured it is compatible... I've also done some manual tests
to verify
>>>>>>> this, and haven't seen any issues across our tests, even without
>>>>>>> re-generating the thrift classes in ACCUMULO-2773; If I roll
>>>>>>> ACCUMULO-1691 in, I'd also include ACCUMULO-2773.)
>>>>>>>
>>>>>>> --
>>>>>>> Christopher L Tubbs II
>>>>>>> http://gravatar.com/ctubbsii
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Sean
>>>>>
>>>>
>

Mime
View raw message