From dev-return-30573-archive-asf-public=cust-asf.ponee.io@ignite.apache.org Thu Feb 1 10:46:58 2018 Return-Path: X-Original-To: archive-asf-public@eu.ponee.io Delivered-To: archive-asf-public@eu.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by mx-eu-01.ponee.io (Postfix) with ESMTP id 3496E180652 for ; Thu, 1 Feb 2018 10:46:58 +0100 (CET) Received: by cust-asf.ponee.io (Postfix) id 242C4160C44; Thu, 1 Feb 2018 09:46:58 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 1F650160C26 for ; Thu, 1 Feb 2018 10:46:56 +0100 (CET) Received: (qmail 81497 invoked by uid 500); 1 Feb 2018 09:46:56 -0000 Mailing-List: contact dev-help@ignite.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@ignite.apache.org Delivered-To: mailing list dev@ignite.apache.org Received: (qmail 81485 invoked by uid 99); 1 Feb 2018 09:46:55 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd3-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 01 Feb 2018 09:46:55 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd3-us-west.apache.org (ASF Mail Server at spamd3-us-west.apache.org) with ESMTP id 0E5B2199702 for ; Thu, 1 Feb 2018 09:46:55 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd3-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 3.292 X-Spam-Level: *** X-Spam-Status: No, score=3.292 tagged_above=-999 required=6.31 tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, HTML_MESSAGE=2, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, SPF_PASS=-0.001, URI_HEX=1.313] autolearn=disabled Authentication-Results: spamd3-us-west.apache.org (amavisd-new); dkim=pass (2048-bit key) header.d=gridgain-com.20150623.gappssmtp.com Received: from mx1-lw-us.apache.org ([10.40.0.8]) by localhost (spamd3-us-west.apache.org [10.40.0.10]) (amavisd-new, port 10024) with ESMTP id xSdVXAHZUL99 for ; Thu, 1 Feb 2018 09:46:51 +0000 (UTC) Received: from mail-wm0-f48.google.com (mail-wm0-f48.google.com [74.125.82.48]) by mx1-lw-us.apache.org (ASF Mail Server at mx1-lw-us.apache.org) with ESMTPS id B81B05F2C5 for ; Thu, 1 Feb 2018 09:46:50 +0000 (UTC) Received: by mail-wm0-f48.google.com with SMTP id i186so4497954wmi.4 for ; Thu, 01 Feb 2018 01:46:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gridgain-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to; bh=JZ2YM84g3ZvYc2ygzCPZYPqHrf10jSQh9mw7BhQtTpQ=; b=XhAkgMRyqepu65fPfHPptgWjNEKYnpZzIOE8MySLgHLXsbVQklh6UbGcORVOrbSFwL h20h8HmTbsm/yuDgDlLW2YvRfrePx8LeEdS2sM72tsrh39BrEFKO6EDSW8Y6EuQ9zJEG gdwEXV50Nkjb+TWNzvdyUxcqIpK8RN++f9WQcBFeCFQ4abL2JLgQO+ZKJuoOwkywFj8K t2aGqyO7HsnBbHmMlqnfqolc04XyXyUy9ywB4ZSCNhGxHWnS2Y4s1P2bLDix0nt9e0X1 UbX3QtKfFHwhcxhH5+aspTJaZStzobM4pY9g+JFA9JVKfmbMZ9LW/SVE274kPwUxrQ7B xcOQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to; bh=JZ2YM84g3ZvYc2ygzCPZYPqHrf10jSQh9mw7BhQtTpQ=; b=XOwXUPtAYc4jIedmVz49oB4N1hy9FZQG2yuh55cZxUpZTDJOX+C/AYNRmYjKNUg1w2 yscnnIJB0BcWW/Z3JMDRtAL67y/vgAz4zgFRw3ciNA/FrZE7DrKFyCtai+0OCNAW3OZm 3Q3rgfkOxCHJFADcXUjI2fecugQnX5mBRong0clL8+dKliIRs4f+bF1aDf/RZhPTFzRX LyZhyLk8pis89+npNaKFDtE8pHFdLgyHJTWtud2S0RN8FKmSH+mVpCV/kF+aJaci5L30 /tEUc1QSymIt93a5mfWULnnAmykHQ9whe2Crm1t1OZeqIq0211ZlS8hkiAR3W0+KjS++ 9jEQ== X-Gm-Message-State: AKwxytch3BtyCtqsj5/QFG9xjSfI3D9rlswqcQcjpZakAxLlgSNezCYU jhe+1DK1+Y+9EQs3D40IRvyHFAohlluwAFdcsIc9lF4= X-Google-Smtp-Source: AH8x225GE974dHrZA/NCSdN5YzeEIZ9aVZ9DZW9+FgsMnk2DH99mazOYUT7IPRv9JJOabb78puEzECygQjoaQNdtKbQ= X-Received: by 10.28.156.81 with SMTP id f78mr24592765wme.131.1517478409334; Thu, 01 Feb 2018 01:46:49 -0800 (PST) MIME-Version: 1.0 Received: by 10.223.198.193 with HTTP; Thu, 1 Feb 2018 01:46:48 -0800 (PST) In-Reply-To: References: <3AB68BD7-05C1-4CE8-99C9-CF52958A712F@apache.org> <1960D45D-E959-43B9-B21A-5D7CEB212123@apache.org> <8727CBCA-6CC0-463C-8367-73514EE0477B@apache.org> <189CD443-650C-4F1F-8015-780FE7280292@apache.org> <1514548297873-0.post@n4.nabble.com> <8F61AD92-C891-48A0-A682-56B8146ED556@apache.org> <3CEBA111-BF60-4F53-80BE-551E435AF816@apache.org> From: Anton Vinogradov Date: Thu, 1 Feb 2018 12:46:48 +0300 Message-ID: Subject: Re: Annoying extra steps for enabling metrics To: dev@ignite.apache.org Content-Type: multipart/alternative; boundary="001a114b2dc28b0b410564237900" --001a114b2dc28b0b410564237900 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Folks, .setEventsDisabled looks to be a good solution, since we will always call it like .setEventsDisabled(true). Calling .setEventsEnabled(false), since true is a default, looks odd to me. On Wed, Jan 31, 2018 at 11:02 PM, Valentin Kulichenko < valentin.kulichenko@gmail.com> wrote: > Alex, > > As a workaround, you can use boxed Boolean as a field type, and then retu= rn > true from getEventsEnabled in case it's null. Will this work? > > -Val > > On Wed, Jan 31, 2018 at 7:31 AM, Alex Plehanov > wrote: > > > 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 : > > > > > 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 : > > > > > >> Good, closed the original ticket. > > >> > > >> Alex P, do you have time to work on IGNITE-7346 instead to address t= he > > >> issue with the cache events per cache in 2.4 release? > > >> > > >> =E2=80=94 > > >> Denis > > >> > > >> > On Dec 29, 2017, at 3:10 PM, Valentin Kulichenko < > > >> valentin.kulichenko@gmail.com> wrote: > > >> > > > >> > Agree. > > >> > > > >> > -Val > > >> > > > >> > On Fri, Dec 29, 2017 at 3:08 PM, Denis Magda > > 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 use= rs > > >> >> attention. > > >> >> > > >> >> As for the cache events, it=E2=80=99s an oversight that there is = no > parameter > > >> that > > >> >> enables/disables the events per cache. Let=E2=80=99s 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 =E2=80=98false=E2=80= =99 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? > > >> >> > > >> >> =E2=80=94 > > >> >> Denis > > >> >> > > >> >> > > >> >> > > >> >>> On Dec 29, 2017, at 11:10 AM, Valentin Kulichenko < > > >> >> valentin.kulichenko@gmail.com> wrote: > > >> >>> > > >> >>> Guys, > > >> >>> > > >> >>> I'm not sure what issue we're trying to solve. Basically, we hav= e > > >> 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 controlle= d > 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 confirmin= g > > >> this: > > >> >>> https://gist.github.com/vkulichenko/ > 54043c7b75c69eca5515e7d7692b52 > > 76 > > >> >>> 3. Querying cache events via IgniteEvents#*Query methods. This i= s > > the > > >> >> ONLY > > >> >>> piece that requires MemoryEventStorageSpi to be configured. Sinc= e > > >> >> 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 lo= ng > > >> 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 cac= he > > >> 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 > > >> 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? > > >> >>>> > > >> >>>> =E2=80=94 > > >> >>>> Denis > > >> >>>> > > >> >>>>> On Dec 29, 2017, at 3:51 AM, Alexey Plekhanov < > > >> plehanov.alex@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? > > >> >>>>> > > >> >>>>> =E2=80=A2 If we enable statistics on caches, this does not nec= essarily > > mean, > > >> >>>> that > > >> >>>>> we wanted to get some events, we can enable statistics for oth= er > > >> >> reasons. > > >> >>>>> Conversely, not all events need statistics to be enabled on > > caches. > > >> So > > >> >> we > > >> >>>>> can=E2=80=99t bind statistics flag to events (subscribe to eve= nts when > > >> >>>> statistics is > > >> >>>>> enabled or enable statistics, when subscribing to events) > > >> >>>>> =E2=80=A2 We can=E2=80=99t set events of interest, when we set= not a dummy > > >> >>>> EventsStorageSpi, > > >> >>>>> because we don=E2=80=99t know which events are interesting. > > >> >>>>> =E2=80=A2 When we set events of interest, it=E2=80=99s not nec= essary, that these > > >> events > > >> >>>> will > > >> >>>>> be monitored using EventsStorageSpi, we can also subscribe to > > >> events by > > >> >>>>> events listeners, in this case EventsStorageSpi don=E2=80=99t = 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=E2=80=99s simplifying the metrics as a part of this ticke= t: > > >> >>>>>> https://issues.apache.org/jira/browse/IGNITE-5796 > > >> >>>>>> <https://issues.apache.org/jira/browse/IGNITE-5796> > > >> >>>>>> > > >> >>>>>> Expanded its scope. > > >> >>>>>> > > >> >>>>>> =E2=80=94 > > >> >>>>>> 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 on= e > > more > > >> >>>> flag! > > >> >>>>>>>>> > > >> >>>>>>>>> > > >> >>>>>> > > >> >>>>>>>> > > >> >>>>>>>> > > >> >>>>>>>> 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=E2=80=99s 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/ > > >> >>>> > > >> >>>> > > >> >> > > >> >> > > >> > > >> > > > > > > --001a114b2dc28b0b410564237900--