From 9238dfb38d9be350a1dfe9f33ad45dc6fb5935f8 Mon Sep 17 00:00:00 2001 From: Patrick Duin Date: Tue, 24 Oct 2023 16:04:29 +0200 Subject: [PATCH] Added Executor and increased timeout to help with stability (#301) * Added Executor and increased timeout to help with stability --- CHANGELOG.md | 6 ++++++ .../waggledance/api/model/AbstractMetaStore.java | 3 ++- .../client/ThriftMetastoreClientManager.java | 2 +- .../mapping/model/MetaStoreMappingImpl.java | 14 +++++++++----- 4 files changed, 18 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b8c15901f..5a7d16689 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ + +## [3.11.6] - 2023-10-24 +### Fixed +- Switch to ExecutorService instead of the default `ForkJoinPool` for `MetastoreMappingImpl.isAvailable()` calls. Using `ForkJoinPool` may cause threads to wait on each other. +- Increased default `MetastoreMappingImpl.isAvailable()` timeout to `2000ms` (was `500ms`) to set a bit more conservative default. + ## [3.11.5] - 2023-10-23 ### Fixed - Added timeout on `MetastoreMappingImpl.isAvailable()` calls to prevent long waits on unresponsive metastores. diff --git a/waggle-dance-api/src/main/java/com/hotels/bdp/waggledance/api/model/AbstractMetaStore.java b/waggle-dance-api/src/main/java/com/hotels/bdp/waggledance/api/model/AbstractMetaStore.java index 0ad3b2d90..e7eec1616 100644 --- a/waggle-dance-api/src/main/java/com/hotels/bdp/waggledance/api/model/AbstractMetaStore.java +++ b/waggle-dance-api/src/main/java/com/hotels/bdp/waggledance/api/model/AbstractMetaStore.java @@ -1,5 +1,5 @@ /** - * Copyright (C) 2016-2022 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. @@ -247,6 +247,7 @@ public String toString() { .add("metastoreTunnel", metastoreTunnel) .add("accessControlType", accessControlType) .add("writableDatabaseWhiteList", writableDatabaseWhitelist) + .add("latency", latency) .add("status", status) .toString(); } diff --git a/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/client/ThriftMetastoreClientManager.java b/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/client/ThriftMetastoreClientManager.java index a45ff80c6..c20c87cfd 100644 --- a/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/client/ThriftMetastoreClientManager.java +++ b/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/client/ThriftMetastoreClientManager.java @@ -196,7 +196,7 @@ void open(HiveUgiArgs ugiArgs) { // Wait before launching the next round of connection retries. if (!isConnected && (retryDelaySeconds > 0) && ((attempt + 1) < retries)) { try { - LOG.info("Waiting " + retryDelaySeconds + " seconds before next connection attempt."); + LOG.debug("Waiting " + retryDelaySeconds + " seconds before next connection attempt."); Thread.sleep(retryDelaySeconds * 1000); } catch (InterruptedException ignore) {} } diff --git a/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/MetaStoreMappingImpl.java b/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/MetaStoreMappingImpl.java index 30f008e04..8b56020df 100644 --- a/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/MetaStoreMappingImpl.java +++ b/waggle-dance-core/src/main/java/com/hotels/bdp/waggledance/mapping/model/MetaStoreMappingImpl.java @@ -21,6 +21,8 @@ import java.util.Locale; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; @@ -45,7 +47,7 @@ class MetaStoreMappingImpl implements MetaStoreMapping { private final static Logger log = LoggerFactory.getLogger(MetaStoreMappingImpl.class); // MilliSeconds - static final long DEFAULT_AVAILABILITY_TIMEOUT = 500; + static final long DEFAULT_AVAILABILITY_TIMEOUT = 2000; private final String databasePrefix; private final CloseableThriftHiveMetastoreIface client; @@ -53,8 +55,8 @@ class MetaStoreMappingImpl implements MetaStoreMapping { private final String name; private final long latency; private final MetaStoreFilterHook metastoreFilter; - private final ConnectionType connectionType; + private final ExecutorService executor = Executors.newSingleThreadExecutor(); MetaStoreMappingImpl( String databasePrefix, @@ -112,6 +114,7 @@ public String getDatabasePrefix() { @Override public void close() throws IOException { client.close(); + executor.shutdownNow(); } /** @@ -130,11 +133,12 @@ public boolean isAvailable() { log.error("Metastore Mapping {} unavailable", name, e); return false; } - }); + }, executor); + long timeout = DEFAULT_AVAILABILITY_TIMEOUT + getLatency(); try { - return future.get(DEFAULT_AVAILABILITY_TIMEOUT + getLatency(), TimeUnit.MILLISECONDS); + return future.get(timeout, TimeUnit.MILLISECONDS); } catch (TimeoutException e) { - log.info("Took too long to check availability of '" + name + "', assuming unavailable"); + log.info("Took too long (>" + timeout + "ms) to check availability of '" + name + "', assuming unavailable"); future.cancel(true); } catch (InterruptedException | ExecutionException e) { log.error("Error while checking availability '" + name + "', assuming unavailable");