[kudu] 03/04: CapturingLogAppender: synchronize access to captured log text

[al...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

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

commit a9a5001ec9cd61c3806a581ca147ea70c1ee30a0
Author: Adar Dembo <ad...@cloudera.com>
AuthorDate: Mon Oct 14 13:24:09 2019 -0700

    CapturingLogAppender: synchronize access to captured log text
    
    I have no explanation for how this could happen, but the test failure I'm
    looking at speaks for itself:
    
      1) testSessionOnceClosed(org.apache.kudu.client.TestKuduClient)
      java.lang.StringIndexOutOfBoundsException: String index out of range: 419
    	at java.lang.String.<init>(String.java:205)
    	at java.lang.StringBuilder.toString(StringBuilder.java:407)
    	at org.apache.kudu.test.CapturingLogAppender.getAppendedText(CapturingLogAppender.java:72)
    	at org.apache.kudu.client.TestKuduClient.testSessionOnceClosed(TestKuduClient.java:1231)
    
    Change-Id: I84a5d0775cba5aa1d9df5484b5e9e621c908d42d
    Reviewed-on: http://gerrit.cloudera.org:8080/14431
    Reviewed-by: Greg Solovyev <gs...@cloudera.com>
    Tested-by: Kudu Jenkins
    Reviewed-by: Grant Henke <gr...@apache.org>
    (cherry picked from commit 0894eba26380036d6d6d59c29c3efc5b3a817641)
    Reviewed-on: http://gerrit.cloudera.org:8080/14467
---
 .../main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala  |  6 ++++--
 .../main/java/org/apache/kudu/test/CapturingLogAppender.java   | 10 ++++++++--
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala b/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala
index 74da812..907a4d5 100644
--- a/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala
+++ b/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala
@@ -268,8 +268,10 @@ class KuduRelation(
       table.getTableStatistics().getOnDiskSize
     } catch {
       case e: Exception =>
-        log.warn("Error while getting table statistic from master, maybe the current" +
-          " master doesn't support the rpc, please check the version.", e)
+        log.warn(
+          "Error while getting table statistic from master, maybe the current" +
+            " master doesn't support the rpc, please check the version.",
+          e)
         super.sizeInBytes
     }
   }
diff --git a/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java b/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java
index 932e14a..9227016 100644
--- a/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java
+++ b/java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java
@@ -19,6 +19,7 @@ package org.apache.kudu.test;
 import java.io.Closeable;
 import java.io.IOException;
 import java.util.Random;
+import javax.annotation.concurrent.GuardedBy;
 
 import com.google.common.base.Throwables;
 import org.apache.logging.log4j.core.LogEvent;
@@ -42,6 +43,11 @@ public class CapturingLogAppender extends AbstractAppender {
       .withPattern("%d{HH:mm:ss.SSS} [%p - %t] (%F:%L) %m%n")
       .build();
 
+  // The caller should detach the logger before calling getAppendedText().
+  // Nevertheless, for some reason it is still possible for additional
+  // append() calls to happen _after_ the logger is detached, which may race
+  // with getAppendedText().
+  @GuardedBy("this")
   private StringBuilder appended = new StringBuilder();
 
   public CapturingLogAppender() {
@@ -57,7 +63,7 @@ public class CapturingLogAppender extends AbstractAppender {
   }
 
   @Override
-  public void append(LogEvent event) {
+  public synchronized void append(LogEvent event) {
     appended.append(getLayout().toSerializable(event));
     if (event.getThrown() != null) {
       appended.append(Throwables.getStackTraceAsString(event.getThrown()));
@@ -68,7 +74,7 @@ public class CapturingLogAppender extends AbstractAppender {
   /**
    * @return all of the appended messages captured thus far, joined together.
    */
-  public String getAppendedText() {
+  public synchronized String getAppendedText() {
     return appended.toString();
   }