[
https://issues.apache.org/jira/browse/DRILL-4504?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15193971#comment-15193971
]
ASF GitHub Bot commented on DRILL-4504:
---------------------------------------
Github user hnfgns commented on a diff in the pull request:
https://github.com/apache/drill/pull/429#discussion_r56060382
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java ---
@@ -74,73 +74,148 @@
/**
* Thin wrapper around a UserClient that handles connect/close and
transforms
* String into ByteBuf.
+ *
+ * Use the builder class ({@link DrillClient.Builder}) to build objects of
this class.
+ * E.g.
+ * <code>
+ * DrillClient client = DrillClient.newBuilder()
+ * .setConfig(...)
+ * .setIsDirectConnection(true)
+ * .build();
+ * </code>
*/
public class DrillClient implements Closeable, ConnectionThrottle {
private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(DrillClient.class);
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 boolean isDirectConnection;
+ private final int reconnectTimes;
+ private final int reconnectDelay;
+
+ // 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 final boolean ownsZkConnection;
+ private final boolean ownsEventLoopGroup;
+ private final boolean ownsExecutor;
+
+ // if the following variables are set during construction, they are not
overridden during or after #connect call
+ // otherwise, they are set to defaults during #connect call
private EventLoopGroup eventLoopGroup;
private ExecutorService executor;
+ private boolean supportComplexTypes;
+
+ // the following variables are set during connection, and must not be
overridden later
+ private UserClient client;
+ private UserProperties props;
+ private volatile ClusterCoordinator clusterCoordinator;
+ private volatile boolean connected; // = false
- public DrillClient() throws OutOfMemoryException {
+ /**
+ * @deprecated Create a DrillClient using {@link DrillClient.Builder}.
+ */
+ @Deprecated
+ public DrillClient() {
this(DrillConfig.create(), false);
}
- public DrillClient(boolean isDirect) throws OutOfMemoryException {
+ /**
+ * @deprecated Create a DrillClient using {@link DrillClient.Builder}.
+ */
+ @Deprecated
+ public DrillClient(boolean isDirect) {
this(DrillConfig.create(), isDirect);
}
- public DrillClient(String fileName) throws OutOfMemoryException {
+ /**
+ * @deprecated Create a DrillClient using {@link DrillClient.Builder}.
+ */
+ @Deprecated
+ public DrillClient(String fileName) {
this(DrillConfig.create(fileName), false);
}
- public DrillClient(DrillConfig config) throws OutOfMemoryException {
+ /**
+ * @deprecated Create a DrillClient using {@link DrillClient.Builder}.
+ */
+ @Deprecated
+ public DrillClient(DrillConfig config) {
this(config, null, false);
}
- public DrillClient(DrillConfig config, boolean isDirect)
- throws OutOfMemoryException {
+ /**
+ * @deprecated Create a DrillClient using {@link DrillClient.Builder}.
+ */
+ @Deprecated
+ public DrillClient(DrillConfig config, boolean isDirect) {
this(config, null, isDirect);
}
- public DrillClient(DrillConfig config, ClusterCoordinator coordinator)
- throws OutOfMemoryException {
+ /**
+ * @deprecated Create a DrillClient using {@link DrillClient.Builder}.
+ */
+ @Deprecated
+ public DrillClient(DrillConfig config, ClusterCoordinator coordinator) {
this(config, coordinator, null, false);
}
- public DrillClient(DrillConfig config, ClusterCoordinator coordinator,
boolean isDirect)
- throws OutOfMemoryException {
+ /**
+ * @deprecated Create a DrillClient using {@link DrillClient.Builder}.
+ */
+ @Deprecated
+ public DrillClient(DrillConfig config, ClusterCoordinator coordinator,
boolean isDirect) {
this(config, coordinator, null, isDirect);
}
- public DrillClient(DrillConfig config, ClusterCoordinator coordinator,
BufferAllocator allocator)
- throws OutOfMemoryException {
+ /**
+ * @deprecated Create a DrillClient using {@link DrillClient.Builder}.
+ */
+ @Deprecated
+ public DrillClient(DrillConfig config, ClusterCoordinator coordinator,
BufferAllocator allocator) {
this(config, coordinator, allocator, false);
}
+ /**
+ * @deprecated Create a DrillClient using {@link DrillClient.Builder}.
+ */
+ @Deprecated
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;
+ this(config,
+ allocator == null,
+ allocator == null ? RootAllocatorFactory.newRoot(config) :
allocator,
+ coordinator == null && !isDirect,
+ coordinator,
+ isDirect,
+ true,
+ null,
+ true,
+ null,
+ config.getBoolean(ExecConstants.CLIENT_SUPPORT_COMPLEX_TYPES)
+ );
+ }
+
+ // used by DrillClient.Builder
+ protected DrillClient(DrillConfig config, boolean ownsAllocator,
BufferAllocator allocator,
+ boolean ownsZkConnection, ClusterCoordinator
clusterCoordinator, boolean isDirectConnection,
+ boolean ownsEventLoopGroup, EventLoopGroup
eventLoopGroup, boolean ownsExecutor,
+ ExecutorService executor, boolean
supportComplexTypes) {
+ this.config = checkNotNull(config);
+ this.ownsAllocator = ownsAllocator;
+ this.allocator = checkNotNull(allocator);
+ this.ownsZkConnection = ownsZkConnection;
+ this.clusterCoordinator = clusterCoordinator;
+ this.isDirectConnection = isDirectConnection;
+ this.ownsEventLoopGroup = ownsEventLoopGroup;
+ this.eventLoopGroup = eventLoopGroup;
+ this.ownsExecutor = ownsExecutor;
+ this.executor = executor;
+ this.supportComplexTypes = supportComplexTypes;
--- End diff --
Nice work here. Would not it be nice if the builder provided "meaningful"
defaults so that the complex creational patterns in this class particularly in
#connect(...) could be eliminated further? For instance, why would executor
ever be null or cluster coordinator be created later in the lifetime of this
instance given instantiation cost is very minimal?
> Create an event loop for each of [user, control, data] RPC components
> ---------------------------------------------------------------------
>
> Key: DRILL-4504
> URL: https://issues.apache.org/jira/browse/DRILL-4504
> Project: Apache Drill
> Issue Type: Improvement
> Components: Execution - RPC
> Reporter: Sudheesh Katkam
> Assignee: Sudheesh Katkam
>
> + Create an event loop group for each client-server pair (data, client and
> user)
> + Allow DrillClient constructor to specify an event loop group (so user event
> loop can be used for queries from Web API calls). Deprecate old DrillClient
> constructors and create a helper class to build instances.
> Miscellaneous:
> + Move WorkEventBus from exec/rpc/control to exec/work
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
