Bharath Vissapragada has posted comments on this change. Change subject: IMPALA-2347: Reuse metastore client connections in Catalog ......................................................................
Patch Set 4: Dimitris, there is an edge case for this implementation. Check the test added in this commit [1]. That consistently fails with this patch. Reason being, once the HMS is restarted, all the connections in the thread pool become stale and any further operations on them return TTransportException. Although, HMSClient class exposes reconnect() call, there is no way to consistently find out if the other end of the connection actually died (even with TCP keep-alive sockets). So there should be some kind of retry logic from the client side. I can think of three ways. (We didn't hit this prior to the patch because we do a new HMSClient() every-time we call getClient(), which basically makes a connection to the metastore post restart) 1) Call reconnect() every time we poll() a client from the thread pool. This basically defeats the purpose of the patch. 2) Have a Ping() operation for HMS in MetaStoreClientPool and call reconnect() if it fails. Even this has an overhead depending on how costly the Ping() method is. 3) Switch to RetryingMetaStoreClient [2] which has an inbuilt retry mechanism on a per metadata call basis. Basically it uses a base HMSClient class and invokes the method with multiple retries and calls reconnect() between retries I feel the switch to (3) is the best way forward for a long term fix which also adds per operation retries to Impala as well (which can be configured from hive-site.xml settings). Do you see any downside for this? Is there any other way to workaround this problem? [1] https://github.com/cloudera/Impala/commit/b3c0d57ab1a4da92aadd07c48b09bc989106ddb4 [2] https://github.com/cloudera/hive/blob/cdh5-1.1.0_5.5.0/metastore/src/java/org/apache/hadoop/hive/metastore/RetryingMetaStoreClient.java#L81 -- To view, visit http://gerrit.cloudera.org:8080/3381 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I517c0e1efef2584cd8d34017b33574f2ad69bd52 Gerrit-PatchSet: 4 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Huaisi Xu <[email protected]> Gerrit-HasComments: No
