metrics class deprecation Was: HBase 0.94.1

[Ted Yu <yu...@gmail.com>]

Renaming this thread since it is getting more and more technical in a
specific area.

Thanks for the analysis, Elliot.

bq. Remove PersistentMetricsTimeVaryingRate, MetricsString,
ExactCounterMetric, MetricsHistogram in 0.98

If the above classes are deprecated in 0.94.1, we should be able to remove
them in 0.96, right ?

Cheers

On Tue, Jul 10, 2012 at 11:19 AM, Elliott Clark <ec...@stumbleupon.com>wrote:

> Metrics2 does place things in different jmx locations from metrics, so when
> we move over it will be a bigger deal for ops.  We should do this in the
> singularity if it all possible.
>
> One possibility is that the classes in org.apache.hadoop.hbase.metrics can
> be deprecated in 94.1 and we can follow hadoop's lead and make a metrics2
> package in 0.96.
>
>
>    - Deprecate PersistentMetricsTimeVaryingRate, MetricsString,
> ExactCounterMetric, MetricsHistogram
>    in 0.94.1
>       - These are currently public with no annotation
>       - I can actually see people using the MetricsHistogram
>       and PersistentMetricsTimeVaryingRate in external code.
>    - Remove PersistentMetricsTimeVaryingRate, MetricsString,
> ExactCounterMetric, MetricsHistogram in
>    0.98
>    - Deprecate OperationMetrics, RegionServerDynamicMetrics, MasterMetrics,
>    et al.
>       - These are classes that actually wire our code up to the
>       metrics infrastructure and publish numbers
>       - There doesn't seem like all that much chance that people are using
>       these classes in external code.
>    - Remove OperationMetrics, RegionServerDynamicMetrics, MasterMetrics in
>    0.96.0
>       - Since these are very likely to be internal only removing in 0.96.0
>       seems low impact.
>    - In 0.96.0 create an org.apache.hadoop.hbase.metrics2 containing any
>    needed Metric2 classes.
>    - in 0.96.0 create replacement classes for publishing metrics to the
>    metrics2 hadoop infra
>
>
> For my money that seems like a pretty safe path. Thoughts ?
>
> On Tue, Jul 10, 2012 at 10:50 AM, Ted Yu <yu...@gmail.com> wrote:
>
> > Please take a look at my tentative patch on HBASE-6365 and provide your
> > feedback.
> >
> > Cheers
> >
> > On Tue, Jul 10, 2012 at 10:34 AM, Todd Lipcon <to...@cloudera.com> wrote:
> >
> > > I don't think we should be concerned with people directly extending
> > > the various metrics classes. They're not meant to be a "user API" IMO.
> > > We should annotate them as private. But the external-facing interface
> > > (ie the JMX output) should be treated as an interface.
> > >
> > > If we have to break them at some point without a deprecation path, +1
> > > for doing so in 0.96.
> > >
> > > On Tue, Jul 10, 2012 at 10:32 AM, Ted Yu <yu...@gmail.com> wrote:
> > > > Todd brought up a good point.
> > > >
> > > > MetricsBase class only exists in old metrics framework but not
> metrics2
> > > > framework.
> > > > So I am not sure whether the actual names of (all) the metrics
> exposed
> > > > would be kept consistent.
> > > >
> > > > Since MetricsHistogram, etc, are public, we do need to deprecate them
> > in
> > > > 0.94 in case some users extend these classes.
> > > >
> > > > Would listen to metrics experts' comments.
> > > >
> > > > On Tue, Jul 10, 2012 at 10:14 AM, Todd Lipcon <to...@cloudera.com>
> > wrote:
> > > >
> > > >> I think there's an important distinction between the Java API of
> > > >> metrics, and the implicit interface that the metrics themselves
> > > >> expose. IMO, we can completely change the implementation of metrics
> > > >> (e.g. class names and java APIs) so long as the actual names of the
> > > >> metrics exposed are kept consistent. If we make a change there, we
> > > >> should provide a deprecation path if at all possible - otherwise we
> > > >> need a big warning on upgrade so that operators know what they're
> > > >> getting themselves into.
> > > >>
> > > >> -Todd
> > > >>
> > > >> On Tue, Jul 10, 2012 at 9:57 AM, Ted Yu <yu...@gmail.com>
> wrote:
> > > >> > There is no annotation declaring whether the current metrics are
> > > stable
> > > >> API:
> > > >> >
> > > >> > public class MetricsHistogram extends MetricsBase {
> > > >> >
> > > >> > LarsH has endorsed marking the current metrics classes deprecated
> in
> > > his
> > > >> > later reply to this thread.
> > > >> >
> > > >> > Correct me if my interpretation is wrong.
> > > >> >
> > > >> > On Tue, Jul 10, 2012 at 9:23 AM, Stack <st...@duboce.net> wrote:
> > > >> >
> > > >> >> On Tue, Jul 10, 2012 at 1:46 AM, lars hofhansl <
> > lhofhansl@yahoo.com>
> > > >> >> wrote:
> > > >> >> > 0.94 is already out and did not have these deprecated. So
> > > deprecating
> > > >> >> them now in a point release is a bit strange.
> > > >> >> > Not -1'ing it, just raising that thought here.
> > > >> >> >
> > > >> >> > As said below because of HBASE-6311 0.94.1 should get out soon.
> > If
> > > >> push
> > > >> >> comes to shuff are folks ok with:
> > > >> >> > 1. deprecating in a point release
> > > >> >> > 2. maybe doing that in 0.94.2
> > > >> >> > ?
> > > >> >> >
> > > >> >>
> > > >> >> In the past, we'd remove public APIs after deprecating them
> across
> > a
> > > >> >> full major release: i.e. we'd deprecate something we want to
> remove
> > > in
> > > >> >> 0.96.0 in 0.94.0 (not 0.94.1).  Are the metrics changes to public
> > > >> >> "stable" APIs?  If so, I'd ask why change our convention now?
> If
> > > >> >> they are "evolving", we might bend the rules.
> > > >> >>
> > > >> >> Regards, what goes into 0.94.1, its up to the release manager.
> >  They
> > > >> >> can entertain petitions regards what to include but ultimately
> its
> > up
> > > >> >> to the RM when it happens and what is in it.
> > > >> >>
> > > >> >> St.Ack
> > > >> >>
> > > >>
> > > >>
> > > >>
> > > >> --
> > > >> Todd Lipcon
> > > >> Software Engineer, Cloudera
> > > >>
> > >
> > >
> > >
> > > --
> > > Todd Lipcon
> > > Software Engineer, Cloudera
> > >
> >
>