This is an automated email from the ASF dual-hosted git repository.
mgaido pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-livy.git
The following commit(s) were added to refs/heads/master by this push:
new 3a26856 [LIVY-745] Ensure that a single RSCClientFactory gets loaded.
3a26856 is described below
commit 3a2685643670c1cd1530feb7a54516b01784e781
Author: Wing Yew Poon <wyp...@cloudera.com>
AuthorDate: Sat Feb 15 22:42:47 2020 +0100
[LIVY-745] Ensure that a single RSCClientFactory gets loaded.
## What changes were proposed in this pull request?
In LivyClientBuilder, a ServiceLoader is used to load configured
LivyClientFactory providers (HttpClientFactory and RSCClientFactory). Correct
behavior of RSCClientFactory implicitly depends on there being a single
instance of it in the Livy server.
Instead of instantiating a new ServiceLoader every time the build method is
called on a LivyClientBuilder, keep a single ServiceLoader in a static field.
ServiceLoader instances are not safe for use by multiple concurrent threads, so
have the ServiceLoader load the LivyClientFactory implementations into a static
List. Then LivyClientBuilder#build will use the single instance of either
HttpClientFactory or RSCClientFactory in this List to create a LivyClient.
## How was this patch tested?
Tested by having 20 clients connect to the Livy Thrift server
simultaneously. Before the change, the behavior described in
https://issues.apache.org/jira/browse/LIVY-745 was encountered. With the
change, the problem is not encountered.
Also added a unit test that fails before this change to LivyClientBuilder,
and passes with the change.
Author: Wing Yew Poon <wyp...@cloudera.com>
Closes #275 from wypoon/LIVY-745.
---
.../java/org/apache/livy/LivyClientBuilder.java | 26 ++++++++++++++++------
.../java/org/apache/livy/TestClientFactory.java | 12 +++++++++-
.../org/apache/livy/TestLivyClientBuilder.java | 8 +++++++
3 files changed, 38 insertions(+), 8 deletions(-)
diff --git a/api/src/main/java/org/apache/livy/LivyClientBuilder.java
b/api/src/main/java/org/apache/livy/LivyClientBuilder.java
index 5bbd170..1d7ec01 100644
--- a/api/src/main/java/org/apache/livy/LivyClientBuilder.java
+++ b/api/src/main/java/org/apache/livy/LivyClientBuilder.java
@@ -23,6 +23,8 @@ import java.io.Reader;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
+import java.util.ArrayList;
+import java.util.List;
import java.util.Map;
import java.util.Properties;
import java.util.ServiceLoader;
@@ -35,6 +37,19 @@ public final class LivyClientBuilder {
public static final String LIVY_URI_KEY = "livy.uri";
+ private static final ServiceLoader<LivyClientFactory> CLIENT_FACTORY_LOADER =
+ ServiceLoader.load(LivyClientFactory.class, classLoader());
+
+ private static List<LivyClientFactory> getLivyClientFactories() {
+ List<LivyClientFactory> factories = new ArrayList<>();
+ for (LivyClientFactory f : CLIENT_FACTORY_LOADER) {
+ factories.add(f);
+ }
+ return factories;
+ }
+
+ private static final List<LivyClientFactory> CLIENT_FACTORIES =
getLivyClientFactories();
+
private final Properties config;
/**
@@ -118,14 +133,11 @@ public final class LivyClientBuilder {
}
LivyClient client = null;
- ServiceLoader<LivyClientFactory> loader =
ServiceLoader.load(LivyClientFactory.class,
- classLoader());
- if (!loader.iterator().hasNext()) {
+ if (CLIENT_FACTORIES.isEmpty()) {
throw new IllegalStateException("No LivyClientFactory implementation was
found.");
}
- Exception error = null;
- for (LivyClientFactory factory : loader) {
+ for (LivyClientFactory factory : CLIENT_FACTORIES) {
try {
client = factory.createClient(uri, config);
} catch (Exception e) {
@@ -158,10 +170,10 @@ public final class LivyClientBuilder {
return client;
}
- private ClassLoader classLoader() {
+ private static ClassLoader classLoader() {
ClassLoader cl = Thread.currentThread().getContextClassLoader();
if (cl == null) {
- cl = getClass().getClassLoader();
+ cl = LivyClientBuilder.class.getClassLoader();
}
return cl;
}
diff --git a/api/src/test/java/org/apache/livy/TestClientFactory.java
b/api/src/test/java/org/apache/livy/TestClientFactory.java
index 89edeec..622908c 100644
--- a/api/src/test/java/org/apache/livy/TestClientFactory.java
+++ b/api/src/test/java/org/apache/livy/TestClientFactory.java
@@ -21,9 +21,19 @@ import java.io.File;
import java.net.URI;
import java.util.Properties;
import java.util.concurrent.Future;
+import java.util.concurrent.atomic.AtomicLong;
public class TestClientFactory implements LivyClientFactory {
+ private static AtomicLong instanceCount = new AtomicLong();
+ public static long getInstanceCount() {
+ return instanceCount.get();
+ }
+
+ public TestClientFactory() {
+ instanceCount.incrementAndGet();
+ }
+
@Override
public LivyClient createClient(URI uri, Properties config) {
switch (uri.getPath()) {
@@ -81,6 +91,6 @@ public class TestClientFactory implements LivyClientFactory {
throw new UnsupportedOperationException();
}
-}
+ }
}
diff --git a/api/src/test/java/org/apache/livy/TestLivyClientBuilder.java
b/api/src/test/java/org/apache/livy/TestLivyClientBuilder.java
index 568d633..213dc03 100644
--- a/api/src/test/java/org/apache/livy/TestLivyClientBuilder.java
+++ b/api/src/test/java/org/apache/livy/TestLivyClientBuilder.java
@@ -78,4 +78,12 @@ public class TestLivyClientBuilder {
}
}
+ @Test
+ public void testSinglenessOfClientFactory() throws Exception {
+ LivyClient client1 = new LivyClientBuilder().build();
+ assertTrue(client1 instanceof TestClientFactory.Client);
+ LivyClient client2 = new LivyClientBuilder().build();
+ assertTrue(client2 instanceof TestClientFactory.Client);
+ assertEquals(1L, TestClientFactory.getInstanceCount());
+ }
}
