Annoying extra steps for enabling metrics
Locked
12
12
 
|
Igniters,
It was a surprise for me to reveal that even if the default config is used [1] I still will not see the metrics in Web Console or in a custom app. However, the config was left unchanged for a while and from the first look encompassed everything we need to start getting metrics. But, after looking into the issue, it was determined that this bean has to added explicitly: <property name="eventStorageSpi"> <bean class="org.apache.ignite.spi.eventstorage.memory.MemoryEventStorageSpi"/> </property> So, my question, what do we do next? It’s ridiculous to force setting this parameter. The NoopEventStorageSpi (default) simply ignores all the events. Why doesn’t it accept them, send to the subscribers and skips? Then it’s up to a subscriber to decide what to do next. [1] https://github.com/apache/ignite/blob/master/examples/config/example-default.xml <https://github.com/apache/ignite/blob/master/examples/config/example-default.xml> — Denis |
Re: Annoying extra steps for enabling metrics
|
Denis, which metrics are you talking about?
On Thu, Sep 7, 2017 at 7:12 PM, Denis Magda <[hidden email]> wrote: > Igniters, > > It was a surprise for me to reveal that even if the default config is used > [1] I still will not see the metrics in Web Console or in a custom app. > However, the config was left unchanged for a while and from the first look > encompassed everything we need to start getting metrics. > > But, after looking into the issue, it was determined that this bean has to > added explicitly: > > <property name="eventStorageSpi"> > <bean class="org.apache.ignite.spi.eventstorage.memory. > MemoryEventStorageSpi"/> > </property> > > So, my question, what do we do next? It’s ridiculous to force setting this > parameter. The NoopEventStorageSpi (default) simply ignores all the events. > Why doesn’t it accept them, send to the subscribers and skips? Then it’s up > to a subscriber to decide what to do next. > > [1] https://github.com/apache/ignite/blob/master/examples/ > config/example-default.xml <https://github.com/apache/ > ignite/blob/master/examples/config/example-default.xml> > > — > Denis |
|
|
Any metric from of org.apache.ignite.events.EventType (cache operations, compute, etc.)
— Denis > On Sep 7, 2017, at 7:15 PM, Dmitriy Setrakyan <[hidden email]> wrote: > > Denis, which metrics are you talking about? > > On Thu, Sep 7, 2017 at 7:12 PM, Denis Magda <[hidden email]> wrote: > >> Igniters, >> >> It was a surprise for me to reveal that even if the default config is used >> [1] I still will not see the metrics in Web Console or in a custom app. >> However, the config was left unchanged for a while and from the first look >> encompassed everything we need to start getting metrics. >> >> But, after looking into the issue, it was determined that this bean has to >> added explicitly: >> >> <property name="eventStorageSpi"> >> <bean class="org.apache.ignite.spi.eventstorage.memory. >> MemoryEventStorageSpi"/> >> </property> >> >> So, my question, what do we do next? It’s ridiculous to force setting this >> parameter. The NoopEventStorageSpi (default) simply ignores all the events. >> Why doesn’t it accept them, send to the subscribers and skips? Then it’s up >> to a subscriber to decide what to do next. >> >> [1] https://github.com/apache/ignite/blob/master/examples/ >> config/example-default.xml <https://github.com/apache/ >> ignite/blob/master/examples/config/example-default.xml> >> >> — >> Denis |
Re: Annoying extra steps for enabling metrics
|
|
On Thu, Sep 7, 2017 at 7:19 PM, Denis Magda <[hidden email]> wrote:
> Any metric from of org.apache.ignite.events.EventType (cache operations, > compute, etc.) > If you enable them all, Ignite will spend 99% of its time doing event notifications. > > — > Denis > > > On Sep 7, 2017, at 7:15 PM, Dmitriy Setrakyan <[hidden email]> > wrote: > > > > Denis, which metrics are you talking about? > > > > On Thu, Sep 7, 2017 at 7:12 PM, Denis Magda <[hidden email]> wrote: > > > >> Igniters, > >> > >> It was a surprise for me to reveal that even if the default config is > used > >> [1] I still will not see the metrics in Web Console or in a custom app. > >> However, the config was left unchanged for a while and from the first > look > >> encompassed everything we need to start getting metrics. > >> > >> But, after looking into the issue, it was determined that this bean has > to > >> added explicitly: > >> > >> <property name="eventStorageSpi"> > >> <bean class="org.apache.ignite.spi.eventstorage.memory. > >> MemoryEventStorageSpi"/> > >> </property> > >> > >> So, my question, what do we do next? It’s ridiculous to force setting > this > >> parameter. The NoopEventStorageSpi (default) simply ignores all the > events. > >> Why doesn’t it accept them, send to the subscribers and skips? Then > it’s up > >> to a subscriber to decide what to do next. > >> > >> [1] https://github.com/apache/ignite/blob/master/examples/ > >> config/example-default.xml <https://github.com/apache/ > >> ignite/blob/master/examples/config/example-default.xml> > >> > >> — > >> Denis > > |
|
|
My point is different. Before I had to do this only assuming that “Ignite will spend 99%” sending events:
<property name="includeEventTypes"> <list> <!--Task execution events--> <util:constant static-field="org.apache.ignite.events.EventType.EVT_TASK_STARTED"/> <util:constant static-field="org.apache.ignite.events.EventType.EVT_TASK_FINISHED"/> <util:constant static-field="org.apache.ignite.events.EventType.EVT_TASK_FAILED"/> <util:constant static-field="org.apache.ignite.events.EventType.EVT_TASK_TIMEDOUT"/> </list> </property> Now the platform forces me to do that (probably thinking that I’m crazy if I want to waste resources for metrics and giving one more change to contemplate the decision): <property name="eventStorageSpi"> <bean class="org.apache.ignite.spi.eventstorage.memory.MemoryEventStorageSpi"/> </property> Does the issue make sense to you know? — Denis > On Sep 7, 2017, at 7:25 PM, Dmitriy Setrakyan <[hidden email]> wrote: > > On Thu, Sep 7, 2017 at 7:19 PM, Denis Magda <[hidden email]> wrote: > >> Any metric from of org.apache.ignite.events.EventType (cache operations, >> compute, etc.) >> > > If you enable them all, Ignite will spend 99% of its time doing event > notifications. > > >> >> — >> Denis >> >>> On Sep 7, 2017, at 7:15 PM, Dmitriy Setrakyan <[hidden email]> >> wrote: >>> >>> Denis, which metrics are you talking about? >>> >>> On Thu, Sep 7, 2017 at 7:12 PM, Denis Magda <[hidden email]> wrote: >>> >>>> Igniters, >>>> >>>> It was a surprise for me to reveal that even if the default config is >> used >>>> [1] I still will not see the metrics in Web Console or in a custom app. >>>> However, the config was left unchanged for a while and from the first >> look >>>> encompassed everything we need to start getting metrics. >>>> >>>> But, after looking into the issue, it was determined that this bean has >> to >>>> added explicitly: >>>> >>>> <property name="eventStorageSpi"> >>>> <bean class="org.apache.ignite.spi.eventstorage.memory. >>>> MemoryEventStorageSpi"/> >>>> </property> >>>> >>>> So, my question, what do we do next? It’s ridiculous to force setting >> this >>>> parameter. The NoopEventStorageSpi (default) simply ignores all the >> events. >>>> Why doesn’t it accept them, send to the subscribers and skips? Then >> it’s up >>>> to a subscriber to decide what to do next. >>>> >>>> [1] https://github.com/apache/ignite/blob/master/examples/ >>>> config/example-default.xml <https://github.com/apache/ >>>> ignite/blob/master/examples/config/example-default.xml> >>>> >>>> — >>>> Denis >> >> |
Re: Annoying extra steps for enabling metrics
|
|
On Thu, Sep 7, 2017 at 7:30 PM, Denis Magda <[hidden email]> wrote:
> My point is different. Before I had to do this only assuming that “Ignite > will spend 99%” sending events: > > > <property name="includeEventTypes"> > <list> > <!--Task execution events--> > <util:constant static-field="org.apache. > ignite.events.EventType.EVT_TASK_STARTED"/> > <util:constant static-field="org.apache. > ignite.events.EventType.EVT_TASK_FINISHED"/> > <util:constant static-field="org.apache. > ignite.events.EventType.EVT_TASK_FAILED"/> > <util:constant static-field="org.apache. > ignite.events.EventType.EVT_TASK_TIMEDOUT"/> > </list> > </property> > > Now the platform forces me to do that (probably thinking that I’m crazy if > I want to waste resources for metrics and giving one more change to > contemplate the decision): > > <property name="eventStorageSpi"> > <bean class="org.apache.ignite.spi.eventstorage.memory. > MemoryEventStorageSpi"/> > </property> > > Does the issue make sense to you know? > I understand now. Why did we change this behavior? Can someone comment? |
Re: Annoying extra steps for enabling metrics
|
|
First of all, these are events, not metrics, and to my knowledge Ignite
itself does not expose any metrics that depend on events. The fact that Web Console (or any other tool) uses events to track and show something is a different story. Events API has two parts: listening (one simply registers a listener which is invoked every time an event is fired) and querying (all events are stored somewhere and one can query events that happened in the past). All events are disabled by default for performance reasons, so to listen for an event you need to enable it via includeEventTypes property. Event storage is also disabled by default, because if you only listen but never query, the storage would be a waste memory. I believe the default behavior is correct. But on the other hand, usability of Web Console obviously suffers. I think we should do the following to solve this: - Add an ability to enable event storage dynamically. BTW, ability to enable/disable events in runtime already exists. - Use the above in Web Console. I.e., it would dynamically enable required events and event storage on first connect to the cluster. This way Web Console will work seamlessly without requiring additional explicit config changes. -Val On Thu, Sep 7, 2017 at 7:34 PM Dmitriy Setrakyan <[hidden email]> wrote: > On Thu, Sep 7, 2017 at 7:30 PM, Denis Magda <[hidden email]> wrote: > > > My point is different. Before I had to do this only assuming that “Ignite > > will spend 99%” sending events: > > > > > > <property name="includeEventTypes"> > > <list> > > <!--Task execution events--> > > <util:constant static-field="org.apache. > > ignite.events.EventType.EVT_TASK_STARTED"/> > > <util:constant static-field="org.apache. > > ignite.events.EventType.EVT_TASK_FINISHED"/> > > <util:constant static-field="org.apache. > > ignite.events.EventType.EVT_TASK_FAILED"/> > > <util:constant static-field="org.apache. > > ignite.events.EventType.EVT_TASK_TIMEDOUT"/> > > </list> > > </property> > > > > Now the platform forces me to do that (probably thinking that I’m crazy > if > > I want to waste resources for metrics and giving one more change to > > contemplate the decision): > > > > <property name="eventStorageSpi"> > > <bean class="org.apache.ignite.spi.eventstorage.memory. > > MemoryEventStorageSpi"/> > > </property> > > > > Does the issue make sense to you know? > > > > I understand now. Why did we change this behavior? Can someone comment? > |
|
|
In reply to this post by dsetrakyan
Surprise!
If you want to see cache events then you have to enable one more flag! <property name="StatisticsEnabled" value="true"/> 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. — Denis > On Sep 7, 2017, at 7:33 PM, Dmitriy Setrakyan <[hidden email]> wrote: > > On Thu, Sep 7, 2017 at 7:30 PM, Denis Magda <[hidden email]> wrote: > >> My point is different. Before I had to do this only assuming that “Ignite >> will spend 99%” sending events: >> >> >> <property name="includeEventTypes"> >> <list> >> <!--Task execution events--> >> <util:constant static-field="org.apache. >> ignite.events.EventType.EVT_TASK_STARTED"/> >> <util:constant static-field="org.apache. >> ignite.events.EventType.EVT_TASK_FINISHED"/> >> <util:constant static-field="org.apache. >> ignite.events.EventType.EVT_TASK_FAILED"/> >> <util:constant static-field="org.apache. >> ignite.events.EventType.EVT_TASK_TIMEDOUT"/> >> </list> >> </property> >> >> Now the platform forces me to do that (probably thinking that I’m crazy if >> I want to waste resources for metrics and giving one more change to >> contemplate the decision): >> >> <property name="eventStorageSpi"> >> <bean class="org.apache.ignite.spi.eventstorage.memory. >> MemoryEventStorageSpi"/> >> </property> >> >> Does the issue make sense to you know? >> > > I understand now. Why did we change this behavior? Can someone comment? |
Re: Annoying extra steps for enabling metrics
|
|
On Sat, Sep 9, 2017 at 8:56 AM, Denis Magda <[hidden email]> 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? > > — > Denis > > > On Sep 7, 2017, at 7:33 PM, Dmitriy Setrakyan <[hidden email]> > wrote: > > > > On Thu, Sep 7, 2017 at 7:30 PM, Denis Magda <[hidden email]> wrote: > > > >> My point is different. Before I had to do this only assuming that > “Ignite > >> will spend 99%” sending events: > >> > >> > >> <property name="includeEventTypes"> > >> <list> > >> <!--Task execution events--> > >> <util:constant static-field="org.apache. > >> ignite.events.EventType.EVT_TASK_STARTED"/> > >> <util:constant static-field="org.apache. > >> ignite.events.EventType.EVT_TASK_FINISHED"/> > >> <util:constant static-field="org.apache. > >> ignite.events.EventType.EVT_TASK_FAILED"/> > >> <util:constant static-field="org.apache. > >> ignite.events.EventType.EVT_TASK_TIMEDOUT"/> > >> </list> > >> </property> > >> > >> Now the platform forces me to do that (probably thinking that I’m crazy > if > >> I want to waste resources for metrics and giving one more change to > >> contemplate the decision): > >> > >> <property name="eventStorageSpi"> > >> <bean class="org.apache.ignite.spi.eventstorage.memory. > >> MemoryEventStorageSpi"/> > >> </property> > >> > >> Does the issue make sense to you know? > >> > > > > I understand now. Why did we change this behavior? Can someone comment? > > |
Re: Annoying extra steps for enabling metrics
|
|
statisticsEnabled property comes from JCache, BTW.
-Val On Sat, Sep 9, 2017 at 11:09 AM, Dmitriy Setrakyan <[hidden email]> wrote: > On Sat, Sep 9, 2017 at 8:56 AM, Denis Magda <[hidden email]> 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? > > > > > > — > > Denis > > > > > On Sep 7, 2017, at 7:33 PM, Dmitriy Setrakyan <[hidden email]> > > wrote: > > > > > > On Thu, Sep 7, 2017 at 7:30 PM, Denis Magda <[hidden email]> wrote: > > > > > >> My point is different. Before I had to do this only assuming that > > “Ignite > > >> will spend 99%” sending events: > > >> > > >> > > >> <property name="includeEventTypes"> > > >> <list> > > >> <!--Task execution events--> > > >> <util:constant static-field="org.apache. > > >> ignite.events.EventType.EVT_TASK_STARTED"/> > > >> <util:constant static-field="org.apache. > > >> ignite.events.EventType.EVT_TASK_FINISHED"/> > > >> <util:constant static-field="org.apache. > > >> ignite.events.EventType.EVT_TASK_FAILED"/> > > >> <util:constant static-field="org.apache. > > >> ignite.events.EventType.EVT_TASK_TIMEDOUT"/> > > >> </list> > > >> </property> > > >> > > >> Now the platform forces me to do that (probably thinking that I’m > crazy > > if > > >> I want to waste resources for metrics and giving one more change to > > >> contemplate the decision): > > >> > > >> <property name="eventStorageSpi"> > > >> <bean class="org.apache.ignite.spi.eventstorage.memory. > > >> MemoryEventStorageSpi"/> > > >> </property> > > >> > > >> Does the issue make sense to you know? > > >> > > > > > > I understand now. Why did we change this behavior? Can someone comment? > > > > > |
|
|
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 <[hidden email]> wrote: > > statisticsEnabled property comes from JCache, BTW. > > -Val > > On Sat, Sep 9, 2017 at 11:09 AM, Dmitriy Setrakyan <[hidden email]> > wrote: > >> On Sat, Sep 9, 2017 at 8:56 AM, Denis Magda <[hidden email]> 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? >> >> >>> >>> — >>> Denis >>> >>>> On Sep 7, 2017, at 7:33 PM, Dmitriy Setrakyan <[hidden email]> >>> wrote: >>>> >>>> On Thu, Sep 7, 2017 at 7:30 PM, Denis Magda <[hidden email]> wrote: >>>> >>>>> My point is different. Before I had to do this only assuming that >>> “Ignite >>>>> will spend 99%” sending events: >>>>> >>>>> >>>>> <property name="includeEventTypes"> >>>>> <list> >>>>> <!--Task execution events--> >>>>> <util:constant static-field="org.apache. >>>>> ignite.events.EventType.EVT_TASK_STARTED"/> >>>>> <util:constant static-field="org.apache. >>>>> ignite.events.EventType.EVT_TASK_FINISHED"/> >>>>> <util:constant static-field="org.apache. >>>>> ignite.events.EventType.EVT_TASK_FAILED"/> >>>>> <util:constant static-field="org.apache. >>>>> ignite.events.EventType.EVT_TASK_TIMEDOUT"/> >>>>> </list> >>>>> </property> >>>>> >>>>> Now the platform forces me to do that (probably thinking that I’m >> crazy >>> if >>>>> I want to waste resources for metrics and giving one more change to >>>>> contemplate the decision): >>>>> >>>>> <property name="eventStorageSpi"> >>>>> <bean class="org.apache.ignite.spi.eventstorage.memory. >>>>> MemoryEventStorageSpi"/> >>>>> </property> >>>>> >>>>> Does the issue make sense to you know? >>>>> >>>> >>>> I understand now. Why did we change this behavior? Can someone comment? >>> >>> >> |
Re: Annoying extra steps for enabling metrics
|
|
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/ |
|
|
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 <[hidden email]> 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/ |
Re: Annoying extra steps for enabling metrics
|
|
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 <[hidden email]> 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 <[hidden email]> > 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/ > > |
Re: Annoying extra steps for enabling metrics
|
|
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 < [hidden email]>: > 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 <[hidden email]> 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 <[hidden email] > > > > 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/ > > > > > |
|
|
In reply to this post by Valentin Kulichenko
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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> >> 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/ >> >> |
Re: Annoying extra steps for enabling metrics
|
|
Agree.
-Val On Fri, Dec 29, 2017 at 3:08 PM, Denis Magda <[hidden email]> 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 < > [hidden email]> 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 <[hidden email]> 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 <[hidden email] > > > >> 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/ > >> > >> > > |
|
|
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 <[hidden email]> wrote: > > Agree. > > -Val > > On Fri, Dec 29, 2017 at 3:08 PM, Denis Magda <[hidden email]> 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 < >> [hidden email]> 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 <[hidden email]> 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 <[hidden email] >>> >>>> 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/ >>>> >>>> >> >> |
Re: Annoying extra steps for enabling metrics
|
|
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 <[hidden email]>: > 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 < > [hidden email]> wrote: > > > > Agree. > > > > -Val > > > > On Fri, Dec 29, 2017 at 3:08 PM, Denis Magda <[hidden email]> 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 < > >> [hidden email]> 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 <[hidden email]> > 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 < > [hidden email] > >>> > >>>> 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/ > >>>> > >>>> > >> > >> > > |
Re: Annoying extra steps for enabling metrics
|
|
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 <[hidden email]>: > 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 <[hidden email]>: > >> 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 < >> [hidden email]> wrote: >> > >> > Agree. >> > >> > -Val >> > >> > On Fri, Dec 29, 2017 at 3:08 PM, Denis Magda <[hidden email]> 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 < >> >> [hidden email]> 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 <[hidden email]> >> 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 < >> [hidden email] >> >>> >> >>>> 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/ >> >>>> >> >>>> >> >> >> >> >> >> > |
«
Return to Apache Ignite Developers - Legacy Mail Archive
|
1 view|%1 views
| Free forum by Nabble | Edit this page |