I guess following the [DISCUSS] thread, we also need to bring back original signature for "org.apache.hadoop.hbase.tool.LoadIncrementalHFiles.tryAtomicRegionLoad"?
Em seg., 22 de jun. de 2020 às 19:22, Wellington Chevreuil < [email protected]> escreveu: > Had submitted an addendum PR for HBASE-21773. For HBASE-24221, we may try > the same. > > Em seg., 22 de jun. de 2020 às 17:45, Nick Dimiduk <[email protected]> > escreveu: > >> 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 <[email protected]> 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 <[email protected]> >> 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 < >> >> [email protected]> 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) < >> >> [email protected]> >> >> > 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 <[email protected]> 于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 < >> [email protected]> >> >> > > 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 [email protected] 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 >> >> > > > > >> >> > > > >> >> > > >> >> > >> >> >> > >> >
