ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Valentin Kulichenko <valentin.kuliche...@gmail.com>
Subject Re: Annoying extra steps for enabling metrics
Date Fri, 29 Dec 2017 23:10:14 GMT
Agree.

-Val

On Fri, Dec 29, 2017 at 3:08 PM, Denis Magda <dmagda@apache.org> wrote:

> Now I see. Seems I was doing something wrong in my initial reproducer.
>
> Updated cache metrics readme doc by purging any events related parameters
> from there:
> https://apacheignite.readme.io/v2.3/docs/cache-metrics <
> https://apacheignite.readme.io/v2.3/docs/cache-metrics>
>
> The events readme doc looks good to me. Just updated the javadoc of
> IgniteEvents#*Query methods to bring MemoryEventStorageSpi to users
> attention.
>
> As for the cache events, it’s an oversight that there is no parameter that
> enables/disables the events per cache. Let’s add CacheConfiguration.setEventsEnabled
> method and have it enabled by default (current behavior). If the user
> deploys hundreds of caches or just interested in the events of specific
> ones then he can always set this property to ‘false’ in configuration of
> the caches of no interest:
> https://issues.apache.org/jira/browse/IGNITE-7346 <
> https://issues.apache.org/jira/browse/IGNITE-7346>
>
> Agree?
>
> —
> Denis
>
>
>
> > On Dec 29, 2017, at 11:10 AM, Valentin Kulichenko <
> valentin.kulichenko@gmail.com> wrote:
> >
> > Guys,
> >
> > I'm not sure what issue we're trying to solve. Basically, we have three
> > different functionality parts here:
> >
> > 1. Cache metrics exposed via CacheMetrics interface and MBeans (number of
> > puts, average put time, this kind of stuff). These are controlled on per
> > cache level by CacheConfiguration#statisticsEnabled property.
> > 2. Listening to cache events. To achieve this you only need to enable
> > particular event types and register listeners, this does not depend on
> > CacheConfiguration#statisticsEnabled. Here is the test confirming this:
> > https://gist.github.com/vkulichenko/54043c7b75c69eca5515e7d7692b5276
> > 3. Querying cache events via IgniteEvents#*Query methods. This is the
> ONLY
> > piece that requires MemoryEventStorageSpi to be configured. Since
> querying
> > is not very frequently used, and event storage introduces significant
> > memory overhead, I don't think we should ever implicitly enable it. We
> > deliberately made no-op implementation the default one not so long ago.
> >
> > All three points above seem to work without any issues. The only useful
> > addition I see here is ability to enable cache events on per cache level.
> > But for this we can introduce new property to CacheConfiguration. I would
> > not mix metrics and events together as these are different APIs designed
> > for different purposes.
> >
> > Am I missing something?
> >
> > -Val
> >
> > On Fri, Dec 29, 2017 at 8:02 AM, Denis Magda <dmagda@apache.org> wrote:
> >
> >> Alexey,
> >>
> >> I think we should enable memoryEventStorageSPI automatically whenever
> >> statisticsEnabled is toggled on. A special message has to be added to
> the
> >> log pointing out that the automatic activation happened.
> >>
> >> Does it sound like a good solution?
> >>
> >> —
> >> Denis
> >>
> >>> On Dec 29, 2017, at 3:51 AM, Alexey Plekhanov <plehanov.alex@gmail.com
> >
> >> wrote:
> >>>
> >>> Denis, I start working on the issue
> >>> https://issues.apache.org/jira/browse/IGNITE-6925 and now I don't
> >> understand
> >>> why these properties must be bound to each other?
> >>>
> >>> • If we enable statistics on caches, this does not necessarily mean,
> >> that
> >>> we wanted to get some events, we can enable statistics for other
> reasons.
> >>> Conversely, not all events need statistics to be enabled on caches. So
> we
> >>> can’t bind statistics flag to events (subscribe to events when
> >> statistics is
> >>> enabled or enable statistics, when subscribing to events)
> >>> • We can’t set events of interest, when we set not a dummy
> >> EventsStorageSpi,
> >>> because we don’t know which events are interesting.
> >>> • When we set events of interest, it’s not necessary, that these events
> >> will
> >>> be monitored using EventsStorageSpi, we can also subscribe to events by
> >>> events listeners, in this case EventsStorageSpi don’t used.
> >>>
> >>> So, there is no general rule (if ... enabled, then ... must be enabled
> >> too).
> >>> The only implementation I can propose - is "set MemoryEventStorageSPI
> >>> instead of NoopEventStorageSPI when includeEventTypes list is not
> empty",
> >>> but even this implementation may be warranted only in some cases.
> >>>
> >>>
> >>> Denis Magda-2 wrote
> >>>> Let’s simplifying the metrics as a part of this ticket:
> >>>> https://issues.apache.org/jira/browse/IGNITE-5796
> >>>> &lt;https://issues.apache.org/jira/browse/IGNITE-5796&gt;
> >>>>
> >>>> Expanded its scope.
> >>>>
> >>>> —
> >>>> Denis
> >>>>
> >>>>> On Sep 9, 2017, at 2:44 PM, Valentin Kulichenko &lt;
> >>>
> >>>> valentin.kulichenko@
> >>>
> >>>> &gt; wrote:
> >>>>>
> >>>>> statisticsEnabled property comes from JCache, BTW.
> >>>>>
> >>>>> -Val
> >>>>>
> >>>>> On Sat, Sep 9, 2017 at 11:09 AM, Dmitriy Setrakyan &lt;
> >>>
> >>>> dsetrakyan@
> >>>
> >>>> &gt;
> >>>>> wrote:
> >>>>>
> >>>>>> On Sat, Sep 9, 2017 at 8:56 AM, Denis Magda &lt;
> >>>
> >>>> dmagda@
> >>>
> >>>> &gt; wrote:
> >>>>>>
> >>>>>>> Surprise!
> >>>>>>>
> >>>>>>> If you want to see cache events then you have to enable
one more
> >> flag!
> >>>>>>>
> >>>>>>>
> >>>> <property name="StatisticsEnabled" value="true"/>
> >>>>>>
> >>>>>>
> >>>>>> What is the overhead of this statistics collection?
> >>>>>>
> >>>>>>
> >>>>>>> Three flags/beans have to be in the config in total, three!
Just to
> >> see
> >>>>>>> cache events. The API is a mess. Let’s contemplate how
to fix it.
> >>>>>>
> >>>>>>
> >>>>>> Agree, this is horrible. We need to fix it in 2.3. Is there
a
> ticket?
> >>>
> >>>
> >>>
> >>>
> >>>
> >>> --
> >>> Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
> >>
> >>
>
>

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