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 2017/11/01 17:22:43 UTC

kudu git commit: [java client] improve AsyncKuduScanner logging

Repository: kudu
Updated Branches:
  refs/heads/master f2c21b4fa -> b2a5bc82b


[java client] improve AsyncKuduScanner logging

As filed in KUDU-2199, in a jenkin's job build, ITClient test failed
even with fault tolerant scanner. The test log demonstrates that the
issue may cause by re-scan of scanned data. However, loop ITClient
10000 times and did not discover any issues:
  http://dist-test.cloudera.org/job?job_id=hao.hao.1508869382.32617

This patch improves test coverage of ITClient so that error will be
reported if row count of each scan ever unexpectedly decrease. It also
improves logging of AsyncKuduScanner to give more information for
diagnosis if the same issue occur again. Another 2000 runs of ITClient
with current patch passed:
  http://dist-test.cloudera.org/job?job_id=hao.hao.1509045968.14317

Change-Id: I90ffcd01e7f99f3090fa118092fc303e06fb92dc
Reviewed-on: http://gerrit.cloudera.org:8080/8382
Reviewed-by: Dan Burkert <da...@apache.org>
Tested-by: Kudu Jenkins


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

Branch: refs/heads/master
Commit: b2a5bc82b4848467a3173fe303d107836c2d6e57
Parents: f2c21b4
Author: hahao <ha...@cloudera.com>
Authored: Wed Oct 25 11:00:11 2017 -0700
Committer: Dan Burkert <da...@apache.org>
Committed: Wed Nov 1 17:22:31 2017 +0000

----------------------------------------------------------------------
 .../apache/kudu/client/AsyncKuduScanner.java    | 26 +++++------
 .../FaultTolerantScannerExpiredException.java   | 47 ++++++++++++++++++++
 .../kudu/client/ScannerExpiredException.java    | 47 --------------------
 .../java/org/apache/kudu/client/ITClient.java   |  7 ++-
 4 files changed, 65 insertions(+), 62 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/b2a5bc82/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 5755158..c7d9330 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
@@ -502,17 +502,17 @@ public final class AsyncKuduScanner {
       @Override
       public Deferred<RowResultIterator> call(Exception e) throws Exception {
         final RemoteTablet old_tablet = tablet;  // Save before invalidate().
-        String message = old_tablet + " pretends to not know " + AsyncKuduScanner.this;
-        LOG.warn(message, e);
         invalidate();  // If there was an error, don't assume we're still OK.
-        // If encountered ScannerExpiredException, it means the scanner
-        // on the server side expired. This exception is only thrown for fault
-        // tolerant scanner. Therefore, open a new scanner.
-        if (e instanceof ScannerExpiredException) {
+        // If encountered FaultTolerantScannerExpiredException, it means the
+        // fault tolerant scanner on the server side expired. Therefore, open
+        // a new scanner.
+        if (e instanceof FaultTolerantScannerExpiredException) {
           scannerId = null;
           sequenceId = 0;
+          LOG.warn("Scanner expired, creating a new one {}", AsyncKuduScanner.this);
           return nextRows();
         } else {
+          LOG.warn("{} pretends to not know {}", old_tablet, AsyncKuduScanner.this, e);
           return Deferred.fromError(e); // Let the error propagate.
         }
       }
@@ -715,10 +715,10 @@ public final class AsyncKuduScanner {
     }
 
     public String toString() {
-      String ret = "AsyncKuduScanner$Response(scannerId=" + Bytes.pretty(scannerId) +
-          ", data=" + data + ", more=" + more;
+      String ret = "AsyncKuduScanner$Response(scannerId = " + Bytes.pretty(scannerId) +
+          ", data = " + data + ", more = " + more;
       if (scanTimestamp != AsyncKuduClient.NO_TIMESTAMP) {
-        ret += ", responseScanTimestamp =" + scanTimestamp;
+        ret += ", responseScanTimestamp = " + scanTimestamp;
       }
       ret += ")";
       return ret;
@@ -858,7 +858,7 @@ public final class AsyncKuduScanner {
           case SCANNER_EXPIRED:
             if (isFaultTolerant) {
               Status status = Status.fromTabletServerErrorPB(error);
-              throw new ScannerExpiredException(status);
+              throw new FaultTolerantScannerExpiredException(status);
             }
             // fall through
           default:
@@ -870,7 +870,7 @@ public final class AsyncKuduScanner {
           callResponse);
 
       boolean hasMore = resp.getHasMoreResults();
-      if (id.length  != 0 && scannerId != null && !Bytes.equals(scannerId, id)) {
+      if (id.length != 0 && scannerId != null && !Bytes.equals(scannerId, id)) {
         Status statusIllegalState = Status.IllegalState("Scan RPC response was for scanner" +
             " ID " + Bytes.pretty(id) + " but we expected " +
             Bytes.pretty(scannerId));
@@ -883,14 +883,14 @@ public final class AsyncKuduScanner {
                                         : AsyncKuduClient.NO_TIMESTAMP,
           resp.getLastPrimaryKey().toByteArray());
       if (LOG.isDebugEnabled()) {
-        LOG.debug(response.toString());
+        LOG.debug("{} for scanner {}", response.toString(), AsyncKuduScanner.this);
       }
       return new Pair<Response, Object>(response, error);
     }
 
     public String toString() {
       return "ScanRequest(scannerId=" + Bytes.pretty(scannerId) +
-          (tablet != null ? ", tabletSlice=" + tablet.getTabletId() : "") +
+          (tablet != null ? ", tablet=" + tablet.getTabletId() : "") +
           ", attempt=" + attempt + ", " + super.toString() + ")";
     }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/b2a5bc82/java/kudu-client/src/main/java/org/apache/kudu/client/FaultTolerantScannerExpiredException.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/FaultTolerantScannerExpiredException.java b/java/kudu-client/src/main/java/org/apache/kudu/client/FaultTolerantScannerExpiredException.java
new file mode 100644
index 0000000..8216b41
--- /dev/null
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/FaultTolerantScannerExpiredException.java
@@ -0,0 +1,47 @@
+// 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 org.apache.yetus.audience.InterfaceAudience;
+import org.apache.yetus.audience.InterfaceStability;
+
+/**
+ * A scanner expired exception only used for fault tolerant scanner.
+ */
+@InterfaceAudience.Private
+@InterfaceStability.Evolving
+@SuppressWarnings("serial")
+class FaultTolerantScannerExpiredException extends RecoverableException {
+  /**
+   * Constructor.
+   * @param status status object containing the reason for the exception
+   * trace
+   */
+  FaultTolerantScannerExpiredException(Status status) {
+    super(status);
+  }
+
+  /**
+   * Constructor.
+   * @param status status object containing the reason for the exception
+   * @param cause the exception that caused this one to be thrown
+   */
+  FaultTolerantScannerExpiredException(Status status, Throwable cause) {
+    super(status, cause);
+  }
+}

http://git-wip-us.apache.org/repos/asf/kudu/blob/b2a5bc82/java/kudu-client/src/main/java/org/apache/kudu/client/ScannerExpiredException.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/ScannerExpiredException.java b/java/kudu-client/src/main/java/org/apache/kudu/client/ScannerExpiredException.java
deleted file mode 100644
index 7e0f4cb..0000000
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/ScannerExpiredException.java
+++ /dev/null
@@ -1,47 +0,0 @@
-// 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 org.apache.yetus.audience.InterfaceAudience;
-import org.apache.yetus.audience.InterfaceStability;
-
-/**
- * A scanner expired exception.
- */
-@InterfaceAudience.Private
-@InterfaceStability.Evolving
-@SuppressWarnings("serial")
-class ScannerExpiredException extends RecoverableException {
-  /**
-   * Constructor.
-   * @param status status object containing the reason for the exception
-   * trace
-   */
-  ScannerExpiredException(Status status) {
-    super(status);
-  }
-
-  /**
-   * Constructor.
-   * @param status status object containing the reason for the exception
-   * @param cause the exception that caused this one to be thrown
-   */
-  ScannerExpiredException(Status status, Throwable cause) {
-    super(status, cause);
-  }
-}

http://git-wip-us.apache.org/repos/asf/kudu/blob/b2a5bc82/java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java b/java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java
index a9501a7..0c0f88d 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/ITClient.java
@@ -69,7 +69,7 @@ public class ITClient extends BaseKuduTest {
     runtimeInSeconds = runtimeProp == null ? DEFAULT_RUNTIME_SECONDS : Long.parseLong(runtimeProp);
 
     if (runtimeInSeconds < TEST_MIN_RUNTIME_SECONDS || runtimeInSeconds > TEST_TIMEOUT_SECONDS) {
-      Assert.fail("This test needs to run more more than " + TEST_MIN_RUNTIME_SECONDS + " seconds" +
+      Assert.fail("This test needs to run more than " + TEST_MIN_RUNTIME_SECONDS + " seconds" +
           " and less than " + TEST_TIMEOUT_SECONDS + " seconds");
     }
 
@@ -397,9 +397,12 @@ public class ITClient extends BaseKuduTest {
             LOG.info("New row count {}", lastRowCount);
           }
           return true;
+        } else {
+          reportError("Row count unexpectedly decreased from " + lastRowCount + "to " + rowCount,
+              null);
         }
 
-        // Due to the lack of KUDU-430, we need to loop until the row count stops regressing.
+        // Due to the lack of KUDU-430, we need to loop for a while.
         try {
           KEEP_RUNNING_LATCH.await(50, TimeUnit.MILLISECONDS);
         } catch (InterruptedException e) {