Karthik Kambatla commented on YARN-2122:

  public void serviceInit(Configuration conf) {
    this.allocFile = getAllocationFile(conf);
    reloadThread = new Thread() {
      public void run() {
- 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)

  public void serviceStart() {
    if (allocFile == null) {
- 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

Reply via email to