Proposal - provide a callback to compute statistics

[Dan Smith <ds...@pivotal.io>]

Hi,

I'd like to add some new methods to the Statistics interface to compute
statistics using callbacks. My original motivation for this is to make it
easy to record statistics that come from lucene for our lucene integration,
but I think this could simplify recording statistics in a lot of places.

The basic idea is that the user provides a callback that returns the value
of the statistic. Geode will call the callback every sample interval to
recompute the value.

See the JIRA for more details:
https://issues.apache.org/jira/browse/GEODE-1494

Thoughts?

-Dan

Re: Proposal - provide a callback to compute statistics

[Dan Smith <ds...@pivotal.io>]

On Thu, Jun 2, 2016 at 11:23 AM, Jens Deppe <jd...@pivotal.io> wrote:

> If the methods are providing a Supplier (to the Statistic) shouldn't they
> be called 'set{Int,Long,Double}Supplier'?
>

Seems reasonable. I'll change them to be set{Int,Long,Double}Supplier

Re: Proposal - provide a callback to compute statistics

[Dan Smith <ds...@pivotal.io>]

>
> Which thread are you proposing to invoke the new statistic provider SPIs?\
>

Yeah - that's the tricky question :) For the stats I want to collect, the
callbacks could be invoked in the stat sampler thread. But it think it
probably makes sense to invoke these callbacks on a separate thread, just
in case someone installs a bad callback it won't completely take down
statistics collection. The downside is that the stats collected with these
callbacks may be slightly stale. What do you think?

-Dan

Re: Proposal - provide a callback to compute statistics

[Kirk Lund <kl...@pivotal.io>]

Which thread are you proposing to invoke the new statistic provider SPIs?\

-Kirk


On Thu, Jun 2, 2016 at 2:31 PM, Dan Smith <ds...@pivotal.io> wrote:

> On Thu, Jun 2, 2016 at 11:47 AM, Darrel Schneider <ds...@pivotal.io>
> wrote:
>
> > Statistics are supposed to work even if you don't have sampling enabled.
> > For example you could turn off sampling and not have a statistic archive
> > but could still run a gfsh command that fetches a bunch of stats from the
> > running system or use the pulse tool.
> >
> > However you can leave sampling turned on even if you do not have a
> > statistic archive and it is true that our current OSStats (linux,
> windows,
> > solaris) and VMStats are only updated when sampling is enabled.
> > If you go with that then you just need to make clear that your sampler
> will
> > only be called if the config property "statistic-sampling-enabled" is
> true
> > and the frequency of calls will be determined by the config property
> > "statistic-sample-rate".
> >
>
> Interesting. I think it make sense for these stats to be controlled by the
> statistic-sampling-enabled property. It looks like that property defaults
> to true, and the javadocs say that if you turn it off some of your stats
> will display as 0.
>
> -Dan
>

Re: Proposal - provide a callback to compute statistics

[Dan Smith <ds...@pivotal.io>]

On Thu, Jun 2, 2016 at 11:47 AM, Darrel Schneider <ds...@pivotal.io>
wrote:

> Statistics are supposed to work even if you don't have sampling enabled.
> For example you could turn off sampling and not have a statistic archive
> but could still run a gfsh command that fetches a bunch of stats from the
> running system or use the pulse tool.
>
> However you can leave sampling turned on even if you do not have a
> statistic archive and it is true that our current OSStats (linux, windows,
> solaris) and VMStats are only updated when sampling is enabled.
> If you go with that then you just need to make clear that your sampler will
> only be called if the config property "statistic-sampling-enabled" is true
> and the frequency of calls will be determined by the config property
> "statistic-sample-rate".
>

Interesting. I think it make sense for these stats to be controlled by the
statistic-sampling-enabled property. It looks like that property defaults
to true, and the javadocs say that if you turn it off some of your stats
will display as 0.

-Dan

Re: Proposal - provide a callback to compute statistics

[Darrel Schneider <ds...@pivotal.io>]

Statistics are supposed to work even if you don't have sampling enabled.
For example you could turn off sampling and not have a statistic archive
but could still run a gfsh command that fetches a bunch of stats from the
running system or use the pulse tool.

However you can leave sampling turned on even if you do not have a
statistic archive and it is true that our current OSStats (linux, windows,
solaris) and VMStats are only updated when sampling is enabled.
If you go with that then you just need to make clear that your sampler will
only be called if the config property "statistic-sampling-enabled" is true
and the frequency of calls will be determined by the config property
"statistic-sample-rate".

On Thu, Jun 2, 2016 at 11:23 AM, Jens Deppe <jd...@pivotal.io> wrote:

> If the methods are providing a Supplier (to the Statistic) shouldn't they
> be called 'set{Int,Long,Double}Supplier'?
>
> On Thu, Jun 2, 2016 at 11:04 AM, Dan Smith <ds...@pivotal.io> wrote:
>
> > Replies inline.
> >
> > On Thu, Jun 2, 2016 at 10:04 AM, Darrel Schneider <dschneider@pivotal.io
> >
> > wrote:
> >
> > > It is not clear to me how the new apis behave.
> > > Is the supplier for a particular id/name/descriptor remembered by the
> > > Statistics instance? So if you wanted to add an intSupplier for a int
> > > statistic you would do it once by calling sampleInt?
> > >
> >
> > Yeah, that's what I was going for. You install the supplier once and then
> > it gets called every sample.
> >
> >
> >
> > > The name of these methods give the impression that calling it will
> take a
> > > sample. Perhaps it should be setSampler? Do you need the type (int,
> long,
> > > etc) in the name or is overloading ok?
> > >
> >
> > I'll rename it to setSampler, that seems clearer. Name overloading should
> > work, but I was trying to be consistent with the existing methods. We
> have
> > incInt, intLong, incDouble. Should I follow a different pattern for these
> > setSampler methods?
> >
> >
> > > Does the supplier need to match the type of the stat otherwise you get
> > > IllegalArgumentException?
> > >
> >
> > That seems like a good idea, I'll clarify that in the javadocs.
> >
> >
> > > Can you only have one sampler registered for a particular stat? If so
> > > should these methods return the old one?
> > >
> >
> > Seems like a good idea. I was also thinking maybe for completeness if you
> > set the sampler to null it will remove it. I'll clarify the javadocs.
> >
> >
> > > How does a sampler interact with the existing methods on the same stat?
> > Do
> > > the "get" methods implicitly call the sampler to compute the result
> they
> > > return? If so then the "set" and "inc" methods become noops.
> > >
> > > I was thinking get would just return the last sampled value - it would
> > not
> > trigger an immediate recomputation. But it's true that set and inc are
> > basically useless if you have a supplier set.
> >
> > -Dan
> >
>

Re: Proposal - provide a callback to compute statistics

[Jens Deppe <jd...@pivotal.io>]

If the methods are providing a Supplier (to the Statistic) shouldn't they
be called 'set{Int,Long,Double}Supplier'?

On Thu, Jun 2, 2016 at 11:04 AM, Dan Smith <ds...@pivotal.io> wrote:

> Replies inline.
>
> On Thu, Jun 2, 2016 at 10:04 AM, Darrel Schneider <ds...@pivotal.io>
> wrote:
>
> > It is not clear to me how the new apis behave.
> > Is the supplier for a particular id/name/descriptor remembered by the
> > Statistics instance? So if you wanted to add an intSupplier for a int
> > statistic you would do it once by calling sampleInt?
> >
>
> Yeah, that's what I was going for. You install the supplier once and then
> it gets called every sample.
>
>
>
> > The name of these methods give the impression that calling it will take a
> > sample. Perhaps it should be setSampler? Do you need the type (int, long,
> > etc) in the name or is overloading ok?
> >
>
> I'll rename it to setSampler, that seems clearer. Name overloading should
> work, but I was trying to be consistent with the existing methods. We have
> incInt, intLong, incDouble. Should I follow a different pattern for these
> setSampler methods?
>
>
> > Does the supplier need to match the type of the stat otherwise you get
> > IllegalArgumentException?
> >
>
> That seems like a good idea, I'll clarify that in the javadocs.
>
>
> > Can you only have one sampler registered for a particular stat? If so
> > should these methods return the old one?
> >
>
> Seems like a good idea. I was also thinking maybe for completeness if you
> set the sampler to null it will remove it. I'll clarify the javadocs.
>
>
> > How does a sampler interact with the existing methods on the same stat?
> Do
> > the "get" methods implicitly call the sampler to compute the result they
> > return? If so then the "set" and "inc" methods become noops.
> >
> > I was thinking get would just return the last sampled value - it would
> not
> trigger an immediate recomputation. But it's true that set and inc are
> basically useless if you have a supplier set.
>
> -Dan
>

Re: Proposal - provide a callback to compute statistics

[Dan Smith <ds...@pivotal.io>]

Replies inline.

On Thu, Jun 2, 2016 at 10:04 AM, Darrel Schneider <ds...@pivotal.io>
wrote:

> It is not clear to me how the new apis behave.
> Is the supplier for a particular id/name/descriptor remembered by the
> Statistics instance? So if you wanted to add an intSupplier for a int
> statistic you would do it once by calling sampleInt?
>

Yeah, that's what I was going for. You install the supplier once and then
it gets called every sample.



> The name of these methods give the impression that calling it will take a
> sample. Perhaps it should be setSampler? Do you need the type (int, long,
> etc) in the name or is overloading ok?
>

I'll rename it to setSampler, that seems clearer. Name overloading should
work, but I was trying to be consistent with the existing methods. We have
incInt, intLong, incDouble. Should I follow a different pattern for these
setSampler methods?


> Does the supplier need to match the type of the stat otherwise you get
> IllegalArgumentException?
>

That seems like a good idea, I'll clarify that in the javadocs.


> Can you only have one sampler registered for a particular stat? If so
> should these methods return the old one?
>

Seems like a good idea. I was also thinking maybe for completeness if you
set the sampler to null it will remove it. I'll clarify the javadocs.


> How does a sampler interact with the existing methods on the same stat? Do
> the "get" methods implicitly call the sampler to compute the result they
> return? If so then the "set" and "inc" methods become noops.
>
> I was thinking get would just return the last sampled value - it would not
trigger an immediate recomputation. But it's true that set and inc are
basically useless if you have a supplier set.

-Dan

Re: Proposal - provide a callback to compute statistics

[Darrel Schneider <ds...@pivotal.io>]

It is not clear to me how the new apis behave.
Is the supplier for a particular id/name/descriptor remembered by the
Statistics instance? So if you wanted to add an intSupplier for a int
statistic you would do it once by calling sampleInt?

The name of these methods give the impression that calling it will take a
sample. Perhaps it should be setSampler? Do you need the type (int, long,
etc) in the name or is overloading ok?
Does the supplier need to match the type of the stat otherwise you get
IllegalArgumentException?
Can you only have one sampler registered for a particular stat? If so
should these methods return the old one?
How does a sampler interact with the existing methods on the same stat? Do
the "get" methods implicitly call the sampler to compute the result they
return? If so then the "set" and "inc" methods become noops.



On Wed, Jun 1, 2016 at 4:33 PM, Dan Smith <ds...@pivotal.io> wrote:

> I'm suggesting using standard Java 8 function interfaces for the
> callbacks - IntSupplier, DoubleSupplier, etc. I can change the name of
> the argument to supplier.
>
> -Dan
>
> On Wed, Jun 1, 2016 at 4:18 PM, Xiaojian Zhou <gz...@pivotal.io> wrote:
> > what's the difference btw supplier and sampler?
> >
> > On Wed, Jun 1, 2016 at 4:12 PM, Dan Smith <ds...@pivotal.io> wrote:
> >
> >> Hi,
> >>
> >> I'd like to add some new methods to the Statistics interface to compute
> >> statistics using callbacks. My original motivation for this is to make
> it
> >> easy to record statistics that come from lucene for our lucene
> integration,
> >> but I think this could simplify recording statistics in a lot of places.
> >>
> >> The basic idea is that the user provides a callback that returns the
> value
> >> of the statistic. Geode will call the callback every sample interval to
> >> recompute the value.
> >>
> >> See the JIRA for more details:
> >> https://issues.apache.org/jira/browse/GEODE-1494
> >>
> >> Thoughts?
> >>
> >> -Dan
> >>
>

Re: Proposal - provide a callback to compute statistics

[Dan Smith <ds...@pivotal.io>]

I'm suggesting using standard Java 8 function interfaces for the
callbacks - IntSupplier, DoubleSupplier, etc. I can change the name of
the argument to supplier.

-Dan

On Wed, Jun 1, 2016 at 4:18 PM, Xiaojian Zhou <gz...@pivotal.io> wrote:
> what's the difference btw supplier and sampler?
>
> On Wed, Jun 1, 2016 at 4:12 PM, Dan Smith <ds...@pivotal.io> wrote:
>
>> Hi,
>>
>> I'd like to add some new methods to the Statistics interface to compute
>> statistics using callbacks. My original motivation for this is to make it
>> easy to record statistics that come from lucene for our lucene integration,
>> but I think this could simplify recording statistics in a lot of places.
>>
>> The basic idea is that the user provides a callback that returns the value
>> of the statistic. Geode will call the callback every sample interval to
>> recompute the value.
>>
>> See the JIRA for more details:
>> https://issues.apache.org/jira/browse/GEODE-1494
>>
>> Thoughts?
>>
>> -Dan
>>

Re: Proposal - provide a callback to compute statistics

[Xiaojian Zhou <gz...@pivotal.io>]

what's the difference btw supplier and sampler?

On Wed, Jun 1, 2016 at 4:12 PM, Dan Smith <ds...@pivotal.io> wrote:

> Hi,
>
> I'd like to add some new methods to the Statistics interface to compute
> statistics using callbacks. My original motivation for this is to make it
> easy to record statistics that come from lucene for our lucene integration,
> but I think this could simplify recording statistics in a lot of places.
>
> The basic idea is that the user provides a callback that returns the value
> of the statistic. Geode will call the callback every sample interval to
> recompute the value.
>
> See the JIRA for more details:
> https://issues.apache.org/jira/browse/GEODE-1494
>
> Thoughts?
>
> -Dan
>