You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by da...@apache.org on 2016/06/17 23:00:37 UTC
[2/2] incubator-kudu git commit: Update Java client for new master
GetTableLocations semantics
Update Java client for new master GetTableLocations semantics
The Java client was relying on the master returning an empty tablet locations
response when the table was still in the process of being created. Now, the
master returns a specific error. This code fixes the table locations lookup code
in AsyncKuduClient for look for that specific error code.
This change isn't tested since the locateTablets codepath is well covered by
existing tests.
Change-Id: I80bf5661aed1ba435800211411b3273874e5bbcf
Reviewed-on: http://gerrit.cloudera.org:8080/3303
Reviewed-by: Jean-Daniel Cryans
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
Project: http://git-wip-us.apache.org/repos/asf/incubator-kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-kudu/commit/1a97c42c
Tree: http://git-wip-us.apache.org/repos/asf/incubator-kudu/tree/1a97c42c
Diff: http://git-wip-us.apache.org/repos/asf/incubator-kudu/diff/1a97c42c
Branch: refs/heads/master
Commit: 1a97c42c678c22ac1aebfbb159402958e5c93952
Parents: c0ba535
Author: Dan Burkert <da...@cloudera.com>
Authored: Tue Jun 14 17:32:51 2016 -0700
Committer: Jean-Daniel Cryans <jd...@gerrit.cloudera.org>
Committed: Fri Jun 17 21:06:59 2016 +0000
----------------------------------------------------------------------
.../java/org/kududb/client/AsyncKuduClient.java | 65 ++++++++++++--------
.../org/kududb/client/TestAsyncKuduClient.java | 9 +--
2 files changed, 45 insertions(+), 29 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/1a97c42c/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java b/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
index b2b5348..4dacd75 100644
--- a/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
+++ b/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
@@ -664,6 +664,17 @@ public class AsyncKuduClient implements AutoCloseable {
return d;
}
+ /**
+ * Sends the provided {@link KuduRpc} to the tablet server hosting the leader
+ * of the tablet identified by the RPC's table and partition key.
+ *
+ * Note: despite the name, this method is also used for routing master
+ * requests to the leader master instance since it's also handled like a tablet.
+ *
+ * @param request the RPC to send
+ * @param <R> the expected return type of the RPC
+ * @return a {@code Deferred} which will contain the response
+ */
<R> Deferred<R> sendRpcToTablet(final KuduRpc<R> request) {
if (cannotRetryRequest(request)) {
return tooManyAttemptsOrTimeout(request, null);
@@ -726,11 +737,15 @@ public class AsyncKuduClient implements AutoCloseable {
}
}
- // Right after creating a table a request will fall into locateTablet since we don't know yet
- // if the table is ready or not. If discoverTablets() didn't get any tablets back,
- // then on retry we'll fall into the following block. It will sleep, then call the master to
- // see if the table was created. We'll spin like this until the table is created and then
- // we'll try to locate the tablet again.
+ // We fall through to here in two cases:
+ //
+ // 1) This client has not yet discovered the tablet which is responsible for
+ // the RPC's table and partition key. This can happen when the client's
+ // tablet location cache is cold because the client is new, or the table
+ // is new.
+ //
+ // 2) The tablet is known, but we do not have an active client for the
+ // leader replica.
if (tablesNotServed.contains(tableId)) {
return delayedIsCreateTableDone(request.getTable(), request,
new RetryRpcCB<R, Master.IsCreateTableDoneResponsePB>(request),
@@ -1034,13 +1049,15 @@ public class AsyncKuduClient implements AutoCloseable {
return Deferred.fromResult(null); // Looks like no lookup needed.
}
}
+ // Leave the end of the partition key range empty in order to pre-fetch tablet locations.
GetTableLocationsRequest rpc =
- new GetTableLocationsRequest(masterTable, partitionKey, partitionKey, tableId);
+ new GetTableLocationsRequest(masterTable, partitionKey, null, tableId);
rpc.setTimeoutMillis(defaultAdminOperationTimeoutMs);
final Deferred<Master.GetTableLocationsResponsePB> d;
- // If we know this is going to the master, check the master consensus configuration (as specified by
- // 'masterAddresses' field) to determine and cache the current leader.
+ // If we know this is going to the master, check the master consensus
+ // configuration (as specified by 'masterAddresses' field) to determine and
+ // cache the current leader.
if (isMasterTable(tableId)) {
d = getMasterTableLocationsPB();
} else {
@@ -1239,19 +1256,24 @@ public class AsyncKuduClient implements AutoCloseable {
MasterLookupCB(KuduTable table) {
this.table = table;
}
- public Object call(final Master.GetTableLocationsResponsePB arg) {
- try {
- discoverTablets(table, arg);
- } catch (NonRecoverableException e) {
- // Returning the exception means we early out and errback to the user.
- return e;
+ public Object call(final GetTableLocationsResponsePB response) {
+ if (response.hasError()) {
+ if (response.getError().getCode() == Master.MasterErrorPB.Code.TABLET_NOT_RUNNING) {
+ // Keep a note that the table exists but at least one tablet is not yet running.
+ LOG.debug("Table {} has a non-running tablet", table.getName());
+ tablesNotServed.add(table.getTableId());
+ } else {
+ return new MasterErrorException("GetTableLocations error", response.getError());
+ }
+ } else {
+ discoverTablets(table, response.getTabletLocationsList());
}
return null;
}
public String toString() {
return "get tablet locations from the master for table " + table.getName();
}
- };
+ }
boolean acquireMasterLookupPermit() {
try {
@@ -1273,18 +1295,11 @@ public class AsyncKuduClient implements AutoCloseable {
}
@VisibleForTesting
- void discoverTablets(KuduTable table, Master.GetTableLocationsResponsePB response)
+ void discoverTablets(KuduTable table, List<Master.TabletLocationsPB> locations)
throws NonRecoverableException {
String tableId = table.getTableId();
String tableName = table.getName();
- if (response.getTabletLocationsCount() == 0) {
- // Keep a note that the table exists but it's not served yet, we'll retry.
- if (LOG.isDebugEnabled()) {
- LOG.debug("Table {} has not been created yet", tableName);
- }
- tablesNotServed.add(tableId);
- return;
- }
+
// Doing a get first instead of putIfAbsent to avoid creating unnecessary CSLMs because in
// the most common case the table should already be present
ConcurrentSkipListMap<byte[], RemoteTablet> tablets = tabletsCache.get(tableId);
@@ -1297,7 +1312,7 @@ public class AsyncKuduClient implements AutoCloseable {
}
}
- for (Master.TabletLocationsPB tabletPb : response.getTabletLocationsList()) {
+ for (Master.TabletLocationsPB tabletPb : locations) {
// Early creating the tablet so that it parses out the pb
RemoteTablet rt = createTabletFromPb(tableId, tabletPb);
Slice tabletId = rt.tabletId;
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/1a97c42c/java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduClient.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduClient.java b/java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduClient.java
index e66a531..abec53f 100644
--- a/java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduClient.java
+++ b/java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduClient.java
@@ -21,6 +21,8 @@ import com.google.common.base.Stopwatch;
import com.google.protobuf.ByteString;
import com.stumbleupon.async.Deferred;
+import java.util.ArrayList;
+import java.util.List;
import java.util.concurrent.TimeUnit;
import org.junit.BeforeClass;
@@ -117,8 +119,7 @@ public class TestAsyncKuduClient extends BaseKuduTest {
assertTrue(ex.getMessage().contains(badHostname));
}
- Master.GetTableLocationsResponsePB.Builder builder =
- Master.GetTableLocationsResponsePB.newBuilder();
+ List<Master.TabletLocationsPB> tabletLocations = new ArrayList<>();
// Builder three bad locations.
Master.TabletLocationsPB.Builder tabletPb = Master.TabletLocationsPB.newBuilder();
@@ -139,14 +140,14 @@ public class TestAsyncKuduClient extends BaseKuduTest {
replicaBuilder.setTsInfo(tsInfoBuilder);
replicaBuilder.setRole(Metadata.RaftPeerPB.Role.FOLLOWER);
tabletPb.addReplicas(replicaBuilder);
- builder.addTabletLocations(tabletPb);
+ tabletLocations.add(tabletPb.build());
}
// Test that a tablet full of unreachable replicas won't make us retry.
try {
KuduTable badTable = new KuduTable(client, "Invalid table name",
"Invalid table ID", null, null);
- client.discoverTablets(badTable, builder.build());
+ client.discoverTablets(badTable, tabletLocations);
fail("This should have failed quickly");
} catch (Exception ex) {
assertTrue(ex instanceof NonRecoverableException);