You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by to...@apache.org on 2018/03/15 17:23:41 UTC
[1/2] kudu git commit: java: key ConnectionCache by address,
improve stringification
Repository: kudu
Updated Branches:
refs/heads/master f2479e21d -> ead756844
java: key ConnectionCache by address, improve stringification
This changes the ConnectionCache in the Java client to be keyed by
InetSocketAddress instead of server UUID. Although we currently assume
that an address and UUID have a 1:1 correspondence, the logical purpose
of the connection cache is to cache a connection to a specific server
endpoint, and if a server were to re-register at a new IP, we'd really
need to reconnect.
Along the way this also adds and improves toString() methods for a
number of core structures in the Java client. These new stringifications
helped me debug a recent issue in the client where it wasn't clear that
we were connecting to the wrong host.
Change-Id: I431b5b6b8af6b81e6ab494d7f84fa2260bb0f941
Reviewed-on: http://gerrit.cloudera.org:8080/9643
Tested-by: Kudu Jenkins
Reviewed-by: Grant Henke <gr...@gmail.com>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/8204f43f
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/8204f43f
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/8204f43f
Branch: refs/heads/master
Commit: 8204f43f0204c172c4a799f9ccda0592d3b02138
Parents: f2479e2
Author: Todd Lipcon <to...@apache.org>
Authored: Wed Mar 14 14:05:34 2018 -0700
Committer: Grant Henke <gr...@gmail.com>
Committed: Thu Mar 15 16:13:09 2018 +0000
----------------------------------------------------------------------
.../apache/kudu/client/AsyncKuduScanner.java | 2 +-
.../java/org/apache/kudu/client/Connection.java | 5 +-
.../org/apache/kudu/client/ConnectionCache.java | 31 +++++-----
.../org/apache/kudu/client/RemoteTablet.java | 23 +++++++-
.../java/org/apache/kudu/client/ServerInfo.java | 13 ++++-
.../apache/kudu/client/TableLocationsCache.java | 30 +++++-----
.../apache/kudu/client/TestConnectionCache.java | 30 +++-------
.../apache/kudu/client/TestRemoteTablet.java | 50 +++++++++-------
.../org/apache/kudu/client/TestServerInfo.java | 9 +--
.../kudu/client/TestTableLocationsCache.java | 61 ++++++++++++++++++++
10 files changed, 167 insertions(+), 87 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/8204f43f/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java b/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
index 0863148..24581af 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
@@ -473,7 +473,7 @@ public final class AsyncKuduScanner {
sequenceId = 0;
return nextRows();
} else {
- LOG.warn("Can not open scanner", e);
+ LOG.debug("Can not open scanner", e);
// Don't let the scanner think it's opened on this tablet.
return Deferred.fromError(e); // Let the error propogate.
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/8204f43f/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java b/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
index 96395e4..b7513e4 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
@@ -17,7 +17,6 @@
package org.apache.kudu.client;
-import java.net.InetSocketAddress;
import java.nio.channels.ClosedChannelException;
import java.util.ArrayList;
import java.util.HashMap;
@@ -483,7 +482,7 @@ class Connection extends SimpleChannelUpstreamHandler {
/** @return string representation of the peer information suitable for logging */
String getLogPrefix() {
- return "[peer " + serverInfo.getUuid() + "]";
+ return "[peer " + serverInfo + "]";
}
/**
@@ -704,7 +703,7 @@ class Connection extends SimpleChannelUpstreamHandler {
Preconditions.checkState(lock.isHeldByCurrentThread());
Preconditions.checkState(state == State.NEW);
state = State.CONNECTING;
- channel.connect(new InetSocketAddress(serverInfo.getResolvedAddress(), serverInfo.getPort()));
+ channel.connect(serverInfo.getResolvedAddress());
}
/** Enumeration to represent the internal state of the Connection object. */
http://git-wip-us.apache.org/repos/asf/kudu/blob/8204f43f/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 db37425..c586886 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
@@ -17,6 +17,7 @@
package org.apache.kudu.client;
+import java.net.InetSocketAddress;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
@@ -40,7 +41,7 @@ import org.jboss.netty.util.HashedWheelTimer;
* <p>
* Disconnected instances of the {@link Connection} class are replaced in the cache with new ones
* when {@link #getConnection(ServerInfo, Connection.CredentialsPolicy)} method is called with the
- * same destination and matching credentials policy. Since the map is keyed by the UUID of the
+ * same destination and matching credentials policy. Since the map is keyed by the address of the
* target server, the theoretical maximum number of elements in the cache is twice the number of
* all servers in the cluster (i.e. both masters and tablet servers). However, in practice it's
* 2 * number of masters + number of tablet servers since tablet servers do not require connections
@@ -65,15 +66,13 @@ class ConnectionCache {
private final ClientSocketChannelFactory channelFactory;
/**
- * Container mapping server UUID into the established connection from the client to the server.
- * It may be up to two connections per server: one established with secondary credentials
- * (e.g. authn token), another with primary ones (e.g. Kerberos credentials).
- *
- * TODO(todd) it would make more sense to key this by IP address rather than by UUID in
- * case a server actually changes address and re-registers to the cluster.
+ * Container mapping server IP/port into the established connection from the client to the
+ * server. It may be up to two connections per server: one established with secondary
+ * credentials (e.g. authn token), another with primary ones (e.g. Kerberos credentials).
*/
- @GuardedBy("uuid2connection")
- private final HashMultimap<String, Connection> uuid2connection = HashMultimap.create();
+ @GuardedBy("connsByAddress")
+ private final HashMultimap<InetSocketAddress, Connection> connsByAddress =
+ HashMultimap.create();
/** Create a new empty ConnectionCache given the specified parameters. */
ConnectionCache(SecurityContext securityContext,
@@ -98,7 +97,7 @@ class ConnectionCache {
public Connection getConnection(final ServerInfo serverInfo,
Connection.CredentialsPolicy credentialsPolicy) {
Connection result = null;
- synchronized (uuid2connection) {
+ synchronized (connsByAddress) {
// Create and register a new connection object into the cache if one of the following is true:
//
// * There isn't a registered connection to the specified destination.
@@ -115,7 +114,7 @@ class ConnectionCache {
// special to the old connection to shut it down since it may be still in use. We rely
// on the server to close inactive connections in accordance with their TTL settings.
//
- final Set<Connection> connections = uuid2connection.get(serverInfo.getUuid());
+ final Set<Connection> connections = connsByAddress.get(serverInfo.getResolvedAddress());
Iterator<Connection> it = connections.iterator();
while (it.hasNext()) {
Connection c = it.next();
@@ -147,9 +146,9 @@ class ConnectionCache {
/** Asynchronously terminate every connection. This cancels all the pending and in-flight RPCs. */
Deferred<ArrayList<Void>> disconnectEverything() {
- synchronized (uuid2connection) {
- List<Deferred<Void>> deferreds = new ArrayList<>(uuid2connection.size());
- for (Connection c : uuid2connection.values()) {
+ synchronized (connsByAddress) {
+ List<Deferred<Void>> deferreds = new ArrayList<>(connsByAddress.size());
+ for (Connection c : connsByAddress.values()) {
deferreds.add(c.shutdown());
}
return Deferred.group(deferreds);
@@ -165,8 +164,8 @@ class ConnectionCache {
*/
@VisibleForTesting
List<Connection> getConnectionListCopy() {
- synchronized (uuid2connection) {
- return ImmutableList.copyOf(uuid2connection.values());
+ synchronized (connsByAddress) {
+ return ImmutableList.copyOf(connsByAddress.values());
}
}
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/8204f43f/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java b/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
index a6025f7..bb2ca27 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
@@ -17,6 +17,8 @@
package org.apache.kudu.client;
+import java.util.ArrayList;
+import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -24,6 +26,7 @@ import java.util.concurrent.atomic.AtomicReference;
import javax.annotation.Nullable;
import javax.annotation.concurrent.GuardedBy;
+import com.google.common.base.Joiner;
import com.google.common.base.Objects;
import com.google.common.collect.ComparisonChain;
import com.google.common.collect.ImmutableList;
@@ -92,7 +95,22 @@ class RemoteTablet implements Comparable<RemoteTablet> {
@Override
public String toString() {
- return getTabletId();
+ StringBuilder sb = new StringBuilder();
+ sb.append(tabletId).append("@[");
+ List<String> tsStrings;
+ synchronized (tabletServers) {
+ tsStrings = new ArrayList<>(tabletServers.size());
+ for (ServerInfo e : tabletServers.values()) {
+ String flag = e.getUuid().equals(leaderUuid) ? "[L]" : "";
+ tsStrings.add(e.toString() + flag);
+ }
+ }
+ // Sort so that we have a consistent iteration order regardless of
+ // HashSet ordering.
+ Collections.sort(tsStrings);
+ sb.append(Joiner.on(',').join(tsStrings));
+ sb.append(']');
+ return sb.toString();
}
/**
@@ -168,6 +186,9 @@ class RemoteTablet implements Comparable<RemoteTablet> {
return e;
}
}
+ // TODO(KUDU-2348) this doesn't return a random server, but rather returns
+ // whichever one's hashcode places it last. That might be the same
+ // "random" choice across all clients, which is not so good.
return last;
}
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/8204f43f/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 4858a16..bed3e21 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
@@ -18,6 +18,7 @@
package org.apache.kudu.client;
import java.net.InetAddress;
+import java.net.InetSocketAddress;
import java.net.UnknownHostException;
import java.util.concurrent.ConcurrentHashMap;
@@ -34,7 +35,7 @@ import org.apache.kudu.util.NetUtil;
public class ServerInfo {
private final String uuid;
private final HostAndPort hostPort;
- private final InetAddress resolvedAddr;
+ private final InetSocketAddress resolvedAddr;
private final boolean local;
private static final ConcurrentHashMap<InetAddress, Boolean> isLocalAddressCache =
new ConcurrentHashMap<>();
@@ -48,9 +49,10 @@ public class ServerInfo {
*/
public ServerInfo(String uuid, HostAndPort hostPort, InetAddress resolvedAddr) {
Preconditions.checkNotNull(uuid);
+ Preconditions.checkArgument(hostPort.getPort() > 0);
this.uuid = uuid;
this.hostPort = hostPort;
- this.resolvedAddr = resolvedAddr;
+ this.resolvedAddr = new InetSocketAddress(resolvedAddr, hostPort.getPort());
Boolean isLocal = isLocalAddressCache.get(resolvedAddr);
if (isLocal == null) {
isLocal = NetUtil.isLocalAddress(resolvedAddr);
@@ -98,7 +100,12 @@ public class ServerInfo {
/**
* @return the cached resolved address for this server
*/
- public InetAddress getResolvedAddress() {
+ public InetSocketAddress getResolvedAddress() {
return resolvedAddr;
}
+
+ @Override
+ public String toString() {
+ return uuid + "(" + hostPort + ")";
+ }
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/8204f43f/java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java b/java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java
index eac658c..3e299ef 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/TableLocationsCache.java
@@ -29,8 +29,10 @@ import java.util.concurrent.locks.ReentrantReadWriteLock;
import javax.annotation.concurrent.GuardedBy;
import javax.annotation.concurrent.ThreadSafe;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.MoreObjects;
import com.google.common.base.Preconditions;
+import com.google.common.base.Ticker;
import com.google.common.primitives.UnsignedBytes;
import org.apache.yetus.audience.InterfaceAudience;
import org.slf4j.Logger;
@@ -51,6 +53,9 @@ class TableLocationsCache {
@GuardedBy("rwl")
private final NavigableMap<byte[], Entry> entries = new TreeMap<>(COMPARATOR);
+ @VisibleForTesting
+ static Ticker ticker = Ticker.systemTicker();
+
public Entry get(byte[] partitionKey) {
if (partitionKey == null) {
@@ -97,7 +102,7 @@ class TableLocationsCache {
byte[] requestPartitionKey,
int requestedBatchSize,
long ttl) {
- long deadline = System.nanoTime() + ttl * TimeUnit.MILLISECONDS.toNanos(1);
+ long deadline = ticker.read() + ttl * TimeUnit.MILLISECONDS.toNanos(1);
if (requestPartitionKey == null) {
// Master lookup.
Preconditions.checkArgument(tablets.size() == 1);
@@ -290,7 +295,7 @@ class TableLocationsCache {
}
private long ttl() {
- return TimeUnit.NANOSECONDS.toMillis(deadline - System.nanoTime());
+ return TimeUnit.NANOSECONDS.toMillis(deadline - ticker.read());
}
public boolean isStale() {
@@ -299,20 +304,13 @@ class TableLocationsCache {
@Override
public String toString() {
- if (isNonCoveredRange()) {
- return MoreObjects.toStringHelper("NonCoveredRange")
- .add("lowerBoundPartitionKey", Bytes.hex(lowerBoundPartitionKey))
- .add("upperBoundPartitionKey", Bytes.hex(upperBoundPartitionKey))
- .add("ttl", ttl())
- .toString();
- } else {
- return MoreObjects.toStringHelper("Tablet")
- .add("lowerBoundPartitionKey", Bytes.hex(getLowerBoundPartitionKey()))
- .add("upperBoundPartitionKey", Bytes.hex(getUpperBoundPartitionKey()))
- .add("tablet-id", tablet.getTabletId())
- .add("ttl", ttl())
- .toString();
- }
+ return MoreObjects.toStringHelper(isNonCoveredRange() ? "NonCoveredRange" : "Tablet")
+ .omitNullValues()
+ .add("lowerBoundPartitionKey", Bytes.hex(getLowerBoundPartitionKey()))
+ .add("upperBoundPartitionKey", Bytes.hex(getUpperBoundPartitionKey()))
+ .add("ttl", ttl())
+ .add("tablet", tablet)
+ .toString();
}
}
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/8204f43f/java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java
index 6759d59..84dacbf 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectionCache.java
@@ -22,9 +22,6 @@ import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNotSame;
import static org.junit.Assert.assertTrue;
-import java.util.List;
-
-import com.google.common.collect.Lists;
import com.google.common.net.HostAndPort;
import com.stumbleupon.async.Deferred;
import org.junit.Test;
@@ -41,34 +38,23 @@ public class TestConnectionCache {
final AsyncKuduClient client =
new AsyncKuduClient.AsyncKuduClientBuilder(cluster.getMasterAddresses()).build();
- final List<HostAndPort> addresses = cluster.getMasterHostPorts();
-
// Below we ping the masters directly using RpcProxy, so if they aren't ready to process
// RPCs we'll get an error. Here by listing the tables we make sure this won't happen since
// it won't return until a master leader is found.
client.getTablesList().join();
- final List<ServerInfo> serverInfos = Lists.newArrayList();
- int i = 0;
- for (HostAndPort hp : addresses) {
- serverInfos.add(new ServerInfo("" + i, hp, NetUtil.getInetAddress(hp.getHost())));
- ++i;
- }
-
- // Ping the process so we go through the whole connection process.
- for (ServerInfo si : serverInfos) {
- final RpcProxy h = client.newRpcProxy(si);
- assertNotNull(h.getConnection());
- pingConnection(h);
- }
+ HostAndPort masterHostPort = cluster.getMasterHostPorts().get(0);
+ ServerInfo firstMaster = new ServerInfo("fake-uuid", masterHostPort,
+ NetUtil.getInetAddress(masterHostPort.getHost()));
- // 3 masters and 3 connections from the newRpcProxy() in the loop above.
+ // 3 masters in the cluster. Connections should have been cached since we forced
+ // a cluster connection above.
// No tservers have been connected to by the client since we haven't accessed
// any data.
- assertEquals(3 + 3, client.getConnectionListCopy().size());
+ assertEquals(3, client.getConnectionListCopy().size());
assertFalse(allConnectionsTerminated(client));
- final RpcProxy proxy = client.newRpcProxy(serverInfos.get(0));
+ final RpcProxy proxy = client.newRpcProxy(firstMaster);
// Disconnect from the server.
proxy.getConnection().disconnect().awaitUninterruptibly();
@@ -80,7 +66,7 @@ public class TestConnectionCache {
assertFalse(allConnectionsTerminated(client));
// For a new RpcProxy instance, a new connection to the same destination is established.
- final RpcProxy newHelper = client.newRpcProxy(serverInfos.get(0));
+ final RpcProxy newHelper = client.newRpcProxy(firstMaster);
final Connection newConnection = newHelper.getConnection();
assertNotNull(newConnection);
assertNotSame(proxy.getConnection(), newConnection);
http://git-wip-us.apache.org/repos/asf/kudu/blob/8204f43f/java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java
index 5516648..9ba6d00 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java
@@ -35,33 +35,34 @@ import org.apache.kudu.consensus.Metadata;
import org.apache.kudu.master.Master;
public class TestRemoteTablet {
+ private static final String[] kUuids = { "uuid-0", "uuid-1", "uuid-2" };
@Test
public void testLeaderLastRemovedLast() {
RemoteTablet tablet = getTablet(2);
// Demote the wrong leader, no-op.
- assertEquals("2", tablet.getLeaderServerInfo().getUuid());
- tablet.demoteLeader("1");
- assertEquals("2", tablet.getLeaderServerInfo().getUuid());
+ assertEquals(kUuids[2], tablet.getLeaderServerInfo().getUuid());
+ tablet.demoteLeader(kUuids[1]);
+ assertEquals(kUuids[2], tablet.getLeaderServerInfo().getUuid());
// Tablet at server 1 was deleted.
- assertTrue(tablet.removeTabletClient("1"));
- assertEquals("2", tablet.getLeaderServerInfo().getUuid());
+ assertTrue(tablet.removeTabletClient(kUuids[1]));
+ assertEquals(kUuids[2], tablet.getLeaderServerInfo().getUuid());
// Simulate another thread trying to remove 1.
- assertFalse(tablet.removeTabletClient("1"));
+ assertFalse(tablet.removeTabletClient(kUuids[1]));
// Tablet at server 0 was deleted.
- assertTrue(tablet.removeTabletClient("0"));
- assertEquals("2", tablet.getLeaderServerInfo().getUuid());
+ assertTrue(tablet.removeTabletClient(kUuids[0]));
+ assertEquals(kUuids[2], tablet.getLeaderServerInfo().getUuid());
// Leader was demoted.
- tablet.demoteLeader("2");
+ tablet.demoteLeader(kUuids[2]);
assertNull(tablet.getLeaderServerInfo());
// Simulate another thread doing the same.
- tablet.demoteLeader("2");
+ tablet.demoteLeader(kUuids[2]);
assertNull(tablet.getLeaderServerInfo());
}
@@ -70,11 +71,11 @@ public class TestRemoteTablet {
RemoteTablet tablet = getTablet(2);
// Test we can remove it.
- assertTrue(tablet.removeTabletClient("2"));
+ assertTrue(tablet.removeTabletClient("uuid-2"));
assertNull(tablet.getLeaderServerInfo());
// Test demoting it doesn't break anything.
- tablet.demoteLeader("2");
+ tablet.demoteLeader("uuid-2");
assertNull(tablet.getLeaderServerInfo());
}
@@ -83,22 +84,22 @@ public class TestRemoteTablet {
RemoteTablet tablet = getTablet(0);
// Test we can remove it.
- assertTrue(tablet.removeTabletClient("0"));
+ assertTrue(tablet.removeTabletClient("uuid-0"));
assertNull(tablet.getLeaderServerInfo());
// Test demoting it doesn't break anything.
- tablet.demoteLeader("0");
+ tablet.demoteLeader("uuid-0");
assertNull(tablet.getLeaderServerInfo());
// Test removing a server with no leader doesn't break.
- assertTrue(tablet.removeTabletClient("2"));
+ assertTrue(tablet.removeTabletClient("uuid-2"));
}
@Test
public void testLocalReplica() {
RemoteTablet tablet = getTablet(0, 0);
- assertEquals("0", tablet.getClosestServerInfo().getUuid());
+ assertEquals(kUuids[0], tablet.getClosestServerInfo().getUuid());
}
@Test
@@ -113,17 +114,24 @@ public class TestRemoteTablet {
public void testReplicaSelection() {
RemoteTablet tablet = getTablet(0, 1);
- assertEquals("0",
+ assertEquals(kUuids[0],
tablet.getReplicaSelectedServerInfo(ReplicaSelection.LEADER_ONLY).getUuid());
- assertEquals("1",
+ assertEquals(kUuids[1],
tablet.getReplicaSelectedServerInfo(ReplicaSelection.CLOSEST_REPLICA).getUuid());
}
+ @Test
+ public void testToString() {
+ RemoteTablet tablet = getTablet(0, 1);
+ assertEquals("fake tablet@[uuid-0(host:1000)[L],uuid-1(host:1001),uuid-2(host:1002)]",
+ tablet.toString());
+ }
+
private RemoteTablet getTablet(int leaderIndex) {
return getTablet(leaderIndex, -1);
}
- private RemoteTablet getTablet(int leaderIndex, int localReplicaIndex) {
+ static RemoteTablet getTablet(int leaderIndex, int localReplicaIndex) {
Master.TabletLocationsPB.Builder tabletPb = Master.TabletLocationsPB.newBuilder();
tabletPb.setPartition(TestUtils.getFakePartitionPB());
@@ -141,9 +149,9 @@ public class TestRemoteTablet {
throw new RuntimeException(e);
}
- String uuid = i + "";
+ String uuid = kUuids[i];
servers.add(new ServerInfo(uuid,
- HostAndPort.fromParts("host", i),
+ HostAndPort.fromParts("host", 1000 + i),
addr));
tabletPb.addReplicas(TestUtils.getFakeTabletReplicaPB(
uuid, "host", i,
http://git-wip-us.apache.org/repos/asf/kudu/blob/8204f43f/java/kudu-client/src/test/java/org/apache/kudu/client/TestServerInfo.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestServerInfo.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestServerInfo.java
index a26e4f2..38df64d 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestServerInfo.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestServerInfo.java
@@ -28,7 +28,7 @@ public class TestServerInfo {
@Test(timeout = 500)
public void testConstructorNotSlow() throws Exception {
String uuid = "nevermind";
- HostAndPort hap = HostAndPort.fromString("nevermind");
+ HostAndPort hap = HostAndPort.fromString("nevermind:12345");
// ip to force NetworkInterface.getByInetAddress call
InetAddress ia = InetAddress.getByName("8.8.8.8");
for (int i = 0; i < 100; ++i) {
@@ -45,7 +45,7 @@ public class TestServerInfo {
ServerInfo serverInfo = new ServerInfo(
"nevermind",
- HostAndPort.fromHost("master2.example.com"),
+ HostAndPort.fromParts("master2.example.com", 12345),
InetAddress.getByName("10.1.2.3"));
Assert.assertEquals("master2.example.com", serverInfo.getAndCanonicalizeHostname());
@@ -60,11 +60,12 @@ public class TestServerInfo {
installFakeDNS("master1.example.com", "server123.example.com", "10.1.2.3");
ServerInfo serverInfo = new ServerInfo(
- "nevermind",
- HostAndPort.fromHost("master1.example.com"),
+ "abcdef", // uuid
+ HostAndPort.fromParts("master1.example.com", 12345),
InetAddress.getByName("10.1.2.3"));
Assert.assertEquals("server123.example.com", serverInfo.getAndCanonicalizeHostname());
+ Assert.assertEquals("abcdef(master1.example.com:12345)", serverInfo.toString());
}
/**
http://git-wip-us.apache.org/repos/asf/kudu/blob/8204f43f/java/kudu-client/src/test/java/org/apache/kudu/client/TestTableLocationsCache.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestTableLocationsCache.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestTableLocationsCache.java
new file mode 100644
index 0000000..d61fb8e
--- /dev/null
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestTableLocationsCache.java
@@ -0,0 +1,61 @@
+// 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.kudu.client;
+
+import static org.junit.Assert.*;
+
+import java.util.List;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import com.google.common.base.Ticker;
+import com.google.common.collect.ImmutableList;
+
+public class TestTableLocationsCache {
+ private TableLocationsCache cache = new TableLocationsCache();
+
+ /**
+ * Prevent time from advancing during the test by mocking the time.
+ */
+ @Before
+ public void mockTime() {
+ TableLocationsCache.ticker = Mockito.mock(Ticker.class);
+ }
+ @After
+ public void unmockTime() {
+ TableLocationsCache.ticker = Ticker.systemTicker();
+ }
+
+ @Test
+ public void testToString() {
+ RemoteTablet tablet = TestRemoteTablet.getTablet(0, 1);
+ List<RemoteTablet> tablets = ImmutableList.of(tablet);
+ cache.cacheTabletLocations(tablets,
+ tablet.getPartition().getPartitionKeyStart(),
+ 1, // requested batch size,
+ 100); // ttl
+ // Mock as if the time increased by 10ms (the ticker is in nanoseconds).
+ // This will result in a remaining TTL of 90.
+ Mockito.when(TableLocationsCache.ticker.read()).thenReturn(10 * 1000000L);
+ assertEquals("[Tablet{lowerBoundPartitionKey=0x, upperBoundPartitionKey=0x, " +
+ "ttl=90, tablet=" + tablet.toString() + "}]",
+ cache.toString());
+ }
+}
[2/2] kudu git commit: java: reduce scary exception spew and improve
logging
Posted by to...@apache.org.
java: reduce scary exception spew and improve logging
In real environments there are various types of failures that are
"expected". In particular, it's not unlikely for the client to
occasionally connect to a server which has gone down or which is not the
leader for a given tablet anymore. In that case, we should make sure
that the logging output is easy to read and doesn't include irrelevant
stack traces stemming from Netty internals.
This patch addresses several of those cases. In particular, it gets rid
of "unexpected exception" messages (and associated stacks) in the case
of "connection refused" errors. It also helpfully includes the
underlying error when invalidating the tablet cache for a location.
There's a relatively simple new unit test, and I also ran some
before/after comparisons using the "loadgen" sample from the
kudu-examples repo against a local cluster with one TS:
Case 1: Master down when we start the client:
============================================================
Before the patch
--------------
[New I/O worker #12] WARN org.apache.kudu.client.ConnectToCluster - Error receiving response from localhost:7051
org.apache.kudu.client.RecoverableException: connection closed
at org.apache.kudu.client.Connection.channelClosed(Connection.java:254)
at org.apache.kudu.shaded.org.jboss.netty.channel.SimpleChannelUpstreamHandler.handleUpstream(SimpleChannelUpstreamHandler.java:88)
at org.apache.kudu.client.Connection.handleUpstream(Connection.java:238)
at org.apache.kudu.shaded.org.jboss.netty.channel.DefaultChannelPipeline.sendUpstream(DefaultChannelPipeline.java:564)
at org.apache.kudu.shaded.org.jboss.netty.channel.DefaultChannelPipeline$DefaultChannelHandlerContext.sendUpstream(DefaultChannelPipeline.java:791)
at org.apache.kudu.shaded.org.jboss.netty.handler.timeout.ReadTimeoutHandler.channelClosed(ReadTimeoutHandler.java:176)
at org.apache.kudu.shaded.org.jboss.netty.channel.SimpleChannelUpstreamHandler.handleUpstream(SimpleChannelUpstreamHandler.java:88)
at org.apache.kudu.shaded.org.jboss.netty.channel.DefaultChannelPipeline.sendUpstream(DefaultChannelPipeline.java:564)
at org.apache.kudu.shaded.org.jboss.netty.channel.DefaultChannelPipeline$DefaultChannelHandlerContext.sendUpstream(DefaultChannelPipeline.java:791)
at org.apache.kudu.shaded.org.jboss.netty.handler.codec.oneone.OneToOneDecoder.handleUpstream(OneToOneDecoder.java:60)
at org.apache.kudu.shaded.org.jboss.netty.channel.DefaultChannelPipeline.sendUpstream(DefaultChannelPipeline.java:564)
at org.apache.kudu.shaded.org.jboss.netty.channel.DefaultChannelPipeline$DefaultChannelHandlerContext.sendUpstream(DefaultChannelPipeline.java:791)
at org.apache.kudu.shaded.org.jboss.netty.handler.codec.frame.FrameDecoder.cleanup(FrameDecoder.java:493)
at org.apache.kudu.shaded.org.jboss.netty.handler.codec.frame.FrameDecoder.channelClosed(FrameDecoder.java:371)
at org.apache.kudu.shaded.org.jboss.netty.channel.SimpleChannelUpstreamHandler.handleUpstream(SimpleChannelUpstreamHandler.java:88)
at org.apache.kudu.shaded.org.jboss.netty.channel.DefaultChannelPipeline.sendUpstream(DefaultChannelPipeline.java:564)
at org.apache.kudu.shaded.org.jboss.netty.channel.DefaultChannelPipeline.sendUpstream(DefaultChannelPipeline.java:559)
at org.apache.kudu.shaded.org.jboss.netty.channel.Channels.fireChannelClosed(Channels.java:468)
at org.apache.kudu.shaded.org.jboss.netty.channel.Channels$6.run(Channels.java:457)
at org.apache.kudu.shaded.org.jboss.netty.channel.socket.ChannelRunnableWrapper.run(ChannelRunnableWrapper.java:40)
at org.apache.kudu.shaded.org.jboss.netty.channel.socket.nio.AbstractNioSelector.processTaskQueue(AbstractNioSelector.java:391)
at org.apache.kudu.shaded.org.jboss.netty.channel.socket.nio.AbstractNioSelector.run(AbstractNioSelector.java:315)
at org.apache.kudu.shaded.org.jboss.netty.channel.socket.nio.AbstractNioWorker.run(AbstractNioWorker.java:89)
at org.apache.kudu.shaded.org.jboss.netty.channel.socket.nio.NioWorker.run(NioWorker.java:178)
at org.apache.kudu.shaded.org.jboss.netty.util.ThreadRenamingRunnable.run(ThreadRenamingRunnable.java:108)
at org.apache.kudu.shaded.org.jboss.netty.util.internal.DeadLockProofWorker$1.run(DeadLockProofWorker.java:42)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
at java.lang.Thread.run(Thread.java:748)
[New I/O boss #17] ERROR org.apache.kudu.client.Connection - [peer master-localhost:7051(localhost:7051)] unexpected exception from downstream on [id: 0xf9759051]
java.net.ConnectException: Connection refused: localhost/127.0.0.1:7051
at sun.nio.ch.SocketChannelImpl.checkConnect(Native Method)
at sun.nio.ch.SocketChannelImpl.finishConnect(SocketChannelImpl.java:717)
at org.apache.kudu.shaded.org.jboss.netty.channel.socket.nio.NioClientBoss.connect(NioClientBoss.java:152)
at org.apache.kudu.shaded.org.jboss.netty.channel.socket.nio.NioClientBoss.processSelectedKeys(NioClientBoss.java:105)
at org.apache.kudu.shaded.org.jboss.netty.channel.socket.nio.NioClientBoss.process(NioClientBoss.java:79)
at org.apache.kudu.shaded.org.jboss.netty.channel.socket.nio.AbstractNioSelector.run(AbstractNioSelector.java:337)
at org.apache.kudu.shaded.org.jboss.netty.channel.socket.nio.NioClientBoss.run(NioClientBoss.java:42)
at org.apache.kudu.shaded.org.jboss.netty.util.ThreadRenamingRunnable.run(ThreadRenamingRunnable.java:108)
at org.apache.kudu.shaded.org.jboss.netty.util.internal.DeadLockProofWorker$1.run(DeadLockProofWorker.java:42)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
at java.lang.Thread.run(Thread.java:748)
[New I/O worker #12] WARN org.apache.kudu.client.ConnectToCluster - Unable to find the leader master localhost:7051; will retry
... repeats with exponential backoff
After the patch
--------------
[New I/O boss #17] INFO org.apache.kudu.client.Connection - Failed to connect to peer master-localhost:7051(localhost:7051): Connection refused: localhost/127.0.0.1:7051
[New I/O worker #1] INFO org.apache.kudu.client.ConnectToCluster - Unable to connect to master localhost:7051: Connection refused: localhost/127.0.0.1:7051
[New I/O worker #1] WARN org.apache.kudu.client.ConnectToCluster - Unable to find the leader master localhost:7051; will retry
... repeats with exponential backoff
Case 2: Tserver is down
============================================================
Before the patch
----------------
[New I/O worker #11] INFO org.apache.kudu.client.AsyncKuduClient - Removing server fc9f6ab955cc47a8ab3653d0170305b0 from this tablet's cache 9da164fbcd93404ea8e38e9491eb3fa8
[New I/O boss #17] ERROR org.apache.kudu.client.Connection - [peer fc9f6ab955cc47a8ab3653d0170305b0(todd-laptop:7050)] unexpected exception from downstream on [id: 0x270e3a82]
java.net.ConnectException: Connection refused: todd-laptop/127.0.1.1:7050
at sun.nio.ch.SocketChannelImpl.checkConnect(Native Method)
at sun.nio.ch.SocketChannelImpl.finishConnect(SocketChannelImpl.java:717)
at org.apache.kudu.shaded.org.jboss.netty.channel.socket.nio.NioClientBoss.connect(NioClientBoss.java:152)
at org.apache.kudu.shaded.org.jboss.netty.channel.socket.nio.NioClientBoss.processSelectedKeys(NioClientBoss.java:105)
at org.apache.kudu.shaded.org.jboss.netty.channel.socket.nio.NioClientBoss.process(NioClientBoss.java:79)
at org.apache.kudu.shaded.org.jboss.netty.channel.socket.nio.AbstractNioSelector.run(AbstractNioSelector.java:337)
at org.apache.kudu.shaded.org.jboss.netty.channel.socket.nio.NioClientBoss.run(NioClientBoss.java:42)
at org.apache.kudu.shaded.org.jboss.netty.util.ThreadRenamingRunnable.run(ThreadRenamingRunnable.java:108)
at org.apache.kudu.shaded.org.jboss.netty.util.internal.DeadLockProofWorker$1.run(DeadLockProofWorker.java:42)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
at java.lang.Thread.run(Thread.java:748)
... repeats with exponential backoff
After the patch
---------------
[New I/O boss #17] INFO org.apache.kudu.client.Connection - Failed to connect to peer fc9f6ab955cc47a8ab3653d0170305b0(todd-laptop:7050): Connection refused: todd-laptop/127.0.1.1:7050
[New I/O worker #15] INFO org.apache.kudu.client.AsyncKuduClient - Invalidating location fc9f6ab955cc47a8ab3653d0170305b0(todd-laptop:7050) for tablet 478ddbaafb5b494ea0de8e2d0ed45a00: Connection refused: todd-laptop/127.0.1.1:7050
... repeats with exponential backoff
Case 3: A tablet is bad
============================================================
In this case I manually delete a non-replicated tablet that is being written to
using 'kudu remote_replica delete'. This is unrecoverable since there is
no other replica.
Before the patch
-----------------
[New I/O worker #10] INFO org.apache.kudu.client.AsyncKuduClient - Removing server fc9f6ab955cc47a8ab3653d0170305b0 from this tablet's cache a633f4a0c14b4404a8ae1f825b378867
... repeats with exponential backoff
After the patch
----------------
[New I/O worker #9] INFO org.apache.kudu.client.AsyncKuduClient - Invalidating location 36622c46b25f4da9ada43f8591728053(todd-laptop:7050) for tablet c0b9488597e8447595ad0ee0a3378f95: Tablet not RUNNING: STOPPED
... repeats with exponential backoff
Change-Id: I4b8be871693fecc1ee46a4e238dd2ed8f0730d4b
Reviewed-on: http://gerrit.cloudera.org:8080/9644
Reviewed-by: Grant Henke <gr...@gmail.com>
Tested-by: Grant Henke <gr...@gmail.com>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/ead75684
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/ead75684
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/ead75684
Branch: refs/heads/master
Commit: ead756844ce9ada904fcc3666df25692f63e76b8
Parents: 8204f43
Author: Todd Lipcon <to...@apache.org>
Authored: Wed Mar 14 15:33:27 2018 -0700
Committer: Grant Henke <gr...@gmail.com>
Committed: Thu Mar 15 16:14:23 2018 +0000
----------------------------------------------------------------------
.../org/apache/kudu/client/AsyncKuduClient.java | 8 ++++--
.../apache/kudu/client/ConnectToCluster.java | 2 +-
.../java/org/apache/kudu/client/Connection.java | 28 +++++++++++++++++--
.../kudu/client/TestClientFailoverSupport.java | 18 ++++++++++++
.../org/apache/kudu/client/TestKuduClient.java | 29 ++++++++++++++++++++
5 files changed, 78 insertions(+), 7 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/ead75684/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 4b93a34..9c52748 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
@@ -1643,7 +1643,7 @@ public class AsyncKuduClient implements AutoCloseable {
* We're in the context of decode() meaning we need to either callback or retry later.
*/
<R> void handleTabletNotFound(final KuduRpc<R> rpc, KuduException ex, ServerInfo info) {
- invalidateTabletCache(rpc.getTablet(), info);
+ invalidateTabletCache(rpc.getTablet(), info, ex.getMessage());
handleRetryableError(rpc, ex);
}
@@ -1732,9 +1732,11 @@ public class AsyncKuduClient implements AutoCloseable {
* Remove the tablet server from the RemoteTablet's locations. Right now nothing is removing
* the tablet itself from the caches.
*/
- private void invalidateTabletCache(RemoteTablet tablet, ServerInfo info) {
+ private void invalidateTabletCache(RemoteTablet tablet, ServerInfo info,
+ String errorMessage) {
final String uuid = info.getUuid();
- LOG.info("Removing server {} from this tablet's cache {}", uuid, tablet.getTabletId());
+ LOG.info("Invalidating location {} for tablet {}: {}",
+ info, tablet.getTabletId(), errorMessage);
tablet.removeTabletClient(uuid);
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/ead75684/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 3e925c5..911a83d 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
@@ -360,7 +360,7 @@ final class ConnectToCluster {
@Override
public Void call(Exception e) throws Exception {
- LOG.warn("Error receiving response from {}", hostAndPort, e);
+ LOG.info("Unable to connect to master {}: {}", hostAndPort, e.getMessage());
exceptionsReceived.add(e);
incrementCountAndCheckExhausted();
return null;
http://git-wip-us.apache.org/repos/asf/kudu/blob/ead75684/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java b/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
index b7513e4..ff04ab0 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
@@ -17,6 +17,7 @@
package org.apache.kudu.client;
+import java.net.ConnectException;
import java.nio.channels.ClosedChannelException;
import java.util.ArrayList;
import java.util.HashMap;
@@ -168,6 +169,11 @@ class Connection extends SimpleChannelUpstreamHandler {
@GuardedBy("lock")
private int nextCallId = 0;
+ @Nullable
+ @GuardedBy("lock")
+ /** The future for the connection attempt. Set only once connect() is called. */
+ private ChannelFuture connectFuture;
+
/**
* Create a new Connection object to the specified destination.
*
@@ -249,9 +255,21 @@ class Connection extends SimpleChannelUpstreamHandler {
/** {@inheritDoc} */
@Override
public void channelClosed(final ChannelHandlerContext ctx, final ChannelStateEvent e) {
+ String msg = "connection closed";
+ // Connection failures are reported as channelClosed() before exceptionCaught() is called.
+ // We can detect this case by looking at whether connectFuture has been marked complete
+ // and grabbing the exception from there.
+ lock.lock();
+ try {
+ if (connectFuture != null && connectFuture.getCause() != null) {
+ msg = connectFuture.getCause().getMessage();
+ }
+ } finally {
+ lock.unlock();
+ }
// No need to call super.channelClosed(ctx, e) -- there should be nobody in the upstream
// pipeline after Connection itself. So, just handle the close event ourselves.
- cleanup(new RecoverableException(Status.NetworkError("connection closed")));
+ cleanup(new RecoverableException(Status.NetworkError(msg)));
}
/** {@inheritDoc} */
@@ -415,7 +433,11 @@ class Connection extends SimpleChannelUpstreamHandler {
explicitlyDisconnected ? "%s disconnected from peer" : "%s lost connection to peer",
getLogPrefix());
error = new RecoverableException(Status.NetworkError(message), e);
- LOG.info(message, e);
+ LOG.info(message);
+ } else if (e instanceof ConnectException) {
+ String message = "Failed to connect to peer " + serverInfo + ": " + e.getMessage();
+ error = new RecoverableException(Status.NetworkError(message), e);
+ LOG.info(message);
} else if (e instanceof SSLException && explicitlyDisconnected) {
// There's a race in Netty where, when we call Channel.close(), it tries
// to send a TLS 'shutdown' message and enters a shutdown state. If another
@@ -703,7 +725,7 @@ class Connection extends SimpleChannelUpstreamHandler {
Preconditions.checkState(lock.isHeldByCurrentThread());
Preconditions.checkState(state == State.NEW);
state = State.CONNECTING;
- channel.connect(serverInfo.getResolvedAddress());
+ connectFuture = channel.connect(serverInfo.getResolvedAddress());
}
/** Enumeration to represent the internal state of the Connection object. */
http://git-wip-us.apache.org/repos/asf/kudu/blob/ead75684/java/kudu-client/src/test/java/org/apache/kudu/client/TestClientFailoverSupport.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestClientFailoverSupport.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestClientFailoverSupport.java
index 6f8f51f..31e8b77 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestClientFailoverSupport.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestClientFailoverSupport.java
@@ -20,16 +20,21 @@ import static org.apache.kudu.util.AssertHelpers.assertEventuallyTrue;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
+import java.io.Closeable;
import java.io.IOException;
import java.util.List;
import org.apache.kudu.util.AssertHelpers.BooleanExpression;
+import org.apache.kudu.util.CapturingLogAppender;
import org.junit.After;
+import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
import com.google.common.net.HostAndPort;
public class TestClientFailoverSupport extends BaseKuduTest {
+ private CapturingLogAppender cla = new CapturingLogAppender();
+ private Closeable claAttach;
enum MasterFailureType {
RESTART,
@@ -42,12 +47,25 @@ public class TestClientFailoverSupport extends BaseKuduTest {
BaseKuduTest.doSetup(3, NUM_TABLET_SERVERS);
}
+ @Before
+ public void attachToLog() {
+ claAttach = cla.attach();
+ }
+
@After
public void restartKilledMaster() throws IOException {
miniCluster.restartDeadMasters();
miniCluster.restartDeadTservers();
}
+ @After
+ public void checkLogs() throws IOException {
+ claAttach.close();
+ String log = cla.getAppendedText();
+ assertFalse("Log should not contain Netty internals",
+ log.contains("socket.nio.AbstractNioSelector"));
+ }
+
private void waitUntilRowCount(final KuduTable table, final int rowCount, long timeoutMs)
throws Exception {
assertEventuallyTrue(String.format("Read count should be %s", rowCount),
http://git-wip-us.apache.org/repos/asf/kudu/blob/ead75684/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
index 52e8fdb..d527bd0 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
@@ -906,6 +906,35 @@ public class TestKuduClient extends BaseKuduTest {
exception_text.contains("lost connection to peer"));
}
+ /**
+ * Test that, if the masters are down when we attempt to connect, we don't end up
+ * logging any nonsensical stack traces including Netty internals.
+ */
+ @Test(timeout = 100000)
+ public void testNoLogSpewOnConnectionRefused() throws Exception {
+ CapturingLogAppender cla = new CapturingLogAppender();
+ try (Closeable c = cla.attach()) {
+ miniCluster.killMasters();
+ try (KuduClient localClient = new KuduClient.KuduClientBuilder(masterAddresses).build()) {
+ // Force the client to connect to the masters.
+ localClient.exportAuthenticationCredentials();
+ } catch (NoLeaderFoundException e) {
+ assertTrue("Bad exception string: " + e.getMessage(),
+ e.getMessage().matches(".*Master config .+ has no leader. " +
+ "Exceptions received:.*Connection refused.*Connection refused" +
+ ".*Connection refused.*"));
+ }
+ } finally {
+ miniCluster.restartDeadMasters();
+ }
+ // Ensure there is no log spew due to an unexpected lost connection.
+ String logText = cla.getAppendedText();
+ assertFalse("Should not claim to have lost a connection in the log",
+ logText.contains("lost connection to peer"));
+ assertFalse("Should not have netty spew in log",
+ logText.contains("socket.nio.AbstractNioSelector"));
+ }
+
@Test(timeout = 100000)
public void testCustomNioExecutor() throws Exception {
long startTime = System.nanoTime();