Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 0 additions & 90 deletions common/src/main/java/org/apache/drill/version/Generator.java

This file was deleted.

79 changes: 79 additions & 0 deletions exec/java-exec/src/main/codegen/templates/DrillVersionInfo.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/*******************************************************************************
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
******************************************************************************/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice use of Freemarker!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Credits to @julienledem for the idea

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:)
and @jaltekruse

<@pp.dropOutputFile />

<@pp.changeOutputFile name="/org/apache/drill/common/util/DrillVersionInfo.java" />

<#include "/@includes/license.ftl" />

package org.apache.drill.common.util;

import java.text.MessageFormat;
import java.text.ParseException;

/*
* This file is generated with Freemarker using the template src/main/codegen/templates/DrillVersionInfo.java
*/
/**
* Give access to Drill version as captured during the build
*
* <strong>Caution</strong> don't rely on major, minor and patch versions only to compare two
* Drill versions. Instead you should use the whole string, and apply the same semver algorithm
* as Maven (see {@code org.apache.maven.artifact.versioning.ComparableVersion}).
*
*/
public class DrillVersionInfo {
private static final String VERSION = "${maven.project.version}";

private static final int MAJOR_VERSION = ${maven.project.artifact.selectedVersion.majorVersion};
private static final int MINOR_VERSION = ${maven.project.artifact.selectedVersion.minorVersion};
private static final int PATCH_VERSION = ${maven.project.artifact.selectedVersion.incrementalVersion};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allow a suffix? Drill 1.9.0-SNAPSHOT or Drill 1.9.0-Fix17

There is a big difference between, say a SNAPSHOT version and the released version...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, that's why I added a big javadoc notice to rely on the version string for semver. Technically we should also add the build version if we wanted to be maven compatible


/**
* Get the Drill version from pom
* @return the version number as x.y.z
*/
public static String getVersion() {
return VERSION;
}

/**
* Get the Drill major version from pom
* @return x if assuming the version number is x.y.z
*/
public static int getMajorVersion() {
return MAJOR_VERSION;
}

/**
* Get the Drill minor version from pom
* @return y if assuming the version number is x.y.z
*/
public static int getMinorVersion() {
return MINOR_VERSION;
}

/**
* Get the Drill patch version from pom
* @return z if assuming the version number is x.y.z(-suffix)
*/
public static int getPatchVersion() {
return PATCH_VERSION;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package org.apache.drill.exec.client;

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
import static org.apache.drill.exec.proto.UserProtos.QueryResultsMode.STREAM_FULL;
import static org.apache.drill.exec.proto.UserProtos.RunQuery.newBuilder;
Expand Down Expand Up @@ -66,6 +67,7 @@
import org.apache.drill.exec.proto.UserProtos.PreparedStatementHandle;
import org.apache.drill.exec.proto.UserProtos.Property;
import org.apache.drill.exec.proto.UserProtos.QueryPlanFragments;
import org.apache.drill.exec.proto.UserProtos.RpcEndpointInfos;
import org.apache.drill.exec.proto.UserProtos.RpcType;
import org.apache.drill.exec.proto.UserProtos.RunQuery;
import org.apache.drill.exec.proto.UserProtos.UserProperties;
Expand Down Expand Up @@ -98,6 +100,8 @@
* String into ByteBuf.
*/
public class DrillClient implements Closeable, ConnectionThrottle {
public static final String DEFAULT_CLIENT_NAME = "Apache Drill Java client";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't there a C++ client? Perhaps the default name should be "Undefined Drill client" to allow for this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The C++ client will have a default Apache Drill C++ client, with an option to override it by the user

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, since I already have a big C++ patch in progress, the C++ change will be done in that branch


private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(DrillClient.class);

private static final ObjectMapper objectMapper = new ObjectMapper();
Expand All @@ -115,6 +119,7 @@ public class DrillClient implements Closeable, ConnectionThrottle {
private final boolean isDirectConnection; // true if the connection bypasses zookeeper and connects directly to a drillbit
private EventLoopGroup eventLoopGroup;
private ExecutorService executor;
private String clientName = DEFAULT_CLIENT_NAME;

public DrillClient() throws OutOfMemoryException {
this(DrillConfig.create(), false);
Expand Down Expand Up @@ -175,6 +180,23 @@ public void setAutoRead(boolean enableAutoRead) {
client.setAutoRead(enableAutoRead);
}

/**
* Sets the client name.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ambiguous. What "client name"? The name of the user? The name of the client application? The name of the client API? ...

Please explain.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Client name is more of the user agent, like JDBC client, ODBC client, C++ connector, etc...
For Java the application name is captured using the RuntimeMXBean, but maybe it should be overrideable too...

*
* If not set, default is {@code DrillClient#DEFAULT_CLIENT_NAME}.
*
* @param name the client name
*
* @throws IllegalStateException if called after a connection has been established.
* @throws NullPointerException if client name is null
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Old clients won't know to provide a name. The name will thus be null. But, we throw an NPE in this case. Two issues.

First, is an NPE the right way to communicate to a (new) client that it failed to follow the (now required) protocol? Should this be translated to some kind of protocol exception somewhere? If so, how will we know that this is an NPE from a protocol error vs just a code bug?

Second, where do we handle the case that a client does not provide a name? There should be a reasonable default (which seems to be the default name, but this bit is no exactly clear.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DrillClient have a default name, so name won't be null. Setting a null name though will cause an exception...

*/
public void setClientName(String name) {
if (connected) {
throw new IllegalStateException("Attempted to modify client connection property after connection has been established.");
}
this.clientName = checkNotNull(name, "client name should not be null");
}

/**
* Sets whether the application is willing to accept complex types (Map, Arrays) in the returned result set.
* Default is {@code true}. If set to {@code false}, the complex types are returned as JSON encoded VARCHAR type.
Expand Down Expand Up @@ -252,7 +274,7 @@ protected void afterExecute(final Runnable r, final Throwable t) {
super.afterExecute(r, t);
}
};
client = new UserClient(config, supportComplexTypes, allocator, eventLoopGroup, executor);
client = new UserClient(clientName, config, supportComplexTypes, allocator, eventLoopGroup, executor);
logger.debug("Connecting to server {}:{}", endpoint.getAddress(), endpoint.getUserPort());
connect(endpoint);
connected = true;
Expand Down Expand Up @@ -330,6 +352,19 @@ public void close() {
connected = false;
}


/**
* Return the server infos. Only available after connecting
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please explain what the "server info" is and we get it from the client object. Is this information about the server associated with this particular client connection? Information provided to the server by this client? Something else?

*
* The result might be null if the server doesn't provide the informations.
*
* @return the server informations, or null if not connected or if the server
* doesn't provide the information
*/
public RpcEndpointInfos getServerInfos() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Infos -> Info

"Information" is already plural.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I learned something :)

return client != null ? client.getServerInfos() : null;
}

/**
* Submits a string based query plan for execution and returns the result batches. Supported query types are:
* <p><ul>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@
*/
package org.apache.drill.exec.rpc.user;

import io.netty.buffer.ByteBuf;
import io.netty.channel.EventLoopGroup;

import java.util.concurrent.Executor;

import org.apache.drill.common.config.DrillConfig;
Expand All @@ -39,6 +36,7 @@
import org.apache.drill.exec.proto.UserProtos.GetTablesResp;
import org.apache.drill.exec.proto.UserProtos.HandshakeStatus;
import org.apache.drill.exec.proto.UserProtos.QueryPlanFragments;
import org.apache.drill.exec.proto.UserProtos.RpcEndpointInfos;
import org.apache.drill.exec.proto.UserProtos.RpcType;
import org.apache.drill.exec.proto.UserProtos.RunQuery;
import org.apache.drill.exec.proto.UserProtos.UserProperties;
Expand All @@ -55,14 +53,20 @@

import com.google.protobuf.MessageLite;

import io.netty.buffer.ByteBuf;
import io.netty.channel.EventLoopGroup;

public class UserClient extends BasicClientWithConnection<RpcType, UserToBitHandshake, BitToUserHandshake> {
private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(UserClient.class);

private final QueryResultHandler queryResultHandler = new QueryResultHandler();
private final String clientName;
private RpcEndpointInfos serverInfos = null;

private boolean supportComplexTypes = true;

public UserClient(DrillConfig config, boolean supportComplexTypes, BufferAllocator alloc,
EventLoopGroup eventLoopGroup, Executor eventExecutor) {
public UserClient(String clientName, DrillConfig config, boolean supportComplexTypes,
BufferAllocator alloc, EventLoopGroup eventLoopGroup, Executor eventExecutor) {
super(
UserRpcConfig.getMapping(config, eventExecutor),
alloc,
Expand All @@ -71,9 +75,14 @@ public UserClient(DrillConfig config, boolean supportComplexTypes, BufferAllocat
BitToUserHandshake.class,
BitToUserHandshake.PARSER,
"user client");
this.clientName = clientName;
this.supportComplexTypes = supportComplexTypes;
}

public RpcEndpointInfos getServerInfos() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Infos -> Info

return serverInfos;
}

public void submitQuery(UserResultsListener resultsListener, RunQuery query) {
send(queryResultHandler.getWrappedListener(resultsListener), RpcType.RUN_QUERY, query, QueryId.class);
}
Expand All @@ -85,7 +94,8 @@ public void connect(RpcConnectionHandler<ServerConnection> handler, DrillbitEndp
.setSupportListening(true)
.setSupportComplexTypes(supportComplexTypes)
.setSupportTimeout(true)
.setCredentials(credentials);
.setCredentials(credentials)
.setClientInfos(UserRpcUtils.getRpcEndpointInfos(clientName));

if (props != null) {
hsBuilder.setProperties(props);
Expand Down Expand Up @@ -141,6 +151,9 @@ protected Response handleReponse(ConnectionThrottle throttle, int rpcType, ByteB
@Override
protected void validateHandshake(BitToUserHandshake inbound) throws RpcException {
// logger.debug("Handling handshake from bit to user. {}", inbound);
if (inbound.hasServerInfos()) {
serverInfos = inbound.getServerInfos();
}
if (inbound.getStatus() != HandshakeStatus.SUCCESS) {
final String errMsg = String.format("Status: %s, Error Id: %s, Error message: %s",
inbound.getStatus(), inbound.getErrorId(), inbound.getErrorMessage());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.drill.exec.rpc.user;

import java.lang.management.ManagementFactory;
import java.lang.management.RuntimeMXBean;

import org.apache.drill.common.util.DrillVersionInfo;
import org.apache.drill.exec.proto.UserProtos.RpcEndpointInfos;

import com.google.common.base.Preconditions;

/**
* Utility class for User RPC
*
*/
public final class UserRpcUtils {
private UserRpcUtils() {}

/**
* Returns a {@code RpcEndpointInfos} instance
*
* The instance is populated based on Drill version informations
* from the classpath and runtime information for the application
* name.
*
* @param name the endpoint name.
* @return a {@code RpcEndpointInfos} instance
* @throws NullPointerException if name is null
*/
public static RpcEndpointInfos getRpcEndpointInfos(String name) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Infos -> Info

RuntimeMXBean mxBean = ManagementFactory.getRuntimeMXBean();
RpcEndpointInfos infos = RpcEndpointInfos.newBuilder()
.setName(Preconditions.checkNotNull(name))
.setApplication(mxBean.getName())
.setVersion(DrillVersionInfo.getVersion())
.setMajorVersion(DrillVersionInfo.getMajorVersion())
.setMinorVersion(DrillVersionInfo.getMinorVersion())
.setPatchVersion(DrillVersionInfo.getPatchVersion())
.build();

return infos;
}
}
Loading