You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@hbase.apache.org by "张铎 (Duo Zhang)" <pa...@gmail.com> on 2020/11/29 04:07:02 UTC

[DISCUSS] The breaking changes in HBASE-24966

In HBASE-24966, we found that in AsyncTableRegionLocator, we accidentally
declared 3 methods

getStartKeys
getEndKeys
getStartEndKeys

to throw IOException directly.

This should be a copy paste mistake, as typically, for a method which
returns CompletableFuture, the exception should be returned through the
CompletableFuture, and this is exactly the behavior of these methods.

So the actual problem is only that we have a wrong method signature. but
since this interface is IA.Public, and it has already been included in
several releases, according to our compatibility rule, we can not just
remove the throws part from the method. Instead, we need to deprecate them
and create new methods. But there will be another problem that we want to
align the method names between the sync and async client, so if we change
the names of the methods, we'd better also change the name of methods for
sync client, which will make our users do more unnecessary work.

So here I want to discuss that, since we all know that, this is a mistake,
and the methods will never throw IOException directly, is it OK for us to
just remove the throws part and tell users directly that this is a mistake,
and release it in the next minor release or a major release as an
incompatible change?

Thanks.

Re: [DISCUSS] The breaking changes in HBASE-24966

Posted by Nick Dimiduk <nd...@apache.org>.
On Tue, Dec 1, 2020 at 6:45 AM 张铎(Duo Zhang) <pa...@gmail.com> wrote:
>
> If no other objections tomorrow, will follow what Sean proposed above, to
> include the patch in 3.0.0 and mark the issue as incompatible change.

Belated +1.

> 张铎(Duo Zhang) <pa...@gmail.com> 于2020年11月30日周一 上午11:04写道:
>
> > I think it could introduce compilation error when removing the throws part
> > of a method signature. As if there is no exception thrown but you have a
> > 'try...catch' then there will be a compilation error...
> >
> > It requires a code change sometimes but anyway, you just need to remove
> > the 'try...catch', no other big impacts. So for me I would also like to
> > mark it as incomplete and change it directly in 3.0.0.
> >
> > Thanks.
> >
> > Sean Busbey <bu...@apache.org> 于2020年11月30日周一 上午2:51写道:
> >
> >> I think we should change what they throw directly and label it
> >> incompatible. I think this is in line with our previous expectation
> >> setting
> >> about how we'll handle mistakes in the API.
> >>
> >> That change would be source incompatible but would still be binary
> >> compatible.
> >>
> >> I think we should do it in a major release. esp since there's not a way in
> >> Java to say "this deprecation is just about the thrown exceptions" and it
> >> will be awkward to write code that is source compatible with the existing
> >> api and with the exception removed.
> >>
> >> On Sat, Nov 28, 2020, 22:07 张铎(Duo Zhang) <pa...@gmail.com> wrote:
> >>
> >> > In HBASE-24966, we found that in AsyncTableRegionLocator, we
> >> accidentally
> >> > declared 3 methods
> >> >
> >> > getStartKeys
> >> > getEndKeys
> >> > getStartEndKeys
> >> >
> >> > to throw IOException directly.
> >> >
> >> > This should be a copy paste mistake, as typically, for a method which
> >> > returns CompletableFuture, the exception should be returned through the
> >> > CompletableFuture, and this is exactly the behavior of these methods.
> >> >
> >> > So the actual problem is only that we have a wrong method signature. but
> >> > since this interface is IA.Public, and it has already been included in
> >> > several releases, according to our compatibility rule, we can not just
> >> > remove the throws part from the method. Instead, we need to deprecate
> >> them
> >> > and create new methods. But there will be another problem that we want
> >> to
> >> > align the method names between the sync and async client, so if we
> >> change
> >> > the names of the methods, we'd better also change the name of methods
> >> for
> >> > sync client, which will make our users do more unnecessary work.
> >> >
> >> > So here I want to discuss that, since we all know that, this is a
> >> mistake,
> >> > and the methods will never throw IOException directly, is it OK for us
> >> to
> >> > just remove the throws part and tell users directly that this is a
> >> mistake,
> >> > and release it in the next minor release or a major release as an
> >> > incompatible change?
> >> >
> >> > Thanks.
> >> >
> >>
> >

Re: [DISCUSS] The breaking changes in HBASE-24966

Posted by Nick Dimiduk <nd...@apache.org>.
On Tue, Dec 1, 2020 at 6:45 AM 张铎(Duo Zhang) <pa...@gmail.com> wrote:
>
> If no other objections tomorrow, will follow what Sean proposed above, to
> include the patch in 3.0.0 and mark the issue as incompatible change.

Belated +1.

> 张铎(Duo Zhang) <pa...@gmail.com> 于2020年11月30日周一 上午11:04写道:
>
> > I think it could introduce compilation error when removing the throws part
> > of a method signature. As if there is no exception thrown but you have a
> > 'try...catch' then there will be a compilation error...
> >
> > It requires a code change sometimes but anyway, you just need to remove
> > the 'try...catch', no other big impacts. So for me I would also like to
> > mark it as incomplete and change it directly in 3.0.0.
> >
> > Thanks.
> >
> > Sean Busbey <bu...@apache.org> 于2020年11月30日周一 上午2:51写道:
> >
> >> I think we should change what they throw directly and label it
> >> incompatible. I think this is in line with our previous expectation
> >> setting
> >> about how we'll handle mistakes in the API.
> >>
> >> That change would be source incompatible but would still be binary
> >> compatible.
> >>
> >> I think we should do it in a major release. esp since there's not a way in
> >> Java to say "this deprecation is just about the thrown exceptions" and it
> >> will be awkward to write code that is source compatible with the existing
> >> api and with the exception removed.
> >>
> >> On Sat, Nov 28, 2020, 22:07 张铎(Duo Zhang) <pa...@gmail.com> wrote:
> >>
> >> > In HBASE-24966, we found that in AsyncTableRegionLocator, we
> >> accidentally
> >> > declared 3 methods
> >> >
> >> > getStartKeys
> >> > getEndKeys
> >> > getStartEndKeys
> >> >
> >> > to throw IOException directly.
> >> >
> >> > This should be a copy paste mistake, as typically, for a method which
> >> > returns CompletableFuture, the exception should be returned through the
> >> > CompletableFuture, and this is exactly the behavior of these methods.
> >> >
> >> > So the actual problem is only that we have a wrong method signature. but
> >> > since this interface is IA.Public, and it has already been included in
> >> > several releases, according to our compatibility rule, we can not just
> >> > remove the throws part from the method. Instead, we need to deprecate
> >> them
> >> > and create new methods. But there will be another problem that we want
> >> to
> >> > align the method names between the sync and async client, so if we
> >> change
> >> > the names of the methods, we'd better also change the name of methods
> >> for
> >> > sync client, which will make our users do more unnecessary work.
> >> >
> >> > So here I want to discuss that, since we all know that, this is a
> >> mistake,
> >> > and the methods will never throw IOException directly, is it OK for us
> >> to
> >> > just remove the throws part and tell users directly that this is a
> >> mistake,
> >> > and release it in the next minor release or a major release as an
> >> > incompatible change?
> >> >
> >> > Thanks.
> >> >
> >>
> >

Re: [DISCUSS] The breaking changes in HBASE-24966

Posted by "张铎 (Duo Zhang)" <pa...@gmail.com>.
If no other objections tomorrow, will follow what Sean proposed above, to
include the patch in 3.0.0 and mark the issue as incompatible change.

Thanks.

张铎(Duo Zhang) <pa...@gmail.com> 于2020年11月30日周一 上午11:04写道:

> I think it could introduce compilation error when removing the throws part
> of a method signature. As if there is no exception thrown but you have a
> 'try...catch' then there will be a compilation error...
>
> It requires a code change sometimes but anyway, you just need to remove
> the 'try...catch', no other big impacts. So for me I would also like to
> mark it as incomplete and change it directly in 3.0.0.
>
> Thanks.
>
> Sean Busbey <bu...@apache.org> 于2020年11月30日周一 上午2:51写道:
>
>> I think we should change what they throw directly and label it
>> incompatible. I think this is in line with our previous expectation
>> setting
>> about how we'll handle mistakes in the API.
>>
>> That change would be source incompatible but would still be binary
>> compatible.
>>
>> I think we should do it in a major release. esp since there's not a way in
>> Java to say "this deprecation is just about the thrown exceptions" and it
>> will be awkward to write code that is source compatible with the existing
>> api and with the exception removed.
>>
>> On Sat, Nov 28, 2020, 22:07 张铎(Duo Zhang) <pa...@gmail.com> wrote:
>>
>> > In HBASE-24966, we found that in AsyncTableRegionLocator, we
>> accidentally
>> > declared 3 methods
>> >
>> > getStartKeys
>> > getEndKeys
>> > getStartEndKeys
>> >
>> > to throw IOException directly.
>> >
>> > This should be a copy paste mistake, as typically, for a method which
>> > returns CompletableFuture, the exception should be returned through the
>> > CompletableFuture, and this is exactly the behavior of these methods.
>> >
>> > So the actual problem is only that we have a wrong method signature. but
>> > since this interface is IA.Public, and it has already been included in
>> > several releases, according to our compatibility rule, we can not just
>> > remove the throws part from the method. Instead, we need to deprecate
>> them
>> > and create new methods. But there will be another problem that we want
>> to
>> > align the method names between the sync and async client, so if we
>> change
>> > the names of the methods, we'd better also change the name of methods
>> for
>> > sync client, which will make our users do more unnecessary work.
>> >
>> > So here I want to discuss that, since we all know that, this is a
>> mistake,
>> > and the methods will never throw IOException directly, is it OK for us
>> to
>> > just remove the throws part and tell users directly that this is a
>> mistake,
>> > and release it in the next minor release or a major release as an
>> > incompatible change?
>> >
>> > Thanks.
>> >
>>
>

Re: [DISCUSS] The breaking changes in HBASE-24966

Posted by "张铎 (Duo Zhang)" <pa...@gmail.com>.
If no other objections tomorrow, will follow what Sean proposed above, to
include the patch in 3.0.0 and mark the issue as incompatible change.

Thanks.

张铎(Duo Zhang) <pa...@gmail.com> 于2020年11月30日周一 上午11:04写道:

> I think it could introduce compilation error when removing the throws part
> of a method signature. As if there is no exception thrown but you have a
> 'try...catch' then there will be a compilation error...
>
> It requires a code change sometimes but anyway, you just need to remove
> the 'try...catch', no other big impacts. So for me I would also like to
> mark it as incomplete and change it directly in 3.0.0.
>
> Thanks.
>
> Sean Busbey <bu...@apache.org> 于2020年11月30日周一 上午2:51写道:
>
>> I think we should change what they throw directly and label it
>> incompatible. I think this is in line with our previous expectation
>> setting
>> about how we'll handle mistakes in the API.
>>
>> That change would be source incompatible but would still be binary
>> compatible.
>>
>> I think we should do it in a major release. esp since there's not a way in
>> Java to say "this deprecation is just about the thrown exceptions" and it
>> will be awkward to write code that is source compatible with the existing
>> api and with the exception removed.
>>
>> On Sat, Nov 28, 2020, 22:07 张铎(Duo Zhang) <pa...@gmail.com> wrote:
>>
>> > In HBASE-24966, we found that in AsyncTableRegionLocator, we
>> accidentally
>> > declared 3 methods
>> >
>> > getStartKeys
>> > getEndKeys
>> > getStartEndKeys
>> >
>> > to throw IOException directly.
>> >
>> > This should be a copy paste mistake, as typically, for a method which
>> > returns CompletableFuture, the exception should be returned through the
>> > CompletableFuture, and this is exactly the behavior of these methods.
>> >
>> > So the actual problem is only that we have a wrong method signature. but
>> > since this interface is IA.Public, and it has already been included in
>> > several releases, according to our compatibility rule, we can not just
>> > remove the throws part from the method. Instead, we need to deprecate
>> them
>> > and create new methods. But there will be another problem that we want
>> to
>> > align the method names between the sync and async client, so if we
>> change
>> > the names of the methods, we'd better also change the name of methods
>> for
>> > sync client, which will make our users do more unnecessary work.
>> >
>> > So here I want to discuss that, since we all know that, this is a
>> mistake,
>> > and the methods will never throw IOException directly, is it OK for us
>> to
>> > just remove the throws part and tell users directly that this is a
>> mistake,
>> > and release it in the next minor release or a major release as an
>> > incompatible change?
>> >
>> > Thanks.
>> >
>>
>

Re: [DISCUSS] The breaking changes in HBASE-24966

Posted by "张铎 (Duo Zhang)" <pa...@gmail.com>.
I think it could introduce compilation error when removing the throws part
of a method signature. As if there is no exception thrown but you have a
'try...catch' then there will be a compilation error...

It requires a code change sometimes but anyway, you just need to remove the
'try...catch', no other big impacts. So for me I would also like to mark it
as incomplete and change it directly in 3.0.0.

Thanks.

Sean Busbey <bu...@apache.org> 于2020年11月30日周一 上午2:51写道:

> I think we should change what they throw directly and label it
> incompatible. I think this is in line with our previous expectation setting
> about how we'll handle mistakes in the API.
>
> That change would be source incompatible but would still be binary
> compatible.
>
> I think we should do it in a major release. esp since there's not a way in
> Java to say "this deprecation is just about the thrown exceptions" and it
> will be awkward to write code that is source compatible with the existing
> api and with the exception removed.
>
> On Sat, Nov 28, 2020, 22:07 张铎(Duo Zhang) <pa...@gmail.com> wrote:
>
> > In HBASE-24966, we found that in AsyncTableRegionLocator, we accidentally
> > declared 3 methods
> >
> > getStartKeys
> > getEndKeys
> > getStartEndKeys
> >
> > to throw IOException directly.
> >
> > This should be a copy paste mistake, as typically, for a method which
> > returns CompletableFuture, the exception should be returned through the
> > CompletableFuture, and this is exactly the behavior of these methods.
> >
> > So the actual problem is only that we have a wrong method signature. but
> > since this interface is IA.Public, and it has already been included in
> > several releases, according to our compatibility rule, we can not just
> > remove the throws part from the method. Instead, we need to deprecate
> them
> > and create new methods. But there will be another problem that we want to
> > align the method names between the sync and async client, so if we change
> > the names of the methods, we'd better also change the name of methods for
> > sync client, which will make our users do more unnecessary work.
> >
> > So here I want to discuss that, since we all know that, this is a
> mistake,
> > and the methods will never throw IOException directly, is it OK for us to
> > just remove the throws part and tell users directly that this is a
> mistake,
> > and release it in the next minor release or a major release as an
> > incompatible change?
> >
> > Thanks.
> >
>

Re: [DISCUSS] The breaking changes in HBASE-24966

Posted by "张铎 (Duo Zhang)" <pa...@gmail.com>.
I think it could introduce compilation error when removing the throws part
of a method signature. As if there is no exception thrown but you have a
'try...catch' then there will be a compilation error...

It requires a code change sometimes but anyway, you just need to remove the
'try...catch', no other big impacts. So for me I would also like to mark it
as incomplete and change it directly in 3.0.0.

Thanks.

Sean Busbey <bu...@apache.org> 于2020年11月30日周一 上午2:51写道:

> I think we should change what they throw directly and label it
> incompatible. I think this is in line with our previous expectation setting
> about how we'll handle mistakes in the API.
>
> That change would be source incompatible but would still be binary
> compatible.
>
> I think we should do it in a major release. esp since there's not a way in
> Java to say "this deprecation is just about the thrown exceptions" and it
> will be awkward to write code that is source compatible with the existing
> api and with the exception removed.
>
> On Sat, Nov 28, 2020, 22:07 张铎(Duo Zhang) <pa...@gmail.com> wrote:
>
> > In HBASE-24966, we found that in AsyncTableRegionLocator, we accidentally
> > declared 3 methods
> >
> > getStartKeys
> > getEndKeys
> > getStartEndKeys
> >
> > to throw IOException directly.
> >
> > This should be a copy paste mistake, as typically, for a method which
> > returns CompletableFuture, the exception should be returned through the
> > CompletableFuture, and this is exactly the behavior of these methods.
> >
> > So the actual problem is only that we have a wrong method signature. but
> > since this interface is IA.Public, and it has already been included in
> > several releases, according to our compatibility rule, we can not just
> > remove the throws part from the method. Instead, we need to deprecate
> them
> > and create new methods. But there will be another problem that we want to
> > align the method names between the sync and async client, so if we change
> > the names of the methods, we'd better also change the name of methods for
> > sync client, which will make our users do more unnecessary work.
> >
> > So here I want to discuss that, since we all know that, this is a
> mistake,
> > and the methods will never throw IOException directly, is it OK for us to
> > just remove the throws part and tell users directly that this is a
> mistake,
> > and release it in the next minor release or a major release as an
> > incompatible change?
> >
> > Thanks.
> >
>

Re: [DISCUSS] The breaking changes in HBASE-24966

Posted by Sean Busbey <bu...@apache.org>.
I think we should change what they throw directly and label it
incompatible. I think this is in line with our previous expectation setting
about how we'll handle mistakes in the API.

That change would be source incompatible but would still be binary
compatible.

I think we should do it in a major release. esp since there's not a way in
Java to say "this deprecation is just about the thrown exceptions" and it
will be awkward to write code that is source compatible with the existing
api and with the exception removed.

On Sat, Nov 28, 2020, 22:07 张铎(Duo Zhang) <pa...@gmail.com> wrote:

> In HBASE-24966, we found that in AsyncTableRegionLocator, we accidentally
> declared 3 methods
>
> getStartKeys
> getEndKeys
> getStartEndKeys
>
> to throw IOException directly.
>
> This should be a copy paste mistake, as typically, for a method which
> returns CompletableFuture, the exception should be returned through the
> CompletableFuture, and this is exactly the behavior of these methods.
>
> So the actual problem is only that we have a wrong method signature. but
> since this interface is IA.Public, and it has already been included in
> several releases, according to our compatibility rule, we can not just
> remove the throws part from the method. Instead, we need to deprecate them
> and create new methods. But there will be another problem that we want to
> align the method names between the sync and async client, so if we change
> the names of the methods, we'd better also change the name of methods for
> sync client, which will make our users do more unnecessary work.
>
> So here I want to discuss that, since we all know that, this is a mistake,
> and the methods will never throw IOException directly, is it OK for us to
> just remove the throws part and tell users directly that this is a mistake,
> and release it in the next minor release or a major release as an
> incompatible change?
>
> Thanks.
>

Re: [DISCUSS] The breaking changes in HBASE-24966

Posted by Sean Busbey <bu...@apache.org>.
I think we should change what they throw directly and label it
incompatible. I think this is in line with our previous expectation setting
about how we'll handle mistakes in the API.

That change would be source incompatible but would still be binary
compatible.

I think we should do it in a major release. esp since there's not a way in
Java to say "this deprecation is just about the thrown exceptions" and it
will be awkward to write code that is source compatible with the existing
api and with the exception removed.

On Sat, Nov 28, 2020, 22:07 张铎(Duo Zhang) <pa...@gmail.com> wrote:

> In HBASE-24966, we found that in AsyncTableRegionLocator, we accidentally
> declared 3 methods
>
> getStartKeys
> getEndKeys
> getStartEndKeys
>
> to throw IOException directly.
>
> This should be a copy paste mistake, as typically, for a method which
> returns CompletableFuture, the exception should be returned through the
> CompletableFuture, and this is exactly the behavior of these methods.
>
> So the actual problem is only that we have a wrong method signature. but
> since this interface is IA.Public, and it has already been included in
> several releases, according to our compatibility rule, we can not just
> remove the throws part from the method. Instead, we need to deprecate them
> and create new methods. But there will be another problem that we want to
> align the method names between the sync and async client, so if we change
> the names of the methods, we'd better also change the name of methods for
> sync client, which will make our users do more unnecessary work.
>
> So here I want to discuss that, since we all know that, this is a mistake,
> and the methods will never throw IOException directly, is it OK for us to
> just remove the throws part and tell users directly that this is a mistake,
> and release it in the next minor release or a major release as an
> incompatible change?
>
> Thanks.
>