[
https://issues.apache.org/jira/browse/DRILL-4606?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15289452#comment-15289452
]
ASF GitHub Bot commented on DRILL-4606:
---------------------------------------
Github user adeneche commented on a diff in the pull request:
https://github.com/apache/drill/pull/480#discussion_r63751306
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java ---
@@ -73,81 +78,173 @@
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.node.ArrayNode;
-import com.google.common.base.Strings;
-import com.google.common.util.concurrent.AbstractCheckedFuture;
-import com.google.common.util.concurrent.SettableFuture;
/**
* Thin wrapper around a UserClient that handles connect/close and
transforms
* String into ByteBuf.
+ *
+ * To create non-default objects, use {@link DrillClient.Builder the
builder class}.
+ * E.g.
+ * <code>
+ * DrillClient client = DrillClient.newBuilder()
+ * .setConfig(...)
+ * .setIsDirectConnection(true)
+ * .build();
+ * </code>
+ *
+ * Except for {@link #runQuery} and {@link #cancelQuery}, this class is
generally not thread safe.
*/
public class DrillClient implements Closeable, ConnectionThrottle {
private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(DrillClient.class);
private static final ObjectMapper objectMapper = new ObjectMapper();
private final DrillConfig config;
- private UserClient client;
- private UserProperties props = null;
- private volatile ClusterCoordinator clusterCoordinator;
- private volatile boolean connected = false;
private final BufferAllocator allocator;
- private int reconnectTimes;
- private int reconnectDelay;
- private boolean supportComplexTypes;
- private final boolean ownsZkConnection;
+ private final EventLoopGroup eventLoopGroup;
+ private final ExecutorService executor;
+ private final boolean isDirectConnection;
+ private final int reconnectTimes;
+ private final int reconnectDelay;
+
+ // TODO: clusterCoordinator should be initialized in the constructor.
+ // Currently, initialization is tightly coupled with #connect.
+ private ClusterCoordinator clusterCoordinator;
+
+ // checks if this client owns these resources (used when closing)
private final boolean ownsAllocator;
- private final boolean isDirectConnection; // true if the connection
bypasses zookeeper and connects directly to a drillbit
- private EventLoopGroup eventLoopGroup;
- private ExecutorService executor;
+ private final boolean ownsZkConnection;
+ private final boolean ownsEventLoopGroup;
+ private final boolean ownsExecutor;
- public DrillClient() throws OutOfMemoryException {
- this(DrillConfig.create(), false);
+ // once #setSupportComplexTypes() is removed, make this final
+ private boolean supportComplexTypes;
+
+ private UserClient client;
+ private UserProperties props;
+ private boolean connected;
+
+ public DrillClient() {
+ this(newBuilder());
}
- public DrillClient(boolean isDirect) throws OutOfMemoryException {
- this(DrillConfig.create(), isDirect);
+ /**
+ * @deprecated Create a DrillClient using {@link DrillClient.Builder}.
+ */
+ @Deprecated
+ public DrillClient(boolean isDirect) {
+ this(newBuilder()
+ .setDirectConnection(isDirect));
}
- public DrillClient(String fileName) throws OutOfMemoryException {
- this(DrillConfig.create(fileName), false);
+ /**
+ * @deprecated Create a DrillClient using {@link DrillClient.Builder}.
+ */
+ @Deprecated
+ public DrillClient(String fileName) {
+ this(newBuilder()
+ .setConfigFromFile(fileName));
}
- public DrillClient(DrillConfig config) throws OutOfMemoryException {
- this(config, null, false);
+ /**
+ * @deprecated Create a DrillClient using {@link DrillClient.Builder}.
+ */
+ @Deprecated
+ public DrillClient(DrillConfig config) {
+ this(newBuilder()
+ .setConfig(config));
}
- public DrillClient(DrillConfig config, boolean isDirect)
- throws OutOfMemoryException {
- this(config, null, isDirect);
+ /**
+ * @deprecated Create a DrillClient using {@link DrillClient.Builder}.
+ */
+ @Deprecated
+ public DrillClient(DrillConfig config, boolean isDirect) {
+ this(newBuilder()
+ .setConfig(config)
+ .setDirectConnection(isDirect));
}
- public DrillClient(DrillConfig config, ClusterCoordinator coordinator)
- throws OutOfMemoryException {
- this(config, coordinator, null, false);
+ /**
+ * @deprecated Create a DrillClient using {@link DrillClient.Builder}.
+ */
+ @Deprecated
+ public DrillClient(DrillConfig config, ClusterCoordinator coordinator) {
+ this(newBuilder()
+ .setConfig(config)
+ .setClusterCoordinator(coordinator));
}
- public DrillClient(DrillConfig config, ClusterCoordinator coordinator,
boolean isDirect)
- throws OutOfMemoryException {
- this(config, coordinator, null, isDirect);
+ /**
+ * @deprecated Create a DrillClient using {@link DrillClient.Builder}.
+ */
+ @Deprecated
+ public DrillClient(DrillConfig config, ClusterCoordinator coordinator,
boolean isDirect) {
+ this(newBuilder()
+ .setConfig(config)
+ .setClusterCoordinator(coordinator)
+ .setDirectConnection(isDirect));
}
- public DrillClient(DrillConfig config, ClusterCoordinator coordinator,
BufferAllocator allocator)
- throws OutOfMemoryException {
- this(config, coordinator, allocator, false);
+ /**
+ * @deprecated Create a DrillClient using {@link DrillClient.Builder}.
+ */
+ @Deprecated
+ public DrillClient(DrillConfig config, ClusterCoordinator coordinator,
BufferAllocator allocator) {
+ this(newBuilder()
+ .setConfig(config)
+ .setClusterCoordinator(coordinator)
+ .setAllocator(allocator));
+ }
+
+ /**
+ * @deprecated Create a DrillClient using {@link DrillClient.Builder}.
+ */
+ @Deprecated
+ public DrillClient(DrillConfig config, ClusterCoordinator coordinator,
BufferAllocator allocator,
+ boolean isDirect) {
+ this(newBuilder()
+ .setConfig(config)
+ .setClusterCoordinator(coordinator)
+ .setAllocator(allocator)
+ .setDirectConnection(isDirect));
}
- public DrillClient(DrillConfig config, ClusterCoordinator coordinator,
BufferAllocator allocator, boolean isDirect) {
- // if isDirect is true, the client will connect directly to the
drillbit instead of
- // going thru the zookeeper
- this.isDirectConnection = isDirect;
- this.ownsZkConnection = coordinator == null && !isDirect;
- this.ownsAllocator = allocator == null;
- this.allocator = ownsAllocator ? RootAllocatorFactory.newRoot(config)
: allocator;
- this.config = config;
- this.clusterCoordinator = coordinator;
+ // used by DrillClient.Builder
+ private DrillClient(final Builder builder) {
+ this.config = builder.config != null ? builder.config :
DrillConfig.create();
+
+ this.ownsAllocator = builder.allocator == null;
+ this.allocator = !ownsAllocator ? builder.allocator :
RootAllocatorFactory.newRoot(config);
+
+ this.isDirectConnection = builder.isDirectConnection;
+ this.ownsZkConnection = builder.clusterCoordinator == null &&
!isDirectConnection;
+ this.clusterCoordinator = builder.clusterCoordinator; // could be null
+
+ this.ownsEventLoopGroup = builder.eventLoopGroup == null;
+ this.eventLoopGroup = !ownsEventLoopGroup ? builder.eventLoopGroup :
+
TransportCheck.createEventLoopGroup(config.getInt(ExecConstants.CLIENT_RPC_THREADS),
"Client-");
+
+ this.ownsExecutor = builder.executor == null;
--- End diff --
what is the motivation behind moving initiating `eventLoopGroup` and
`executor` from `connect(String connect, Properties props)` to here, assuming
the `connect` method was never called more than once ?
> Create DrillClient.Builder class
> --------------------------------
>
> Key: DRILL-4606
> URL: https://issues.apache.org/jira/browse/DRILL-4606
> Project: Apache Drill
> Issue Type: Improvement
> Reporter: Sudheesh Katkam
> Assignee: Sudheesh Katkam
>
> + Create a helper class to build DrillClient instances, and deprecate
> DrillClient constructors
> + Allow DrillClient to specify an event loop group (so user event loop can be
> used for queries from Web API calls)
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
