> On May 14, 2014, 12:42 a.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java, line 198
> > <https://reviews.apache.org/r/21349/diff/2/?file=580984#file580984line198>
> >
> >     This is problematic in two ways - it synthetically slows the build by 5 
> > seconds, and can never be sped up.  It also will be flaky depending on the 
> > behavior of the machine running tests.
> >     
> >     I'm actually not sure there's a clean way to cover this through 
> > SchedulerIT.  Ideally the test coverage is narrow enough to mock the 
> > ScheduledExecutorService and test without actually waiting for the delay.  
> > I think this is doable within the sla module without too much plumbing (you 
> > just need to be able to inject the ScheduledExecutorService to SlaModule).
> 
> Maxim Khutornenko wrote:
>     This is actually 5 msec not seconds. I could set it even lower, just 
> wanted to control the CPU load. Also, the awaitSlaReady() loops at 50 msec 
> interval and bails out immediately after detecting a specified sla stat.
> 
> Bill Farner wrote:
>     Ah, my mistake.  The flakiness concern remains, though.  Making 
> assumptions about thread scheduling is likely to fail at some point.

Not sure I get your point. The SchedulerIT is already heavily multithreaded in 
the way it detects the running scheduler (including the use of executors). The 
awaidSlaReady() does not kick in until all other checks are done and the 
scheduler is up. By that time, the SchedulerActive event is already fired and 
the SLA thread most likely done a few iterations (given the 5 msec interval). 
I'd say this test is quite safe given it's 10 sec timeout on the callable get.  


- Maxim


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21349/#review42920
-----------------------------------------------------------


On May 13, 2014, 10:09 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21349/
> -----------------------------------------------------------
> 
> (Updated May 13, 2014, 10:09 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-410
>     https://issues.apache.org/jira/browse/AURORA-410
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Starting SLA calculations on SchedulerActive event.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 3dc15aded2743dd39ca307e1ab5589fa2e4507a3 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> bf3d7a36a575bb9d64f4dd851c63fbebda1e61b8 
>   src/main/java/org/apache/aurora/scheduler/sla/MetricCalculator.java 
> 13449ccae12bd906a4d948f4b4a83cf755eeca77 
>   src/main/java/org/apache/aurora/scheduler/sla/SlaModule.java 
> 8a0707ec2dfb695847d93a6e60adbbb488c22195 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> df5a5da120b754f7da5122feed99039c6ce7b613 
> 
> Diff: https://reviews.apache.org/r/21349/diff/
> 
> 
> Testing
> -------
> 
> gradle build
> gradle run
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Reply via email to