Added timeout on isAvailable calls (#300)

* Added timeout * Update CHANGELOG.md
ExpediaGroup · Oct 20, 2023 · 24cbd56 · 24cbd56
1 parent 774f670
commit 24cbd56
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 11 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,7 @@
+## [3.11.5] - 2023-10-23
+### Fixed
+- Added timeout on `MetastoreMappingImpl.isAvailable()` calls to prevent long waits on unresponsive metastores.
+
 ## [3.11.4] - 2023-08-23
 ### Changed
 - Metrics have been incorporated into Waggle Dance with the inclusion of tags, which will facilitate filtering within Datadog.

diff --git a/pom.xml b/pom.xml
@@ -42,7 +42,6 @@
     <hive.version>2.3.7</hive.version>
     <junit.version>4.13.1</junit.version>
     <mockito.version>3.5.15</mockito.version>
-    <dropwizard.metrics.version>3.1.5</dropwizard.metrics.version>
     <aspectj-maven-plugin.version>1.9</aspectj-maven-plugin.version>
     <aspectj.version>1.8.9</aspectj.version>
     <beeju.version>4.0.0</beeju.version>

diff --git a/...nce-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/MetaStoreMappingImpl.java b/...nce-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/MetaStoreMappingImpl.java
@@ -1,5 +1,5 @@
 /**
- * Copyright (C) 2016-2021 Expedia, Inc.
+ * Copyright (C) 2016-2023 Expedia, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -19,6 +19,11 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Locale;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 
 import org.apache.hadoop.hive.metastore.MetaStoreFilterHook;
 import org.apache.hadoop.hive.metastore.api.AlreadyExistsException;
@@ -39,6 +44,9 @@ class MetaStoreMappingImpl implements MetaStoreMapping {
 
   private final static Logger log = LoggerFactory.getLogger(MetaStoreMappingImpl.class);
 
+  // MilliSeconds
+  static final long DEFAULT_AVAILABILITY_TIMEOUT = 500;
+
   private final String databasePrefix;
   private final CloseableThriftHiveMetastoreIface client;
   private final AccessControlHandler accessControlHandler;
@@ -106,18 +114,32 @@ public void close() throws IOException {
     client.close();
   }
 
+  /**
+   * This is potentially slow so a best effort is made and false is returned after a timeout.
+   */
   @Override
   public boolean isAvailable() {
-    try {
-      boolean isOpen = client.isOpen();
-      if (isOpen && connectionType == ConnectionType.TUNNELED) {
-        client.getStatus();
+    Future<Boolean> future = CompletableFuture.supplyAsync(() -> {
+      try {
+        boolean isOpen = client.isOpen();
+        if (isOpen && connectionType == ConnectionType.TUNNELED) {
+          client.getStatus();
+        }
+        return isOpen;
+      } catch (Exception e) {
+        log.error("Metastore Mapping {} unavailable", name, e);
+        return false;
       }
-      return isOpen;
-    } catch (Exception e) {
-      log.error("Metastore Mapping {} unavailable", name, e);
-      return false;
+    });
+    try {
+      return future.get(DEFAULT_AVAILABILITY_TIMEOUT + getLatency(), TimeUnit.MILLISECONDS);
+    } catch (TimeoutException e) {
+      log.info("Took too long to check availability of '" + name + "', assuming unavailable");
+      future.cancel(true);
+    } catch (InterruptedException | ExecutionException e) {
+      log.error("Error while checking availability '" + name + "', assuming unavailable");
     }
+    return false;
   }
 
   @Override

diff --git a/...core/src/test/java/com/hotels/bdp/waggledance/mapping/model/MetaStoreMappingImplTest.java b/...core/src/test/java/com/hotels/bdp/waggledance/mapping/model/MetaStoreMappingImplTest.java
@@ -1,5 +1,5 @@
 /**
- * Copyright (C) 2016-2021 Expedia, Inc.
+ * Copyright (C) 2016-2023 Expedia, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -19,12 +19,14 @@
 import static org.hamcrest.CoreMatchers.sameInstance;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.fail;
+import static org.mockito.AdditionalAnswers.answersWithDelay;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import static com.hotels.bdp.waggledance.api.model.ConnectionType.DIRECT;
 import static com.hotels.bdp.waggledance.api.model.ConnectionType.TUNNELED;
+import static com.hotels.bdp.waggledance.mapping.model.MetaStoreMappingImpl.DEFAULT_AVAILABILITY_TIMEOUT;
 
 import java.io.IOException;
 
@@ -115,6 +117,12 @@ public void isNotAvailable() {
     assertThat(metaStoreMapping.isAvailable(), is(false));
   }
 
+  @Test
+  public void isNotAvailableTimeout() throws Exception {
+    when(client.isOpen()).then(answersWithDelay(DEFAULT_AVAILABILITY_TIMEOUT+LATENCY+10, a -> true));
+    assertThat(metaStoreMapping.isAvailable(), is(false));
+  }
+
   @Test
   public void isAvailableTunnelled() throws Exception {
     when(client.isOpen()).thenReturn(true);