You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ignite.apache.org by Alexey Plekhanov <pl...@gmail.com> on 2017/12/29 11:51:37 UTC
Re: Annoying extra steps for enabling metrics
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
> <https://issues.apache.org/jira/browse/IGNITE-5796>
>
> Expanded its scope.
>
> —
> Denis
>
>> On Sep 9, 2017, at 2:44 PM, Valentin Kulichenko <
> valentin.kulichenko@
> > wrote:
>>
>> statisticsEnabled property comes from JCache, BTW.
>>
>> -Val
>>
>> On Sat, Sep 9, 2017 at 11:09 AM, Dmitriy Setrakyan <
> dsetrakyan@
> >
>> wrote:
>>
>>> On Sat, Sep 9, 2017 at 8:56 AM, Denis Magda <
> dmagda@
> > 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/
Re: Annoying extra steps for enabling metrics
Posted by Alex Plehanov <pl...@gmail.com>.
Ok, then I will rename methods to setEventsDisabled/getEventsDisabled.
2018-02-01 12:46 GMT+03:00 Anton Vinogradov <av...@gridgain.com>:
> Folks,
>
> .setEventsDisabled looks to be a good solution, since we will always call
> it like .setEventsDisabled(true).
>
> Calling .setEventsEnabled(false), since true is a default, looks odd to me.
>
> On Wed, Jan 31, 2018 at 11:02 PM, Valentin Kulichenko <
> valentin.kulichenko@gmail.com> wrote:
>
> > 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 <pl...@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 <pl...@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 <dm...@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 <dm...@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
> > > >> >>>>>> <https://issues.apache.org/jira/browse/IGNITE-5796>
> > > >> >>>>>>
> > > >> >>>>>> Expanded its scope.
> > > >> >>>>>>
> > > >> >>>>>> —
> > > >> >>>>>> Denis
> > > >> >>>>>>
> > > >> >>>>>>> On Sep 9, 2017, at 2:44 PM, Valentin Kulichenko <
> > > >> >>>>>
> > > >> >>>>>> valentin.kulichenko@
> > > >> >>>>>
> > > >> >>>>>> > wrote:
> > > >> >>>>>>>
> > > >> >>>>>>> statisticsEnabled property comes from JCache, BTW.
> > > >> >>>>>>>
> > > >> >>>>>>> -Val
> > > >> >>>>>>>
> > > >> >>>>>>> On Sat, Sep 9, 2017 at 11:09 AM, Dmitriy Setrakyan <
> > > >> >>>>>
> > > >> >>>>>> dsetrakyan@
> > > >> >>>>>
> > > >> >>>>>> >
> > > >> >>>>>>> wrote:
> > > >> >>>>>>>
> > > >> >>>>>>>> On Sat, Sep 9, 2017 at 8:56 AM, Denis Magda <
> > > >> >>>>>
> > > >> >>>>>> dmagda@
> > > >> >>>>>
> > > >> >>>>>> > 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/
> > > >> >>>>
> > > >> >>>>
> > > >> >>
> > > >> >>
> > > >>
> > > >>
> > > >
> > >
> >
>
Re: Annoying extra steps for enabling metrics
Posted by Anton Vinogradov <av...@gridgain.com>.
Folks,
.setEventsDisabled looks to be a good solution, since we will always call
it like .setEventsDisabled(true).
Calling .setEventsEnabled(false), since true is a default, looks odd to me.
On Wed, Jan 31, 2018 at 11:02 PM, Valentin Kulichenko <
valentin.kulichenko@gmail.com> wrote:
> 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 <pl...@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 <pl...@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 <dm...@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 <dm...@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 <dm...@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
> > >> >>>>>> <https://issues.apache.org/jira/browse/IGNITE-5796>
> > >> >>>>>>
> > >> >>>>>> Expanded its scope.
> > >> >>>>>>
> > >> >>>>>> —
> > >> >>>>>> Denis
> > >> >>>>>>
> > >> >>>>>>> On Sep 9, 2017, at 2:44 PM, Valentin Kulichenko <
> > >> >>>>>
> > >> >>>>>> valentin.kulichenko@
> > >> >>>>>
> > >> >>>>>> > wrote:
> > >> >>>>>>>
> > >> >>>>>>> statisticsEnabled property comes from JCache, BTW.
> > >> >>>>>>>
> > >> >>>>>>> -Val
> > >> >>>>>>>
> > >> >>>>>>> On Sat, Sep 9, 2017 at 11:09 AM, Dmitriy Setrakyan <
> > >> >>>>>
> > >> >>>>>> dsetrakyan@
> > >> >>>>>
> > >> >>>>>> >
> > >> >>>>>>> wrote:
> > >> >>>>>>>
> > >> >>>>>>>> On Sat, Sep 9, 2017 at 8:56 AM, Denis Magda <
> > >> >>>>>
> > >> >>>>>> dmagda@
> > >> >>>>>
> > >> >>>>>> > 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/
> > >> >>>>
> > >> >>>>
> > >> >>
> > >> >>
> > >>
> > >>
> > >
> >
>
Re: Annoying extra steps for enabling metrics
Posted by Valentin Kulichenko <va...@gmail.com>.
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 <pl...@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 <pl...@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 <dm...@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 <dm...@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 <dm...@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
> >> >>>>>> <https://issues.apache.org/jira/browse/IGNITE-5796>
> >> >>>>>>
> >> >>>>>> Expanded its scope.
> >> >>>>>>
> >> >>>>>> —
> >> >>>>>> Denis
> >> >>>>>>
> >> >>>>>>> On Sep 9, 2017, at 2:44 PM, Valentin Kulichenko <
> >> >>>>>
> >> >>>>>> valentin.kulichenko@
> >> >>>>>
> >> >>>>>> > wrote:
> >> >>>>>>>
> >> >>>>>>> statisticsEnabled property comes from JCache, BTW.
> >> >>>>>>>
> >> >>>>>>> -Val
> >> >>>>>>>
> >> >>>>>>> On Sat, Sep 9, 2017 at 11:09 AM, Dmitriy Setrakyan <
> >> >>>>>
> >> >>>>>> dsetrakyan@
> >> >>>>>
> >> >>>>>> >
> >> >>>>>>> wrote:
> >> >>>>>>>
> >> >>>>>>>> On Sat, Sep 9, 2017 at 8:56 AM, Denis Magda <
> >> >>>>>
> >> >>>>>> dmagda@
> >> >>>>>
> >> >>>>>> > 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/
> >> >>>>
> >> >>>>
> >> >>
> >> >>
> >>
> >>
> >
>
Re: Annoying extra steps for enabling metrics
Posted by Alex Plehanov <pl...@gmail.com>.
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 <pl...@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 <dm...@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 <dm...@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 <dm...@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
>> >>>>>> <https://issues.apache.org/jira/browse/IGNITE-5796>
>> >>>>>>
>> >>>>>> Expanded its scope.
>> >>>>>>
>> >>>>>> —
>> >>>>>> Denis
>> >>>>>>
>> >>>>>>> On Sep 9, 2017, at 2:44 PM, Valentin Kulichenko <
>> >>>>>
>> >>>>>> valentin.kulichenko@
>> >>>>>
>> >>>>>> > wrote:
>> >>>>>>>
>> >>>>>>> statisticsEnabled property comes from JCache, BTW.
>> >>>>>>>
>> >>>>>>> -Val
>> >>>>>>>
>> >>>>>>> On Sat, Sep 9, 2017 at 11:09 AM, Dmitriy Setrakyan <
>> >>>>>
>> >>>>>> dsetrakyan@
>> >>>>>
>> >>>>>> >
>> >>>>>>> wrote:
>> >>>>>>>
>> >>>>>>>> On Sat, Sep 9, 2017 at 8:56 AM, Denis Magda <
>> >>>>>
>> >>>>>> dmagda@
>> >>>>>
>> >>>>>> > 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/
>> >>>>
>> >>>>
>> >>
>> >>
>>
>>
>
Re: Annoying extra steps for enabling metrics
Posted by Alex Plehanov <pl...@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 <dm...@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 <dm...@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 <dm...@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
> >>>>>> <https://issues.apache.org/jira/browse/IGNITE-5796>
> >>>>>>
> >>>>>> Expanded its scope.
> >>>>>>
> >>>>>> —
> >>>>>> Denis
> >>>>>>
> >>>>>>> On Sep 9, 2017, at 2:44 PM, Valentin Kulichenko <
> >>>>>
> >>>>>> valentin.kulichenko@
> >>>>>
> >>>>>> > wrote:
> >>>>>>>
> >>>>>>> statisticsEnabled property comes from JCache, BTW.
> >>>>>>>
> >>>>>>> -Val
> >>>>>>>
> >>>>>>> On Sat, Sep 9, 2017 at 11:09 AM, Dmitriy Setrakyan <
> >>>>>
> >>>>>> dsetrakyan@
> >>>>>
> >>>>>> >
> >>>>>>> wrote:
> >>>>>>>
> >>>>>>>> On Sat, Sep 9, 2017 at 8:56 AM, Denis Magda <
> >>>>>
> >>>>>> dmagda@
> >>>>>
> >>>>>> > 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/
> >>>>
> >>>>
> >>
> >>
>
>
Re: Annoying extra steps for enabling metrics
Posted by Denis Magda <dm...@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 <va...@gmail.com> wrote:
>
> Agree.
>
> -Val
>
> On Fri, Dec 29, 2017 at 3:08 PM, Denis Magda <dm...@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 <dm...@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
>>>>>> <https://issues.apache.org/jira/browse/IGNITE-5796>
>>>>>>
>>>>>> Expanded its scope.
>>>>>>
>>>>>> —
>>>>>> Denis
>>>>>>
>>>>>>> On Sep 9, 2017, at 2:44 PM, Valentin Kulichenko <
>>>>>
>>>>>> valentin.kulichenko@
>>>>>
>>>>>> > wrote:
>>>>>>>
>>>>>>> statisticsEnabled property comes from JCache, BTW.
>>>>>>>
>>>>>>> -Val
>>>>>>>
>>>>>>> On Sat, Sep 9, 2017 at 11:09 AM, Dmitriy Setrakyan <
>>>>>
>>>>>> dsetrakyan@
>>>>>
>>>>>> >
>>>>>>> wrote:
>>>>>>>
>>>>>>>> On Sat, Sep 9, 2017 at 8:56 AM, Denis Magda <
>>>>>
>>>>>> dmagda@
>>>>>
>>>>>> > 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/
>>>>
>>>>
>>
>>
Re: Annoying extra steps for enabling metrics
Posted by Valentin Kulichenko <va...@gmail.com>.
Agree.
-Val
On Fri, Dec 29, 2017 at 3:08 PM, Denis Magda <dm...@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 <dm...@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
> >>>> <https://issues.apache.org/jira/browse/IGNITE-5796>
> >>>>
> >>>> Expanded its scope.
> >>>>
> >>>> —
> >>>> Denis
> >>>>
> >>>>> On Sep 9, 2017, at 2:44 PM, Valentin Kulichenko <
> >>>
> >>>> valentin.kulichenko@
> >>>
> >>>> > wrote:
> >>>>>
> >>>>> statisticsEnabled property comes from JCache, BTW.
> >>>>>
> >>>>> -Val
> >>>>>
> >>>>> On Sat, Sep 9, 2017 at 11:09 AM, Dmitriy Setrakyan <
> >>>
> >>>> dsetrakyan@
> >>>
> >>>> >
> >>>>> wrote:
> >>>>>
> >>>>>> On Sat, Sep 9, 2017 at 8:56 AM, Denis Magda <
> >>>
> >>>> dmagda@
> >>>
> >>>> > 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/
> >>
> >>
>
>
Re: Annoying extra steps for enabling metrics
Posted by Denis Magda <dm...@apache.org>.
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 <va...@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 <dm...@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 <pl...@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
>>>> <https://issues.apache.org/jira/browse/IGNITE-5796>
>>>>
>>>> Expanded its scope.
>>>>
>>>> —
>>>> Denis
>>>>
>>>>> On Sep 9, 2017, at 2:44 PM, Valentin Kulichenko <
>>>
>>>> valentin.kulichenko@
>>>
>>>> > wrote:
>>>>>
>>>>> statisticsEnabled property comes from JCache, BTW.
>>>>>
>>>>> -Val
>>>>>
>>>>> On Sat, Sep 9, 2017 at 11:09 AM, Dmitriy Setrakyan <
>>>
>>>> dsetrakyan@
>>>
>>>> >
>>>>> wrote:
>>>>>
>>>>>> On Sat, Sep 9, 2017 at 8:56 AM, Denis Magda <
>>>
>>>> dmagda@
>>>
>>>> > 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/
>>
>>
Re: Annoying extra steps for enabling metrics
Posted by Alex Plehanov <pl...@gmail.com>.
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 <dm...@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
> > >> <https://issues.apache.org/jira/browse/IGNITE-5796>
> > >>
> > >> Expanded its scope.
> > >>
> > >> —
> > >> Denis
> > >>
> > >>> On Sep 9, 2017, at 2:44 PM, Valentin Kulichenko <
> > >
> > >> valentin.kulichenko@
> > >
> > >> > wrote:
> > >>>
> > >>> statisticsEnabled property comes from JCache, BTW.
> > >>>
> > >>> -Val
> > >>>
> > >>> On Sat, Sep 9, 2017 at 11:09 AM, Dmitriy Setrakyan <
> > >
> > >> dsetrakyan@
> > >
> > >> >
> > >>> wrote:
> > >>>
> > >>>> On Sat, Sep 9, 2017 at 8:56 AM, Denis Magda <
> > >
> > >> dmagda@
> > >
> > >> > 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/
> >
> >
>
Re: Annoying extra steps for enabling metrics
Posted by Valentin Kulichenko <va...@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 <dm...@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 <pl...@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
> >> <https://issues.apache.org/jira/browse/IGNITE-5796>
> >>
> >> Expanded its scope.
> >>
> >> —
> >> Denis
> >>
> >>> On Sep 9, 2017, at 2:44 PM, Valentin Kulichenko <
> >
> >> valentin.kulichenko@
> >
> >> > wrote:
> >>>
> >>> statisticsEnabled property comes from JCache, BTW.
> >>>
> >>> -Val
> >>>
> >>> On Sat, Sep 9, 2017 at 11:09 AM, Dmitriy Setrakyan <
> >
> >> dsetrakyan@
> >
> >> >
> >>> wrote:
> >>>
> >>>> On Sat, Sep 9, 2017 at 8:56 AM, Denis Magda <
> >
> >> dmagda@
> >
> >> > 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/
>
>
Re: Annoying extra steps for enabling metrics
Posted by Denis Magda <dm...@apache.org>.
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 <pl...@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
>> <https://issues.apache.org/jira/browse/IGNITE-5796>
>>
>> Expanded its scope.
>>
>> —
>> Denis
>>
>>> On Sep 9, 2017, at 2:44 PM, Valentin Kulichenko <
>
>> valentin.kulichenko@
>
>> > wrote:
>>>
>>> statisticsEnabled property comes from JCache, BTW.
>>>
>>> -Val
>>>
>>> On Sat, Sep 9, 2017 at 11:09 AM, Dmitriy Setrakyan <
>
>> dsetrakyan@
>
>> >
>>> wrote:
>>>
>>>> On Sat, Sep 9, 2017 at 8:56 AM, Denis Magda <
>
>> dmagda@
>
>> > 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/