kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ha...@apache.org
Subject kudu git commit: KUDU-2343. java: properly reconnect to new leader master after failover
Date Thu, 15 Mar 2018 02:41:15 GMT
Repository: kudu
Updated Branches:
  refs/heads/branch-1.5.x 8c9dcc0ee -> 011a70b91


KUDU-2343. java: properly reconnect to new leader master after failover

This fixes the "fake" location information returned in response to a
ConnectToMaster RPC to include a distinct "fake UUID" for each master.
Previously, we were using an empty string for the UUID of the masters.
This caused collisions in the ConnectionCache, which is keyed by server
UUIDs.

The fake UUID added by this patch matches the fake UUID already in use
by AsyncKuduClient.newMasterRpcProxy. This should allow us to share the
RPC connection between the ConnectToMaster RPCs and the subsequent
GetTableLocation RPCs, which is also a benefit for latency after a
failover or on a fresh client.

Additionally, this will help with various log messages that previously
would print an empty UUID string.

A prior version of this patch solved the problem by changing the key for
the ConnectionCache to be based on IP address, which has other benefits
in terms of future support for servers changing their DNS resolution at
runtime. However, since this patch is intended for backport into prior
releases, this simpler approach is taken for now. A TODO is added for
the longer-term idea.

An existing test which tested killing a master now runs in a second mode
which restarts the master. This reproduced the bug prior to the fix.
This patch also cleans up that test somewhat - it was doing some buggy
logic to attempt to kill more than one tablet server, but in fact just
called "killTabletServer" three times on the same one. Killing three
tablet servers never made sense, either, since the table in the test
only had three replicas. Neither did it make sense to start six tablet
servers for the test.

Change-Id: I36f96c6712800e398ed46887d97d4b09fd993b04
Reviewed-on: http://gerrit.cloudera.org:8080/9612
Reviewed-by: Alexey Serbin <aserbin@cloudera.com>
Tested-by: Todd Lipcon <todd@apache.org>
Reviewed-on: http://gerrit.cloudera.org:8080/9638


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/011a70b9
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/011a70b9
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/011a70b9

Branch: refs/heads/branch-1.5.x
Commit: 011a70b911e75068aa0cafc2e96cc2a49bc1caf2
Parents: 8c9dcc0
Author: Todd Lipcon <todd@apache.org>
Authored: Tue Mar 13 11:43:47 2018 -0700
Committer: Todd Lipcon <todd@apache.org>
Committed: Wed Mar 14 23:46:13 2018 +0000

----------------------------------------------------------------------
 .../org/apache/kudu/client/AsyncKuduClient.java |  6 +-
 .../kudu/client/ConnectToClusterResponse.java   |  3 +-
 .../org/apache/kudu/client/ConnectionCache.java |  3 +
 .../kudu/client/TestClientFailoverSupport.java  | 64 +++++++++++++++-----
 .../apache/kudu/client/TestConnectionCache.java |  6 +-
 5 files changed, 63 insertions(+), 19 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/011a70b9/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 aec75e7..1292d20 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
@@ -279,7 +279,11 @@ public class AsyncKuduClient implements AutoCloseable {
       return null;
     }
     return newRpcProxy(
-        new ServerInfo("master-" + hostPort.toString(), hostPort, inetAddress), credentialsPolicy);
+        new ServerInfo(getFakeMasterUuid(hostPort), hostPort, inetAddress), credentialsPolicy);
+  }
+
+  static String getFakeMasterUuid(HostAndPort hostPort) {
+    return "master-" + hostPort.toString();
   }
 
   void reconnectToCluster(Callback<Void, Boolean> cb,

http://git-wip-us.apache.org/repos/asf/kudu/blob/011a70b9/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToClusterResponse.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToClusterResponse.java
b/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToClusterResponse.java
index 6cb687f..bfa1f2b 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToClusterResponse.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/ConnectToClusterResponse.java
@@ -61,6 +61,7 @@ class ConnectToClusterResponse {
    * if it were a tablet.
    */
   public GetTableLocationsResponsePB getAsTableLocations() {
+    String fakeUuid = AsyncKuduClient.getFakeMasterUuid(leaderHostAndPort);
     return GetTableLocationsResponsePB.newBuilder()
         .addTabletLocations(TabletLocationsPB.newBuilder()
             .setPartition(PartitionPB.newBuilder()
@@ -70,7 +71,7 @@ class ConnectToClusterResponse {
             .addReplicas(ReplicaPB.newBuilder()
                 .setTsInfo(TSInfoPB.newBuilder()
                     .addRpcAddresses(ProtobufHelper.hostAndPortToPB(leaderHostAndPort))
-                    .setPermanentUuid(ByteString.EMPTY)) // required field, but unused for
master
+                    .setPermanentUuid(ByteString.copyFromUtf8(fakeUuid)))
                 .setRole(connectResponse.getRole()))).build();
   }
 }

http://git-wip-us.apache.org/repos/asf/kudu/blob/011a70b9/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 53e8e59..db37425 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
@@ -68,6 +68,9 @@ class ConnectionCache {
    * 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.
    */
   @GuardedBy("uuid2connection")
   private final HashMultimap<String, Connection> uuid2connection = HashMultimap.create();

http://git-wip-us.apache.org/repos/asf/kudu/blob/011a70b9/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 b1cb403..a23b9d0 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
@@ -22,17 +22,29 @@ import static org.junit.Assert.assertFalse;
 
 import java.util.List;
 import org.apache.kudu.util.AssertHelpers.BooleanExpression;
+import org.junit.After;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
 public class TestClientFailoverSupport extends BaseKuduTest {
 
+  enum MasterFailureType {
+    RESTART,
+    KILL
+  }
+
   @BeforeClass
   public static void setUpBeforeClass() throws Exception {
-    final int NUM_TABLET_SERVERS = 6;
+    final int NUM_TABLET_SERVERS = 3;
     BaseKuduTest.doSetup(3, NUM_TABLET_SERVERS);
   }
 
+  @After
+  public void restartKilledMaster() throws Exception {
+    miniCluster.restartDeadMasters();
+    miniCluster.restartDeadTabletServers();
+  }
+
   private void waitUntilRowCount(final KuduTable table, final int rowCount, long timeoutMs)
       throws Exception {
     assertEventuallyTrue(String.format("Read count should be %s", rowCount),
@@ -46,25 +58,35 @@ public class TestClientFailoverSupport extends BaseKuduTest {
         }, timeoutMs);
   }
 
+  @Test(timeout = 100000)
+  public void testRestartLeaderMaster() throws Exception {
+    doTestMasterFailover(MasterFailureType.RESTART);
+  }
+
+  @Test(timeout = 100000)
+  public void testKillLeaderMaster() throws Exception {
+    doTestMasterFailover(MasterFailureType.KILL);
+  }
+
   /**
    * Tests that the Java client will appropriately failover when a new master leader is elected.
-   * We force a metadata update by killing the active tablet servers.
-   * Then we kill the master leader.
-   * We write some more rows.
+   *
+   * We inject some failure on the master, based on 'failureType'. Then we force a tablet
+   * re-election by killing the leader replica. The client then needs to reconnect to the
masters
+   * to find the new location information.
+   *
    * If we can successfully read back the rows written, that shows the client handled the
failover
    * correctly.
    */
-  @Test(timeout = 100000)
-  public void testMultipleFailover() throws Exception {
-    final String TABLE_NAME = TestClientFailoverSupport.class.getName();
-
+  private void doTestMasterFailover(MasterFailureType failureType) throws Exception {
+    final String TABLE_NAME = TestClientFailoverSupport.class.getName()
+        + "-" + failureType;
     createTable(TABLE_NAME, basicSchema, getBasicCreateTableOptions());
 
     KuduTable table = openTable(TABLE_NAME);
     KuduSession session = syncClient.newSession();
 
     final int TOTAL_ROWS_TO_INSERT = 10;
-    final int TSERVER_LEADERS_TO_KILL = 3;
 
     for (int i = 0; i < TOTAL_ROWS_TO_INSERT; i++) {
       session.apply(createBasicSchemaInsert(table, i));
@@ -72,14 +94,26 @@ public class TestClientFailoverSupport extends BaseKuduTest {
 
     waitUntilRowCount(table, TOTAL_ROWS_TO_INSERT, DEFAULT_SLEEP);
 
-    for (int i = 0; i < TSERVER_LEADERS_TO_KILL; i++) {
-      List<LocatedTablet> tablets = table.getTabletsLocations(DEFAULT_SLEEP);
-      assertEquals(1, tablets.size());
-      final int leaderPort = findLeaderTabletServerPort(tablets.get(0));
-      miniCluster.killTabletServerOnPort(leaderPort);
+    // Kill or restart the leader master.
+    switch (failureType) {
+    case KILL:
+      killMasterLeader();
+      break;
+    case RESTART:
+      restartLeaderMaster();
+      break;
     }
-    killMasterLeader();
 
+    // Kill the tablet server leader. This will force us to go back to the
+    // master to find the new location. At that point, the client will
+    // notice that the old leader master is no longer current and fail over
+    // to the new one.
+    List<LocatedTablet> tablets = table.getTabletsLocations(DEFAULT_SLEEP);
+    assertEquals(1, tablets.size());
+    final int leaderPort = findLeaderTabletServerPort(tablets.get(0));
+    miniCluster.killTabletServerOnPort(leaderPort);
+
+    // Insert some more rows.
     for (int i = TOTAL_ROWS_TO_INSERT; i < 2*TOTAL_ROWS_TO_INSERT; i++) {
       session.apply(createBasicSchemaInsert(table, i));
     }

http://git-wip-us.apache.org/repos/asf/kudu/blob/011a70b9/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 5b5d4cf..6759d59 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
@@ -62,8 +62,10 @@ public class TestConnectionCache {
         pingConnection(h);
       }
 
-      // 1 tserver and 3 masters and 3 connections from the newRpcProxy() in the loop above.
-      assertEquals(1 + 3 + 3, client.getConnectionListCopy().size());
+      // 3 masters and 3 connections from the newRpcProxy() in the loop above.
+      // No tservers have been connected to by the client since we haven't accessed
+      // any data.
+      assertEquals(3 + 3, client.getConnectionListCopy().size());
       assertFalse(allConnectionsTerminated(client));
 
       final RpcProxy proxy = client.newRpcProxy(serverInfos.get(0));


Mime
View raw message