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.kuliche...@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 <dma...@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.a...@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/ >> >>
