Andrey, Yakov, An MBean for eviction policy is registered in GridCacheProcessor#prepare().
--AG 2017-02-09 18:53 GMT+03:00 Yakov Zhdanov <yzhda...@apache.org>: > Wow! This is the regression (from long ago version) if true. > > As far as having mbean to manage eviction policy on the fly - why not? This > is handy. > > -- > Yakov Zhdanov > > On Feb 9, 2017 9:09 PM, "Andrey Mashenkov" <andrey.mashen...@gmail.com> > wrote: > > > Folks, > > > > I've found no mention in ignite code where EvictionPolicy used as MBean > and > > it seems it is never registered as MBean. > > Is it really need to have MBean interfaces for EvictionPolicy > > implementations? > > > > > > > > On Wed, Feb 8, 2017 at 7:23 AM, Yakov Zhdanov <yzhda...@apache.org> > wrote: > > > > > +1 to Vladimir suggestion > > > > > > --Yakov > > > > > > 2017-02-07 20:50 GMT+07:00 Vladimir Ozerov <voze...@gridgain.com>: > > > > > > > Andrey, Valya, > > > > > > > > There is another problem here. What is we decide to add some existing > > > > setter method to MBean? If it has signature "T setSomething(...)", we > > > will > > > > not be able to do so. We need to understand how to deal with it, so > > that > > > > possible further improvements to MBean-s are not compromised. Any > > ideas? > > > > May be we should fully decouple MBeans into separate classes? > > > > > > > > E.g. instead of: > > > > FifoEvictionPolicy implements FifoEvictionPolicyMBean > > > > > > > > we will have > > > > FifoEvictionPolicy > > > > FifoEvictionPolicyMBeanImpl implements FifoEvictionPolicyMBean > > > > > > > > This way public API will be fully decoupled form JMX what seems > > > reasonable > > > > to me. Thoughts? > > > > > > > > On Tue, Feb 7, 2017 at 4:31 PM, Andrey Mashenkov < > > > > andrey.mashen...@gmail.com > > > > > wrote: > > > > > > > > > Val, > > > > > > > > > > void setBatchSize(int batchSize) > > > > > void setMaxMemorySize(long maxMemSize) > > > > > void setMaxSize(int max) > > > > > void setExcludePaths(Collection<String> excludePaths) > > > > > void setMaxBlocks(int maxBlocks) > > > > > void setParallelJobsNumber(int num) > > > > > void setWaitingJobsNumber(int num) > > > > > > > > > > https://ignite.apache.org/releases/1.8.0/javadoc/org/ > > > > > apache/ignite/cache/eviction/fifo/FifoEvictionPolicyMBean.html > > > > > https://ignite.apache.org/releases/1.8.0/javadoc/org/ > > > > > apache/ignite/cache/eviction/igfs/IgfsPerBlockLruEvictionPolicyM > > > > XBean.html > > > > > https://ignite.apache.org/releases/1.8.0/javadoc/org/ > > > > > apache/ignite/cache/eviction/lru/LruEvictionPolicyMBean.html > > > > > https://ignite.apache.org/releases/1.8.0/javadoc/org/ > > > > > apache/ignite/cache/eviction/sorted/SortedEvictionPolicyMBean.html > > > > > https://ignite.apache.org/releases/1.8.0/javadoc/org/ > > > > > apache/ignite/spi/collision/fifoqueue/FifoQueueCollisionSpiMBean. > > html > > > > > > > > > > On Tue, Feb 7, 2017 at 2:18 AM, Valentin Kulichenko < > > > > > valentin.kuliche...@gmail.com> wrote: > > > > > > > > > > > Andrey, > > > > > > > > > > > > Can you list all setters that we have on MBeans? > > > > > > > > > > > > -Val > > > > > > > > > > > > On Mon, Feb 6, 2017 at 2:21 PM, Andrey Mashenkov < > > > > > > andrey.mashen...@gmail.com > > > > > > > wrote: > > > > > > > > > > > > > Folks, > > > > > > > > > > > > > > Changing MBeans setters signature is bad idea. AOP tests failed > > on > > > TC > > > > > > with > > > > > > > this change. > > > > > > > > > > > > > > On Mon, Feb 6, 2017 at 11:21 AM, Vladimir Ozerov < > > > > voze...@gridgain.com > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > Val, > > > > > > > > > > > > > > > > Good catch! Can we try leaving SPIs and base methods > untouched? > > > > Will > > > > > it > > > > > > > API > > > > > > > > be consistent in this case? > > > > > > > > > > > > > > > > On Fri, Feb 3, 2017 at 10:22 PM, Valentin Kulichenko < > > > > > > > > valentin.kuliche...@gmail.com> wrote: > > > > > > > > > > > > > > > > > Folks, > > > > > > > > > > > > > > > > > > I tend to think that the problem is that we try to apply > > > 'builder > > > > > > > > approach' > > > > > > > > > to *ALL* setters. Let's approach this smarter. > > > > > > > > > > > > > > > > > > This approach is actually applicable only for configuration > > > > setters > > > > > > > > > available on public API, i.e. it's only about setters on > > > > > > > ***Configuration > > > > > > > > > classes and SPI *implementations*. For SPI interface > methods > > > like > > > > > > > > > 'CollisionSpi.setExternalCollisionListener' this makes no > > > > sense, I > > > > > > > would > > > > > > > > > not touch them. > > > > > > > > > > > > > > > > > > The only thing I still don't like is MBeans. Returning > > > something > > > > > > except > > > > > > > > > void on MBean interfaces look ugly, but without doing this > we > > > > will > > > > > > > break > > > > > > > > > API consistency on the implementation. Any ideas on how to > > > > approach > > > > > > > this? > > > > > > > > > > > > > > > > > > -Val > > > > > > > > > > > > > > > > > > On Fri, Feb 3, 2017 at 10:29 AM, Denis Magda < > > > dma...@apache.org> > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > Sorry, “public modifications” -> “public APIs” > > > > > > > > > > > > > > > > > > > > — > > > > > > > > > > Denis > > > > > > > > > > > > > > > > > > > > > On Feb 3, 2017, at 10:03 AM, Denis Magda < > > > dma...@apache.org> > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > Andrey, > > > > > > > > > > > > > > > > > > > > > > If the changes affect public modifications don’t forget > > to > > > > > update > > > > > > > > this > > > > > > > > > > page: > > > > > > > > > > > https://cwiki.apache.org/confluence/display/IGNITE/ > > > > > > > > > > Apache+Ignite+2.0+Migration+Guide < > > https://cwiki.apache.org/ > > > > > > > > > > confluence/display/IGNITE/Apache+Ignite+2.0+Migration+ > > Guide> > > > > > > > > > > > > > > > > > > > > > > — > > > > > > > > > > > Denis > > > > > > > > > > > > > > > > > > > > > >> On Feb 3, 2017, at 12:24 AM, Andrey Mashenkov < > > > > > > > > > > andrey.mashen...@gmail.com> wrote: > > > > > > > > > > >> > > > > > > > > > > >> Vladimir, > > > > > > > > > > >> > > > > > > > > > > >> Ok. I'll go ahead with changing SPI interfaces and run > > TC > > > > > test. > > > > > > > > > > >> I think, it would be better to have this branch merged > > to > > > > > master > > > > > > > as > > > > > > > > 2 > > > > > > > > > > >> separate commits at least. > > > > > > > > > > >> And may be we should make changes of SPI interfaces in > > > > > separate > > > > > > > Jira > > > > > > > > > > >> ticket? > > > > > > > > > > >> > > > > > > > > > > >> On Fri, Feb 3, 2017 at 11:08 AM, Vladimir Ozerov < > > > > > > > > > voze...@gridgain.com> > > > > > > > > > > >> wrote: > > > > > > > > > > >> > > > > > > > > > > >>> Andrey, > > > > > > > > > > >>> > > > > > > > > > > >>> This is very important change from usability > > standpoint. > > > > But > > > > > my > > > > > > > > main > > > > > > > > > > >>> concern is changes to SPI *interfaces*. If we do so > > users > > > > who > > > > > > > > > > implemented > > > > > > > > > > >>> custom SPIs will have broken compatibility. On the > > other > > > > > hand, > > > > > > I > > > > > > > > > doubt > > > > > > > > > > >>> there will be too much affected users, and we break > > > > > compilation > > > > > > > in > > > > > > > > AI > > > > > > > > > > 2.0 > > > > > > > > > > >>> anyway. So looks like we can go ahead with it. > > > > > > > > > > >>> > > > > > > > > > > >>> Thoughts? > > > > > > > > > > >>> > > > > > > > > > > >>> On Fri, Feb 3, 2017 at 7:46 AM, Valentin Kulichenko < > > > > > > > > > > >>> valentin.kuliche...@gmail.com> wrote: > > > > > > > > > > >>> > > > > > > > > > > >>>> My only concern is MBean interfaces. These are not > > > called > > > > > from > > > > > > > > code, > > > > > > > > > > but > > > > > > > > > > >>>> from MBean viewers, and I'm not sure setters not > > > returning > > > > > > voids > > > > > > > > > will > > > > > > > > > > be > > > > > > > > > > >>>> properly treated as setters by these viewers. This > > needs > > > > to > > > > > be > > > > > > > > > > checked. > > > > > > > > > > >>>> > > > > > > > > > > >>>> -Val > > > > > > > > > > >>>> > > > > > > > > > > >>>> On Thu, Feb 2, 2017 at 8:32 PM, Andrey Mashenkov < > > > > > > > > > > >>>> andrey.mashen...@gmail.com > > > > > > > > > > >>>>> wrote: > > > > > > > > > > >>>> > > > > > > > > > > >>>>> Val, > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> Yes, you are right. I don't think we should care > > about > > > > > > > > compilation > > > > > > > > > > >>>>> error on user side, as we break compatibility with > > > > previous > > > > > > > > > versions. > > > > > > > > > > >>>>> But we talk about public interfaces and may be > > someone > > > > has > > > > > > some > > > > > > > > > cons > > > > > > > > > > >>>>> or suggestions? > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> On Fri, Feb 3, 2017 at 5:31 AM, Valentin > Kulichenko < > > > > > > > > > > >>>>> valentin.kuliche...@gmail.com> wrote: > > > > > > > > > > >>>>> > > > > > > > > > > >>>>>> Andrey, > > > > > > > > > > >>>>>> > > > > > > > > > > >>>>>> In which case compatibility is broken? If there > is a > > > > > method > > > > > > > that > > > > > > > > > > >>>> returns > > > > > > > > > > >>>>>> void and you change it to return some type, it > > doesn't > > > > > break > > > > > > > > > > >>> anything, > > > > > > > > > > >>>>>> because currently nobody can assign the result of > > this > > > > > > method > > > > > > > > to a > > > > > > > > > > >>>>>> variable. I.e. in old code the returned value will > > be > > > > > always > > > > > > > > > > ignored, > > > > > > > > > > >>>>>> therefore it can be of any type. > > > > > > > > > > >>>>>> > > > > > > > > > > >>>>>> Is there anything else that I'm missing? > > > > > > > > > > >>>>>> > > > > > > > > > > >>>>>> -Val > > > > > > > > > > >>>>>> > > > > > > > > > > >>>>>> On Thu, Feb 2, 2017 at 3:49 AM, Andrey Mashenkov < > > > > > > > > > > >>>>>> andrey.mashen...@gmail.com > > > > > > > > > > >>>>>>> wrote: > > > > > > > > > > >>>>>> > > > > > > > > > > >>>>>>> Hi Igniters, > > > > > > > > > > >>>>>>> > > > > > > > > > > >>>>>>> > > > > > > > > > > >>>>>>> I'm working on IGNITE-4564 [1] to make our > > > > configuration > > > > > > > > classes > > > > > > > > > > >>> and > > > > > > > > > > >>>>> SPI > > > > > > > > > > >>>>>>> classes more convenient. > > > > > > > > > > >>>>>>> > > > > > > > > > > >>>>>>> There is no problem to change return type in > setter > > > > > method > > > > > > > > > > >>> signatures > > > > > > > > > > >>>>>>> and override methods in child child classes to > make > > > > them > > > > > > > return > > > > > > > > > > >>> more > > > > > > > > > > >>>>>>> accurate type. > > > > > > > > > > >>>>>>> > > > > > > > > > > >>>>>>> But, I found that we have set methods in some of > > our > > > > > > > interfaces > > > > > > > > > and > > > > > > > > > > >>>>>>> changing its signature may broke compatibility > with > > > > user > > > > > > > > > > >>>>> implementations. > > > > > > > > > > >>>>>>> > > > > > > > > > > >>>>>>> Here are example interfaces with setters: > > > > > > > > > > >>>>>>> org.apache.ignite.cache.eviction.fifo. > > > > > > > FifoEvictionPolicyMBean > > > > > > > > > > >>>>>>> org.apache.ignite.cache.eviction.igfs. > > > > > > > > > > >>> IgfsPerBlockLruEvictionPolicyM > > > > > > > > > > >>>>>> XBean > > > > > > > > > > >>>>>>> org.apache.ignite.cache.eviction.lru. > > > > > > LruEvictionPolicyMBean > > > > > > > > > > >>>>>>> org.apache.ignite.cache.eviction.sorted. > > > > > > > > > SortedEvictionPolicyMBean > > > > > > > > > > >>>>>>> org.apache.ignite.spi.checkpoint.CheckpointSpi > > > > > > > > > > >>>>>>> org.apache.ignite.spi.collision.CollisionSpi > > > > > > > > > > >>>>>>> org.apache.ignite.spi.collision.fifoqueue. > > > > > > > > > > >>> FifoQueueCollisionSpiMBean > > > > > > > > > > >>>>>>> > > > > > > > > > > >>>>>>> However we have interfaces with NO setters > > > > > > > > > > >>>>>>> org.apache.ignite.spi.loadbalancing.adaptive. > > > > > > > > > > >>>>>>> AdaptiveLoadBalancingSpiMBean. > > > > > > > > > > >>>>>>> > > > > > > > > > > >>>>>>> What can we do with it? > > > > > > > > > > >>>>>>> Change signature of setters without regarding > > > > > > compatibility? > > > > > > > Or > > > > > > > > > may > > > > > > > > > > >>>> be > > > > > > > > > > >>>>> it > > > > > > > > > > >>>>>>> is possible to remove setters from some > interfaces? > > > > > > > > > > >>>>>>> > > > > > > > > > > >>>>>>> Thought? > > > > > > > > > > >>>>>>> > > > > > > > > > > >>>>>>> > > > > > > > > > > >>>>>>> [1] https://issues.apache.org/ > > > jira/browse/IGNITE-4564 > > > > > > > > > > >>>>>>> > > > > > > > > > > >>>>>> > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> -- > > > > > > > > > > >>>>> С уважением, > > > > > > > > > > >>>>> Машенков Андрей Владимирович > > > > > > > > > > >>>>> Тел. +7-921-932-61-82 > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> Best regards, > > > > > > > > > > >>>>> Andrey V. Mashenkov > > > > > > > > > > >>>>> Cerr: +7-921-932-61-82 > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > > >> -- > > > > > > > > > > >> С уважением, > > > > > > > > > > >> Машенков Андрей Владимирович > > > > > > > > > > >> Тел. +7-921-932-61-82 > > > > > > > > > > >> > > > > > > > > > > >> Best regards, > > > > > > > > > > >> Andrey V. Mashenkov > > > > > > > > > > >> Cerr: +7-921-932-61-82 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > С уважением, > > > > > > > Машенков Андрей Владимирович > > > > > > > Тел. +7-921-932-61-82 > > > > > > > > > > > > > > Best regards, > > > > > > > Andrey V. Mashenkov > > > > > > > Cerr: +7-921-932-61-82 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > Best regards, > > > > > Andrey V. Mashenkov > > > > > > > > > > > > > > > > > > > > -- > > Best regards, > > Andrey V. Mashenkov > > >
