That's unfortunate, but needs must, IMHO. A potential benefit of also marking the impls LP(COPROC) is this captures any implicit dependency on semantics and functionality of the implementation classes not directly exposed in the hbase-metrics-api facade.
So, let's do both? (Facade improvement, raise to LP the impl classes) On Thu, Jun 11, 2020 at 12:00 PM Geoffrey Jacoby <gjac...@apache.org> wrote: > 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 > > >>>> > > >>> > > >> > > > > > > > > > -- Best regards, Andrew Words like orphans lost among the crosstalk, meaning torn from truth's decrepit hands - A23, Crosstalk
