Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust the Greengrass Discovery client to use its own executor service #427

Merged
merged 7 commits into from
Jun 15, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
import java.util.Map;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;

/**
* Class for performing network-based discovery of the connectivity properties of registered greengrass cores
Expand All @@ -35,12 +37,27 @@ public class DiscoveryClient implements AutoCloseable {

private final HttpClientConnectionManager httpClientConnectionManager;

/**
* We need to use a code defined executor to avoid leaving threads alive when the discovery client is closed
* It also fixes issues with the SecurityManager as well - as created ExecutorServices created via code
* inherit the permissions of the application, unlike the default common thread pool that is used by default
* with supplyAsync otherwise.
*/
private ExecutorService executorService = null;
private boolean cleanExecutor = false;

/**
*
* @param config Greengrass discovery client configuration
*/
public DiscoveryClient(final DiscoveryClientConfig config) {
this.httpClientConnectionManager = HttpClientConnectionManager.create(
executorService = config.getDiscoveryExecutor();
if (executorService == null) {
// If an executor is not set, then create one and make sure to clean it when finished.
executorService = Executors.newFixedThreadPool(1);
cleanExecutor = true;
}
httpClientConnectionManager = HttpClientConnectionManager.create(
new HttpClientConnectionManagerOptions()
.withClientBootstrap(config.getBootstrap())
.withProxyOptions(config.getProxyOptions())
Expand Down Expand Up @@ -102,11 +119,11 @@ public void onResponseComplete(HttpStream httpStream, int errorCode) {
catch(InterruptedException | ExecutionException e) {
throw new RuntimeException(e.getMessage(), e);
}
});
}, executorService);
}

private static String getHostname(final DiscoveryClientConfig config) {
//allow greengrass server endpoint to be manualy set for unique endpoints
//allow greengrass server endpoint to be manually set for unique endpoints
if (config.getGGServerName().equals("")) {
return String.format("greengrass-ats.iot.%s.%s",
config.getRegion(), AWS_DOMAIN_SUFFIX_MAP.getOrDefault(config.getRegion(), AWS_DOMAIN_DEFAULT));
Expand All @@ -120,5 +137,9 @@ public void close() {
if(httpClientConnectionManager != null) {
httpClientConnectionManager.close();
}
if (cleanExecutor == true && executorService != null) {
sbSteveK marked this conversation as resolved.
Show resolved Hide resolved
executorService.shutdown();
executorService = null;
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package software.amazon.awssdk.iot.discovery;

import java.util.concurrent.ExecutorService;

import software.amazon.awssdk.crt.http.HttpProxyOptions;
import software.amazon.awssdk.crt.io.ClientBootstrap;
import software.amazon.awssdk.crt.io.SocketOptions;
Expand All @@ -17,6 +19,7 @@ public class DiscoveryClientConfig implements AutoCloseable {
final private int maxConnections;
final private HttpProxyOptions proxyOptions;
final private String ggServerName;
private ExecutorService discoveryExecutor;

/**
* Constructor for DiscoveryClientConfig creates the correct endpoint if not in a special region
Expand Down Expand Up @@ -44,6 +47,7 @@ public DiscoveryClientConfig(
this.maxConnections = maxConnections;
this.proxyOptions = proxyOptions;
this.ggServerName = "";
this.discoveryExecutor = null;
}

/**
Expand All @@ -70,7 +74,7 @@ public DiscoveryClientConfig(

/**
* Default Constructor for DiscoveryClientConfig that allows the specification of a specific ggServerName to use in special regions
*
*
* @param bootstrap client bootstrap to use to establish network connections
* @param tlsContextOptions tls configuration for client network connections. For greengrass discovery, the
* tls context must be initialized with the certificate and private key of the
Expand All @@ -94,6 +98,7 @@ public DiscoveryClientConfig(
this.maxConnections = maxConnections;
this.proxyOptions = proxyOptions;
this.ggServerName = ggServerName;
this.discoveryExecutor = null;
}

/**
Expand Down Expand Up @@ -164,6 +169,27 @@ public String getGGServerName() {
return ggServerName;
}

/**
* @return the executor set for this discover client, if one is set.
sbSteveK marked this conversation as resolved.
Show resolved Hide resolved
*/
public ExecutorService getDiscoveryExecutor() {
return this.discoveryExecutor;
}

/**
* Sets the executor that is used when calling discover().
* If set using this function, you are expected to close/shutdown the executor when finished with it.
*
* If it is not set, the discovery client will internally manage its own executor.
*
* @param override The executor to use
* @return The client config
*/
public DiscoveryClientConfig setDiscoveryExecutor(ExecutorService override) {
this.discoveryExecutor = override;
return this;
}

@Override
public void close() {
if(tlsContext != null) {
Expand Down