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 Sat, 30 Dec 2017 07:12:22 GMT
Due to holidays I can start work on this ticket only after 8 jan 2018

2017-12-30 2:12 GMT+03:00 Denis Magda <dmagda@apache.org>:

> Good, closed the original ticket.
>
> Alex P, do you have time to work on IGNITE-7346 instead to address the
> issue with the cache events per cache in 2.4 release?
>
> —
> Denis
>
> > On Dec 29, 2017, at 3:10 PM, Valentin Kulichenko <
> valentin.kulichenko@gmail.com> wrote:
> >
> > 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