Fix all high priority Findbugs issues (linkedin#502)

saniyanajeeb · Jan 3, 2019 · 440fa4e · 440fa4e
1 parent 1a16321
commit 440fa4e
Show file tree

Hide file tree

Showing 15 changed files with 65 additions and 46 deletions.
diff --git a/app/com/linkedin/drelephant/analysis/AnalyticJobGeneratorHadoop2.java b/app/com/linkedin/drelephant/analysis/AnalyticJobGeneratorHadoop2.java
@@ -44,7 +44,7 @@ public class AnalyticJobGeneratorHadoop2 implements AnalyticJobGenerator {
   private static final String RM_NODE_STATE_URL = "http://%s/ws/v1/cluster/info";
   private static final String FETCH_INITIAL_WINDOW_MS = "drelephant.analysis.fetch.initial.windowMillis";
 
-  private static Configuration configuration;
+  private Configuration configuration;
 
   // We provide one minute job fetch delay due to the job sending lag from AM/NM to JobHistoryServer HDFS
   private static final long FETCH_DELAY = 60000;

diff --git a/app/com/linkedin/drelephant/clients/azkaban/AzkabanJobStatusUtil.java b/app/com/linkedin/drelephant/clients/azkaban/AzkabanJobStatusUtil.java
@@ -39,9 +39,9 @@ public class AzkabanJobStatusUtil {
   private static final Logger logger = Logger.getLogger(AzkabanJobStatusUtil.class);
   private HashMap<String, AzkabanWorkflowClient> workflowClients = new HashMap<String, AzkabanWorkflowClient>();
   private String scheduler = "azkaban";
-  private static String USERNAME = "username";
-  private static String PRIVATE_KEY = "private_key";
-  private static String PASSWORD = "password";
+  private static final String USERNAME = "username";
+  private static final String PRIVATE_KEY = "private_key";
+  private static final String PASSWORD = "password";
   private static final long TOKEN_UPDATE_INTERVAL = AutoTuner.ONE_MIN * 60 * 1;
 
   public AzkabanWorkflowClient getWorkflowClient(String url) throws MalformedURLException {
@@ -67,7 +67,7 @@ public WorkflowClient doLogin(AzkabanWorkflowClient workflowClient) {
       SchedulerConfigurationData schedulerData = InfoExtractor.getSchedulerData(scheduler);
 
       if (schedulerData == null) {
-        throw new RuntimeException(String.format("Cannot find scheduler %s for url %s", scheduler));
+        throw new RuntimeException(String.format("Cannot find scheduler %s", scheduler));
       }
 
       if (!schedulerData.getParamMap().containsKey(USERNAME)) {

diff --git a/app/com/linkedin/drelephant/clients/azkaban/AzkabanWorkflowClient.java b/app/com/linkedin/drelephant/clients/azkaban/AzkabanWorkflowClient.java
@@ -32,6 +32,7 @@
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URL;
+import java.nio.charset.Charset;
 import java.security.InvalidKeyException;
 import java.security.KeyFactory;
 import java.security.KeyManagementException;
@@ -285,7 +286,7 @@ private String parseContent(InputStream response)
     BufferedReader reader = null;
     StringBuilder result = new StringBuilder();
     try {
-      reader = new BufferedReader(new InputStreamReader(response));
+      reader = new BufferedReader(new InputStreamReader(response, Charset.forName("UTF-8")));
 
       String line = null;
       while ((line = reader.readLine()) != null) {

diff --git a/app/com/linkedin/drelephant/exceptions/MRClient.java b/app/com/linkedin/drelephant/exceptions/MRClient.java
@@ -75,7 +75,7 @@ public String getMRJobLog(String mrJobId) {
     String mrJobHistoryURL = "http://" + jhistoryAddr + "/ws/v1/history/mapreduce/jobs/" + mrJobId;
     try {
       JsonNode response = fetchJson(new URL(mrJobHistoryURL));
-      if (response.get("job").get("state").toString() != "SUCCEEDED") {
+      if (!response.get("job").get("state").toString().equals("SUCCEEDED")) {
         return response.get("job").get("diagnostics").getTextValue();
       }
     } catch (MalformedURLException e) {

diff --git a/app/com/linkedin/drelephant/mapreduce/heuristics/GenericMemoryHeuristic.java b/app/com/linkedin/drelephant/mapreduce/heuristics/GenericMemoryHeuristic.java
@@ -58,9 +58,8 @@ private long getContainerMemDefaultMBytes() {
     if (paramMap.containsKey(CONTAINER_MEM_DEFAULT_MB)) {
       String strValue = paramMap.get(CONTAINER_MEM_DEFAULT_MB);
       try {
-        return Long.valueOf(strValue);
-      }
-      catch (NumberFormatException e) {
+        return Long.parseLong(strValue);
+      } catch (NumberFormatException e) {
         logger.warn(CONTAINER_MEM_DEFAULT_MB + ": expected number [" + strValue + "]");
       }
     }

diff --git a/app/com/linkedin/drelephant/math/Statistics.java b/app/com/linkedin/drelephant/math/Statistics.java
@@ -34,8 +34,8 @@ public final class Statistics {
   public static final long MINUTE_IN_MS = 60L * SECOND_IN_MS;
   public static final long HOUR_IN_MS = 60L * MINUTE_IN_MS;
 
-  public static long MINUTE = 60L;
-  public static long HOUR = 60*MINUTE;
+  public static final long MINUTE = 60L;
+  public static final long HOUR = 60L * MINUTE;
 
   private Statistics() {
   }

diff --git a/app/com/linkedin/drelephant/tez/heuristics/GenericMemoryHeuristic.java b/app/com/linkedin/drelephant/tez/heuristics/GenericMemoryHeuristic.java
@@ -98,24 +98,19 @@ public HeuristicResult apply(TezApplicationData data) {
     if(!data.getSucceeded()) {
       return null;
     }
-
-
-    TezTaskData[] tasks = getTasks(data);
 
+    TezTaskData[] tasks = getTasks(data);
 
     List<Long> totalPhysicalMemory = new LinkedList<Long>();
     List<Long> totalVirtualMemory = new LinkedList<Long>();
     List<Long> runTime = new LinkedList<Long>();
-  
+
     for (TezTaskData task : tasks) {
-
       if (task.isSampled()) {
         totalPhysicalMemory.add(task.getCounters().get(TezCounterData.CounterName.PHYSICAL_MEMORY_BYTES));
         totalVirtualMemory.add(task.getCounters().get(TezCounterData.CounterName.VIRTUAL_MEMORY_BYTES));
         runTime.add(task.getTotalRunTimeMs());
       }
-
-
     }
 
     long averagePMem = Statistics.average(totalPhysicalMemory);
@@ -125,7 +120,7 @@ public HeuristicResult apply(TezApplicationData data) {
     try{
       maxPMem = Collections.max(totalPhysicalMemory);
       minPMem = Collections.min(totalPhysicalMemory);
-      
+
     }
     catch(Exception exception){
       maxPMem = 0;
@@ -136,20 +131,22 @@ public HeuristicResult apply(TezApplicationData data) {
     String containerSizeStr;
 
 
-    if(!Strings.isNullOrEmpty(data.getConf().getProperty(TEZ_MAPPER_MEMORY_CONF)) && Long.valueOf(data.getConf().getProperty(TEZ_MAPPER_MEMORY_CONF)) > 0){
+    if(!Strings.isNullOrEmpty(data.getConf().getProperty(TEZ_MAPPER_MEMORY_CONF))
+        && Long.parseLong(data.getConf().getProperty(TEZ_MAPPER_MEMORY_CONF)) > 0) {
       containerSizeStr = data.getConf().getProperty(TEZ_MAPPER_MEMORY_CONF);
-    }
-    else if(!Strings.isNullOrEmpty(data.getConf().getProperty(HIVE_MAPPER_MEMORY_CONF)) && Long.valueOf(data.getConf().getProperty(HIVE_MAPPER_MEMORY_CONF)) > 0){
+    } else if(!Strings.isNullOrEmpty(data.getConf().getProperty(HIVE_MAPPER_MEMORY_CONF))
+        && Long.parseLong(data.getConf().getProperty(HIVE_MAPPER_MEMORY_CONF)) > 0) {
       containerSizeStr = data.getConf().getProperty(HIVE_MAPPER_MEMORY_CONF);
     }
-    else if(!Strings.isNullOrEmpty(data.getConf().getProperty(_mapredContainerMemConf)) && Long.valueOf(data.getConf().getProperty(_mapredContainerMemConf)) > 0) {
+    else if(!Strings.isNullOrEmpty(data.getConf().getProperty(_mapredContainerMemConf))
+        && Long.parseLong(data.getConf().getProperty(_mapredContainerMemConf)) > 0) {
       containerSizeStr = data.getConf().getProperty(_mapredContainerMemConf);
     }
     else {
       containerSizeStr = getContainerMemDefaultMBytes();
     }
-   
-    long containerSize = Long.valueOf(containerSizeStr) * FileUtils.ONE_MB;
+
+    long containerSize = Long.parseLong(containerSizeStr) * FileUtils.ONE_MB;
 
     double averageMemMb = (double)((averagePMem) /FileUtils.ONE_MB) ;
 
@@ -181,7 +178,7 @@ else if(!Strings.isNullOrEmpty(data.getConf().getProperty(_mapredContainerMemCon
     result.addResultDetail("Requested Container Memory (MB)",
             (tasks.length == 0 || containerSize == 0 || containerSize == -1) ? "0" : String.valueOf(containerSize / FileUtils.ONE_MB));
 
-   
+
     return result;
 
   }

diff --git a/app/com/linkedin/drelephant/tuning/AutoTuningAPIHelper.java b/app/com/linkedin/drelephant/tuning/AutoTuningAPIHelper.java
@@ -447,7 +447,6 @@ private void insertParamSet(JobDefinition job, TuningAlgorithm tuningAlgorithm,
    */
   @SuppressWarnings("unchecked")
   private void insertParameterValues(JobSuggestedParamSet jobSuggestedParamSet, Map<String, Double> paramValueMap) {
-    ObjectMapper mapper = new ObjectMapper();
     if (paramValueMap != null) {
       for (Map.Entry<String, Double> paramValue : paramValueMap.entrySet()) {
         insertParameterValue(jobSuggestedParamSet, paramValue.getKey(), paramValue.getValue());

diff --git a/app/com/linkedin/drelephant/tuning/PSOParamGenerator.java b/app/com/linkedin/drelephant/tuning/PSOParamGenerator.java
@@ -21,6 +21,7 @@
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStreamReader;
+import java.nio.charset.Charset;
 import java.util.ArrayList;
 import java.util.List;
 import org.apache.hadoop.conf.Configuration;
@@ -85,8 +86,10 @@ public JobTuningInfo generateParamSet(JobTuningInfo jobTuningInfo) {
       Process p = Runtime.getRuntime()
           .exec(
               PYTHON_PATH + " " + TUNING_SCRIPT_PATH + " " + stringTunerState + " " + parametersToTune + " " + jobType);
-      BufferedReader inputStream = new BufferedReader(new InputStreamReader(p.getInputStream()));
-      BufferedReader errorStream = new BufferedReader(new InputStreamReader(p.getErrorStream()));
+      BufferedReader inputStream = new BufferedReader(
+          new InputStreamReader(p.getInputStream(), Charset.forName("UTF-8")));
+      BufferedReader errorStream = new BufferedReader(
+          new InputStreamReader(p.getErrorStream(), Charset.forName("UTF-8")));
       String updatedStringTunerState = inputStream.readLine();
       newJobTuningInfo.setTunerState(updatedStringTunerState);
       String errorLine;

diff --git a/app/com/linkedin/drelephant/tuning/ParamGenerator.java b/app/com/linkedin/drelephant/tuning/ParamGenerator.java
@@ -21,6 +21,7 @@
 import com.fasterxml.jackson.databind.node.JsonNodeFactory;
 import com.fasterxml.jackson.databind.node.ObjectNode;
 import controllers.AutoTuningMetricsController;
+import java.nio.charset.Charset;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -198,7 +199,7 @@ private List<JobTuningInfo> getJobsTuningInfo(List<TuningJobDefinition> tuningJo
 
       boolean validSavedState = true;
       if (jobSavedState != null && jobSavedState.isValid()) {
-        String savedState = new String(jobSavedState.savedState);
+        String savedState = new String(jobSavedState.savedState, Charset.forName("UTF-8"));
         ObjectNode jsonSavedState = (ObjectNode) Json.parse(savedState);
         JsonNode jsonCurrentPopulation = jsonSavedState.get(JSON_CURRENT_POPULATION_KEY);
         List<Particle> currentPopulation = jsonToParticleList(jsonCurrentPopulation);
@@ -481,7 +482,7 @@ private void saveTunerState(List<JobTuningInfo> jobTuningInfoList) {
         jobSavedState = new JobSavedState();
         jobSavedState.jobDefinitionId = jobTuningInfo.getTuningJob().id;
       }
-      jobSavedState.savedState = jobTuningInfo.getTunerState().getBytes();
+      jobSavedState.savedState = jobTuningInfo.getTunerState().getBytes(Charset.forName("UTF-8"));
       jobSavedState.save();
     }
   }

diff --git a/app/com/linkedin/drelephant/util/ThreadContextMR2.java b/app/com/linkedin/drelephant/util/ThreadContextMR2.java
@@ -18,6 +18,7 @@
 public final class ThreadContextMR2 {
     private static final Logger logger = Logger.getLogger(com.linkedin.drelephant.util.ThreadContextMR2.class);
 
+    private static final Random RANDOM_GENERATOR = new Random();
     private static final AtomicInteger THREAD_ID = new AtomicInteger(1);
 
     private static final ThreadLocal<Integer> _LOCAL_THREAD_ID = new ThreadLocal<Integer>() {
@@ -45,7 +46,7 @@ public Pattern initialValue() {
                 public AuthenticatedURL.Token initialValue() {
                     _LOCAL_LAST_UPDATED.set(System.currentTimeMillis());
                     // Random an interval for each executor to avoid update token at the same time
-                    _LOCAL_UPDATE_INTERVAL.set(Statistics.MINUTE_IN_MS * 30 + new Random().nextLong()
+                    _LOCAL_UPDATE_INTERVAL.set(Statistics.MINUTE_IN_MS * 30 + RANDOM_GENERATOR.nextLong()
                             % (3 * Statistics.MINUTE_IN_MS));
                     logger.info("Executor " + _LOCAL_THREAD_ID.get() + " update interval " + _LOCAL_UPDATE_INTERVAL.get() * 1.0
                             / Statistics.MINUTE_IN_MS);

diff --git a/app/controllers/api/v1/JsonKeys.java b/app/controllers/api/v1/JsonKeys.java
@@ -55,11 +55,11 @@ public class JsonKeys {
   public static final String EXCEPTION_STATUS = "exception-status";
   public static final String SCHEDULERS = "schedulers";
   public static final String EXCEPTION_ENABLED = "exceptionenabled";
-  public static String EXCEPTION_SUMMARY = "exceptionSummary";
-  public static String STATUS = "status";
-  public static String TYPE = "type";
-  public static String TASKS = "tasks";
-  public static String WORKFLOW_EXCEPTIONS = "workflow-exceptions";
+  public static final String EXCEPTION_SUMMARY = "exceptionSummary";
+  public static final String STATUS = "status";
+  public static final String TYPE = "type";
+  public static final String TASKS = "tasks";
+  public static final String WORKFLOW_EXCEPTIONS = "workflow-exceptions";
 
 
   // Workflows

diff --git a/app/controllers/api/v1/Web.java b/app/controllers/api/v1/Web.java
@@ -1397,11 +1397,11 @@ public static Result search() {
     int total = 0;
 
     if (form.get("offset") != null && form.get("offset") != "") {
-      offset = Integer.valueOf(form.get("offset"));
+      offset = Integer.parseInt(form.get("offset"));
     }
 
     if (form.get("limit") != null && form.get("limit") != "") {
-      limit = Integer.valueOf(form.get("limit"));
+      limit = Integer.parseInt(form.get("limit"));
     }
 
     if (offset < 0) {
@@ -1550,11 +1550,11 @@ public static Result restGetUsersSummaryStats() {
     int total = 0;
 
     if (form.get("offset") != null && form.get("offset") != "") {
-      offset = Integer.valueOf(form.get("offset"));
+      offset = Integer.parseInt(form.get("offset"));
     }
 
     if (form.get("limit") != null && form.get("limit") != "") {
-      limit = Integer.valueOf(form.get("limit"));
+      limit = Integer.parseInt(form.get("limit"));
     }
 
     if (offset < 0) {
@@ -1770,10 +1770,10 @@ public static Result restExceptions() throws URISyntaxException, MalformedURLExc
     HadoopSecurity _hadoopSeverity = HadoopSecurity.getInstance();
 
 
-    logger.info(String.format("scheduler + ", scheduler));
-    if(scheduler==null) {
+    logger.info("Scheduler is " + scheduler);
+    if (scheduler == null) {
       scheduler = "azkaban";
-      logger.info(String.format("Setting scheduler ", scheduler));
+      logger.info(String.format("Setting default scheduler as %s", scheduler));
     }
     if(!InfoExtractor.getSchedulersConfiguredForException().contains(scheduler)) {
       logger.info("scheduler not found ");

diff --git a/project/findbugs-exclude-filters.xml b/project/findbugs-exclude-filters.xml
@@ -32,4 +32,22 @@
   <Match>
       <Class name="~.*controllers\.routes(.[a-z\$]+)?"/>
   </Match>
+
+  <!-- Other exclusions -->
+  <!-- play.db.ebean.Model.Finder is not modified hence ignore the warning -->
+  <Match>
+      <Class name="~models.*" />
+      <Field name="find" />
+      <Bug pattern="MS_SHOULD_BE_FINAL" />
+  </Match>
+  <!--
+      Random is meant to be used only once in calculating TOKEN_UPDATE_INTERVAL hence this
+      bug can be ignored. As TOKEN_UPDATE_INTERVAL is a static field and will be initialized
+      in static initializer code, filtering it out by using <clinit>
+  -->
+  <Match>
+      <Class name="com.linkedin.drelephant.analysis.AnalyticJobGeneratorHadoop2" />
+      <Method name='&lt;clinit&gt;' />
+      <Bug pattern="DMI_RANDOM_USED_ONLY_ONCE" />
+  </Match>
 </FindBugsFilter>
diff --git a/travis.sh b/travis.sh
@@ -65,7 +65,7 @@ function runFindbugs() {
   totalBugs=`echo $totalBugs | awk -F'="' '{print $2}'`
   if [ $totalBugs -gt 0 ];then
     echo -e "$ERROR_COLOR_PREFIX Build failed due to "$totalBugs" Findbugs issues..."
-    #exit 1;
+    exit 1;
   fi
 }