Deprecate force server mode for clients.

[Andrew Mashenkov]

Hi Igniters,

I've found that KernalContext.clientNode() and TcpDusciveryNode.isClient()
means different and it looks confusing.

1. The first one return true if "node is configured as client" and false
otherwise,
2. The second one can return false if server mode is forced for client (via
TcpDiscoverySpi.setForceServerMode(true)) ,
 so actually true value means "a client node is out of the ring".

Thus, TcpDiscoveryNode.isClient() semantic is broken and  we can get
confusing results on client node with forced server mode,
when kernalContext.clientNode() return true and
kernalContex.localNode().isClient() return false.

Also, we have a number of places in code where node.isClient() method is
used.

My suggestion is
1. Deprecate force server mode and plan to remove it to next major release.
2. Fix TcpDiscoveryNode,isClient() semantic e.g. to use node attributes to
make ClusterNode.isClient method consistent with node configuration.
2. Add isRingNode (possibly package private) with current
TcpDiscoveryNode.isClient() semantic to preserve compatibility.

Any pros or cons?

--
Best regards,
Andrey V. Mashenkov

[Pavel Kovalenko]

Andrey,

Huge +1 for that.
"Force server mode" increases the complexity of understanding how cluster
in ring mode works without real benefits.
If a user wants to start client node first before server nodes have
started, he can adjust timeouts on connection and spin till a client is
connected. Ability to join client node to topology without server nodes
looks dubious.
As we have thin clients and Zookeeper Discovery SPI this mode looks like a
crutch and should be removed.

2018-08-09 12:41 GMT+03:00 Andrey Mashenkov <[hidden email]>:

> Hi Igniters,
>
> I've found that KernalContext.clientNode() and TcpDusciveryNode.isClient()
> means different and it looks confusing.
>
> 1. The first one return true if "node is configured as client" and false
> otherwise,
> 2. The second one can return false if server mode is forced for client (via
> TcpDiscoverySpi.setForceServerMode(true)) ,
>  so actually true value means "a client node is out of the ring".
>
> Thus, TcpDiscoveryNode.isClient() semantic is broken and  we can get
> confusing results on client node with forced server mode,
> when kernalContext.clientNode() return true and
> kernalContex.localNode().isClient() return false.
>
> Also, we have a number of places in code where node.isClient() method is
> used.
>
> My suggestion is
> 1. Deprecate force server mode and plan to remove it to next major release.
> 2. Fix TcpDiscoveryNode,isClient() semantic e.g. to use node attributes to
> make ClusterNode.isClient method consistent with node configuration.
> 2. Add isRingNode (possibly package private) with current
> TcpDiscoveryNode.isClient() semantic to preserve compatibility.
>
> Any pros or cons?
>
> --
> Best regards,
> Andrey V. Mashenkov
>

[Nikolay Izhikov-2]

+1 from me.

В Чт, 09/08/2018 в 12:51 +0300, Pavel Kovalenko пишет:

> Andrey,
>
> Huge +1 for that.
> "Force server mode" increases the complexity of understanding how cluster
> in ring mode works without real benefits.
> If a user wants to start client node first before server nodes have
> started, he can adjust timeouts on connection and spin till a client is
> connected. Ability to join client node to topology without server nodes
> looks dubious.
> As we have thin clients and Zookeeper Discovery SPI this mode looks like a
> crutch and should be removed.
>
> 2018-08-09 12:41 GMT+03:00 Andrey Mashenkov <[hidden email]>:
>
> > Hi Igniters,
> >
> > I've found that KernalContext.clientNode() and TcpDusciveryNode.isClient()
> > means different and it looks confusing.
> >
> > 1. The first one return true if "node is configured as client" and false
> > otherwise,
> > 2. The second one can return false if server mode is forced for client (via
> > TcpDiscoverySpi.setForceServerMode(true)) ,
> >  so actually true value means "a client node is out of the ring".
> >
> > Thus, TcpDiscoveryNode.isClient() semantic is broken and  we can get
> > confusing results on client node with forced server mode,
> > when kernalContext.clientNode() return true and
> > kernalContex.localNode().isClient() return false.
> >
> > Also, we have a number of places in code where node.isClient() method is
> > used.
> >
> > My suggestion is
> > 1. Deprecate force server mode and plan to remove it to next major release.
> > 2. Fix TcpDiscoveryNode,isClient() semantic e.g. to use node attributes to
> > make ClusterNode.isClient method consistent with node configuration.
> > 2. Add isRingNode (possibly package private) with current
> > TcpDiscoveryNode.isClient() semantic to preserve compatibility.
> >
> > Any pros or cons?
> >
> > --
> > Best regards,
> > Andrey V. Mashenkov
> >

[Andrew Mashenkov]

Thanks.
I've created a ticket [1]  for this issue.

[1] https://issues.apache.org/jira/browse/IGNITE-9241

On Thu, Aug 9, 2018 at 4:23 PM Nikolay Izhikov <[hidden email]> wrote:

> +1 from me.
>
> В Чт, 09/08/2018 в 12:51 +0300, Pavel Kovalenko пишет:
> > Andrey,
> >
> > Huge +1 for that.
> > "Force server mode" increases the complexity of understanding how cluster
> > in ring mode works without real benefits.
> > If a user wants to start client node first before server nodes have
> > started, he can adjust timeouts on connection and spin till a client is
> > connected. Ability to join client node to topology without server nodes
> > looks dubious.
> > As we have thin clients and Zookeeper Discovery SPI this mode looks like
> a
> > crutch and should be removed.
> >
> > 2018-08-09 12:41 GMT+03:00 Andrey Mashenkov <[hidden email]
> >:
> >
> > > Hi Igniters,
> > >
> > > I've found that KernalContext.clientNode() and
> TcpDusciveryNode.isClient()
> > > means different and it looks confusing.
> > >
> > > 1. The first one return true if "node is configured as client" and
> false
> > > otherwise,
> > > 2. The second one can return false if server mode is forced for client
> (via
> > > TcpDiscoverySpi.setForceServerMode(true)) ,
> > >  so actually true value means "a client node is out of the ring".
> > >
> > > Thus, TcpDiscoveryNode.isClient() semantic is broken and  we can get
> > > confusing results on client node with forced server mode,
> > > when kernalContext.clientNode() return true and
> > > kernalContex.localNode().isClient() return false.
> > >
> > > Also, we have a number of places in code where node.isClient() method
> is
> > > used.
> > >
> > > My suggestion is
> > > 1. Deprecate force server mode and plan to remove it to next major
> release.
> > > 2. Fix TcpDiscoveryNode,isClient() semantic e.g. to use node
> attributes to
> > > make ClusterNode.isClient method consistent with node configuration.
> > > 2. Add isRingNode (possibly package private) with current
> > > TcpDiscoveryNode.isClient() semantic to preserve compatibility.
> > >
> > > Any pros or cons?
> > >
> > > --
> > > Best regards,
> > > Andrey V. Mashenkov
> > >

--
Best regards,
Andrey V. Mashenkov

[Eduard Shangareev]

Andrey,

It was discussed already here [1].

And the fix is almost merged.

[1]
http://apache-ignite-developers.2346864.n4.nabble.com/Issues-with-forceServerMode-on-clients-and-unclear-contract-of-isClient-isClientMode-methods-td31152.html

On Mon, Aug 13, 2018 at 9:07 AM Andrey Mashenkov <[hidden email]>
wrote:

> Thanks.
> I've created a ticket [1]  for this issue.
>
> [1] https://issues.apache.org/jira/browse/IGNITE-9241
>
> On Thu, Aug 9, 2018 at 4:23 PM Nikolay Izhikov <[hidden email]>
> wrote:
>
> > +1 from me.
> >
> > В Чт, 09/08/2018 в 12:51 +0300, Pavel Kovalenko пишет:
> > > Andrey,
> > >
> > > Huge +1 for that.
> > > "Force server mode" increases the complexity of understanding how
> cluster
> > > in ring mode works without real benefits.
> > > If a user wants to start client node first before server nodes have
> > > started, he can adjust timeouts on connection and spin till a client is
> > > connected. Ability to join client node to topology without server nodes
> > > looks dubious.
> > > As we have thin clients and Zookeeper Discovery SPI this mode looks
> like
> > a
> > > crutch and should be removed.
> > >
> > > 2018-08-09 12:41 GMT+03:00 Andrey Mashenkov <
> [hidden email]
> > >:
> > >
> > > > Hi Igniters,
> > > >
> > > > I've found that KernalContext.clientNode() and
> > TcpDusciveryNode.isClient()
> > > > means different and it looks confusing.
> > > >
> > > > 1. The first one return true if "node is configured as client" and
> > false
> > > > otherwise,
> > > > 2. The second one can return false if server mode is forced for
> client
> > (via
> > > > TcpDiscoverySpi.setForceServerMode(true)) ,
> > > >  so actually true value means "a client node is out of the ring".
> > > >
> > > > Thus, TcpDiscoveryNode.isClient() semantic is broken and  we can get
> > > > confusing results on client node with forced server mode,
> > > > when kernalContext.clientNode() return true and
> > > > kernalContex.localNode().isClient() return false.
> > > >
> > > > Also, we have a number of places in code where node.isClient() method
> > is
> > > > used.
> > > >
> > > > My suggestion is
> > > > 1. Deprecate force server mode and plan to remove it to next major
> > release.
> > > > 2. Fix TcpDiscoveryNode,isClient() semantic e.g. to use node
> > attributes to
> > > > make ClusterNode.isClient method consistent with node configuration.
> > > > 2. Add isRingNode (possibly package private) with current
> > > > TcpDiscoveryNode.isClient() semantic to preserve compatibility.
> > > >
> > > > Any pros or cons?
> > > >
> > > > --
> > > > Best regards,
> > > > Andrey V. Mashenkov
> > > >
>
>
>
> --
> Best regards,
> Andrey V. Mashenkov
>