pvary commented on a change in pull request #2119:
URL: https://github.com/apache/iceberg/pull/2119#discussion_r560715337



##########
File path: 
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveClientPool.java
##########
@@ -83,6 +83,20 @@ protected HiveMetaStoreClient reconnect(HiveMetaStoreClient 
client) {
     return client;
   }
 
+  @Override
+  protected boolean failureDetection(HiveMetaStoreClient client, Exception ex) 
{
+    if (null != ex && ex instanceof MetaException) {
+      try {
+        client.getDatabase("default");

Review comment:
       I would prefer to have a configurable timeout for this, so we do not end 
up doubling every failed metastore call. MetaException can be returned even if 
the HMS is working correctly. Also it would be good to be able to turn off this 
check with the same config.
   
   One more thing to consider is the time of the duplicated call. Would it be 
possible to return the original result and do the check asynchronously? Or in 
case of failure we always retry now?
   
   If I remember correctly theoretically you can habe a HMS without a "default" 
db.
   I remember that there was an API call to get the HMS version which should be 
good, or if it does not exist in  Hive 2.3 then maybe 
`get_current_notificationEventId` would be good for the quick health check.
   
   What do you think?




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

Reply via email to