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