belugabehr commented on a change in pull request #1946:
URL: https://github.com/apache/hive/pull/1946#discussion_r571131651
##########
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 "
Review comment:
OK, I think we should definitely add a timeout value. But perhaps in a
separate effort.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
