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.kuliche...@gmail.com> wrote: > > Agree. > > -Val > > On Fri, Dec 29, 2017 at 3:08 PM, Denis Magda <dma...@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.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/ >>>> >>>> >> >>
