[ https://issues.apache.org/jira/browse/HIVE-24739?focusedWorklogId=565808&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-565808 ]
ASF GitHub Bot logged work on HIVE-24739: ----------------------------------------- Author: ASF GitHub Bot Created on: 13/Mar/21 22:43 Start Date: 13/Mar/21 22:43 Worklog Time Spent: 10m Work Description: belugabehr commented on a change in pull request #1946: URL: https://github.com/apache/hive/pull/1946#discussion_r593810591 ########## File path: service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java ########## @@ -113,43 +113,68 @@ 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(); - if (sessionHandle != null) { - LOG.info("Session disconnected without closing properly. "); + + final ThriftCLIServerContext context = (ThriftCLIServerContext) serverContext; + final Optional<SessionHandle> sessionHandle = context.getSessionHandle(); + + if (sessionHandle.isPresent()) { + // Normally, the client should politely inform the server it is + // closing its session with Hive before closing its network + // connection. However, if the client connection dies for any reason + // (load-balancer round-robin configuration, firewall 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 during its lifetime of {}ms. Inspect the client connection " + + "for time-out, firewall killing the connection, invalid load balancer configuration, etc.", + sessionHandle, context.getMessagesProcessedCount(), context.getDuration().toMillis()); try { - boolean close = cliService.getSessionManager().getSession(sessionHandle).getHiveConf() + final boolean close = cliService.getSessionManager().getSession(sessionHandle.get()).getHiveConf() .getBoolVar(ConfVars.HIVE_SERVER2_CLOSE_SESSION_ON_DISCONNECT); - LOG.info((close ? "" : "Not ") + "Closing the session: " + sessionHandle); if (close) { - cliService.closeSession(sessionHandle); + cliService.closeSession(sessionHandle.get()); + } else { + LOG.warn("Session not actually closed because configuration {} is set to false", + ConfVars.HIVE_SERVER2_CLOSE_SESSION_ON_DISCONNECT.varname); } } catch (HiveSQLException e) { - LOG.warn("Failed to close session: " + e, e); + LOG.warn("Failed to close session", e); + } + } else { + // There is no session handle because the client gracefully closed + // the session *or* because the client had some issue and never was + // able to create one in the first place + if (!context.getSessionHandle().isPresent()) { Review comment: Yes. Excellent. Thanks. ---------------------------------------------------------------- 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: us...@infra.apache.org Issue Time Tracking ------------------- Worklog Id: (was: 565808) Time Spent: 8h 20m (was: 8h 10m) > Clarify Usage of Thrift TServerEventHandler and Count Number of Messages > Processed > ---------------------------------------------------------------------------------- > > Key: HIVE-24739 > URL: https://issues.apache.org/jira/browse/HIVE-24739 > Project: Hive > Issue Type: Improvement > Reporter: David Mollitor > Assignee: David Mollitor > Priority: Major > Labels: pull-request-available > Time Spent: 8h 20m > Remaining Estimate: 0h > > Make the messages emitted from {{TServerEventHandler}} more meaningful. > Also, track the number of messages that each client sends to aid in > troubleshooting. > I run into this issue all the time with and this would greatly help clarify > the logging. -- This message was sent by Atlassian Jira (v8.3.4#803005)