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