On Wed, Jun 24, 2020 at 8:11 AM Wellington Chevreuil <
wellington.chevre...@gmail.com> wrote:
> I guess following the [DISCUSS] thread, we also need to bring back original
> signature for
> "org.apache.hadoop.hbase.tool.LoadIncrementalHFiles.tryAtomicRegionLoad"?
>
That is what I have proposed, yes.
Em seg., 22 de jun. de 2020 às 19:22, Wellington Chevreuil <
> wellington.chevre...@gmail.com> 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 <ndimi...@apache.org>
> > 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 <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
> >> >> > > > >
> >> >> > > >
> >> >> > >
> >> >> >
> >> >>
> >> >
> >>
> >
>
