You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ignite.apache.org by Nikita Amelchev <ns...@gmail.com> on 2020/01/16 13:40:46 UTC
Re: Unclear to use methods in the DiscoverySpiCustomMessage interface
Hi, Igniters.
I have created the issue [1] and prepared PR [2] to remove the
stopProcess method from interfaces.
I have removed the method from the internal interface
(DiscoveryCustomMessage) and marked deprecated in the public interface
(DiscoverySpiCustomMessage).
Could anyone review my changes, please?
[1] https://issues.apache.org/jira/browse/IGNITE-12400
[2] https://github.com/apache/ignite/pull/7268/files
пт, 25 окт. 2019 г. в 11:24, Nikolay Izhikov <ni...@apache.org>:
>
> Nikita, please, feel free:
>
> 1. To create a ticket to remove `stopProcess` method from the interface.
> This improvement looks obvious to me.
> Seems we can implement it easily.
>
> 2. To make all Discovery messages immutable.
> This one more complex.
> I think we should discuss every single mutable message separately.
> Lets' do it in a separate discussion.
>
> WDYT?
>
> В Пт, 25/10/2019 в 11:10 +0300, Ivan Pavlukhin пишет:
> > There are at least a couple more mutable messages:
> > StartRoutineDiscoveryMessage and SchemaProposeDiscoveryMessage.
> >
> > I agree that here that making all messages immutable are highly
> > desired (e.g. it will allow implementation of new discovery
> > protocols). But so far I do not understand why marker interface is a
> > better approach. Pavel, could you please elaborate?
> >
> > чт, 24 окт. 2019 г. в 17:55, Pavel Kovalenko <jo...@gmail.com>:
> > >
> > > Hi Nikita,
> > >
> > > 1. As I can understand, this functionality is needed if a custom message is
> > > needed to be processed only on the coordinator node. I'm not following for
> > > what purposes it was implemented and I see the real usage only in tests.
> > > As far as this functionality is not used in production code, I suggest to
> > > remove it.
> > > 2. This method needs if we want to perform a message mutation during
> > > sending it over the ring. Each node after message receive can mutate its
> > > state and resend to another node. I know only 2 mechanisms where this
> > > functionality is needed:
> > > binary metadata propagation and cluster-wide metrics collecting. From my
> > > side of view, this is poor approach and we should avoid discovery messages
> > > mutation and use only immutable messages. It can be removed from the custom
> > > message interface and marker interface should be introduced instead.
> > >
> > > чт, 24 окт. 2019 г. в 16:35, Nikita Amelchev <ns...@gmail.com>:
> > >
> > > > Hi, Igniters.
> > > >
> > > > The discovery custom message interface has two methods the use of
> > > > which is not clear to developers:
> > > >
> > > > 1. The stopProcess() method. Currently, it works only if the zookeeper
> > > > discovery configured. It doesn't work in TcpDiscoverySpi. I did not
> > > > find any usages of this method except tests. I suggest to remove it
> > > > from the discovery custom message interface.
> > > >
> > > > 2. The isMutable() method. It works in TcpDiscoverySpi but not in the
> > > > zookeeper SPI. Using this method is opaque to the developer when he
> > > > wants to add new features to the project. In general, it cannot be
> > > > used because may not be supported depending on the configuration.
> > > > Perhaps some functionality that uses it is not working correctly with
> > > > zookeeper configured. It seems we should avoid it in the custom
> > > > message interface too.
> > > >
> > > > WDYT?
> > > >
> > > > --
> > > > Best wishes,
> > > > Amelchev Nikita
> > > >
> >
> >
> >
--
Best wishes,
Amelchev Nikita
Re: Unclear to use methods in the DiscoverySpiCustomMessage interface
Posted by Nikita Amelchev <ns...@gmail.com>.
Hi, Igniters.
Could someone review my changes, please? [1] [2]
I have removed the method from the internal interface
(DiscoveryCustomMessage) and marked deprecated in the public interface
(DiscoverySpiCustomMessage).
[1] https://issues.apache.org/jira/browse/IGNITE-12400
[2] https://github.com/apache/ignite/pull/7268/files
чт, 16 янв. 2020 г. в 16:40, Nikita Amelchev <ns...@gmail.com>:
>
> Hi, Igniters.
>
> I have created the issue [1] and prepared PR [2] to remove the
> stopProcess method from interfaces.
>
> I have removed the method from the internal interface
> (DiscoveryCustomMessage) and marked deprecated in the public interface
> (DiscoverySpiCustomMessage).
>
> Could anyone review my changes, please?
>
> [1] https://issues.apache.org/jira/browse/IGNITE-12400
> [2] https://github.com/apache/ignite/pull/7268/files
>
> пт, 25 окт. 2019 г. в 11:24, Nikolay Izhikov <ni...@apache.org>:
> >
> > Nikita, please, feel free:
> >
> > 1. To create a ticket to remove `stopProcess` method from the interface.
> > This improvement looks obvious to me.
> > Seems we can implement it easily.
> >
> > 2. To make all Discovery messages immutable.
> > This one more complex.
> > I think we should discuss every single mutable message separately.
> > Lets' do it in a separate discussion.
> >
> > WDYT?
> >
> > В Пт, 25/10/2019 в 11:10 +0300, Ivan Pavlukhin пишет:
> > > There are at least a couple more mutable messages:
> > > StartRoutineDiscoveryMessage and SchemaProposeDiscoveryMessage.
> > >
> > > I agree that here that making all messages immutable are highly
> > > desired (e.g. it will allow implementation of new discovery
> > > protocols). But so far I do not understand why marker interface is a
> > > better approach. Pavel, could you please elaborate?
> > >
> > > чт, 24 окт. 2019 г. в 17:55, Pavel Kovalenko <jo...@gmail.com>:
> > > >
> > > > Hi Nikita,
> > > >
> > > > 1. As I can understand, this functionality is needed if a custom message is
> > > > needed to be processed only on the coordinator node. I'm not following for
> > > > what purposes it was implemented and I see the real usage only in tests.
> > > > As far as this functionality is not used in production code, I suggest to
> > > > remove it.
> > > > 2. This method needs if we want to perform a message mutation during
> > > > sending it over the ring. Each node after message receive can mutate its
> > > > state and resend to another node. I know only 2 mechanisms where this
> > > > functionality is needed:
> > > > binary metadata propagation and cluster-wide metrics collecting. From my
> > > > side of view, this is poor approach and we should avoid discovery messages
> > > > mutation and use only immutable messages. It can be removed from the custom
> > > > message interface and marker interface should be introduced instead.
> > > >
> > > > чт, 24 окт. 2019 г. в 16:35, Nikita Amelchev <ns...@gmail.com>:
> > > >
> > > > > Hi, Igniters.
> > > > >
> > > > > The discovery custom message interface has two methods the use of
> > > > > which is not clear to developers:
> > > > >
> > > > > 1. The stopProcess() method. Currently, it works only if the zookeeper
> > > > > discovery configured. It doesn't work in TcpDiscoverySpi. I did not
> > > > > find any usages of this method except tests. I suggest to remove it
> > > > > from the discovery custom message interface.
> > > > >
> > > > > 2. The isMutable() method. It works in TcpDiscoverySpi but not in the
> > > > > zookeeper SPI. Using this method is opaque to the developer when he
> > > > > wants to add new features to the project. In general, it cannot be
> > > > > used because may not be supported depending on the configuration.
> > > > > Perhaps some functionality that uses it is not working correctly with
> > > > > zookeeper configured. It seems we should avoid it in the custom
> > > > > message interface too.
> > > > >
> > > > > WDYT?
> > > > >
> > > > > --
> > > > > Best wishes,
> > > > > Amelchev Nikita
> > > > >
> > >
> > >
> > >
>
>
>
> --
> Best wishes,
> Amelchev Nikita
--
Best wishes,
Amelchev Nikita