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 2016/06/21 01:13:07 UTC
incubator-kudu git commit: [java client] Fix GetTableLocations error
handling
Repository: incubator-kudu
Updated Branches:
refs/heads/master bd549e137 -> 0c8f72b4c
[java client] Fix GetTableLocations error handling
When GetTableLocations get an error, e.g. table not found, in some
scenario the callback will silently drop the error, not notifying
the RPC which triggered this GetTableLocations call, causing the
RPC to hang.
This usually happens when user is inserting data, but underlying
table is deleted and recreated.
Change-Id: I79d246a83e9f0071063a23fb9a8d4b4c0220e507
Reviewed-on: http://gerrit.cloudera.org:8080/3295
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans
Project: http://git-wip-us.apache.org/repos/asf/incubator-kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-kudu/commit/0c8f72b4
Tree: http://git-wip-us.apache.org/repos/asf/incubator-kudu/tree/0c8f72b4
Diff: http://git-wip-us.apache.org/repos/asf/incubator-kudu/diff/0c8f72b4
Branch: refs/heads/master
Commit: 0c8f72b4c5d637dba4c0c0da4f55ea918c287d88
Parents: bd549e1
Author: Binglin Chang <de...@gmail.com>
Authored: Fri Jun 3 11:05:54 2016 +0800
Committer: Jean-Daniel Cryans <jd...@gerrit.cloudera.org>
Committed: Tue Jun 21 01:11:32 2016 +0000
----------------------------------------------------------------------
.../java/org/kududb/client/AsyncKuduClient.java | 11 +--
.../org/kududb/client/ListTabletsRequest.java | 70 ++++++++++++++++++++
.../org/kududb/client/ListTabletsResponse.java | 40 +++++++++++
.../org/kududb/client/TestAsyncKuduSession.java | 40 +++++++++++
4 files changed, 157 insertions(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/0c8f72b4/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 4dacd75..e5f23a4 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
@@ -785,7 +785,8 @@ public class AsyncKuduClient implements AutoCloseable {
/**
* "Errback" used to delayed-retry a RPC if it fails due to no leader master being found.
- * Other exceptions are passed through to be handled by the caller.
+ * Other exceptions are used to notify request RPC error, and passed through to be handled
+ * by the caller.
* <p>
* Use {@code AsyncUtil.addCallbacksDeferring} to add this as the "errback" and
* {@link RetryRpcCB} as the callback to the {@code Deferred} returned by
@@ -805,14 +806,16 @@ public class AsyncKuduClient implements AutoCloseable {
if (arg instanceof NoLeaderMasterFoundException) {
// If we could not find the leader master, try looking up the leader master
// again.
- Deferred<R> d = request.getDeferred();
// TODO: Handle the situation when multiple in-flight RPCs are queued waiting
// for the leader master to be determine (either after a failure or at initialization
// time). This could re-use some of the existing piping in place for non-master tablets.
delayedSendRpcToTablet(request, (NoLeaderMasterFoundException) arg);
- return d;
+ } else {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug(String.format("Notify RPC %s after lookup exception", request), arg);
+ }
+ request.errback(arg);
}
- // Pass all other exceptions through.
return Deferred.fromError(arg);
}
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/0c8f72b4/java/kudu-client/src/main/java/org/kududb/client/ListTabletsRequest.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/kududb/client/ListTabletsRequest.java b/java/kudu-client/src/main/java/org/kududb/client/ListTabletsRequest.java
new file mode 100644
index 0000000..8a29b7b
--- /dev/null
+++ b/java/kudu-client/src/main/java/org/kududb/client/ListTabletsRequest.java
@@ -0,0 +1,70 @@
+// 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.kududb.client;
+
+import com.google.protobuf.Message;
+import org.kududb.annotations.InterfaceAudience;
+import org.kududb.tablet.Tablet;
+import org.kududb.tserver.Tserver;
+import org.kududb.tserver.TserverService;
+import org.kududb.util.Pair;
+import org.jboss.netty.buffer.ChannelBuffer;
+
+import java.util.ArrayList;
+import java.util.List;
+
+@InterfaceAudience.Private
+class ListTabletsRequest extends KuduRpc<ListTabletsResponse> {
+
+ ListTabletsRequest() {
+ super(null);
+ }
+
+ @Override
+ ChannelBuffer serialize(Message header) {
+ assert header.isInitialized();
+ final Tserver.ListTabletsRequestPB.Builder builder =
+ Tserver.ListTabletsRequestPB.newBuilder();
+ return toChannelBuffer(header, builder.build());
+ }
+
+ @Override
+ String serviceName() { return TABLET_SERVER_SERVICE_NAME; }
+
+ @Override
+ String method() {
+ return "ListTablets";
+ }
+
+ @Override
+ Pair<ListTabletsResponse, Object> deserialize(CallResponse callResponse,
+ String tsUUID) throws Exception {
+ final Tserver.ListTabletsResponsePB.Builder respBuilder =
+ Tserver.ListTabletsResponsePB.newBuilder();
+ readProtobuf(callResponse.getPBMessage(), respBuilder);
+ int serversCount = respBuilder.getStatusAndSchemaCount();
+ List<String> tablets = new ArrayList<String>(serversCount);
+ for (Tserver.ListTabletsResponsePB.StatusAndSchemaPB info
+ : respBuilder.getStatusAndSchemaList()) {
+ tablets.add(info.getTabletStatus().getTabletId());
+ }
+ ListTabletsResponse response = new ListTabletsResponse(deadlineTracker.getElapsedMillis(),
+ tsUUID, tablets);
+ return new Pair<ListTabletsResponse, Object>(
+ response, respBuilder.hasError() ? respBuilder.getError() : null);
+ }
+}
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/0c8f72b4/java/kudu-client/src/main/java/org/kududb/client/ListTabletsResponse.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/kududb/client/ListTabletsResponse.java b/java/kudu-client/src/main/java/org/kududb/client/ListTabletsResponse.java
new file mode 100644
index 0000000..be2ed65
--- /dev/null
+++ b/java/kudu-client/src/main/java/org/kududb/client/ListTabletsResponse.java
@@ -0,0 +1,40 @@
+// 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.kududb.client;
+
+import org.kududb.annotations.InterfaceAudience;
+
+import java.util.List;
+
+@InterfaceAudience.Private
+public class ListTabletsResponse extends KuduRpcResponse {
+
+ private final List<String> tabletsList;
+
+ ListTabletsResponse(long ellapsedMillis, String tsUUID, List<String> tabletsList) {
+ super(ellapsedMillis, tsUUID);
+ this.tabletsList = tabletsList;
+ }
+
+ /**
+ * Get the list of tablets as specified in the request.
+ * @return a list of tablet uuids
+ */
+ public List<String> getTabletsList() {
+ return tabletsList;
+ }
+}
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/0c8f72b4/java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduSession.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduSession.java b/java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduSession.java
index 89b2384..94dc55b 100644
--- a/java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduSession.java
+++ b/java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduSession.java
@@ -18,6 +18,7 @@ package org.kududb.client;
import org.kududb.Schema;
import org.kududb.WireProtocol.AppStatusPB;
+import org.kududb.client.AsyncKuduClient.RemoteTablet;
import org.kududb.tserver.Tserver.TabletServerErrorPB;
import com.stumbleupon.async.Callback;
@@ -108,6 +109,45 @@ public class TestAsyncKuduSession extends BaseKuduTest {
}
}
+ /**
+ * Regression test for case when tablet lookup error causes original RPC to get stuck.
+ * @throws Exception
+ */
+ @Test(timeout = 100000)
+ public void testGetTableLocationsErrorCauseSessionStuck() throws Exception {
+ AsyncKuduSession session = client.newSession();
+ // Make sure tablet locations is cached.
+ Insert insert = createInsert(1);
+ session.apply(insert).join(DEFAULT_SLEEP);
+ RemoteTablet rt = client.getTablet(table.getTableId(), insert.partitionKey());
+ String tabletId = rt.getTabletIdAsString();
+ TabletClient tc = client.clientFor(rt);
+ try {
+ // Delete table so we get table not found error.
+ client.deleteTable(TABLE_NAME).join();
+ // Wait until tablet is deleted on TS.
+ while (true) {
+ ListTabletsRequest req = new ListTabletsRequest();
+ tc.sendRpc(req);
+ ListTabletsResponse resp = req.getDeferred().join();
+ if (!resp.getTabletsList().contains(tabletId)) {
+ break;
+ }
+ Thread.sleep(100);
+ }
+ try {
+ session.apply(createInsert(1)).join(DEFAULT_SLEEP);
+ fail("Insert should not succeed");
+ } catch (MasterErrorException e) {
+ // Expect NOT_FOUND, because the table was deleted.
+ } catch (Throwable e) {
+ fail("Should not throw other error: " + e);
+ }
+ } finally {
+ table = createTable(TABLE_NAME, schema, getBasicCreateTableOptions());
+ }
+ }
+
@Test(timeout = 100000)
public void test() throws Exception {