You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by jd...@apache.org on 2017/02/17 17:46:45 UTC
[1/3] kudu git commit: java: refactor logic for fanning out master
connection into a class
Repository: kudu
Updated Branches:
refs/heads/master f0af095e5 -> 47409f4a5
java: refactor logic for fanning out master connection into a class
This moves some code out of AsyncKuduClient into a couple of static
methods in the ConnectToCluster class. No functional changes, just
cleanup.
Change-Id: Ie463b21aef4745863c303ea975aa61696deeef18
Reviewed-on: http://gerrit.cloudera.org:8080/6028
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans <jd...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/866287c5
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/866287c5
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/866287c5
Branch: refs/heads/master
Commit: 866287c53a0c3ef14f83fafe3f9454e0685fb7aa
Parents: f0af095
Author: Todd Lipcon <to...@apache.org>
Authored: Wed Feb 15 13:15:41 2017 -0800
Committer: Todd Lipcon <to...@apache.org>
Committed: Fri Feb 17 17:40:34 2017 +0000
----------------------------------------------------------------------
.../org/apache/kudu/client/AsyncKuduClient.java | 64 +----------------
.../apache/kudu/client/ConnectToCluster.java | 74 ++++++++++++++++++--
.../kudu/client/ConnectToMasterRequest.java | 4 +-
.../org/apache/kudu/client/ConnectionCache.java | 8 +++
.../kudu/client/TestConnectToCluster.java | 8 +--
5 files changed, 83 insertions(+), 75 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/866287c5/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java b/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
index 6f404a1..680773e 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
@@ -1067,30 +1067,10 @@ public class AsyncKuduClient implements AutoCloseable {
* @return An initialized Deferred object to hold the response.
*/
Deferred<Master.GetTableLocationsResponsePB> getMasterTableLocationsPB(KuduRpc<?> parentRpc) {
- final Deferred<Master.GetTableLocationsResponsePB> responseD = new Deferred<>();
- final ConnectToCluster connector =
- new ConnectToCluster(masterAddresses, responseD);
- for (HostAndPort hostAndPort : masterAddresses) {
- Deferred<ConnectToClusterResponse> d;
- // Note: we need to create a client for that host first, as there's a
- // chicken and egg problem: since there is no source of truth beyond
- // the master, the only way to get information about a master host is
- // by making an RPC to that host.
- TabletClient clientForHostAndPort = newMasterClient(hostAndPort);
- if (clientForHostAndPort == null) {
- String message = "Couldn't resolve this master's address " + hostAndPort.toString();
- LOG.warn(message);
- Status statusIOE = Status.IOError(message);
- d = Deferred.fromError(new NonRecoverableException(statusIOE));
- } else {
- d = getMasterRegistration(clientForHostAndPort, parentRpc);
- }
- d.addCallbacks(connector.callbackForNode(hostAndPort), connector.errbackForNode(hostAndPort));
- }
- return responseD;
+ return ConnectToCluster.run(masterAddresses, parentRpc,
+ connectionCache, defaultAdminOperationTimeoutMs);
}
-
/**
* Get all or some tablets for a given table. This may query the master multiple times if there
* are a lot of tablets.
@@ -1486,46 +1466,6 @@ public class AsyncKuduClient implements AutoCloseable {
}
/**
- * Retrieve the master registration (see {@link ConnectToClusterResponse}
- * for a replica.
- * @param masterClient an initialized client for the master replica
- * @param parentRpc RPC that prompted a master lookup, can be null
- * @return a Deferred object for the master replica's current registration
- */
- Deferred<ConnectToClusterResponse> getMasterRegistration(
- TabletClient masterClient, KuduRpc<?> parentRpc) {
- // TODO: Handle the situation when multiple in-flight RPCs all want to query the masters,
- // basically reuse in some way the master permits.
- ConnectToMasterRequest rpc = new ConnectToMasterRequest(masterTable);
- if (parentRpc != null) {
- rpc.setTimeoutMillis(parentRpc.deadlineTracker.getMillisBeforeDeadline());
- rpc.setParentRpc(parentRpc);
- } else {
- rpc.setTimeoutMillis(defaultAdminOperationTimeoutMs);
- }
- Deferred<ConnectToClusterResponse> d = rpc.getDeferred();
- rpc.attempt++;
- masterClient.sendRpc(rpc);
- return d;
- }
-
- /**
- * If a live client already exists for the specified master server, returns that client;
- * otherwise, creates a new client for the specified master server.
- * @param masterHostPort The RPC host and port for the master server.
- * @return A live and initialized client for the specified master server.
- */
- TabletClient newMasterClient(HostAndPort masterHostPort) {
- // We should pass a UUID here but we have a chicken and egg problem, we first need to
- // communicate with the masters to find out about them, and that's what we're trying to do.
- // The UUID is used for logging, so instead we're passing the "master table name" followed by
- // host and port which is enough to identify the node we're connecting to.
- return connectionCache.newClient(
- MASTER_TABLE_NAME_PLACEHOLDER + " - " + masterHostPort.toString(),
- masterHostPort);
- }
-
- /**
* Invokes {@link #shutdown()} and waits. This method returns
* void, so consider invoking shutdown directly if there's a need to handle dangling RPCs.
* @throws Exception if an error happens while closing the connections
http://git-wip-us.apache.org/repos/asf/kudu/blob/866287c5/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java b/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java
index 1de40d0..eb21c77 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToCluster.java
@@ -23,6 +23,7 @@ import java.util.List;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Functions;
import com.google.common.base.Joiner;
import com.google.common.collect.Lists;
@@ -37,6 +38,7 @@ import org.apache.kudu.Common;
import org.apache.kudu.annotations.InterfaceAudience;
import org.apache.kudu.consensus.Metadata;
import org.apache.kudu.master.Master;
+import org.apache.kudu.master.Master.GetTableLocationsResponsePB;
import org.apache.kudu.util.NetUtil;
/**
@@ -71,13 +73,73 @@ final class ConnectToCluster {
* @param responseD Deferred object that will hold the GetTableLocationsResponsePB object for
* the master table.
*/
- public ConnectToCluster(List<HostAndPort> masterAddrs,
- Deferred<Master.GetTableLocationsResponsePB> responseD) {
+ ConnectToCluster(List<HostAndPort> masterAddrs) {
this.masterAddrs = masterAddrs;
- this.responseD = responseD;
+ this.responseD = new Deferred<>();
this.numMasters = masterAddrs.size();
}
+ public Deferred<GetTableLocationsResponsePB> getDeferred() {
+ return responseD;
+ }
+
+ private static Deferred<ConnectToClusterResponse> getMasterRegistration(
+ TabletClient masterClient, KuduRpc<?> parentRpc,
+ long defaultTimeoutMs) {
+ // TODO: Handle the situation when multiple in-flight RPCs all want to query the masters,
+ // basically reuse in some way the master permits.
+ // TODO: is null below for table object ok?
+ ConnectToMasterRequest rpc = new ConnectToMasterRequest();
+ if (parentRpc != null) {
+ rpc.setTimeoutMillis(parentRpc.deadlineTracker.getMillisBeforeDeadline());
+ rpc.setParentRpc(parentRpc);
+ } else {
+ rpc.setTimeoutMillis(defaultTimeoutMs);
+ }
+ Deferred<ConnectToClusterResponse> d = rpc.getDeferred();
+ rpc.attempt++;
+ masterClient.sendRpc(rpc);
+ return d;
+ }
+
+ /**
+ * Retrieve the master registration (see {@link ConnectToClusterResponse}
+ * from the leader master.
+ *
+ * @param masterAddresses the addresses of masters to fetch from
+ * @param parentRpc RPC that prompted a master lookup, can be null
+ * @param connCache the client's connection cache, used for creating connections
+ * to masters
+ * @param defaultTimeoutMs timeout to use for RPCs if the parentRpc has no timeout
+ * @return a Deferred object for the master replica's current registration
+ */
+ public static Deferred<GetTableLocationsResponsePB> run(
+ List<HostAndPort> masterAddresses,
+ KuduRpc<?> parentRpc,
+ ConnectionCache connCache,
+ long defaultTimeoutMs) {
+ ConnectToCluster connector = new ConnectToCluster(masterAddresses);
+
+ // Try to connect to each master. The ConnectToCluster instance
+ // waits until it gets a good response before firing the returned
+ // deferred.
+ for (HostAndPort hostAndPort : masterAddresses) {
+ Deferred<ConnectToClusterResponse> d;
+ TabletClient client = connCache.newMasterClient(hostAndPort);
+ if (client == null) {
+ String message = "Couldn't resolve this master's address " + hostAndPort.toString();
+ LOG.warn(message);
+ Status statusIOE = Status.IOError(message);
+ d = Deferred.fromError(new NonRecoverableException(statusIOE));
+ } else {
+ d = getMasterRegistration(client, parentRpc, defaultTimeoutMs);
+ }
+ d.addCallbacks(connector.callbackForNode(hostAndPort),
+ connector.errbackForNode(hostAndPort));
+ }
+ return connector.responseD;
+ }
+
/**
* Creates a callback for a GetMasterRegistrationRequest that was sent to 'hostAndPort'.
* @see GetMasterRegistrationCB
@@ -85,7 +147,8 @@ final class ConnectToCluster {
* be valid.
* @return The callback object that can be added to the RPC request.
*/
- public Callback<Void, ConnectToClusterResponse> callbackForNode(HostAndPort hostAndPort) {
+ @VisibleForTesting
+ Callback<Void, ConnectToClusterResponse> callbackForNode(HostAndPort hostAndPort) {
return new GetMasterRegistrationCB(hostAndPort);
}
@@ -96,7 +159,8 @@ final class ConnectToCluster {
* purposes.
* @return The errback object that can be added to the RPC request.
*/
- public Callback<Void, Exception> errbackForNode(HostAndPort hostAndPort) {
+ @VisibleForTesting
+ Callback<Void, Exception> errbackForNode(HostAndPort hostAndPort) {
return new GetMasterRegistrationErrCB(hostAndPort);
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/866287c5/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToMasterRequest.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToMasterRequest.java b/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToMasterRequest.java
index a98ac38..dc77a8a 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToMasterRequest.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToMasterRequest.java
@@ -34,8 +34,8 @@ import org.apache.kudu.util.Pair;
public class ConnectToMasterRequest extends KuduRpc<ConnectToClusterResponse> {
private static final String GET_MASTER_REGISTRATION = "GetMasterRegistration";
- public ConnectToMasterRequest(KuduTable masterTable) {
- super(masterTable);
+ public ConnectToMasterRequest() {
+ super(null); // no KuduTable
}
@Override
http://git-wip-us.apache.org/repos/asf/kudu/blob/866287c5/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java b/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java
index 3c0a055..3a6d7b4 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java
@@ -121,6 +121,14 @@ class ConnectionCache {
return newClient(new ServerInfo(uuid, hostPort, inetAddress)).getServerInfo();
}
+ TabletClient newMasterClient(HostAndPort hostPort) {
+ // We should pass a UUID here but we have a chicken and egg problem, we first need to
+ // communicate with the masters to find out about them, and that's what we're trying to do.
+ // The UUID is just used for logging and cache key, so instead we just use a constructed
+ // string with the master host and port as.
+ return newClient("master-" + hostPort.toString(), hostPort);
+ }
+
TabletClient newClient(String uuid, HostAndPort hostPort) {
InetAddress inetAddress = NetUtil.getInetAddress(hostPort.getHostText());
if (inetAddress == null) {
http://git-wip-us.apache.org/repos/asf/kudu/blob/866287c5/java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectToCluster.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectToCluster.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectToCluster.java
index eb10443..eb3cb5c 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectToCluster.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectToCluster.java
@@ -27,12 +27,10 @@ import java.util.List;
import com.google.common.collect.ImmutableList;
import com.google.common.net.HostAndPort;
import com.stumbleupon.async.Callback;
-import com.stumbleupon.async.Deferred;
import org.junit.Test;
import org.apache.kudu.WireProtocol;
import org.apache.kudu.consensus.Metadata;
-import org.apache.kudu.master.Master;
public class TestConnectToCluster {
@@ -161,9 +159,7 @@ public class TestConnectToCluster {
// Here we basically do what AsyncKuduClient would do, add all the callbacks and then we also
// add the responses. We then check for the right response.
- Deferred<Master.GetTableLocationsResponsePB> d = new Deferred<>();
-
- ConnectToCluster grrm = new ConnectToCluster(MASTERS, d);
+ ConnectToCluster grrm = new ConnectToCluster(MASTERS);
Callback<Void, ConnectToClusterResponse> cb0 = grrm.callbackForNode(MASTERS.get(0));
Callback<Void, ConnectToClusterResponse> cb1 = grrm.callbackForNode(MASTERS.get(1));
@@ -178,7 +174,7 @@ public class TestConnectToCluster {
callTheRightCallback(cb2, eb2, response2);
try {
- d.join(); // Don't care about the response.
+ grrm.getDeferred().join(); // Don't care about the response.
if ((expectedResponse instanceof Exception)) {
fail("Should not work " + expectedResponse.getClass());
} else {
[3/3] kudu git commit: [java client] Fix javadoc in ServerInfo
Posted by jd...@apache.org.
[java client] Fix javadoc in ServerInfo
This was missed during code review.
Change-Id: Ic4b415c9a436d568dd32448c677b200ad0e2c43c
Reviewed-on: http://gerrit.cloudera.org:8080/6039
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/47409f4a
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/47409f4a
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/47409f4a
Branch: refs/heads/master
Commit: 47409f4a59a9d52ca69618534793cb3eb0b0e748
Parents: de69932
Author: Jean-Daniel Cryans <jd...@apache.org>
Authored: Thu Feb 16 12:54:01 2017 -0800
Committer: Todd Lipcon <to...@apache.org>
Committed: Fri Feb 17 17:45:19 2017 +0000
----------------------------------------------------------------------
.../src/main/java/org/apache/kudu/client/ServerInfo.java | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/47409f4a/java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java b/java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java
index ce72f0e..6d26b8f 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java
@@ -38,9 +38,8 @@ public class ServerInfo {
* Constructor for all the fields. The intent is that there should only be one ServerInfo
* instance per UUID the client is connected to.
* @param uuid server's UUID
- * @param hostname server's hostname, only one of them
- * @param port server's port
- * @param local if the server is hosted on the same machine where this client is running
+ * @param hostPort server's hostname and port
+ * @param resolvedAddr resolved address used to check if the server is local
*/
public ServerInfo(String uuid, HostAndPort hostPort, InetAddress resolvedAddr) {
this.uuid = uuid;
[2/3] kudu git commit: Tag kerberos_principal flag as unsafe
Posted by jd...@apache.org.
Tag kerberos_principal flag as unsafe
See the new comment and KUDU-1884 for rationale.
Change-Id: I6b296265c9a62a2908222d28903a47ea31719db2
Reviewed-on: http://gerrit.cloudera.org:8080/6036
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/de699327
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/de699327
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/de699327
Branch: refs/heads/master
Commit: de69932749e33663bb8d49db5a4d2f3fc74bbbaf
Parents: 866287c
Author: Todd Lipcon <to...@apache.org>
Authored: Wed Feb 15 23:42:00 2017 -0800
Committer: Todd Lipcon <to...@apache.org>
Committed: Fri Feb 17 17:41:42 2017 +0000
----------------------------------------------------------------------
src/kudu/security/init.cc | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/de699327/src/kudu/security/init.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/init.cc b/src/kudu/security/init.cc
index b5e2e41..7e21d89 100644
--- a/src/kudu/security/init.cc
+++ b/src/kudu/security/init.cc
@@ -37,10 +37,11 @@ DEFINE_string(kerberos_principal, "kudu/_HOST",
"Kerberos principal that this daemon will log in as. The special token "
"_HOST will be replaced with the FQDN of the local host.");
TAG_FLAG(kerberos_principal, experimental);
-
-// TODO(todd): this currently only affects the keytab login which is used
-// for client credentials, but doesn't affect the SASL server code path.
-// We probably need to plumb the same configuration into the RPC code.
+// This is currently tagged as unsafe because there is no way for users to configure
+// clients to expect a non-default principal. As such, configuring a server to login
+// as a different one would end up with a cluster that can't be connected to.
+// See KUDU-1884.
+TAG_FLAG(kerberos_principal, unsafe);
using std::mt19937;
using std::random_device;