hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Apekshit Sharma <a...@cloudera.com>
Subject Re: [DISCUSS] Merge hbase-metrics:oahh.metrics.impl into hbase-metrics-api
Date Thu, 26 Oct 2017 17:55:48 GMT
bq. .....integration with metrics-library1 to support the metrics platform
for their $business1. $business2 might want to use metrics-library2....
Makes sense Josh.

bq.  One final concern, bundling our "default" DropwizardMetrics 3-based
implementation could cause headaches for users who want to build their own
metrics-api implementation.
Right. Do you have something in mind to avoid this?

Thanks Josh!

So the only remaining thing is, we should either
- remove this (
https://github.com/apache/hbase/blob/master/hbase-metrics-api/src/main/java/org/apache/hadoop/hbase/metrics/MetricRegistriesLoader.java#L67)
which seems unnecessary because Service Provider will pick up
"MetricRegistriesImpl" if it's there.
- Or move MetricRegistriesImpl in the same module as 'default'
implementation. (until we have multiple implementation).

-- Appy


On Wed, Oct 25, 2017 at 9:59 PM, Misty Stanley-Jones <misty@apache.org>
wrote:

> If we keep it as is, how and where can we capture this knowledge to save
> future contributors from having to ask again?
>
> On Wed, Oct 25, 2017 at 9:22 AM Josh Elser <elserj@apache.org> wrote:
>
> > :) no worries, Appy. These are good questions.
> >
> > It really comes down to the question: would HBase ever provide multiple
> > metrics implementations for users?
> >
> > As we know metrics in HBase now, there isn't any reason to support
> > multiple implementations of metrics. We have one implementation which is
> > optimal for people to use.
> >
> > One plausible use-case I can see being built is a metrics implementation
> > that natively uses something _other than_ Hadoop metrics2. Something you
> > may not have seen yet is the translation-layer from Dropwizard Metrics
> > to Metrics2 via HBaseMetrics2HadoopMetricsAdapter.java.
> >
> > I could see a world where folks would want to build an integration with
> > metrics-library1 to support the metrics platform for their $business1.
> > $business2 might want to use metrics-library2 that can automatically
> > push to something. As we know all too well, there may be conflicting
> > dependencies between metrics-library1 and metrics-library2 to prevent
> > them from successfully functioning on the classpath at the same time.
> > Additionally, $business1 may have a hard requirement to not deploy
> > metrics-library2 (and vice versa). One final concern, bundling our
> > "default" DropwizardMetrics 3-based implementation could cause headaches
> > for users who want to build their own metrics-api implementation.
> >
> > Granted, all of the above is hypothetical, but I think leaving this
> > flexibility in place is worth the separation of logic.
> >
> > On 10/25/17 3:35 AM, Apekshit Sharma wrote:
> > > Hi Josh
> > >
> > > You're right that naming doesn't make it clear, but Enis added a really
> > > great description in README of hbase-metrics-api to explain what's in
> > each
> > > module. However it's not obvious why are we splitting things into
> > separate
> > > module, as opposed to say, separating implementation in a separate
> "sub"
> > > package.
> > >
> > > I don't know Avatica, but as you clearly state, one of the requirements
> > was
> > > being able to plug in different implementations. Using service provider
> > > clearly makes sense in that case, so does keeping the implementation
> in a
> > > separate jar to avoid loading it if not needed.
> > > But in case of HBase, even if we have multiple implementations
> > > (hypothetically, because it doesn't seem probable), since the only user
> > > will be HBase and all modules will always be in classpath, it doesn't
> > > really get us anything. Right?
> > > I dug into this code first time today and just learnt about service
> > > providers, so i may be off, in which case please correct me. Thank you!
> > >
> > > On Tue, Oct 24, 2017 at 8:25 PM, Josh Elser <elserj@apache.org> wrote:
> > >
> > >> Hey Appy,
> > >>
> > >> I think Enis essentially copied what I was originally trying to do. Up
> > in
> > >> Avatica[1], I made a similar API Maven module which just had
> interfaces.
> > >> The implementation was them split up into a different module to avoid
> > the
> > >> implementation details of the API (specifically, using Dropwizard3)
> from
> > >> "infecting" anyone who just wanted the API. Implementations of the
> > metrics
> > >> API could be picked up via ServiceLoader.
> > >>
> > >> So, here in HBase, our "hbase-metrics" module is really an
> > implementation
> > >> of hbase-metrics-api for dropwizard-metrics 3.2.1 (the naming just
> loses
> > >> that detail).
> > >>
> > >> In Avatica, I wanted to make sure that people could use a metrics
> > library
> > >> implementation of their choosing (e.g. tied to whatever kind of
> > container
> > >> or app-server you're deploying Avatica). In HBase, that isn't such a
> > >> concern -- we publish via Metrics2 and that's it.
> > >>
> > >> That said, I could see a place where this separation enables some
> extra
> > >> functionality with less headache, but I, admittedly, don't have a
> > concrete
> > >> use-case behind it.
> > >>
> > >> - Josh
> > >>
> > >> [1] https://github.com/apache/calcite-avatica
> > >>
> > >> On 10/24/17 6:09 PM, Apekshit Sharma wrote:
> > >>
> > >>> Hi
> > >>>
> > >>> Am confused why do we have this weird circular dependency between the
> > two
> > >>> modules: hbase-metrics-api & hbase-metrics. And since maven doesn't
> > allow
> > >>> it explicitly, this was tucked and hidden by using reflection?
> > >>>
> > >>> MetricRegistriesImpl[hbase-metrics] implements
> > >>> MetricRegistries[hbase-metrics-api].
> > >>> MetricRegistriesLoader[hbase-metrics-api] depends on this
> > implementation,
> > >>> MetricRegistriesImpl, because it instantiates it via reflection
> > >>> <https://github.com/apache/hbase/blob/eee3b0180ead73c09b33f9
> > >>> 583bfee9c01bc3aed2/hbase-metrics-api/src/main/java/org/
> > >>> apache/hadoop/hbase/metrics/MetricRegistriesLoader.java#L39>
> > >>> [1].
> > >>>
> > >>> Can we just move all interfaces and default implementation together
> > into
> > >>> single module. But i like the idea of keeping implementations in
> > separate
> > >>> package with "impl" explicitly in package name.
> > >>>
> > >>> [1]
> > >>> https://github.com/apache/hbase/blob/eee3b0180ead73c09b33f95
> > >>> 83bfee9c01bc3aed2/hbase-metrics-api/src/main/java/org/
> > >>> apache/hadoop/hbase/metrics/MetricRegistriesLoader.java#L39
> > >>>
> > >>> -- Appy
> > >>>
> > >>>
> > >
> > >
> >
>



-- 

-- Appy

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message