commit 0894eba26380036d6d6d59c29c3efc5b3a817641
author Adar Dembo <adar@cloudera.com> Mon Oct 14 13:24:09 2019 -0700
committer Adar Dembo <adar@cloudera.com> Wed Oct 16 16:11:51 2019 +0000
tree 0dcd0781c2860a4c2e245f8eb935f3ceb1120b26
parent 3bd293d0f32003e877e7f160365ffd238927e164

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 <gsolovyev@cloudera.com>
Tested-by: Kudu Jenkins
Reviewed-by: Grant Henke <granthenke@apache.org>

java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala

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 @@
       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
     }
   }

java/kudu-test-utils/src/main/java/org/apache/kudu/test/CapturingLogAppender.java

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 @@
 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 @@
       .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 @@
   }
 
   @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 @@
   /**
    * @return all of the appended messages captured thus far, joined together.
    */
-  public String getAppendedText() {
+  public synchronized String getAppendedText() {
     return appended.toString();
   }