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

Wilfred Spiegelenburg commented on YARN-7622:
---------------------------------------------

Sorry for the delay in the review,

Using the AtomicLong is overkill in this case. You are only using the get and 
set on them which negates the whole idea about the atomicity. We are not 
updating or incrementing the value and the use of volatile will thus be enough, 
see the documentation of 
[atomic|https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/atomic/package-summary.html]

I also looked a bit closer at the change and tried to see if current failures 
would still fail and that we are just allowing hdfs as the storage. With the 
change that is made I can currently get any configuration value to pass. We 
should not do this. It will then go wrong when the serviceInit is called before 
we start the reload thread:
* Using a HTTP URL which passes the tests. This will then later cause failures 
while trying to load the file since the filesystem is wrong.
* A bogus URL which would not work in the previous code setup but now passes 
and fails in the service init with an exception.

Two things:
# limit the URL that can be set to filesystems we support loading the file from 
(HDFS, S3?)
# negative tests (config we don't allow should not work)

> Allow fair-scheduler configuration on HDFS
> ------------------------------------------
>
>                 Key: YARN-7622
>                 URL: https://issues.apache.org/jira/browse/YARN-7622
>             Project: Hadoop YARN
>          Issue Type: Improvement
>          Components: fairscheduler, resourcemanager
>            Reporter: Greg Phillips
>            Assignee: Greg Phillips
>            Priority: Minor
>         Attachments: YARN-7622.001.patch, YARN-7622.002.patch, 
> YARN-7622.003.patch
>
>
> The FairScheduler requires the allocation file to be hosted on the local 
> filesystem on the RM node(s). Allowing HDFS to store the allocation file will 
> provide improved redundancy, more options for scheduler updates, and RM 
> failover consistency in HA.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org

Reply via email to