ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrey Mashenkov <andrey.mashen...@gmail.com>
Subject Re: Ensure that builder approach is used for all setters in public API
Date Tue, 07 Feb 2017 13:31:44 GMT
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/IgfsPerBlockLruEvictionPolicyMXBean.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.kulichenko@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.mashenkov@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 <vozerov@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.kulichenko@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 <dmagda@apache.org>
> > wrote:
> > > >
> > > > > Sorry, “public modifications” -> “public APIs”
> > > > >
> > > > > —
> > > > > Denis
> > > > >
> > > > > > On Feb 3, 2017, at 10:03 AM, Denis Magda <dmagda@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.mashenkov@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 <
> > > > vozerov@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.kulichenko@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.mashenkov@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.kulichenko@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.mashenkov@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

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message