[GitHub] [hive] belugabehr commented on a change in pull request #1946: HIVE-24739: Clarify Usage of Thrift TServerEventHandler and Count Number of Messages Processed

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

belugabehr commented on a change in pull request #1946:
URL: https://github.com/apache/hive/pull/1946#discussion_r570689174



##########
File path: service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java
##########
@@ -113,43 +113,58 @@ protected void initServer() {
       // TCP Server
       server = new TThreadPoolServer(sargs);
       server.setServerEventHandler(new TServerEventHandler() {
+
         @Override
         public ServerContext createContext(TProtocol input, TProtocol output) {
           Metrics metrics = MetricsFactory.getInstance();
           if (metrics != null) {
-            try {
-              metrics.incrementCounter(MetricsConstant.OPEN_CONNECTIONS);
-              metrics.incrementCounter(MetricsConstant.CUMULATIVE_CONNECTION_COUNT);
-            } catch (Exception e) {
-              LOG.warn("Error Reporting JDO operation to Metrics system", e);
-            }
+            metrics.incrementCounter(MetricsConstant.OPEN_CONNECTIONS);
+            metrics.incrementCounter(MetricsConstant.CUMULATIVE_CONNECTION_COUNT);
           }
           return new ThriftCLIServerContext();
         }
 
+        /**
+         * This is called by the Thrift server when the underlying client
+         * connection is cleaned up by the server because the connection has
+         * been closed.
+         */
         @Override
         public void deleteContext(ServerContext serverContext, TProtocol input, TProtocol output) {
           Metrics metrics = MetricsFactory.getInstance();
           if (metrics != null) {
-            try {
-              metrics.decrementCounter(MetricsConstant.OPEN_CONNECTIONS);
-            } catch (Exception e) {
-              LOG.warn("Error Reporting JDO operation to Metrics system", e);
-            }
+            metrics.decrementCounter(MetricsConstant.OPEN_CONNECTIONS);
           }
+
           ThriftCLIServerContext context = (ThriftCLIServerContext) serverContext;
-          SessionHandle sessionHandle = context.getSessionHandle();
+
+          final SessionHandle sessionHandle = context.getSessionHandle();
           if (sessionHandle != null) {
-            LOG.info("Session disconnected without closing properly. ");
+            // Normally, the client should politely inform the server it is
+            // closing its session with Hive before closing its connection.
+            // However, if the client connection dies for any reason
+            // (load-balancer health check, load-balancer configuration,
+            // fire-wall kills long-running sessions, bad client, failed client,
+            // timed-out client, etc.) then the server will close the connection
+            // without having properly cleaned up the Hive session (resources,
+            // configuration, logging etc.) That needs to be cleaned up now.
+            LOG.warn(
+                "Client connection bound to {} unexpectedly closed: closing this Hive session to release its resources. "
+                    + "The connection processed {} total messages. If total messages processed is zero or one, most likely it is a "
+                    + "client that is opening then immediately closing the socket (i.e., TCP health check or port scanner), otherwise "
+                    + "inspect the client for time-out, fire-wall killing the connection, invalid load balancer configuration, etc.",
+                sessionHandle, context.getMessagesProcessedCount());
             try {
               boolean close = cliService.getSessionManager().getSession(sessionHandle).getHiveConf()
                   .getBoolVar(ConfVars.HIVE_SERVER2_CLOSE_SESSION_ON_DISCONNECT);
-              LOG.info((close ? "" : "Not ") + "Closing the session: " + sessionHandle);

Review comment:
       This is already covered in the message above:
   
   > closing this Hive session to release its resources.




----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org