ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alex Plehanov <plehanov.a...@gmail.com>
Subject Re: Annoying extra steps for enabling metrics
Date Fri, 29 Dec 2017 22:05:50 GMT
Valentine, yes, that's exactly what I'm trying to say. I don't see direct
dependencies between these properties (when a property must be set in all
cases another property is set).

2017-12-29 22:10 GMT+03:00 Valentin Kulichenko <
valentin.kulichenko@gmail.com>:

> 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