Sergey,
Thanks for the detailed explanation and for covering all corner cases.
Considering the improvement's criticality, I would continue moving in the
initial direction and add that particular configuration property.
Potentially, we can put more effort throughout an Ignite 3.0 timeframe and
remove the property altogether. @Valentin Kulichenko
<vkuliche...@gridgain.com>, could you please suggest any alternate naming?
Btw, what are the specifics of the issue with continuous queries? It will
be ideal if we could release this new communication option in the GA state
in 2.9.
-
Denis
On Wed, Jun 10, 2020 at 1:22 AM Sergey Chugunov <sergey.chugu...@gmail.com>
wrote:
> Denis, Val,
>
> Idea of prohibiting servers to open connections to clients and force
> clients to always open "inverse connections" to servers looks promising. To
> be clear, by "inverse connections" I mean here that server needed to
> communicate with client requests client to open a connection back instead
> of opening connection by itself using addresses published by the client.
>
> If we apply the idea it will indeed allow us to simplify our configuration
> (no need for new configuration property), another advantage is clients
> won't need to publish their addresses anymore (with one side note I'll
> cover at the end), it will also simplify our code.
>
> However applying it with current implementation of inverse connection
> request (when request goes across all ring) may bring significant delay of
> opening first connection depending on cluster size and relative positions
> between server that needs to communicate with client (target server) and
> client's router node.
>
> It is possible to overcome this by sending inverse connection request not
> via discovery but directly to router server node via communication and
> convert to discovery message only on the router. We'll still have two hops
> of communication request instead of one plus discovery worker on client may
> be busy working on other stuff slowing down handling of connection request.
> But it should be fine.
>
> However with this solution it is hard to implement failover of router node:
> let me describe it in more details.
> In case of router node failure target server won't be able to determine if
> client received inverse comm request successfully and (even worse) won't be
> able to figure out new router for the client without waiting for discovery
> event of the client reconnect.
> And this brings us to the following choise: we either wait for discovery
> event about client reconnect (this is deadlock-prone as current protocol of
> CQ registration opens comm connection to client right from discovery thread
> in some cases; if we wait for new discovery event from discovery thread, it
> is a deadlock) or we fail opening the connection by timeout thus adding new
> scenarios when opening connection may fail.
>
> Thus implementing communication model "clients connect to servers, servers
> never connect to clients" efficiently requires more work on different parts
> of our functionality and rigorous testing of readiness of our code for more
> communication connection failures.
>
> So to me the least risky decision is not to delete new configuration but
> leave it with experimental status. If we find out that direct request
> (server -> router server -> target client) implementation works well and
> doesn't bring much complexity in failover scenarios we'll remove that
> configuration and prohibit servers to open connections to clients by
> default.
>
> Side note: there are rare but yet possible scenarios where client node
> needs to open communication connection to other client node. If we let
> clients not to publish their addresses these scenarios will stop working
> without additional logic like sending data through router node. As far as I
> know client-client connectivity is involved in p2p class deployment
> scenarios, does anyone know about other cases?
>
> --
> Thanks,
> Sergey Chugunov
>
> On Wed, Jun 3, 2020 at 5:37 PM Denis Magda <dma...@apache.org> wrote:
>
> > Ivan,
> >
> > It feels like Val is driving us in the right direction. Is there any
> reason
> > for keeping the current logic when servers can open connections to
> clients?
> >
> > -
> > Denis
> >
> >
> > On Thu, May 21, 2020 at 4:48 PM Valentin Kulichenko <
> > valentin.kuliche...@gmail.com> wrote:
> >
> > > Ivan,
> > >
> > > Have you considered eliminating server to client connections
> altogether?
> > > Or, at the very least making the "client to server only" mode the
> default
> > > one?
> > >
> > > All the suggested names are confusing and not intuitive, and I doubt we
> > > will be able to find a good one. A server initiating a TCP connection
> > with
> > > a client is confusing in the first place and creates a usability issue.
> > We
> > > now want to solve it by introducing an additional configuration
> > > parameter, and therefore additional complexity. I don't think this is
> the
> > > right approach.
> > >
> > > What are the drawbacks of permanently switching to client-to-server
> > > connections? Is there any value provided by the server-to-client
> option?
> > >
> > > As for pair connections, I'm not sure I understand why there is a
> > > limitation. As far as I know, the idea behind this feature is that we
> > > maintain two connections between two nodes instead of one, so that
> every
> > > connection is used for communication in a single direction only. Why
> does
> > > it matter which node initiates the connection? Why can't one of the
> nodes
> > > (e.g., a client) initiate both connections, and then use them
> > accordingly?
> > > Correct me if I'm wrong, but I don't see why we can't do this.
> > >
> > > -Val
> > >
> > > On Thu, May 21, 2020 at 1:58 PM Denis Magda <dma...@apache.org> wrote:
> > >
> > > > Ivan,
> > > >
> > > > Considering that the setting controls the way a communication SPI
> > > > connection is open I would add the new parameter to CommunicationSpi
> > > > interface naming it as follows:
> > > >
> > > > >
> > > > > CommunicationSpi.connectionInitiationMode
> > > > > {
> > > > > BIDIRECTIONAL, //both clients and servers initiate a connection
> > > > > initiation procedure
> > > > > CLIENTS_TO_SERVERS //servers cannot open a connection to
> clients,
> > > > only
> > > > > clients can do that
> > > > > }
> > > >
> > > >
> > > > The problem with the environment type approach is that private
> networks
> > > of
> > > > bare-metal environments usually impose restrictions similar to
> virtual
> > > > environments powered by Kubernetes. Thus, environmentType.VIRTUALIZED
> > > > doesn't cover all the cases and I'm struggling to come up with a
> > > universal
> > > > alternative.
> > > >
> > > > -
> > > > Denis
> > > >
> > > >
> > > > On Thu, May 21, 2020 at 5:38 AM Ivan Bessonov <bessonov...@gmail.com
> >
> > > > wrote:
> > > >
> > > > > Hello Igniters,
> > > > >
> > > > > I'd like to discuss with you changes related to [1] and [2]. Both
> > > issues
> > > > > are mostly the same so
> > > > > let's discuss the core idea.
> > > > >
> > > > > *Motivation.*
> > > > >
> > > > > There are certain environments that don't allow Ignite server nodes
> > to
> > > > open
> > > > > TCP connections to
> > > > > thick clients, e.g. K8S, AWS Lambda or Azure Functions. To operate
> in
> > > > such
> > > > > environments, the
> > > > > server needs a way to request a client to open an "inverse"
> > > communication
> > > > > connection to it.
> > > > >
> > > > > I've prepared a PR (still in progress) that introduces new
> mechanism
> > of
> > > > > opening connection and
> > > > > related configuration.
> > > > >
> > > > > *Main idea*
> > > > >
> > > > > This mechanism is called "communication via discovery" or "inverse
> > > > > connection", it works as
> > > > > follows:
> > > > > - server that needs to connect to "unreachable" thick client
> sends a
> > > > > specific Discovery message
> > > > > (inverse communication request) to that client;
> > > > > - client node upon receiving the request opens communication
> > > connection
> > > > to
> > > > > that server;
> > > > > - server sees connection opened by client and proceeds with its
> task
> > > > (that
> > > > > required opening
> > > > > connection to the client).
> > > > >
> > > > > Working name for new configuration parameter for this feature is
> > > > > environmentType, it is an
> > > > > enum with two values (again, working names): STANDALONE (default)
> and
> > > > > VIRTUALIZED.
> > > > > It is used as a hint to server to speed-up establishing of
> > connections:
> > > > > when server sees a client
> > > > > with VIRTUALIZED environmentType it doesn't try to open connection
> to
> > > it
> > > > > and sends inverse
> > > > > communication request right away.
> > > > > If environmentType is STANDALONE then server tries to open a
> > connection
> > > > in
> > > > > a regular way
> > > > > (iterating over all client addresses) and sends request only if all
> > > > > attempts failed.
> > > > >
> > > > > There is a concern about naming of the configuration as it catches
> > only
> > > > one
> > > > > use-case: when we
> > > > > deal with some kind of virtualized environment like K8S.
> > > > > There are other options I've encountered in private discussion:
> > > > > - connectionMode - ALWAYS_INITIATE / INITIATE_OR_ACCEPT
> > > > > - networkConnectivity - REACHABLE_ALWAYS / REACHABLE_ONE_WAY
> > > > > - communicationViaDiscovery - ALWAYS / FALLBACK
> > > > > - isReachableFromAllNodes (true/false flag)
> > > > > - initiateConnectionsOnThisNode (true/false flag).
> > > > >
> > > > > *Limitations*
> > > > >
> > > > > The feature cannot be used along with pairedConnection setting as
> > this
> > > > > setting implies
> > > > > establishing connections in both directions. Also current
> > > implementation
> > > > > supports opening only
> > > > > client-to-server connections. Other types of connections like
> > > > > client-to-client or server-to-server
> > > > > will be implemented in separate tickets.
> > > > >
> > > > > [1] https://issues.apache.org/jira/browse/IGNITE-12438
> > > > > [2] https://issues.apache.org/jira/browse/IGNITE-13013
> > > > >
> > > > > --
> > > > > Sincerely yours,
> > > > > Ivan Bessonov
> > > > >
> > > >
> > >
> >
>
