You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by gr...@apache.org on 2019/11/27 18:24:09 UTC

[kudu] branch master updated: [java] Enable and fix low SpotBugs warnings

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

granthenke pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/master by this push:
     new 25ae6c5  [java] Enable and fix low SpotBugs warnings
25ae6c5 is described below

commit 25ae6c5108cc84289f69c467d862e298d3361ea8
Author: Grant Henke <gr...@apache.org>
AuthorDate: Tue Nov 26 16:12:23 2019 -0600

    [java] Enable and fix low SpotBugs warnings
    
    This patch fixes the low SpotBugs issues along with some other minor
    cleanup spotted along the way. Either the SpotBugs issues were fixed or
    they were added to the excludeFilter as a special case.
    
    Follow on patches will enforce SpotBugs in the pre-commit build.
    
    Change-Id: I4e5c8285554ff84e948c0ea095ba476969377440
    Reviewed-on: http://gerrit.cloudera.org:8080/14801
    Tested-by: Kudu Jenkins
    Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
 java/config/spotbugs/excludeFilter.xml             | 154 ++++++++++++++++++---
 java/gradle/quality.gradle                         |   2 +-
 .../scala/org/apache/kudu/backup/BackupIO.scala    |  13 +-
 .../org/apache/kudu/backup/KuduBackupCLI.scala     |  18 ++-
 .../kudu/mapreduce/tools/ExportCsvMapper.java      |   4 +-
 .../tools/IntegrationTestBigLinkedList.java        |   8 +-
 .../src/main/java/org/apache/kudu/Type.java        |  35 +++--
 .../org/apache/kudu/client/AsyncKuduClient.java    |  54 +++++---
 .../main/java/org/apache/kudu/client/Bytes.java    |  40 ++++--
 .../java/org/apache/kudu/client/Connection.java    |  44 +++---
 .../java/org/apache/kudu/client/KeyEncoder.java    |   2 +-
 .../java/org/apache/kudu/client/Negotiator.java    |   5 +-
 .../java/org/apache/kudu/client/PartialRow.java    |   1 -
 .../kudu/client/PleaseThrottleException.java       |   4 +-
 .../org/apache/kudu/client/RowResultIterator.java  |   4 +
 .../org/apache/kudu/client/SecurityContext.java    |  19 ++-
 .../java/org/apache/kudu/client/ServerInfo.java    |  12 +-
 .../java/org/apache/kudu/util/SecurityUtil.java    |   3 +-
 .../org/apache/kudu/client/TestKuduClient.java     |   2 +-
 .../org/apache/kudu/client/TestKuduScanner.java    |  10 +-
 .../org/apache/kudu/client/TestNegotiator.java     |   2 +-
 .../java/org/apache/kudu/client/TestOperation.java |   6 +-
 .../org/apache/kudu/client/TestRemoteTablet.java   |   1 -
 .../apache/kudu/client/TestScannerMultiTablet.java |   1 -
 .../java/org/apache/kudu/mapreduce/JarFinder.java  |   2 +-
 .../org/apache/kudu/mapreduce/TestJarFinder.java   |  49 ++++---
 .../kudu/spark/tools/ImportExportFiles.scala       |   4 +-
 .../spark/tools/IntegrationTestBigLinkedList.scala |  17 +--
 .../org/apache/kudu/spark/kudu/DefaultSource.scala |   5 +-
 .../org/apache/kudu/spark/kudu/KuduContext.scala   |   8 +-
 .../org/apache/kudu/spark/kudu/KuduRDDTest.scala   |   8 +-
 .../java/org/apache/kudu/test/cluster/FakeDNS.java |   2 +-
 .../apache/kudu/test/cluster/MiniKuduCluster.java  |   2 +-
 .../java/org/apache/kudu/test/junit/RetryRule.java |   2 +-
 .../test/cluster/TestKuduBinaryJarExtractor.java   |  68 +++++----
 35 files changed, 392 insertions(+), 219 deletions(-)

diff --git a/java/config/spotbugs/excludeFilter.xml b/java/config/spotbugs/excludeFilter.xml
index dccb69d..4913579 100644
--- a/java/config/spotbugs/excludeFilter.xml
+++ b/java/config/spotbugs/excludeFilter.xml
@@ -67,45 +67,78 @@
              <Bug pattern="NM_CLASS_NAMING_CONVENTION"/>
              <!-- NM_METHOD_NAMING_CONVENTION: Method names should start with a lower case letter. -->
              <Bug pattern="NM_METHOD_NAMING_CONVENTION"/>
+             <!-- NM_FIELD_NAMING_CONVENTION: Field names should start with a lower case letter. -->
+             <Bug pattern="NM_FIELD_NAMING_CONVENTION"/>
              <!-- EC_NULL_ARG: Call to equals(null) -->
              <Bug pattern="EC_NULL_ARG"/>
              <!-- NP_ALWAYS_NULL: Null pointer dereference -->
              <Bug pattern="NP_ALWAYS_NULL"/>
              <!-- MS_CANNOT_BE_FINAL: Field isn't final and can't be protected from malicious code. -->
              <Bug pattern="MS_CANNOT_BE_FINAL"/>
+             <!-- DLS_DEAD_LOCAL_STORE: Dead store to local variable. -->
+             <Bug pattern="DLS_DEAD_LOCAL_STORE"/>
+             <!-- UPM_UNCALLED_PRIVATE_METHOD: Private method is never called. -->
+             <Bug pattern="UPM_UNCALLED_PRIVATE_METHOD"/>
+             <!-- SE_NO_SERIALVERSIONID: Class is Serializable, but doesn't define serialVersionUID. -->
+             <Bug pattern="SE_NO_SERIALVERSIONID"/>
+             <!-- HE_HASHCODE_USE_OBJECT_EQUALS: Class defines hashCode() and uses Object.equals(). -->
+             <Bug pattern="HE_HASHCODE_USE_OBJECT_EQUALS"/>
          </Or>
     </Match>
     <Match>
-        <!-- The retry rule doesn't need to be read.
-        This naming scheme should be used for all RetryRule usage to avoid SpotBugs issues.
-        -->
-        <Field name="~.*retryRule" />
-        <Bug pattern="URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD" />
-    </Match>
-    <Match>
         <!-- Disable warnings about mutable objects and the use of public fields.
          Though these are important concerns, we have many instances of this warning
          and "fixing" the warning could have performance implications. -->
         <Bug pattern="EI_EXPOSE_REP,EI_EXPOSE_REP2" />
     </Match>
     <Match>
-        <!-- This rule often falsely flags side effecting calls. Tests will frequently call
-        methods for the side effects without caring about the return value. -->
-        <Class name="~org\.apache\.kudu.*Test.*"/>
-        <Bug pattern="RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT" />
-    </Match>
-    <Match>
-        <!-- Tests use i % 2 == 1 frequently to alternate behavior. -->
-        <!-- TODO: converting these into negated "check for even" and remove. -->
-        <Class name="~org\.apache\.kudu.*Test.*"/>
-        <Bug pattern="IM_BAD_CHECK_FOR_ODD" />
+        <!-- Bugs that are ignored in all tests. -->
+        <Or>
+            <Class name="~org\.apache\.kudu.*Test.*"/>
+            <Class name="~org\.apache\.kudu\..*\.IT.*"/>
+        </Or>
+        <Or>
+            <!-- Often fields are initialized in the setUp method in tests. -->
+            <Bug pattern="UWF_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR" />
+            <!-- Tests often ignore expected exceptions -->
+            <Bug pattern="DE_MIGHT_IGNORE" />
+            <Bug pattern="REC_CATCH_EXCEPTION" />
+            <!-- Tests of call methods for their side effects or to check for exceptions and
+            ignore the return value -->
+            <Bug pattern="RV_RETURN_VALUE_IGNORED" />
+            <Bug pattern="RV_RETURN_VALUE_IGNORED_BAD_PRACTICE" />
+            <Bug pattern="RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT" />
+            <!-- Tests often cast to a known class. -->
+            <Bug pattern="BC_UNCONFIRMED_CAST_OF_RETURN_VALUE" />
+            <!-- Tests don't need to worry about serialization compatibility. -->
+            <Bug pattern="SE_NO_SERIALVERSIONID" />
+            <!-- Tests use i % 2 == 1 frequently to alternate behavior. -->
+            <!-- TODO: converting these into negated "check for even" and remove. -->
+            <Bug pattern="IM_BAD_CHECK_FOR_ODD" />
+            <!-- The retry rule doesn't need to be read.
+            This naming scheme should be used for all RetryRule usage to avoid SpotBugs issues.
+            -->
+            <And>
+                <Field name="~.*retryRule" />
+                <Bug pattern="URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD" />
+            </And>
+            <!-- We generally don't care about minor performance issues in tests. -->
+            <And>
+                <Bug category="PERFORMANCE" />
+                <Priority value="3" />
+            </And>
+        </Or>
     </Match>
 
     <!-- kudu-backup exclusions -->
     <Match>
-        <!-- The options field doesn't need to be restored in this case. -->
+        <!-- These fields don't need to be restored in this case. -->
         <Class name="org.apache.kudu.backup.KuduBackupRDD"/>
-        <Field name="options" />
+        <Or>
+            <Field name="options" />
+            <Field name="sc" />
+            <Field name="table" />
+        </Or>
         <Bug pattern="SE_TRANSIENT_FIELD_NOT_RESTORED" />
     </Match>
 
@@ -124,6 +157,61 @@
         </Or>
         <Bug pattern="NM_CLASS_NOT_EXCEPTION" />
     </Match>
+    <Match>
+        <!-- Though returning null is a bad practice, changing this now breaks expectations. -->
+        <Or>
+            <And>
+                <Class name="org.apache.kudu.client.KuduRpc"/>
+                <Method name="partitionKey" />
+            </And>
+            <And>
+                <Class name="org.apache.kudu.client.ColumnRangePredicate"/>
+                <Method name="toByteArray" />
+            </And>
+        </Or>
+        <Bug pattern="PZLA_PREFER_ZERO_LENGTH_ARRAYS" />
+    </Match>
+    <Match>
+        <!-- The return value isn't needed. -->
+        <Class name="org.apache.kudu.client.ErrorCollector"/>
+        <Method name="addError" />
+        <Bug pattern="RV_RETURN_VALUE_IGNORED" />
+    </Match>
+    <Match>
+        <!-- Status has manny upper case methods, but changing it would be breaking. -->
+        <Class name="org.apache.kudu.client.Status"/>
+        <Bug pattern="NM_METHOD_NAMING_CONVENTION" />
+    </Match>
+    <Match>
+        <!-- These instances of floating point equality are correct. -->
+        <Or>
+            <And>
+                <Class name="org.apache.kudu.client.KuduPredicate"/>
+                <Method name="areConsecutive" />
+            </And>
+            <And>
+                <Class name="org.apache.kudu.client.PartialRow"/>
+                <Or>
+                    <Method name="incrementColumn" />
+                    <Method name="isCellEqual" />
+                    <Method name="isCellIncremented" />
+                </Or>
+            </And>
+        </Or>
+        <Bug pattern="FE_FLOATING_POINT_EQUALITY" />
+    </Match>
+    <Match>
+        <!-- This cast is safe. -->
+        <Class name="org.apache.kudu.client.Negotiator"/>
+        <Method name="evaluateChallenge" />
+        <Bug pattern="BC_UNCONFIRMED_CAST_OF_RETURN_VALUE" />
+    </Match>
+    <Match>
+        <!-- Changing this would break public API. -->
+        <Class name="org.apache.kudu.client.AlterTableResponse"/>
+        <Method name="getTsUUID" />
+        <Bug pattern="NM_CONFUSING" />
+    </Match>
 
     <Match>
         <!-- The nullable annotation is from Guava and therefore can't be changed.
@@ -154,16 +242,37 @@
         <Bug pattern="URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD" />
     </Match>
 
+    <!-- kudu-mapreduce exclusions -->
+    <Match>
+        <!-- This is a false positive. -->
+        <Class name="org.apache.kudu.mapreduce.TestJarFinder"/>
+        <Method name="writeManifest" />
+        <Bug pattern="OBL_UNSATISFIED_OBLIGATION" />
+    </Match>
+
     <!-- kudu-spark exclusions -->
     <Match>
-        <!-- The options and projectedCols field doesn't need to be restored in this case. -->
+        <!-- These fields don't need to be restored in this case. -->
         <Class name="org.apache.kudu.spark.kudu.KuduRDD"/>
         <Or>
             <Field name="options" />
+            <Field name="predicates"/>
             <Field name="projectedCols"/>
+            <Field name="sc"/>
+            <Field name="table"/>
         </Or>
         <Bug pattern="SE_TRANSIENT_FIELD_NOT_RESTORED" />
     </Match>
+    <Match>
+        <!-- These fields don't need to be restored in this case. -->
+        <Class name="org.apache.kudu.spark.kudu.KuduContext"/>
+        <Bug pattern="SE_TRANSIENT_FIELD_NOT_RESTORED" />
+    </Match>
+    <Match>
+        <!-- This instance of floating point equality is correct. -->
+        <Class name="~org\.apache\.kudu\.spark\.kudu\.KuduContextTest.*"/>
+        <Bug pattern="FE_FLOATING_POINT_EQUALITY" />
+    </Match>
 
     <!-- kudu-test-utils exclusions -->
     <Match>
@@ -176,4 +285,9 @@
         <Class name="org.apache.kudu.test.junit.TestResultReporter$MockFlakyTestServlet"/>
         <Bug pattern="SE_TRANSIENT_FIELD_NOT_RESTORED" />
     </Match>
+    <Match>
+        <!-- This is okay in a test context. -->
+        <Class name="org.apache.kudu.test.cluster.FakeDNS"/>
+        <Bug pattern="DP_DO_INSIDE_DO_PRIVILEGED" />
+    </Match>
 </FindBugsFilter>
\ No newline at end of file
diff --git a/java/gradle/quality.gradle b/java/gradle/quality.gradle
index 1d36919..4a75071 100644
--- a/java/gradle/quality.gradle
+++ b/java/gradle/quality.gradle
@@ -41,7 +41,7 @@ spotbugs {
   toolVersion = versions.spotBugs
   ignoreFailures = true
   effort = "max"
-  reportLevel = "medium"
+  reportLevel = "low"
   excludeFilter = file("$rootProject.projectDir/config/spotbugs/excludeFilter.xml")
 }
 
diff --git a/java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/BackupIO.scala b/java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/BackupIO.scala
index f5c2dfd..355b84d 100644
--- a/java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/BackupIO.scala
+++ b/java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/BackupIO.scala
@@ -242,11 +242,14 @@ class BackupIO(val conf: Configuration, rootPathStr: String) {
    */
   def readTableMetadata(metadataPath: Path): TableMetadataPB = {
     val in = new InputStreamReader(fs.open(metadataPath), StandardCharsets.UTF_8)
-    val json = CharStreams.toString(in)
-    in.close()
-    val builder = TableMetadataPB.newBuilder()
-    JsonFormat.parser().merge(json, builder)
-    builder.build()
+    try {
+      val json = CharStreams.toString(in)
+      val builder = TableMetadataPB.newBuilder()
+      JsonFormat.parser().merge(json, builder)
+      builder.build()
+    } finally {
+      in.close()
+    }
   }
 }
 
diff --git a/java/kudu-backup-tools/src/main/scala/org/apache/kudu/backup/KuduBackupCLI.scala b/java/kudu-backup-tools/src/main/scala/org/apache/kudu/backup/KuduBackupCLI.scala
index a29fb90..1df4a57 100644
--- a/java/kudu-backup-tools/src/main/scala/org/apache/kudu/backup/KuduBackupCLI.scala
+++ b/java/kudu-backup-tools/src/main/scala/org/apache/kudu/backup/KuduBackupCLI.scala
@@ -19,6 +19,7 @@ package org.apache.kudu.backup
 
 import java.time.Duration
 import java.time.temporal.ChronoUnit
+import java.util.Locale
 
 import org.apache.kudu.backup.BackupCLIOptions.DefaultDryRun
 import org.apache.kudu.backup.BackupCLIOptions.DefaultExpirationAge
@@ -90,16 +91,19 @@ object BackupCLIOptions {
         .text("List the backups in the rootPath.")
         .children(
           opt[String]("type")
-            .action((v, o) => o.copy(listType = ListType.withName(v.toUpperCase)))
+            .action((v, o) => o.copy(listType = ListType.withName(v.toUpperCase(Locale.ENGLISH))))
             .text("The type of listing to perform. One of 'latest', 'restore_sequence', 'all'. " +
-              s"Default: ${DefaultListType.toString.toLowerCase()}")
-            .validate(
-              validateEnumeratedOption("type", ListType.values.map(_.toString.toLowerCase))),
+              s"Default: ${DefaultListType.toString.toLowerCase(Locale.ENGLISH)}")
+            .validate(validateEnumeratedOption(
+              "type",
+              ListType.values.map(_.toString.toLowerCase(Locale.ENGLISH)))),
           opt[String]("format")
-            .action((v, o) => o.copy(format = Format.withName(v.toUpperCase)))
+            .action((v, o) => o.copy(format = Format.withName(v.toUpperCase(Locale.ENGLISH))))
             .text(s"The output format. One of 'pretty', 'tsv', 'csv'. " +
-              s"Default: ${DefaultFormat.toString.toLowerCase()}")
-            .validate(validateEnumeratedOption("format", Format.values.map(_.toString.toLowerCase)))
+              s"Default: ${DefaultFormat.toString.toLowerCase(Locale.ENGLISH)}")
+            .validate(validateEnumeratedOption(
+              "format",
+              Format.values.map(_.toString.toLowerCase(Locale.ENGLISH))))
             .optional()
         )
 
diff --git a/java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/ExportCsvMapper.java b/java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/ExportCsvMapper.java
index eac5c8a..d7b4800 100644
--- a/java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/ExportCsvMapper.java
+++ b/java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/ExportCsvMapper.java
@@ -86,6 +86,7 @@ public class ExportCsvMapper extends Mapper<NullWritable, RowResult, NullWritabl
           buf.append(value.getInt(i));
           break;
         case INT64:
+        case UNIXTIME_MICROS:
           buf.append(value.getLong(i));
           break;
         case STRING:
@@ -103,9 +104,6 @@ public class ExportCsvMapper extends Mapper<NullWritable, RowResult, NullWritabl
         case BOOL:
           buf.append(value.getBoolean(i));
           break;
-        case UNIXTIME_MICROS:
-          buf.append(value.getLong(i));
-          break;
         default:
           buf.append("<unknown type!>");
           break;
diff --git a/java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/IntegrationTestBigLinkedList.java b/java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/IntegrationTestBigLinkedList.java
index 164f97d..7730cd8 100644
--- a/java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/IntegrationTestBigLinkedList.java
+++ b/java/kudu-client-tools/src/main/java/org/apache/kudu/mapreduce/tools/IntegrationTestBigLinkedList.java
@@ -736,9 +736,7 @@ public class IntegrationTestBigLinkedList extends Configured implements Tool {
           context.getCounter(Counts.UNREFERENCED).increment(1);
         } else {
           if (refs.size() > 1) {
-            if (refsList != null) {
-              context.write(new Text(keyString), new Text(refsList.toString()));
-            }
+            context.write(new Text(keyString), new Text(refsList.toString()));
             context.getCounter(Counts.EXTRAREFERENCES).increment(refs.size() - 1);
           }
           // node is defined and referenced
@@ -1107,8 +1105,8 @@ public class IntegrationTestBigLinkedList extends Configured implements Tool {
         // Add as many heads as we need, then we skip the rest.
         do {
           if (headsCache.size() < numUpdatesPerMapper) {
-            value = (RowResult)context.getCurrentValue();
-            headsCache.add(new Pair<>(value.getLong(0), value.getLong(1)));
+            RowResult realValue = (RowResult) context.getCurrentValue();
+            headsCache.add(new Pair<>(realValue.getLong(0), realValue.getLong(1)));
           }
         } while (context.nextKeyValue());
 
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/Type.java b/java/kudu-client/src/main/java/org/apache/kudu/Type.java
index 8cd243c..b1cb785 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/Type.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/Type.java
@@ -171,22 +171,33 @@ public enum Type {
    */
   public static Type getTypeForDataType(DataType type) {
     switch (type) {
-      case STRING: return STRING;
-      case BINARY: return BINARY;
-      case VARCHAR: return VARCHAR;
-      case BOOL: return BOOL;
-      case INT8: return INT8;
-      case INT16: return INT16;
-      case INT32: return INT32;
-      case INT64: return INT64;
-      case UNIXTIME_MICROS: return UNIXTIME_MICROS;
-      case FLOAT: return FLOAT;
-      case DOUBLE: return DOUBLE;
+      case STRING:
+        return STRING;
+      case BINARY:
+        return BINARY;
+      case VARCHAR:
+        return VARCHAR;
+      case BOOL:
+      case IS_DELETED:
+        return BOOL;
+      case INT8:
+        return INT8;
+      case INT16:
+        return INT16;
+      case INT32:
+        return INT32;
+      case INT64:
+        return INT64;
+      case UNIXTIME_MICROS:
+        return UNIXTIME_MICROS;
+      case FLOAT:
+        return FLOAT;
+      case DOUBLE:
+        return DOUBLE;
       case DECIMAL32:
       case DECIMAL64:
       case DECIMAL128:
         return DECIMAL;
-      case IS_DELETED: return BOOL;
       default:
         throw new IllegalArgumentException("The provided data type doesn't map" +
             " to know any known one: " + type.getDescriptorForType().getFullName());
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java b/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
index a286116..a399366 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
@@ -1386,27 +1386,7 @@ public class AsyncKuduClient implements AutoCloseable {
       @Nonnull final String method,
       @Nullable final KuduRpc<?> parent,
       long timeoutMs) {
-    KuduRpc<R> rpc = new KuduRpc<R>(null, timer, timeoutMs) {
-      @Override
-      Message createRequestPB() {
-        return null;
-      }
-
-      @Override
-      String serviceName() {
-        return null;
-      }
-
-      @Override
-      String method() {
-        return method;
-      }
-
-      @Override
-      Pair<R, Object> deserialize(CallResponse callResponse, String tsUUID) throws KuduException {
-        return null;
-      }
-    };
+    KuduRpc<R> rpc = new FakeKuduRpc<R>(method, timer, timeoutMs);
     rpc.setParentRpc(parent);
     return rpc;
   }
@@ -1426,6 +1406,38 @@ public class AsyncKuduClient implements AutoCloseable {
   }
 
   /**
+   * A fake RPC that is used for timeouts and will never be sent.
+   */
+  private static class FakeKuduRpc<R> extends KuduRpc<R> {
+    private final String method;
+
+    FakeKuduRpc(String method, Timer timer, long timeoutMillis) {
+      super(null, timer, timeoutMillis);
+      this.method = method;
+    }
+
+    @Override
+    Message createRequestPB() {
+      return null;
+    }
+
+    @Override
+    String serviceName() {
+      return null;
+    }
+
+    @Override
+    String method() {
+      return method;
+    }
+
+    @Override
+    Pair<R, Object> deserialize(CallResponse callResponse, String tsUUID) throws KuduException {
+      return null;
+    }
+  }
+
+  /**
    * Schedules a IsAlterTableDone RPC. When the response comes in, if the table
    * is done altering, the RPC's callback chain is triggered with 'resp' as its
    * value. If not, another IsAlterTableDone RPC is scheduled and the cycle
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/Bytes.java b/java/kudu-client/src/main/java/org/apache/kudu/client/Bytes.java
index 256f6a2..8574abf 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/Bytes.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/Bytes.java
@@ -36,6 +36,8 @@ import java.lang.reflect.Method;
 import java.math.BigDecimal;
 import java.math.BigInteger;
 import java.nio.ByteBuffer;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
 import java.util.Arrays;
 import java.util.BitSet;
 import java.util.Comparator;
@@ -1045,21 +1047,37 @@ public final class Bytes {
     try {
       ReplayingDecoderBuffer = Class.forName("org.jboss.netty.handler.codec." +
           "replay.ReplayingDecoderBuffer");
-      Field field = null;
-      try {
-        field = ReplayingDecoderBuffer.getDeclaredField("buffer");
-        field.setAccessible(true);
-      } catch (NoSuchFieldException e) {
-        // Ignore.  Field has been removed in Netty 3.5.1.
-      }
+      Field field = AccessController.doPrivileged(new PrivilegedAction<Field>() {
+        @Override
+        public Field run() {
+          try {
+            Field bufferField = ReplayingDecoderBuffer.getDeclaredField("buffer");
+            bufferField.setAccessible(true);
+            return bufferField;
+          } catch (NoSuchFieldException e) {
+            // Ignore.  Field has been removed in Netty 3.5.1.
+            return null;
+          }
+        }
+      });
       RDB_buffer = field;
       if (field != null) {  // Netty 3.5.0 or before.
         RDB_buf = null;
       } else {
-        RDB_buf = ReplayingDecoderBuffer.getDeclaredMethod("buf");
-        RDB_buf.setAccessible(true);
+        RDB_buf = AccessController.doPrivileged(new PrivilegedAction<Method>() {
+          @Override
+          public Method run() {
+            try {
+              Method bufMethod = ReplayingDecoderBuffer.getDeclaredMethod("buf");
+              bufMethod.setAccessible(true);
+              return bufMethod;
+            } catch (NoSuchMethodException e) {
+              throw new RuntimeException(e);
+            }
+          }
+        });
       }
-    } catch (Exception e) {
+    } catch (ReflectiveOperationException | RuntimeException e) {
       throw new RuntimeException("static initializer failed", e);
     }
   }
@@ -1078,6 +1096,8 @@ public final class Bytes {
   /** {@link Comparator} for non-{@code null} byte arrays.  */
   private static final class MemCmp implements Comparator<byte[]>, Serializable {
 
+    private static final long serialVersionUID = 914981342853419168L;
+
     private MemCmp() {  // Can't instantiate outside of this class.
     }
 
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java b/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
index d02d27f..69767d1 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
@@ -596,25 +596,37 @@ class Connection extends SimpleChannelUpstreamHandler {
   Deferred<Void> shutdown() {
     final ChannelFuture disconnectFuture = disconnect();
     final Deferred<Void> d = new Deferred<>();
-    disconnectFuture.addListener(new ChannelFutureListener() {
-      @Override
-      public void operationComplete(final ChannelFuture future) {
-        if (future.isSuccess()) {
-          d.callback(null);
-          return;
-        }
-        final Throwable t = future.getCause();
-        if (t instanceof Exception) {
-          d.callback(t);
-        } else {
-          d.callback(new NonRecoverableException(
-              Status.IllegalState("failed to shutdown: " + this), t));
-        }
-      }
-    });
+    disconnectFuture.addListener(new ShutdownListener(d));
     return d;
   }
 
+  /**
+   * A ChannelFutureListener that completes the passed deferred on completion.
+   */
+  private static class ShutdownListener implements ChannelFutureListener {
+
+    private final Deferred<Void> deferred;
+
+    public ShutdownListener(Deferred<Void> deferred) {
+      this.deferred = deferred;
+    }
+
+    @Override
+    public void operationComplete(final ChannelFuture future) {
+      if (future.isSuccess()) {
+        deferred.callback(null);
+        return;
+      }
+      final Throwable t = future.getCause();
+      if (t instanceof Exception) {
+        deferred.callback(t);
+      } else {
+        deferred.callback(new NonRecoverableException(
+            Status.IllegalState("failed to shutdown: " + this), t));
+      }
+    }
+  }
+
   /** @return string representation of this object (suitable for printing into the logs, etc.) */
   @Override
   public String toString() {
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/KeyEncoder.java b/java/kudu-client/src/main/java/org/apache/kudu/client/KeyEncoder.java
index a2cafaf..de268b6 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/KeyEncoder.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/KeyEncoder.java
@@ -254,7 +254,7 @@ class KeyEncoder {
 
     List<Integer> buckets = new ArrayList<>();
 
-    for (HashBucketSchema unused : partitionSchema.getHashBucketSchemas()) {
+    for (int i = 0; i < partitionSchema.getHashBucketSchemas().size(); i++) {
       if (buf.hasRemaining()) {
         buckets.add(buf.getInt());
       } else {
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java b/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
index 7617de4..84a982d 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
@@ -35,6 +35,7 @@ import java.security.PrivilegedActionException;
 import java.security.PrivilegedExceptionAction;
 import java.security.cert.Certificate;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
 import javax.net.ssl.SSLEngine;
@@ -400,7 +401,7 @@ public class Negotiator extends SimpleChannelUpstreamHandler {
     Map<String, String> errorsByMech = Maps.newHashMap();
     Set<SaslMechanism> serverMechs = Sets.newHashSet();
     for (RpcHeader.NegotiatePB.SaslMechanism mech : response.getSaslMechanismsList()) {
-      switch (mech.getMechanism().toUpperCase()) {
+      switch (mech.getMechanism().toUpperCase(Locale.ENGLISH)) {
         case "GSSAPI":
           serverMechs.add(SaslMechanism.GSSAPI);
           break;
@@ -450,7 +451,7 @@ public class Negotiator extends SimpleChannelUpstreamHandler {
                                            "kudu",
                                            remoteHostname,
                                            props,
-            saslCallback);
+                                           saslCallback);
         chosenMech = clientMech;
         break;
       } catch (SaslException e) {
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java b/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
index b3ee835..c118ed4 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java
@@ -1644,7 +1644,6 @@ public class PartialRow {
 
     switch (type) {
       case BOOL:
-        return a.rowAlloc[offset] == b.rowAlloc[offset];
       case INT8:
         return a.rowAlloc[offset] == b.rowAlloc[offset];
       case INT16:
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/PleaseThrottleException.java b/java/kudu-client/src/main/java/org/apache/kudu/client/PleaseThrottleException.java
index b290e62..52d15ae 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/PleaseThrottleException.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/PleaseThrottleException.java
@@ -66,10 +66,10 @@ public final class PleaseThrottleException extends RecoverableException
     implements HasFailedRpcException {
 
   /** The RPC that was failed with this exception.  */
-  private final Operation rpc;
+  private final transient Operation rpc;
 
   /** A deferred one can wait on before retrying the failed RPC.  */
-  private final Deferred deferred;
+  private final transient Deferred deferred;
 
   /**
    * Constructor.
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/RowResultIterator.java b/java/kudu-client/src/main/java/org/apache/kudu/client/RowResultIterator.java
index d4a15aa..b869988 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/RowResultIterator.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/RowResultIterator.java
@@ -18,6 +18,7 @@
 package org.apache.kudu.client;
 
 import java.util.Iterator;
+import java.util.NoSuchElementException;
 
 import org.apache.yetus.audience.InterfaceAudience;
 import org.apache.yetus.audience.InterfaceStability;
@@ -112,6 +113,9 @@ public class RowResultIterator extends KuduRpcResponse implements Iterator<RowRe
 
   @Override
   public RowResult next() {
+    if (!hasNext()) {
+      throw new NoSuchElementException();
+    }
     // If sharedRowResult is not null, we should reuse it for every next call.
     if (sharedRowResult != null) {
       this.sharedRowResult.advancePointerTo(this.currentRow++);
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java b/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java
index da7a08a..8073c4d 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/SecurityContext.java
@@ -17,8 +17,10 @@
 
 package org.apache.kudu.client;
 
+import java.io.IOException;
 import java.security.AccessControlContext;
 import java.security.AccessController;
+import java.security.GeneralSecurityException;
 import java.security.KeyStore;
 import java.security.PrivilegedActionException;
 import java.security.PrivilegedExceptionAction;
@@ -27,6 +29,7 @@ import java.security.cert.CertificateFactory;
 import java.security.cert.X509Certificate;
 import java.util.Collections;
 import java.util.List;
+import java.util.Objects;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 import javax.annotation.Nonnull;
@@ -140,7 +143,7 @@ class SecurityContext {
 
       this.sslContextTrustAny = SSLContext.getInstance("TLS");
       sslContextTrustAny.init(null, new TrustManager[] { new TrustAnyCert() }, null);
-    } catch (Exception e) {
+    } catch (GeneralSecurityException | RuntimeException e) {
       throw new RuntimeException(e);
     }
   }
@@ -217,12 +220,8 @@ class SecurityContext {
       LOG.debug("Refreshing Kerberos credentials...");
       Subject newSubject;
       try {
-        newSubject = Subject.doAs(new Subject(), new PrivilegedExceptionAction<Subject>() {
-          @Override
-          public Subject run() {
-            return SecurityUtil.getSubjectFromTicketCacheOrNull();
-          }
-        });
+        newSubject = Subject.doAs(new Subject(),
+            (PrivilegedExceptionAction<Subject>) SecurityUtil::getSubjectFromTicketCacheOrNull);
       } catch (PrivilegedActionException e) {
         throw new RuntimeException(e.getCause());
       }
@@ -241,7 +240,7 @@ class SecurityContext {
       // so let's just refuse the re-login attempt if the principal switched.
       KerberosPrincipal oldPrincipal = SecurityUtil.getKerberosPrincipalOrNull(localSubject);
       KerberosPrincipal principal = SecurityUtil.getKerberosPrincipalOrNull(newSubject);
-      if (!oldPrincipal.equals(principal)) {
+      if (!Objects.equals(oldPrincipal, principal)) {
         LOG.error("Attempted to refresh Kerberos credentials from ticket cache but found that " +
             "the new Kerberos principal {} did not match the original principal {}. Ignoring.",
             principal, oldPrincipal);
@@ -400,7 +399,7 @@ class SecurityContext {
         throw new RuntimeException("TrustManagerFactory generated multiple TrustManagers");
       }
       return (X509TrustManager) managers[0];
-    } catch (Exception e) {
+    } catch (GeneralSecurityException | IOException | RuntimeException e) {
       Throwables.throwIfInstanceOf(e, CertificateException.class);
       throw new RuntimeException(e);
     }
@@ -435,7 +434,7 @@ class SecurityContext {
 
     @Override
     public X509Certificate[] getAcceptedIssuers() {
-      return null;
+      return new X509Certificate[0];
     }
   }
 
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java b/java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java
index 911b223..17e9674 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java
@@ -20,6 +20,7 @@ package org.apache.kudu.client;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.UnknownHostException;
+import java.util.Locale;
 import java.util.concurrent.ConcurrentHashMap;
 
 import com.google.common.base.Preconditions;
@@ -57,12 +58,8 @@ public class ServerInfo {
     this.hostPort = hostPort;
     this.resolvedAddr = new InetSocketAddress(resolvedAddr, hostPort.getPort());
     this.location = location;
-    Boolean isLocal = isLocalAddressCache.get(resolvedAddr);
-    if (isLocal == null) {
-      isLocal = NetUtil.isLocalAddress(resolvedAddr);
-      isLocalAddressCache.putIfAbsent(resolvedAddr, isLocal);
-    }
-    this.local = isLocal;
+    this.local = isLocalAddressCache.computeIfAbsent(resolvedAddr,
+        inetAddress -> NetUtil.isLocalAddress(resolvedAddr));
   }
 
   /**
@@ -79,7 +76,8 @@ public class ServerInfo {
    */
   public String getAndCanonicalizeHostname() {
     try {
-      return InetAddress.getByName(hostPort.getHost()).getCanonicalHostName().toLowerCase();
+      return InetAddress.getByName(
+          hostPort.getHost()).getCanonicalHostName().toLowerCase(Locale.ENGLISH);
     } catch (UnknownHostException e) {
       return hostPort.getHost();
     }
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java b/java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java
index 9d317ea..cab4f1f 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/util/SecurityUtil.java
@@ -21,6 +21,7 @@ import java.security.MessageDigest;
 import java.security.cert.Certificate;
 import java.security.cert.X509Certificate;
 import java.util.HashMap;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
 import javax.annotation.Nullable;
@@ -142,7 +143,7 @@ public abstract class SecurityUtil {
     // There's no API available to actually find just the digest algorithm,
     // so we resort to some hackery.
     String[] components = sigAlg.split("with", 2);
-    String digestAlg = CERT_DIGEST_TO_MESSAGE_DIGEST.get(components[0].toUpperCase());
+    String digestAlg = CERT_DIGEST_TO_MESSAGE_DIGEST.get(components[0].toUpperCase(Locale.ENGLISH));
     if (digestAlg == null) {
       // RFC 5929: if the certificate's signatureAlgorithm uses no hash functions or
       // uses multiple hash functions, then this channel binding type's channel
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
index b4b3d2d..1792642 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
@@ -1240,7 +1240,7 @@ public class TestKuduClient {
         fail();
       } catch (InvocationTargetException ex) {
         assertTrue(ex.getTargetException() instanceof KuduException);
-        KuduException realEx = (KuduException)ex.getTargetException();
+        KuduException realEx = (KuduException) ex.getTargetException();
         assertTrue(realEx.getStatus().isTimedOut());
       }
     }
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
index 01246b0..c0fbac3 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduScanner.java
@@ -124,7 +124,7 @@ public class TestKuduScanner {
       "--scanner_gc_check_interval_us=500000"}) // 10% of the TTL.
   public void testKeepAlive() throws Exception {
     int rowCount = 500;
-    int shortScannerTtlMs = 5000;
+    long shortScannerTtlMs = 5000;
 
     // Create a simple table with a single partition.
     Schema tableSchema = new Schema(Collections.singletonList(
@@ -154,7 +154,7 @@ public class TestKuduScanner {
     // Test that a keepAlivePeriodMs greater than the scanner ttl fails.
     KuduScanner badScanner = client.newScannerBuilder(table)
         .batchSizeBytes(100) // Set a small batch size so the first scan doesn't read all the rows.
-        .keepAlivePeriodMs(shortScannerTtlMs * 2)
+        .keepAlivePeriodMs(shortScannerTtlMs * 2L)
         .build();
     try {
       processKeepAliveScanner(badScanner, shortScannerTtlMs);
@@ -164,10 +164,12 @@ public class TestKuduScanner {
     }
   }
 
-  private void processKeepAliveScanner(KuduScanner scanner, int scannerTtlMs) throws Exception {
+  private void processKeepAliveScanner(KuduScanner scanner, long scannerTtlMs) throws Exception {
     int i = 0;
+    KuduScannerIterator iterator = scanner.iterator();
     // Ensure reading takes longer than the scanner ttl.
-    for (RowResult unused : scanner) {
+    while (iterator.hasNext()) {
+      iterator.next();
       // Sleep for half the ttl for the first few rows. This ensures
       // we are on the same tablet and will go past the ttl without
       // a new scan request. It also ensures a single row doesn't go
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java
index 32ddade..c60578d 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java
@@ -98,7 +98,7 @@ public class TestNegotiator {
   public RetryRule retryRule = new RetryRule();
 
   @Before
-  public void setup() {
+  public void setUp() {
     serverEngine = createServerEngine();
     serverEngine.setUseClientMode(false);
     secContext = new SecurityContext();
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestOperation.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestOperation.java
index b64fbb0..7280c30 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestOperation.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestOperation.java
@@ -81,12 +81,12 @@ public class TestOperation {
 
       // Check the strings.
       int offset = 3;
-      for (int i = 0; i <= 4; i++) {
+      for (long i = 0; i <= 4; i++) {
         // The offset into the indirect buffer
-        assertEquals(6 * i, Bytes.getLong(rows, offset));
+        assertEquals(6L * i, Bytes.getLong(rows, offset));
         offset += Longs.BYTES;
         // The length of the pointed-to string.
-        assertEquals(6, Bytes.getLong(rows, offset));
+        assertEquals(6L, Bytes.getLong(rows, offset));
         offset += Longs.BYTES;
       }
 
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java
index 25b097c..44d652a 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java
@@ -32,7 +32,6 @@ import org.junit.Rule;
 import org.junit.Test;
 
 import org.apache.kudu.consensus.Metadata;
-import org.apache.kudu.master.Master;
 import org.apache.kudu.test.ProtobufUtils;
 import org.apache.kudu.test.junit.RetryRule;
 
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java
index e7fcda2..03c700a 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java
@@ -26,7 +26,6 @@ import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
 import java.util.ArrayList;
-import java.util.Map;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
diff --git a/java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/JarFinder.java b/java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/JarFinder.java
index f71011c..ce55f22 100644
--- a/java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/JarFinder.java
+++ b/java/kudu-mapreduce/src/main/java/org/apache/kudu/mapreduce/JarFinder.java
@@ -189,7 +189,7 @@ public class JarFinder {
     return null;
   }
 
-  private static File getFileDir() throws IOException {
+  private static synchronized File getFileDir() throws IOException {
     if (fileDir == null) {
       String testDirPath = System.getProperty(FILE_DIR_PROPERTY);
       if (testDirPath == null) {
diff --git a/java/kudu-mapreduce/src/test/java/org/apache/kudu/mapreduce/TestJarFinder.java b/java/kudu-mapreduce/src/test/java/org/apache/kudu/mapreduce/TestJarFinder.java
index b63f61b..03b7c52 100644
--- a/java/kudu-mapreduce/src/test/java/org/apache/kudu/mapreduce/TestJarFinder.java
+++ b/java/kudu-mapreduce/src/test/java/org/apache/kudu/mapreduce/TestJarFinder.java
@@ -82,40 +82,39 @@ public class TestJarFinder {
   @Test
   public void testExistingManifest() throws Exception {
     File dir = new File(testDir, TestJarFinder.class.getName() + "-testExistingManifest");
+    Assert.assertTrue(dir.mkdirs());
+    writeManifest(dir);
+    Assert.assertNotNull(getManifest(dir));
+  }
+
+  @Test
+  public void testNoManifest() throws Exception {
+    File dir = new File(testDir, TestJarFinder.class.getName() + "-testNoManifest");
+    Assert.assertTrue(dir.mkdirs());
+    Assert.assertNotNull(getManifest(dir));
+  }
+
+  private void writeManifest(File dir) throws Exception {
     File metaInfDir = new File(dir, "META-INF");
     Assert.assertTrue(metaInfDir.mkdirs());
     File manifestFile = new File(metaInfDir, "MANIFEST.MF");
     Manifest manifest = new Manifest();
-
     try (OutputStream os = new FileOutputStream(manifestFile)) {
       manifest.write(os);
     }
-
-    File propsFile = new File(dir, "props.properties");
-    Writer writer = Files.newBufferedWriter(propsFile.toPath(), UTF_8);
-    new Properties().store(writer, "");
-    writer.close();
-    ByteArrayOutputStream baos = new ByteArrayOutputStream();
-    JarOutputStream zos = new JarOutputStream(baos);
-    JarFinder.jarDir(dir, "", zos);
-    JarInputStream jis = new JarInputStream(new ByteArrayInputStream(baos.toByteArray()));
-    Assert.assertNotNull(jis.getManifest());
-    jis.close();
   }
 
-  @Test
-  public void testNoManifest() throws Exception {
-    File dir = new File(testDir, TestJarFinder.class.getName() + "-testNoManifest");
-    Assert.assertTrue(dir.mkdirs());
+  private Manifest getManifest(File dir) throws Exception {
     File propsFile = new File(dir, "props.properties");
-    Writer writer = Files.newBufferedWriter(propsFile.toPath(), UTF_8);
-    new Properties().store(writer, "");
-    writer.close();
-    ByteArrayOutputStream baos = new ByteArrayOutputStream();
-    JarOutputStream zos = new JarOutputStream(baos);
-    JarFinder.jarDir(dir, "", zos);
-    JarInputStream jis = new JarInputStream(new ByteArrayInputStream(baos.toByteArray()));
-    Assert.assertNotNull(jis.getManifest());
-    jis.close();
+    try (Writer writer = Files.newBufferedWriter(propsFile.toPath(), UTF_8)) {
+      new Properties().store(writer, "");
+    }
+    try (ByteArrayOutputStream baos = new ByteArrayOutputStream();
+         JarOutputStream zos = new JarOutputStream(baos)) {
+      JarFinder.jarDir(dir, "", zos);
+      try (JarInputStream jis = new JarInputStream(new ByteArrayInputStream(baos.toByteArray()))) {
+        return jis.getManifest();
+      }
+    }
   }
 }
diff --git a/java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/ImportExportFiles.scala b/java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/ImportExportFiles.scala
index 2b04f97..0700c25 100644
--- a/java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/ImportExportFiles.scala
+++ b/java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/ImportExportFiles.scala
@@ -32,7 +32,7 @@ import org.apache.yetus.audience.InterfaceStability
 @InterfaceAudience.Public
 @InterfaceStability.Unstable //TODO: Unstable due to KUDU-2454
 object ImportExportKudu {
-  val LOG: Logger = LoggerFactory.getLogger(ImportExportKudu.getClass)
+  val log: Logger = LoggerFactory.getLogger(ImportExportKudu.getClass)
 
   def fail(msg: String): Nothing = {
     System.err.println(msg)
@@ -68,7 +68,7 @@ object ImportExportKudu {
 
   object ArgsCls {
     private def parseInner(options: ArgsCls, args: List[String]): ArgsCls = {
-      LOG.info(args.mkString(","))
+      log.info(args.mkString(","))
       args match {
         case Nil => options
         case "--help" :: _ =>
diff --git a/java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/IntegrationTestBigLinkedList.scala b/java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/IntegrationTestBigLinkedList.scala
index 056bb19..1282e0b 100644
--- a/java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/IntegrationTestBigLinkedList.scala
+++ b/java/kudu-spark-tools/src/main/scala/org/apache/kudu/spark/tools/IntegrationTestBigLinkedList.scala
@@ -18,6 +18,7 @@
 package org.apache.kudu.spark.tools
 
 import java.net.InetAddress
+import java.util.Locale
 
 import org.apache.kudu.client.SessionConfiguration.FlushMode
 import org.apache.kudu.client.KuduClient
@@ -52,7 +53,7 @@ import scala.util.Try
 @InterfaceStability.Unstable
 object IntegrationTestBigLinkedList {
 
-  val LOG: Logger =
+  val log: Logger =
     LoggerFactory.getLogger(IntegrationTestBigLinkedList.getClass)
 
   def usage: String =
@@ -100,7 +101,7 @@ object IntegrationTestBigLinkedList {
   def main(args: Array[String]): Unit = {
     if (args.isEmpty) { fail(usage) }
 
-    args(0).toLowerCase() match {
+    args(0).toLowerCase(Locale.ENGLISH) match {
       case "generate" => Generator.main(args.slice(1, args.length))
       case "verify" => Verifier.main(args.slice(1, args.length))
       case "loop" => Looper.main(args.slice(1, args.length))
@@ -110,7 +111,7 @@ object IntegrationTestBigLinkedList {
 }
 
 object Generator {
-  import IntegrationTestBigLinkedList.LOG
+  import IntegrationTestBigLinkedList.log
   import IntegrationTestBigLinkedList.defaultMasterAddrs
   import IntegrationTestBigLinkedList.fail
   import IntegrationTestBigLinkedList.nanosToHuman
@@ -223,7 +224,7 @@ object Generator {
       for (_ <- 0 until args.lists) {
         val start = System.nanoTime()
         insertList(clientId, args, table, session, rand)
-        LOG.info(
+        log.info(
           s"$clientId inserted ${args.nodes} node linked list in {}",
           nanosToHuman(System.nanoTime() - start))
       }
@@ -452,7 +453,7 @@ object Verifier {
 }
 
 object Looper {
-  import IntegrationTestBigLinkedList.LOG
+  import IntegrationTestBigLinkedList.log
   import IntegrationTestBigLinkedList.fail
 
   def main(args: Array[String]): Unit = {
@@ -471,9 +472,9 @@ object Looper {
       val expected = verifyArgs.nodes.map(_ + nodesPerLoop)
       Verifier.verify(expected, count).map(fail)
       verifyArgs = verifyArgs.copy(nodes = Some(expected.getOrElse(nodesPerLoop)))
-      LOG.info("*************************************************")
-      LOG.info(s"Completed $n loops. Nodes verified: ${count.referenced}")
-      LOG.info("*************************************************")
+      log.info("*************************************************")
+      log.info(s"Completed $n loops. Nodes verified: ${count.referenced}")
+      log.info("*************************************************")
     }
   }
 }
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 b7f5634..7852906 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
@@ -18,6 +18,7 @@
 package org.apache.kudu.spark.kudu
 
 import java.net.InetAddress
+import java.util.Locale
 
 import scala.collection.JavaConverters._
 import scala.util.Try
@@ -210,7 +211,7 @@ class DefaultSource
   }
 
   private def getScanLocalityType(opParam: String): ReplicaSelection = {
-    opParam.toLowerCase match {
+    opParam.toLowerCase(Locale.ENGLISH) match {
       case "leader_only" => ReplicaSelection.LEADER_ONLY
       case "closest_replica" => ReplicaSelection.CLOSEST_REPLICA
       case _ =>
@@ -223,7 +224,7 @@ class DefaultSource
   }
 
   private def stringToOperationType(opParam: String): OperationType = {
-    opParam.toLowerCase match {
+    opParam.toLowerCase(Locale.ENGLISH) match {
       case "insert" => Insert
       case "insert-ignore" => Insert
       case "upsert" => Upsert
diff --git a/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala b/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
index 5a23862..6cf0ace 100644
--- a/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
+++ b/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala
@@ -470,7 +470,7 @@ class KuduContext(val kuduMaster: String, sc: SparkContext, val socketReadTimeou
 }
 
 private object KuduContext {
-  val Log: Logger = LoggerFactory.getLogger(classOf[KuduContext])
+  val log: Logger = LoggerFactory.getLogger(classOf[KuduContext])
 
   /**
    * Returns a new Kerberos-authenticated [[Subject]] if the Spark context contains
@@ -496,7 +496,7 @@ private object KuduContext {
     val keytab =
       sc.getConf.getOption("spark.yarn.keytab").getOrElse(return subject)
 
-    Log.info(s"Logging in as principal $principal with keytab $keytab")
+    log.info(s"Logging in as principal $principal with keytab $keytab")
 
     val conf = new Configuration {
       override def getAppConfigurationEntry(name: String): Array[AppConfigurationEntry] = {
@@ -524,7 +524,7 @@ private object KuduContext {
 }
 
 private object KuduClientCache {
-  val Log: Logger = LoggerFactory.getLogger(KuduClientCache.getClass)
+  val log: Logger = LoggerFactory.getLogger(KuduClientCache.getClass)
 
   private case class CacheValue(kuduClient: AsyncKuduClient, shutdownHookHandle: Runnable)
 
@@ -546,7 +546,7 @@ private object KuduClientCache {
         try {
           cacheValue.kuduClient.close()
         } catch {
-          case e: Exception => Log.warn("Error while shutting down the test client", e);
+          case e: Exception => log.warn("Error while shutting down the test client", e);
         }
 
         // A client may only be closed once, so once we've close this client,
diff --git a/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduRDDTest.scala b/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduRDDTest.scala
index 88404eb..08b1a04 100644
--- a/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduRDDTest.scala
+++ b/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduRDDTest.scala
@@ -48,7 +48,7 @@ class KuduRDDTest extends KuduTestSuite {
     ))
   def testKeepAlive() {
     val rowCount = 500
-    val shortScannerTtlMs = 5000
+    val shortScannerTtlMs = 5000L
 
     // Create a simple table with a single partition.
     val tableName = "testKeepAlive"
@@ -82,7 +82,7 @@ class KuduRDDTest extends KuduTestSuite {
         // a new scan request. It also ensures a single row doesn't go
         // longer than the ttl.
         if (i < 5) {
-          Thread.sleep(shortScannerTtlMs / 2) // Sleep for half the ttl.
+          Thread.sleep(shortScannerTtlMs / 2L) // Sleep for half the ttl.
           i = i + 1
         }
       }
@@ -95,7 +95,7 @@ class KuduRDDTest extends KuduTestSuite {
       List("key"),
       KuduReadOptions(
         batchSize = 100, // Set a small batch size so the first scan doesn't read all the rows.
-        keepAlivePeriodMs = shortScannerTtlMs / 4)
+        keepAlivePeriodMs = shortScannerTtlMs / 4L)
     )
     processRDD(goodRdd)
 
@@ -106,7 +106,7 @@ class KuduRDDTest extends KuduTestSuite {
       List("key"),
       KuduReadOptions(
         batchSize = 100, // Set a small batch size so the first scan doesn't read all the rows.
-        keepAlivePeriodMs = shortScannerTtlMs * 2)
+        keepAlivePeriodMs = shortScannerTtlMs * 2L)
     )
     try {
       processRDD(badRdd)
diff --git a/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/FakeDNS.java b/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/FakeDNS.java
index 0c37f7d..663c3e7 100644
--- a/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/FakeDNS.java
+++ b/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/FakeDNS.java
@@ -115,7 +115,7 @@ public class FakeDNS {
         // Java 8 or earlier takes a list of NameServices
         field.set(InetAddress.class, Arrays.asList(proxy));
       }
-    } catch (Exception e) {
+    } catch (ReflectiveOperationException e) {
       throw new RuntimeException(e);
     }
     installed = true;
diff --git a/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java b/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java
index a1e1d8a..bd19625 100644
--- a/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java
+++ b/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java
@@ -70,7 +70,7 @@ import org.apache.kudu.util.SecurityUtil;
  */
 @InterfaceAudience.Private
 @InterfaceStability.Unstable
-public class MiniKuduCluster implements AutoCloseable {
+public final class MiniKuduCluster implements AutoCloseable {
 
   private static final Logger LOG = LoggerFactory.getLogger(MiniKuduCluster.class);
 
diff --git a/java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/RetryRule.java b/java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/RetryRule.java
index 17a9413..8698b21 100644
--- a/java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/RetryRule.java
+++ b/java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/RetryRule.java
@@ -69,7 +69,7 @@ public class RetryRule implements TestRule {
         for (String l = br.readLine(); l != null; l = br.readLine()) {
           FLAKY_TESTS.add(l);
         }
-      } catch (Exception e) {
+      } catch (IOException e) {
         throw new RuntimeException(e);
       }
     }
diff --git a/java/kudu-test-utils/src/test/java/org/apache/kudu/test/cluster/TestKuduBinaryJarExtractor.java b/java/kudu-test-utils/src/test/java/org/apache/kudu/test/cluster/TestKuduBinaryJarExtractor.java
index 598c3fd..4604cb8 100644
--- a/java/kudu-test-utils/src/test/java/org/apache/kudu/test/cluster/TestKuduBinaryJarExtractor.java
+++ b/java/kudu-test-utils/src/test/java/org/apache/kudu/test/cluster/TestKuduBinaryJarExtractor.java
@@ -65,44 +65,42 @@ public class TestKuduBinaryJarExtractor {
 
     final Map<String, String> env = new HashMap<>();
     env.put("create", "true");
-    final FileSystem zipFs = FileSystems.newFileSystem(uri, env);
-
-    final Path root = zipFs.getPath("/");
-    final Path src =
-        Paths.get(TestKuduBinaryJarExtractor.class.getResource("/fake-kudu-binary").toURI());
-
-    Files.walkFileTree(src, new SimpleFileVisitor<Path>() {
-      @Override
-      public FileVisitResult visitFile(Path file,
-                                       BasicFileAttributes attrs) throws IOException {
-        final Path dest = zipFs.getPath(root.toString(),
-            src.relativize(file).toString());
-        Files.copy(file, dest, StandardCopyOption.REPLACE_EXISTING);
-        return FileVisitResult.CONTINUE;
-      }
+    try (FileSystem zipFs = FileSystems.newFileSystem(uri, env)) {
+      final Path root = zipFs.getPath("/");
+      final Path src =
+          Paths.get(TestKuduBinaryJarExtractor.class.getResource("/fake-kudu-binary").toURI());
+
+      Files.walkFileTree(src, new SimpleFileVisitor<Path>() {
+        @Override
+        public FileVisitResult visitFile(Path file,
+                                         BasicFileAttributes attrs) throws IOException {
+          final Path dest = zipFs.getPath(root.toString(),
+              src.relativize(file).toString());
+          Files.copy(file, dest, StandardCopyOption.REPLACE_EXISTING);
+          return FileVisitResult.CONTINUE;
+        }
 
-      @Override
-      public FileVisitResult preVisitDirectory(Path dir,
-                                               BasicFileAttributes attrs) throws IOException {
-        final Path dirToCreate = zipFs.getPath(root.toString(),
-            src.relativize(dir).toString());
-        if (Files.notExists(dirToCreate)) {
-          LOG.debug("Creating directory {}", dirToCreate);
-          Files.createDirectories(dirToCreate);
+        @Override
+        public FileVisitResult preVisitDirectory(Path dir,
+                                                 BasicFileAttributes attrs) throws IOException {
+          final Path dirToCreate = zipFs.getPath(root.toString(),
+              src.relativize(dir).toString());
+          if (Files.notExists(dirToCreate)) {
+            LOG.debug("Creating directory {}", dirToCreate);
+            Files.createDirectories(dirToCreate);
+          }
+          return FileVisitResult.CONTINUE;
         }
-        return FileVisitResult.CONTINUE;
+      });
+
+      Path metaInf = zipFs.getPath(root.toString(), "META-INF");
+      Files.createDirectory(metaInf);
+      // Customize the properties file to enable positive and negative test scenarios.
+      Path propsPath = zipFs.getPath(metaInf.toString(), "apache-kudu-test-binary.properties");
+      try (OutputStream propsOutputStream = Files.newOutputStream(propsPath)) {
+        writeProperties(os, propsOutputStream);
       }
-    });
-
-    Path metaInf = zipFs.getPath(root.toString(), "META-INF");
-    Files.createDirectory(metaInf);
-    // Customize the properties file to enable positive and negative test scenarios.
-    Path propsPath = zipFs.getPath(metaInf.toString(), "apache-kudu-test-binary.properties");
-    OutputStream propsOutputStream = Files.newOutputStream(propsPath);
-    writeProperties(os, propsOutputStream);
-    propsOutputStream.close();
-
-    zipFs.close();
+    }
     return path;
   }