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/ > >>>> > >>>> > >> > >> > >
