+1 to "let's do both"
FWIW, our stance on semver is to "strive to follow it", not "must follow
it". I think when we have a concrete change we can look at, we could
make the call to say whether or not we feel comfortable bringing this
into maintenance release lines. The circumstances of this request
certainly feel "sufficient" to me to push a new facade into a bugfix
release (because it would increase later stability by getting Phoenix
off private API).
On 6/11/20 3:07 PM, Andrew Purtell wrote:
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
