Arun Suresh commented on YARN-4526:

Agreed, it does seem wasteful... Thanks for the patch [~kasha]..

I was the just wondering though.. instead of passing the clock instance in the 
constructor and setting it to a field, why not do away with the {{this.clock}} 
field itself and just use {{SystemClock.getInstance()}} whenever you need it. I 
understand the constructor injection was probably put there for improved 
testability (I havnt seen it used though.. but i might be missing something). 
But wouldn't a {{SystemClock.setInstance(clock)}} with an *\@visibleForTesting* 
tag, which you set per testcase serve the same purpose ?

> Make SystemClock singleton so AppSchedulingInfo could use it
> ------------------------------------------------------------
>                 Key: YARN-4526
>                 URL: https://issues.apache.org/jira/browse/YARN-4526
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: scheduler
>    Affects Versions: 2.8.0
>            Reporter: Karthik Kambatla
>            Assignee: Karthik Kambatla
>         Attachments: yarn-4526-1.patch
> To track the time a request is received, we need to get current system time. 
> For better testability of this, we are likely better off using a Clock 
> instance that uses SystemClock by default. Instead of creating umpteen 
> instances of SystemClock, we should just reuse the same instance. 

This message was sent by Atlassian JIRA

Reply via email to