YARN-5967. Fix slider core module findbugs warnings. Contributed by J…

…ian He
apache · Nov 6, 2017 · db96e8a · db96e8a
1 parent c11f4b3
commit db96e8a
Show file tree

Hide file tree

Showing 45 changed files with 222 additions and 1,096 deletions.
diff --git a/.../hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-slider/dev-support/findbugs-exclude.xml b/.../hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-slider/dev-support/findbugs-exclude.xml
@@ -0,0 +1,96 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+   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.
+-->
+<FindBugsFilter>
+    <Match>
+        <Package name="org.apache.slider.api.proto" />
+    </Match>
+    <Match>
+        <class name="~org\.apache\.slider\.*" />
+        <Bug pattern="IS2_INCONSISTENT_SYNC" />
+    </Match>
+    <Match>
+        <Class name="org.apache.slider.core.zk.BlockingZKWatcher" />
+        <Bug pattern="JLM_JSR166_UTILCONCURRENT_MONITORENTER" />
+    </Match>
+    <Match>
+        <Class name="org.apache.slider.server.appmaster.state.ProviderAppState" />
+        <Bug pattern="JLM_JSR166_UTILCONCURRENT_MONITORENTER" />
+    </Match>
+    <Match>
+        <Class name="org.apache.slider.providers.ProviderUtils" />
+        <Bug pattern="SWL_SLEEP_WITH_LOCK_HELD" />
+    </Match>
+    <Match>
+        <Class name="org.apache.slider.server.appmaster.SliderAppMaster" />
+        <Bug pattern="WA_AWAIT_NOT_IN_LOOP" />
+    </Match>
+    <Match>
+        <Class name="org.apache.slider.core.zk.ZKIntegration" />
+        <Bug pattern="SBSC_USE_STRINGBUFFER_CONCATENATION" />
+    </Match>
+    <Match>
+        <Bug pattern="NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE" />
+    </Match>
+    <Match>
+        <Class name="org.apache.slider.server.servicemonitor.ProbeWorker" />
+        <Bug pattern="SF_SWITCH_FALLTHROUGH" />
+    </Match>
+    <Match>
+        <Class name="org.apache.slider.core.persist.JsonSerDeser" />
+        <Bug pattern="UI_INHERITANCE_UNSAFE_GETRESOURCE" />
+    </Match>
+    <Match>
+        <Class name="org.apache.slider.server.appmaster.rpc.SliderAMPolicyProvider" />
+        <Bug pattern="EI_EXPOSE_REP" />
+    </Match>
+    <Match>
+        <Class name="org.apache.slider.server.appmaster.state.OutstandingRequest" />
+        <Bug pattern="EQ_DOESNT_OVERRIDE_EQUALS" />
+    </Match>
+    <Match>
+        <Class name="org.apache.slider.server.appmaster.web.SliderAMWebApp" />
+        <Bug pattern="LG_LOST_LOGGER_DUE_TO_WEAK_REFERENCE" />
+    </Match>
+    <Match>
+        <Class name="org.apache.slider.server.servicemonitor.Probe" />
+        <Bug pattern="URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD" />
+    </Match>
+    <Match>
+        <Class name="org.apache.slider.server.services.workflow.ForkedProcessService" />
+        <Bug pattern="JLM_JSR166_UTILCONCURRENT_MONITORENTER" />
+    </Match>
+    <Match>
+        <Class name="org.apache.slider.server.appmaster.state.RoleInstance"/>
+        <Bug pattern="UWF_UNWRITTEN_PUBLIC_OR_PROTECTED_FIELD"/>
+    </Match>
+    <Match>
+        <Class name="org.apache.slider.client.SliderClient" />
+        <Method name="actionRegistryListConfigsYarn" />
+        <Bug pattern="OS_OPEN_STREAM" />
+    </Match>
+    <Match>
+        <Class name="org.apache.slider.client.SliderClient" />
+        <Method name="actionRegistryListExports" />
+        <Bug pattern="OS_OPEN_STREAM" />
+    </Match>
+    <Match>
+        <Class name="org.apache.slider.common.tools.SliderUtils" />
+        <Method name="getApplicationResourceInputStream" />
+        <Bug pattern="OS_OPEN_STREAM" />
+    </Match>
+</FindBugsFilter>
diff --git a/...-applications/hadoop-yarn-slider/hadoop-yarn-slider-core/dev-support/findbugs-exclude.xml b/...-applications/hadoop-yarn-slider/hadoop-yarn-slider-core/dev-support/findbugs-exclude.xml
diff --git a/...t/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-slider/hadoop-yarn-slider-core/pom.xml b/...t/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-slider/hadoop-yarn-slider-core/pom.xml
@@ -26,6 +26,11 @@
   <packaging>jar</packaging>
   <name>Apache Hadoop YARN Slider Core</name>
 
+  <properties>
+    <!-- Needed for generating FindBugs warnings using parent pom -->
+    <yarn.basedir>${project.parent.basedir}</yarn.basedir>
+  </properties>
+
   <build>
     <!-- resources are filtered for dynamic updates. This gets build info in-->
     <resources>

diff --git a/...lider/hadoop-yarn-slider-core/src/main/java/org/apache/hadoop/security/KerberosDiags.java b/...lider/hadoop-yarn-slider-core/src/main/java/org/apache/hadoop/security/KerberosDiags.java
@@ -35,6 +35,7 @@
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
+import java.io.PrintStream;
 import java.io.PrintWriter;
 import java.lang.reflect.InvocationTargetException;
 import java.net.InetAddress;
@@ -82,7 +83,7 @@ public class KerberosDiags implements Closeable {
 
   private final Configuration conf;
   private final List<String> services;
-  private final PrintWriter out;
+  private final PrintStream out;
   private final File keytab;
   private final String principal;
   private final long minKeyLength;
@@ -97,7 +98,7 @@ public class KerberosDiags implements Closeable {
 
   @SuppressWarnings("IOResourceOpenedButNotSafelyClosed")
   public KerberosDiags(Configuration conf,
-      PrintWriter out,
+      PrintStream out,
       List<String> services,
       File keytab,
       String principal,

diff --git a/...doop-yarn-slider-core/src/main/java/org/apache/slider/api/types/ComponentInformation.java b/...doop-yarn-slider-core/src/main/java/org/apache/slider/api/types/ComponentInformation.java
@@ -31,7 +31,7 @@
  * Serializable version of component data.
  * <p>
  * This is sent in REST calls as a JSON object —but is also marshalled into
- * a protobuf structure. Look at {@link org.apache.slider.api.proto.RestTypeMarshalling}
+ * a protobuf structure. Look at {@link RestTypeMarshalling}
  * for the specifics there.
  * <p>
  * This means that if any fields are added here. they must be added to

diff --git a/...slider/api/proto/RestTypeMarshalling.java → ...slider/api/types/RestTypeMarshalling.java b/...slider/api/proto/RestTypeMarshalling.java → ...slider/api/types/RestTypeMarshalling.java
@@ -16,8 +16,9 @@
  * limitations under the License.
  */
 
-package org.apache.slider.api.proto;
+package org.apache.slider.api.types;
 
+import org.apache.slider.api.proto.Messages;
 import org.apache.slider.api.types.ApplicationLivenessInformation;
 import org.apache.slider.api.types.ComponentInformation;
 import org.apache.slider.api.types.ContainerInformation;
@@ -66,7 +67,7 @@ public class RestTypeMarshalling {
     info.name = wire.getName();
     info.priority = wire.getPriority();
     info.placementPolicy = wire.getPlacementPolicy();
-    
+
     info.actual = wire.getActual();
     info.completed = wire.getCompleted();
     info.desired = wire.getDesired();
@@ -95,7 +96,7 @@ public static Messages.ComponentInformationProto marshall(ComponentInformation i
     builder.setName(info.name);
     builder.setPriority(info.priority);
     builder.setPlacementPolicy(info.placementPolicy);
-    
+
     builder.setActual(info.actual);
     builder.setCompleted(info.completed);
     builder.setDesired(info.desired);
@@ -269,7 +270,7 @@ public static ConfTree unmarshallToConfTree(Messages.WrappedJsonProto wire) thro
       IOException {
     return new ConfTreeSerDeser().fromJson(wire.getJson());
   }
-  
+
   public static ConfTreeOperations unmarshallToCTO(Messages.WrappedJsonProto wire) throws
       IOException {
     return new ConfTreeOperations(new ConfTreeSerDeser().fromJson(wire.getJson()));

diff --git a/...n-slider/hadoop-yarn-slider-core/src/main/java/org/apache/slider/client/SliderClient.java b/...n-slider/hadoop-yarn-slider-core/src/main/java/org/apache/slider/client/SliderClient.java
@@ -192,10 +192,10 @@
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.io.Writer;
-import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.URISyntaxException;
 import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -1322,9 +1322,9 @@ public int actionPackage(ActionPackageArgs actionPackageInfo)
   }
 
   private void initializeOutputStream(String outFile)
-      throws FileNotFoundException {
+      throws IOException {
     if (outFile != null) {
-      clientOutputStream = new PrintStream(new FileOutputStream(outFile));
+      clientOutputStream = new PrintStream(outFile, "UTF-8");
     } else {
       clientOutputStream = System.out;
     }
@@ -3299,7 +3299,7 @@ public int flex(String clustername, Map<String, String> roleInstances)
           int updateCount = Integer.parseInt(updateCountStr);
           // if component was specified before, get the current count
           if (component.get(COMPONENT_INSTANCES) != null) {
-            currentCount = Integer.valueOf(component.get(COMPONENT_INSTANCES));
+            currentCount = Integer.parseInt(component.get(COMPONENT_INSTANCES));
             if (currentCount + updateCount < 0) {
               throw new BadCommandArgumentsException("The requested count " +
                   "of \"%s\" for role %s makes the total number of " +
@@ -3610,16 +3610,16 @@ public int actionResolve(ActionResolveArgs args)
     // as this is an API entry point, validate
     // the arguments
     args.validate();
-    RegistryOperations operations = getRegistryOperations();
     String path = SliderRegistryUtils.resolvePath(args.path);
     ServiceRecordMarshal serviceRecordMarshal = new ServiceRecordMarshal();
     try {
       if (args.list) {
         File destDir = args.destdir;
-        if (destDir != null) {
-          destDir.mkdirs();
+        if (destDir != null && !destDir.exists() && !destDir.mkdirs()) {
+          throw new IOException("Failed to create directory: " + destDir);
         }
 
+
         Map<String, ServiceRecord> recordMap;
         Map<String, RegistryPathStatus> znodes;
         try {
@@ -3656,9 +3656,7 @@ public int actionResolve(ActionResolveArgs args)
           } else {
             String filename = RegistryPathUtils.lastPathEntry(name) + ".json";
             File jsonFile = new File(destDir, filename);
-            write(jsonFile,
-                serviceRecordMarshal.toBytes(instance),
-                true);
+            write(jsonFile, serviceRecordMarshal.toBytes(instance));
           }
         }
       } else  {
@@ -3669,7 +3667,7 @@ public int actionResolve(ActionResolveArgs args)
           outFile = new File(args.destdir, RegistryPathUtils.lastPathEntry(path));
         }
         if (outFile != null) {
-          write(outFile, serviceRecordMarshal.toBytes(instance), true);
+          write(outFile, serviceRecordMarshal.toBytes(instance));
         } else {
           println(serviceRecordMarshal.toJson(instance));
         }
@@ -4062,11 +4060,13 @@ private void actionDiagnosticClient(ActionDiagnosticArgs diagnosticArgs)
   @SuppressWarnings("IOResourceOpenedButNotSafelyClosed")
   private int actionKDiag(ActionKDiagArgs args)
     throws Exception {
-    PrintWriter out = new PrintWriter(System.err);
+    PrintStream out;
     boolean closeStream = false;
     if (args.out != null) {
-      out = new PrintWriter(new FileOutputStream(args.out));
+      out = new PrintStream(args.out, "UTF-8");
       closeStream = true;
+    } else {
+      out = System.err;
     }
     try {
       KerberosDiags kdiags = new KerberosDiags(getConfig(),
@@ -4137,19 +4137,16 @@ public void actionRegistryListConfigsYarn(ActionRegistryArgs registryArgs)
     PrintStream out = null;
     try {
       if (registryArgs.out != null) {
-        out = new PrintStream(new FileOutputStream(registryArgs.out));
+        out = new PrintStream(registryArgs.out, "UTF-8");
       } else {
         out = System.out;
       }
       for (String configName : configurations.keys()) {
         if (!registryArgs.verbose) {
           out.println(configName);
         } else {
-          PublishedConfiguration published =
-              configurations.get(configName);
-          out.printf("%s: %s\n",
-              configName,
-              published.description);
+          PublishedConfiguration published = configurations.get(configName);
+          out.printf("%s: %s%n", configName, published.description);
         }
       }
     } finally {
@@ -4178,7 +4175,7 @@ public void actionRegistryListExports(ActionRegistryArgs registryArgs)
     boolean streaming = false;
     try {
       if (registryArgs.out != null) {
-        out = new PrintStream(new FileOutputStream(registryArgs.out));
+        out = new PrintStream(registryArgs.out, "UTF-8");
         streaming = true;
         log.debug("Saving output to {}", registryArgs.out);
       } else {
@@ -4193,9 +4190,7 @@ public void actionRegistryListExports(ActionRegistryArgs registryArgs)
           out.println(exportName);
         } else {
           PublishedExports published = exports.get(exportName);
-          out.printf("%s: %s\n",
-              exportName,
-              published.description);
+          out.printf("%s: %s%n", exportName, published.description);
         }
       }
     } finally {
@@ -4401,27 +4396,24 @@ public RegistryOperations getRegistryOperations()
    * Output to standard out/stderr (implementation specific detail)
    * @param src source
    */
-  @SuppressWarnings("UseOfSystemOutOrSystemErr")
   private static void print(CharSequence src) {
-    clientOutputStream.append(src);
+    clientOutputStream.print(src);
   }
 
   /**
    * Output to standard out/stderr with a newline after
    * @param message message
    */
   private static void println(String message) {
-    print(message);
-    print("\n");
+    clientOutputStream.println(message);
   }
   /**
    * Output to standard out/stderr with a newline after, formatted
    * @param message message
    * @param args arguments for string formatting
    */
   private static void println(String message, Object ... args) {
-    print(String.format(message, args));
-    print("\n");
+    clientOutputStream.println(String.format(message, args));
   }
 
   /**
@@ -4497,12 +4489,6 @@ private int actionHelp(String actionName) throws YarnException, IOException {
     throw new UsageException(CommonArgs.usage(serviceArgs, actionName));
   }
 
-  private int actionHelp(String errMsg, String actionName)
-      throws YarnException, IOException {
-    throw new UsageException("%s %s", errMsg, CommonArgs.usage(serviceArgs,
-        actionName));
-  }
-
   /**
    * List the nodes in the cluster, possibly filtering by node state or label.
    *

diff --git a/...lider/hadoop-yarn-slider-core/src/main/java/org/apache/slider/client/TokensOperation.java b/...lider/hadoop-yarn-slider-core/src/main/java/org/apache/slider/client/TokensOperation.java
@@ -75,11 +75,10 @@ public int actionTokens(ActionTokensArgs args, FileSystem fs,
       }
       saveTokens(output, credentials);
       String filename = output.getCanonicalPath();
-      footnote = String.format("%d tokens saved to %s\n" +
-              "To use these in the environment:\n" +
-              "export %s=%s",
-          credentials.numberOfTokens(),
-          filename, UserGroupInformation.HADOOP_TOKEN_FILE_LOCATION, filename);
+      footnote = String.format(
+          "%d tokens saved to %s%n" + "To use these in the environment:%n"
+              + "export %s=%s", credentials.numberOfTokens(), filename,
+          UserGroupInformation.HADOOP_TOKEN_FILE_LOCATION, filename);
     } else if (args.source != null) {
       File source = args.source;
       log.info("Reading credentials from file {}", source);