[jira] [Commented] (YARN-3204) Fix new findbug warnings in hadoop-yarn-server-resourcemanager(resourcemanager.scheduler.fair)

["Chengbing Liu (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/YARN-3204?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14339811#comment-14339811 ] 

Chengbing Liu commented on YARN-3204:
-------------------------------------

{code}
-    this.reservedAppSchedulable = (FSAppAttempt) application;
+     if(application instanceof FSAppAttempt){
+    	this.reservedAppSchedulable = (FSAppAttempt) application;
+    }
{code}
Would it be better if we throw an exception if the condition is not met?

{code}
     Set<String> planQueues = new HashSet<String>();
     for (FSQueue fsQueue : queueMgr.getQueues()) {
       String queueName = fsQueue.getName();
-      if (allocConf.isReservable(queueName)) {
+      boolean isReservable = false;
+      synchronized(this){
+    	  isReservable = allocConf.isReservable(queueName);
+      }
+      if (isReservable) {
         planQueues.add(queueName);
       }
     }
{code}
I think we should synchronize the whole function, since {{allocConf}} may be reloaded during this loop. A dedicated lock is better than {{FairScheduler.this}} to me.

> Fix new findbug warnings in hadoop-yarn-server-resourcemanager(resourcemanager.scheduler.fair)
> ----------------------------------------------------------------------------------------------
>
>                 Key: YARN-3204
>                 URL: https://issues.apache.org/jira/browse/YARN-3204
>             Project: Hadoop YARN
>          Issue Type: Bug
>            Reporter: Brahma Reddy Battula
>            Assignee: Brahma Reddy Battula
>         Attachments: YARN-3204-001.patch, YARN-3204-002.patch
>
>
> Please check following findbug report..
> https://builds.apache.org/job/PreCommit-YARN-Build/6644//artifact/patchprocess/newPatchFindbugsWarningshadoop-yarn-server-resourcemanager.html



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)