[GitHub] [ignite] isapego commented on a change in pull request #9817: IGNITE-16462 .NET: Thin client: add heartbeats

[GitBox <gi...@apache.org>]

isapego commented on a change in pull request #9817:
URL: https://github.com/apache/ignite/pull/9817#discussion_r805979653



##########
File path: modules/platforms/dotnet/Apache.Ignite.Core/Impl/Client/ClientSocket.cs
##########
@@ -170,6 +176,29 @@ internal sealed class ClientSocket : IDisposable
 
             _features = Handshake(clientConfiguration, ServerVersion);
 
+            if (_features.HasFeature(ClientBitmaskFeature.Heartbeat) &&
+                clientConfiguration.HeartbeatInterval > TimeSpan.Zero)
+            {
+                _heartbeatInterval = clientConfiguration.HeartbeatInterval;
+
+                var serverIdleTimeout = TimeSpan.FromMilliseconds(
+                    DoOutInOp(ClientOp.GetIdleTimeout, null, r => r.Reader.ReadLong()));
+
+                if (_heartbeatInterval > serverIdleTimeout)

Review comment:
       I'm agree with @ptupitsyn here, I'd let user decide and left a hint that maybe they are doing something they didn't want to. I don't think it's that hard for a user to set up those settings in a proper way by themselves.
   
   @ivandasch originally we wanted a robust and more manageable replacement for TCP KeepAlive native mechanism, and TCP level probes behave as @ptupitsyn described, so I'm on Pavel's side here. Basically, idle connection is a connection that is not used for user-level data, while main purpose of heartbeats mechanism is to detect half-dead connections early, not preventing connection from close by server side.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org