belugabehr commented on a change in pull request #1946:
URL: https://github.com/apache/hive/pull/1946#discussion_r570692697
##########
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:
Thanks. You're correct. I will address this. I was confusing
`sessionHandle` and `serverContext` here. Every connection will get a
`serverContext`, but will only have a `sessionHandle` if they call OpenSession.
So, if the connection is closed, and it never processed any messages, then I
want to log some error about a load balancer health check (or some other health
check). I've worked on the Thrift server code (THRIFT-5297) and every
connection gets a `serverContext`. If there is no data from the client after
some amount of time (time out), then the connection will close on the server
with a read or write time out error.
My motivation for this is that currently health checks cause a ton of SPAM
in the logs and it's very confusing. I want to detect when a valid connection
is dropping (bad client, fire wall, bad load balancer config, JVM Pauses that
cause timeouts) and ignore, or at least clearly and discretely log, when a
client didn't event bother to open a session.
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
