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 2019/10/24 13:35:35 UTC
Unclear to use methods in the DiscoverySpiCustomMessage interface
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
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
Re: Unclear to use methods in the DiscoverySpiCustomMessage interface
Posted by 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
Re: Unclear to use methods in the DiscoverySpiCustomMessage
interface
Posted by 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
> > >
>
>
>
Re: Unclear to use methods in the DiscoverySpiCustomMessage interface
Posted by Ivan Pavlukhin <vo...@gmail.com>.
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 regards,
Ivan Pavlukhin
Re: Unclear to use methods in the DiscoverySpiCustomMessage interface
Posted by 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
>