Couple points: 1. I like Andrew's proposed solution, and we should do it, but I'm not sure it's sufficient for Rushabh's purposes because of semver rules. Phoenix supports HBase 1.3 -1.5 (soon to add 1.6) and HBase 2.0 (soon to gain 2.1 and 2.2, with 2.3 coming shortly after its release here.) If we add the new sizeHistogram and timeHistogram methods to hbase-metrics, they'll be available in Phoenix only in HBase 1.7 and 2.4. (since 2.3 is mostly-frozen)
Since Phoenix will be supporting earlier versions of both HBase branches for a good while, there will need to be a compatibility shim. And the older-version instance of the shim will probably need to access the classes directly. (Please correct me if I'm wrong, Rushabh or Andrew.) So it still might need a LimitedPrivate IA. 2. I agree with Nick that it's better to use LimitedPrivate.COPROC rather than LimitedPrivate.PHOENIX. Geoffrey On Thu, Jun 11, 2020 at 11:28 AM Josh Elser <els...@apache.org> wrote: > Sounds reasonable to me! > > On 6/11/20 1:06 PM, Andrew Purtell wrote: > > hbase-metrics-api is available for coprocessors already and interfaces > > within are already LimitedPrivate(COPROC). However, that package is > mostly > > interface and seems geared toward consuming metrics instantiated and > > registered via private stuff. Or, rather, I didn't see how Phoenix could > choose > > which of MutableSizeHistogram and MutableTimeHistogram to instantiate > using > > those interfaces, there is only Histogram MetricRegistry#histogram(String > > name). So I think it is also worth some time to review the utility of > > hbase-metrics-api and decide if more need be done there. Would the > addition > > of > > > > Histogram MetricRegistry#sizeHistogram(String name) > > Histogram MetricRegistry#timeHistogram(String name) > > > > achieve the desired objective instead? > > > > > > On Thu, Jun 11, 2020 at 9:16 AM Nick Dimiduk <ndimi...@apache.org> > wrote: > > > >> I was just about to reply with the same -- Josh is faster :) +1 on > >> considering the full surface area of the APIs being exposed. > >> > >> I also wonder if exposing the metrics infrastructure is something of > >> interest more broadly than Phoenix. Seems like any coprocessor might > want > >> to provide or monitor some metric value. > >> > >> On Thu, Jun 11, 2020 at 9:08 AM Josh Elser <els...@apache.org> wrote: > >> > >>> My only concern is that you can't just mark these two classes a > >>> LimitedPrivate for Phoenix -- you would also have to mark > >>> MutableRangeHistogram, MutableHistogram (and the rest of the class > >>> hierarchy) to make sure that we don't make it super confusing as to > what > >>> comes from LimitedPrivate classes and what is coming from Private > >> classes. > >>> > >>> Would it be better to just say: make > >>> ./hbase-hadoop2-compat/src/main/java/org/apache/hadoop/metrics2/lib > >>> LimitedPrivate? > >>> > >>> Do you also need the stuff in > >>> hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase to push > >>> metrics back through the HBase metrics subsystem? > >>> > >>> Sorry for the late reply. Just want to make sure we open up the > >>> audience, we open it sufficiently. > >>> > >>> On 6/8/20 1:15 PM, Rushabh Shah wrote: > >>>> Hi, > >>>> Currently the IA for MutableSizeHistogram and MutableTimeHistogram is > >>>> private. We want to use these classes in PHOENIX project and I thought > >> we > >>>> can leverage the existing implementation from hbase histo > >> implementation. > >>>> IIUC the private IA can't be used in other projects. Proposing to make > >> it > >>>> LimitedPrivate and mark HBaseInterfaceAudience.PHOENIX. Please > suggest. > >>>> Related jira: https://issues.apache.org/jira/browse/HBASE-24520 > >>>> > >>> > >> > > > > >
