I have reopened HBASE-22504, HBASE-21773, HBASE-23055, and HBASE-24102 for
addendums based on this thread. I also started a [DISCUSS] thread re:
VisibleForTesting annotation.
Thanks,
Nick
On Mon, Jun 22, 2020 at 9:22 AM Nick Dimiduk <ndimi...@apache.org> wrote:
> > On NettyRpcClientConfigHelper I think it is fine. It is designed to be
> an 'util' class, so in HBASE-23956 we made it final and added a private
> constructor. It only has static methods and is not expected to be extended
> or instantiated by end users.
>
> Very well, let's keep this change.
>
> > On ByteBufferUtils, it is IA.Private on master branch?
>
> It is IA.Public on 2.2.0, the point of reference.
>
> > On the replication related classes, all the constructors are marked as
> IA.Private, so I think they are all fine. Anyway, we should have a better
> design, maybe something like the ClusterMetrics, where we introduce an
> interface get the metrics.
>
> Ah indeed, the constructors are marked IA.Private. That's not very kind to
> our users, but I guess it works.
>
> > For the
> org.apache.hadoop.hbase.tool.LoadIncrementalHFiles.tryAtomicRegionLoad
> change added by HBASE-24221, the method is marked with @VisibleForTesting
> and javadoc says "Protected for testing", so maybe it's fine?
>
> To the best of my knowledge, we do not discuss the @VisibleForTesting
> annotation in our compatibility guidelines, thus I think it's a violation.
>
> > For "org.apache.hadoop.hbase.mapreduce.RowCounter.createSubmittableJob
> -- Method parameter removed via HBASE-21773" and
> "org.apache.hadoop.hbase.client.SnapshotDescription -- Additional
> constructor arguments added in HBASE-22648" I guess we could push amending
> PRs re-adding the methods with original signature as deprecated?
>
> Yes, sounds good for both of them.
>
> > adding property map argument in SnapshotDescription was my doing, let me
> open up Jira to bring back original signature as deprecated (since I am
> familiar with it). I can also help look into other changes if required.
>
> Thank you!
>
> > Moreover, reg test failure for ReplicationStatusSink, opened Jira
> HBASE-24594 to have it separate cluster pair setup and not share resources
> with TestReplicationStatus. This is committed, I will keep an eye on flaky
> and nightly builds.
>
> And thank you again :)
>
> On Sun, Jun 21, 2020 at 10:52 AM Viraj Jasani <vjas...@apache.org> wrote:
>
>> Nick,
>>
>> I just went through above methods and I agree with Duo and Wellington reg
>> @IA.Private, @VisibleForTesting methods and also the fact that we should
>> add original signature for IA.Public methods making them deprecated and
>> internally using new methods. e.g adding property map argument in
>> SnapshotDescription was my doing, let me open up Jira to bring back
>> original signature as deprecated (since I am familiar with it). I can also
>> help look into other changes if required.
>>
>> Moreover, reg test failure for ReplicationStatusSink, opened Jira
>> HBASE-24594 to have it separate cluster pair setup and not share resources
>> with TestReplicationStatus. This is committed, I will keep an eye on flaky
>> and nightly builds. I wish I had taken junit xml output when it failed,
>> apologies. However, I am glad that this is not reported flaky as such and
>> with separate resource allocation, this should go even smoother.
>>
>> Overall, I am hopeful that we should be able to take care of all relevant
>> source incompatibilities sooner.
>>
>>
>> On 2020/06/19 09:51:43, Wellington Chevreuil <
>> wellington.chevre...@gmail.com> wrote:
>> > I agree with Duo regarding the methods that were already marked as
>> > IA.private.
>> >
>> > For the
>> > org.apache.hadoop.hbase.tool.LoadIncrementalHFiles.tryAtomicRegionLoad
>> > change added by HBASE-24221, the method is marked with
>> @VisibleForTesting
>> > and javadoc says "Protected for testing", so maybe it's fine?
>> >
>> > For "org.apache.hadoop.hbase.mapreduce.RowCounter.createSubmittableJob
>> > --Method parameter removed via HBASE-21773" and
>> > "org.apache.hadoop.hbase.client.SnapshotDescription -- Additional
>> > constructor arguments added in HBASE-22648" I guess we could
>> > push amending PRs re-adding the methods with original signature as
>> > deprecated?
>> >
>> >
>> >
>> > Em sex., 19 de jun. de 2020 às 03:01, 张铎(Duo Zhang) <
>> palomino...@gmail.com>
>> > escreveu:
>> >
>> > > On NettyRpcClientConfigHelper I think it is fine. It is designed to
>> be an
>> > > 'util' class, so in HBASE-23956 we made it final and added a private
>> > > constructor. It only has static methods and is not expected to be
>> extended
>> > > or instantiated by end users.
>> > >
>> > > On ByteBufferUtils, it is IA.Private on master branch?
>> > >
>> > > On the replication related classes, all the constructors are marked as
>> > > IA.Private, so I think they are all fine. Anyway, we should have a
>> better
>> > > design, maybe something like the ClusterMetrics, where we introduce an
>> > > interface get the metrics.
>> > >
>> > >
>> > > Nick Dimiduk <ndimi...@apache.org> 于2020年6月19日周五 上午6:26写道：
>> > >
>> > > > I've done some accounting of the source-incompatible changes. I'm
>> not
>> > > > listing every item here, only the ones that I think might raise
>> eyebrows
>> > > or
>> > > > warrant further discussion. Here are my findings.
>> > > >
>> > > > I think these problems sink the RC. I plan to reopen the various
>> tickets
>> > > > here and start a discussion with the authors for getting an addendum
>> > > posted
>> > > > that addresses the problems. Before I do that, please speak up if
>> you
>> > > think
>> > > > anything here I've -1 are actually justified.
>> > > >
>> > > > Thanks,
>> > > > Nick
>> > > >
>> > > > Removed Methods:
>> > > > * org.apache.hadoop.hbase.rest.client.{RemoteAdmin,RemoteHTable} --
>> > > these
>> > > > were dropped intentionally and with discussion via HBASE-24115, +1.
>> > > > * org.apache.hadoop.hbase.util.ByteBufferUtils.findCommonPrefix
>> (...) --
>> > > > looks like this method was refactored away as part of HBASE-22504.
>> > > However,
>> > > > it is present in an IA.Public class, so it needs a deprecation cycle
>> > > before
>> > > > removal, -1.
>> > > > * org.apache.hadoop.hbase.HRegionInfo.compareTo -- the Comparable
>> > > > implementation was moved up to the interface via default method, so
>> I
>> > > think
>> > > > this is acceptable, via HBASE-23753. +1.
>> > > > * org.apache.hadoop.hbase.replication.ReplicationLoadSink --
>> Additional
>> > > > constructor arguments added in HBASE-21406, -1.
>> > > > * org.apache.hadoop.hbase.replication.ReplicationLoadSource --
>> > > Additional
>> > > > constructor arguments added in HBASE-21505, -1.
>> > > > * org.apache.hadoop.hbase.client.SnapshotDescription -- Additional
>> > > > constructor arguments added in HBASE-22648, -1.
>> > > > *
>> > > >
>> > >
>> org.apache.hadoop.hbase.tool.LoadIncrementalHFiles.tryAtomicRegionLoad(...)
>> > > > -- Method signature change on a protected method via HBASE-24221,
>> -1.
>> > > > *
>> org.apache.hadoop.hbase.mapreduce.RowCounter.createSubmittableJob --
>> > > > Method parameter removed via HBASE-21773, -1.
>> > > >
>> > > > Problems with Data Types, High Severity:
>> > > > * org.apache.hadoop.hbase.HConstants#HBASE_NON_USER_TABLE_DIRS --
>> Field
>> > > > removed via HBASE-23055, -1.
>> > > > * org.apache.hadoop.hbase.HRegionInfo Removed super-interface
>> > > > java.lang.Comparable<HRegionInfo> -- same as HBASE-23753 above.
>> Missing
>> > > > interface moved to the super-interface, preserved. +1.
>> > > > * A number of interfaces seeing new methods. These interfaces look
>> like
>> > > > API we expect a client to consume, not implement. Book says under
>> Client
>> > > > API compatibility: "New APIs introduced in a patch version will
>> only be
>> > > > added in a source compatible way [1]: i.e. code that implements
>> public
>> > > APIs
>> > > > will continue to compile." Strictly speaking, I think these are
>> breaking
>> > > > changes for a minor release, but since this is an interface we don't
>> > > expect
>> > > > clients to implement, only to consume, I'm -0.
>> > > > * org.apache.hadoop.hbase.ipc.NettyRpcClientConfigHelper -- class
>> became
>> > > > final, via HBASE-23956. We don't want clients (or anyone) to extend
>> this
>> > > > class. +1.
>> > > > * org.apache.hadoop.hbase.replication.ReplicationLoadSource --
>> class
>> > > > became final, via HBASE-21505. We don't want clients (or anyone) to
>> > > extend
>> > > > this class. +1.
>> > > > * org.apache.hadoop.hbase.mapreduce.RowCounter -- removed interface
>> > > Tool,
>> > > > via HBASE-21773. Now extends AbstractHBaseTool, which does implement
>> > > > Tool. +1.
>> > > > * org.apache.hadoop.hbase.util.RegionMover -- 6 public fields made
>> > > > private, via HBASE-24102. -1.
>> > > >
>> > > > Problems with Methods, High Severity:
>> > > > * org.apache.hadoop.hbase.ipc.NettyRpcClientConfigHelper -- default
>> > > > constructor made private, via HBASE-23956. -1.
>> > > >
>> > > > Problems with Methods, Medium Severity:
>> > > > *
>> org.apache.hadoop.hbase.rest.filter.RestCsrfPreventionFilter.init(...)
>> > > > -- removed declared thrown exception, via HBASE-23661. Clients don't
>> > > extend
>> > > > this class, +1.
>> > > > *
>> org.apache.hadoop.hbase.client.RegionInfo.isEncodedRegionName(...) --
>> > > > removed thrown exception, via HBASE-23326. Clients don't extend this
>> > > > class, +1.
>> > > > *
>> > > >
>> > > >
>> > > > On Tue, Jun 16, 2020 at 9:36 AM Nick Dimiduk <ndimi...@apache.org>
>> > > wrote:
>> > > >
>> > > > > Please vote on this Apache hbase release candidate,
>> > > > > hbase-2.3.0RC0
>> > > > >
>> > > > > The VOTE will remain open for at least 72 hours.
>> > > > >
>> > > > > [ ] +1 Release this package as Apache hbase 2.3.0
>> > > > > [ ] -1 Do not release this package because ...
>> > > > >
>> > > > > The tag to be voted on is 2.3.0RC0:
>> > > > >
>> > > > > https://github.com/apache/hbase/tree/2.3.0RC0
>> > > > >
>> > > > > The release files, including signatures, digests, as well as
>> CHANGES.md
>> > > > > and RELEASENOTES.md included in this RC can be found at:
>> > > > >
>> > > > > https://dist.apache.org/repos/dist/dev/hbase/2.3.0RC0/
>> > > > >
>> > > > > Maven artifacts are available in a staging repository at:
>> > > > >
>> > > > >
>> > > >
>> https://repository.apache.org/content/repositories/orgapachehbase-1393/
>> > > > >
>> > > > > Artifacts were signed with the ndimi...@apache.org key which can
>> be
>> > > > found
>> > > > > in:
>> > > > >
>> > > > > https://dist.apache.org/repos/dist/release/hbase/KEYS
>> > > > >
>> > > > > To learn more about Apache hbase, please see
>> > > > >
>> > > > > http://hbase.apache.org/
>> > > > >
>> > > > > Thanks,
>> > > > > Your HBase Release Manager
>> > > > >
>> > > >
>> > >
>> >
>>
>
