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

Karthik Kambatla commented on YARN-2122:
----------------------------------------

{code}
  public void serviceInit(Configuration conf) {
    this.allocFile = getAllocationFile(conf);
    super.init(conf);
    reloadThread = new Thread() {
      public void run() {
{code}
- We should call super.serviceInit instead of super.init, and that call should 
be the last statement of the method.
- Creation of reloadThread should be guarded by if (allocFile != null)

{code}
  public void serviceStart() {
    if (allocFile == null) {
      return;
    }
    reloadThread.start();
    super.start();
  }
{code}
- It is good practice to call super.serviceStart() at the end of this method 
(not super.start()). So, we should probably not check for (allocFile == null) 
and instead call reloadThread.start() after checking (allocFile != null).

- super.serviceStop instead of super.stop
- For the findbugs warning, I see why we don't have to do any additional 
synchronization. Can we add a findbugs exclusion? 

> In AllocationFileLoaderService, the reloadThread should be created in init() 
> and started in start()
> ---------------------------------------------------------------------------------------------------
>
>                 Key: YARN-2122
>                 URL: https://issues.apache.org/jira/browse/YARN-2122
>             Project: Hadoop YARN
>          Issue Type: Bug
>          Components: scheduler
>    Affects Versions: 2.4.0
>            Reporter: Karthik Kambatla
>            Assignee: Robert Kanter
>         Attachments: YARN-2122.patch, YARN-2122.patch
>
>
> AllcoationFileLoaderService has this reloadThread that is currently created 
> and started in start(). Instead, it should be created in init() and started 
> in start().



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Reply via email to