> 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 > > > > > > > > > > > > > > >