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 Wed, 31 Jan 2018 20:02:45 GMT
Alex,

As a workaround, you can use boxed Boolean as a field type, and then return
true from getEventsEnabled in case it's null. Will this work?

-Val

On Wed, Jan 31, 2018 at 7:31 AM, Alex Plehanov <plehanov.alex@gmail.com>
wrote:

> Denis, there is a question about IGNITE-7346.
>
> CacheConfiguration fields are set to they default values when cache
> configuration is created by constructor. When CacheConfiguration is created
> by deserialization (from another node or from PDS), constructor is not
> called. If it was serialized by older version of Ignite, a new added
> boolean fields are set to false after deserialization by a new Ignite
> versions. We can't change this behavior without methods for custom
> serialization/deserialization (which are not implemented now in
> CacheConfiguration class). It will differ from requirements for "Eve
> ntsEnabled" property (events must be enabled for caches by default).
>
> I think we can reverse the logic and rename method
> CacheConfiguration.setEventsEnabled
> to CacheConfiguration.setEventsDisabled, in this case the field value will
> always be "false" by default.
>
> What do you think about it?
>
> 2017-12-30 10:12 GMT+03:00 Alex Plehanov <plehanov.alex@gmail.com>:
>
> > 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/54043c7b75c69eca5515e7d7692b52
> 76
> >> >>> 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