kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ale...@apache.org
Subject [kudu] 01/03: KUDU-2711 pt 4. Java support for GetTableLocations optimizations
Date Sat, 11 May 2019 03:25:46 GMT
This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit 58f189dac6aa691bd7b8e5ebc1e89756385147a8
Author: Will Berkeley <wdberkeley@gmail.com>
AuthorDate: Tue May 7 10:55:51 2019 -0700

    KUDU-2711 pt 4. Java support for GetTableLocations optimizations
    
    This adds support for the optimized GetTableLocations response format
    added in 586e957f76a547340f2ab93a7eebc3f116ff825e.
    
    There's no new tests, but as this changes the way GetTableLocations
    works, it's tested by all existing tests that do any writes or scans, so
    it's well-tested by existing tests.
    
    Additionally, to test backwards compatibility, I ran the Java client
    test suite while using a set of binaries compiled without support for
    the GetTableLocations optimization.
    
    Change-Id: I5af146fd1984ce683f056877129506cd2068e0e8
    Reviewed-on: http://gerrit.cloudera.org:8080/13287
    Reviewed-by: Adar Dembo <adar@cloudera.com>
    Tested-by: Will Berkeley <wdberkeley@gmail.com>
---
 .../org/apache/kudu/client/AsyncKuduClient.java    | 48 +++++++++++++++++++---
 .../kudu/client/GetTableLocationsRequest.java      |  1 +
 .../java/org/apache/kudu/client/LocatedTablet.java |  2 +
 .../java/org/apache/kudu/client/RemoteTablet.java  | 14 ++++---
 .../apache/kudu/client/TestAsyncKuduClient.java    |  6 ++-
 .../org/apache/kudu/client/TestRemoteTablet.java   | 14 +++----
 6 files changed, 63 insertions(+), 22 deletions(-)

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 afc54d1..668f238 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
@@ -35,6 +35,7 @@ import java.net.UnknownHostException;
 import java.security.cert.CertificateException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.function.Consumer;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Random;
@@ -75,6 +76,7 @@ import org.apache.kudu.Schema;
 import org.apache.kudu.master.Master;
 import org.apache.kudu.master.Master.GetTableLocationsResponsePB;
 import org.apache.kudu.master.Master.TableIdentifierPB;
+import org.apache.kudu.master.Master.TSInfoPB;
 import org.apache.kudu.util.AsyncUtil;
 import org.apache.kudu.util.NetUtil;
 import org.apache.kudu.util.Pair;
@@ -2112,6 +2114,7 @@ public class AsyncKuduClient implements AutoCloseable {
                           partitionKey,
                           requestedBatchSize,
                           response.getTabletLocationsList(),
+                          response.getTsInfosList(),
                           response.getTtlMillis());
         } catch (KuduException e) {
           return e;
@@ -2146,12 +2149,13 @@ public class AsyncKuduClient implements AutoCloseable {
   }
 
   /**
-   * Makes discovered tablet locations visible in the clients caches.
+   * Makes discovered tablet locations visible in the client's caches.
    * @param table the table which the locations belong to
    * @param requestPartitionKey the partition key of the table locations request
    * @param requestedBatchSize the number of tablet locations requested from the master in
the
    *                           original request
    * @param locations the discovered locations
+   * @param tsInfosList a list of ts info that the replicas in 'locations' references by
index.
    * @param ttl the ttl of the locations
    */
   @InterfaceAudience.LimitedPrivate("Test")
@@ -2159,8 +2163,8 @@ public class AsyncKuduClient implements AutoCloseable {
                        byte[] requestPartitionKey,
                        int requestedBatchSize,
                        List<Master.TabletLocationsPB> locations,
+                       List<Master.TSInfoPB> tsInfosList,
                        long ttl) throws KuduException {
-    // TODO(todd): handle "interned" response here
     String tableId = table.getTableId();
     String tableName = table.getName();
 
@@ -2179,20 +2183,48 @@ public class AsyncKuduClient implements AutoCloseable {
 
     // Build the list of discovered remote tablet instances. If we have
     // already discovered the tablet, its locations are refreshed.
+    int numTsInfos = tsInfosList.size();
     List<RemoteTablet> tablets = new ArrayList<>(locations.size());
     for (Master.TabletLocationsPB tabletPb : locations) {
 
-      List<UnknownHostException> lookupExceptions = new ArrayList<>(tabletPb.getReplicasCount());
+      List<Exception> lookupExceptions = new ArrayList<>(tabletPb.getReplicasCount());
       List<ServerInfo> servers = new ArrayList<>(tabletPb.getReplicasCount());
-      for (Master.TabletLocationsPB.ReplicaPB replica : tabletPb.getReplicasList()) {
+
+      // Lambda that does the common handling of a ts info.
+      Consumer<Master.TSInfoPB> updateServersAndCollectExceptions = tsInfo -> {
         try {
-          ServerInfo serverInfo = resolveTS(replica.getTsInfo());
+          ServerInfo serverInfo = resolveTS(tsInfo);
           if (serverInfo != null) {
             servers.add(serverInfo);
           }
         } catch (UnknownHostException ex) {
           lookupExceptions.add(ex);
         }
+      };
+
+      // Handle "old-style" non-interned replicas.
+      for (Master.TabletLocationsPB.ReplicaPB replica : tabletPb.getReplicasList()) {
+        updateServersAndCollectExceptions.accept(replica.getTsInfo());
+      }
+
+      // Handle interned replicas. As a shim, we also need to create a list of "old-style"
ReplicaPBs
+      // to be stored inside the RemoteTablet.
+      // TODO(wdberkeley): Change this so ReplicaPBs aren't used by the client at all anymore.
+      List<Master.TabletLocationsPB.ReplicaPB> replicas = new ArrayList<>();
+      for (Master.TabletLocationsPB.InternedReplicaPB replica : tabletPb.getInternedReplicasList())
{
+        int tsInfoIdx = replica.getTsInfoIdx();
+        if (tsInfoIdx >= numTsInfos) {
+          lookupExceptions.add(new NonRecoverableException(Status.Corruption(
+              String.format("invalid response from master: referenced tablet idx %d but only
%d present",
+                            tsInfoIdx, numTsInfos))));
+          continue;
+        }
+        TSInfoPB tsInfo = tsInfosList.get(tsInfoIdx);
+        updateServersAndCollectExceptions.accept(tsInfo);
+        Master.TabletLocationsPB.ReplicaPB.Builder builder = Master.TabletLocationsPB.ReplicaPB.newBuilder();
+        builder.setRole(replica.getRole());
+        builder.setTsInfo(tsInfo);
+        replicas.add(builder.build());
       }
 
       if (!lookupExceptions.isEmpty() &&
@@ -2202,7 +2234,11 @@ public class AsyncKuduClient implements AutoCloseable {
         throw new NonRecoverableException(statusIOE);
       }
 
-      RemoteTablet rt = new RemoteTablet(tableId, tabletPb, servers);
+      RemoteTablet rt = new RemoteTablet(tableId,
+                                         tabletPb.getTabletId().toStringUtf8(),
+                                         ProtobufHelper.pbToPartition(tabletPb.getPartition()),
+                                         replicas.isEmpty() ? tabletPb.getReplicasList()
: replicas,
+                                         servers);
 
       LOG.debug("Learned about tablet {} for table '{}' with partition {}",
                 rt.getTabletId(), tableName, rt.getPartition());
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/GetTableLocationsRequest.java
b/java/kudu-client/src/main/java/org/apache/kudu/client/GetTableLocationsRequest.java
index a7fe825..3be2770 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/GetTableLocationsRequest.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/GetTableLocationsRequest.java
@@ -91,6 +91,7 @@ class GetTableLocationsRequest extends KuduRpc<Master.GetTableLocationsResponseP
       builder.setPartitionKeyEnd(UnsafeByteOperations.unsafeWrap(endKey));
     }
     builder.setMaxReturnedLocations(maxReturnedLocations);
+    builder.setInternTsInfosInResponse(true);
     return builder.build();
   }
 }
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/LocatedTablet.java b/java/kudu-client/src/main/java/org/apache/kudu/client/LocatedTablet.java
index 655f800..ae31903 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/LocatedTablet.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/LocatedTablet.java
@@ -107,6 +107,8 @@ public class LocatedTablet {
   @InterfaceAudience.Public
   @InterfaceStability.Evolving
   public static class Replica {
+    // TODO(wdberkeley): The ReplicaPB is deprecated server-side, so we ought to redo how
this
+    // class stores its information.
     private final ReplicaPB pb;
 
     Replica(ReplicaPB pb) {
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 fd0eb3a..2241f58 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
@@ -69,11 +69,13 @@ public class RemoteTablet implements Comparable<RemoteTablet> {
   private String leaderUuid;
 
   RemoteTablet(String tableId,
-               Master.TabletLocationsPB tabletLocations,
+               String tabletId,
+               Partition partition,
+               List<Master.TabletLocationsPB.ReplicaPB> replicas,
                List<ServerInfo> serverInfos) {
-    this.tabletId = tabletLocations.getTabletId().toStringUtf8();
+    this.tabletId = tabletId;
     this.tableId = tableId;
-    this.partition = ProtobufHelper.pbToPartition(tabletLocations.getPartition());
+    this.partition = partition;
     this.tabletServers = new HashMap<>(serverInfos.size());
 
     for (ServerInfo serverInfo : serverInfos) {
@@ -81,18 +83,18 @@ public class RemoteTablet implements Comparable<RemoteTablet> {
     }
 
     ImmutableList.Builder<LocatedTablet.Replica> replicasBuilder = new ImmutableList.Builder<>();
-    for (Master.TabletLocationsPB.ReplicaPB replica : tabletLocations.getReplicasList())
{
+    for (Master.TabletLocationsPB.ReplicaPB replica : replicas) {
       String uuid = replica.getTsInfo().getPermanentUuid().toStringUtf8();
       replicasBuilder.add(new LocatedTablet.Replica(replica));
       if (replica.getRole().equals(Metadata.RaftPeerPB.Role.LEADER)) {
-        leaderUuid = uuid;
+        this.leaderUuid = uuid;
       }
     }
 
     if (leaderUuid == null) {
       LOG.warn("No leader provided for tablet {}", getTabletId());
     }
-    replicas.set(replicasBuilder.build());
+    this.replicas.set(replicasBuilder.build());
   }
 
   @Override
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java
b/java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java
index d79d114..bef643d 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java
@@ -173,7 +173,8 @@ public class TestAsyncKuduClient {
     try {
       KuduTable badTable = new KuduTable(asyncClient, "Invalid table name",
           "Invalid table ID", null, null, 3);
-      asyncClient.discoverTablets(badTable, null, requestBatchSize, tabletLocations, 1000);
+      asyncClient.discoverTablets(badTable, null, requestBatchSize,
+                                  tabletLocations, new ArrayList<>(), 1000);
       fail("This should have failed quickly");
     } catch (NonRecoverableException ex) {
       assertTrue(ex.getMessage().contains(badHostname));
@@ -205,7 +206,8 @@ public class TestAsyncKuduClient {
         "master", leader.getRpcHost(), leader.getRpcPort(), Metadata.RaftPeerPB.Role.FOLLOWER));
     tabletLocations.add(tabletPb.build());
     try {
-      asyncClient.discoverTablets(table, new byte[0], requestBatchSize, tabletLocations,
1000);
+      asyncClient.discoverTablets(table, new byte[0], requestBatchSize,
+                                  tabletLocations, new ArrayList<>(), 1000);
       fail("discoverTablets should throw an exception if there's no leader");
     } catch (NoLeaderFoundException ex) {
       // Expected.
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 43ca37a..c4372c1 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
@@ -223,10 +223,8 @@ public class TestRemoteTablet {
   static RemoteTablet getTablet(int leaderIndex,
                                 int localReplicaIndex,
                                 int sameLocationReplicaIndex) {
-    Master.TabletLocationsPB.Builder tabletPb = Master.TabletLocationsPB.newBuilder();
-
-    tabletPb.setPartition(ProtobufUtils.getFakePartitionPB());
-    tabletPb.setTabletId(ByteString.copyFromUtf8("fake tablet"));
+    Partition partition = ProtobufHelper.pbToPartition(ProtobufUtils.getFakePartitionPB().build());
+    List<Master.TabletLocationsPB.ReplicaPB> replicas = new ArrayList<>();
     List<ServerInfo> servers = new ArrayList<>();
     for (int i = 0; i < 3; i++) {
       InetAddress addr;
@@ -246,11 +244,11 @@ public class TestRemoteTablet {
                                  new HostAndPort("host", 1000 + i),
                                  addr,
                                  location));
-      tabletPb.addReplicas(ProtobufUtils.getFakeTabletReplicaPB(
-          uuid, "host", i,
-          leaderIndex == i ? Metadata.RaftPeerPB.Role.LEADER : Metadata.RaftPeerPB.Role.FOLLOWER));
+      Metadata.RaftPeerPB.Role role = leaderIndex == i ? Metadata.RaftPeerPB.Role.LEADER
:
+                                                         Metadata.RaftPeerPB.Role.FOLLOWER;
+      replicas.add(ProtobufUtils.getFakeTabletReplicaPB(uuid, "host", i, role).build());
     }
 
-    return new RemoteTablet("fake table", tabletPb.build(), servers);
+    return new RemoteTablet("fake table", "fake tablet", partition, replicas, servers);
   }
 }


Mime
View raw message