Re: Review Request 30010: [AURORA-184] Remove hardcoded 'host' and 'rack' limit constraints

2015-01-21 Thread Bill Farner


 On Jan. 21, 2015, 6:53 p.m., Zameer Manji wrote:
  I talked with Florian offline and the next step for this patch is to add 
  CLI flags so this legacy behaviour can be enabled. This way we don't break 
  backwards compatability and users who rely on this behaviour don't get a 
  nasty surprise when they update to the next release.

Thanks Zameer!


- Bill


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


On Jan. 20, 2015, 9:46 p.m., Florian Pfeiffer wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/30010/
 ---
 
 (Updated Jan. 20, 2015, 9:46 p.m.)
 
 
 Review request for Aurora, Bill Farner and Zameer Manji.
 
 
 Bugs: AURORA-184
 https://issues.apache.org/jira/browse/AURORA-184
 
 
 Repository: aurora
 
 
 Description
 ---
 
 [AURORA-184] Remove hardcoded 'host' and 'rack' limit constraints
 
 This is the first step for AURORA-184, that removes the default hostrack 
 limit constraints.
 The second step that's still missing would be to add s.th. like 
 --default-constraints as start parameter to the scheduler. 
 
 AURORA-174 could probably be closed with this?(since the rack limit 
 constraint can be configured in the .aurora file)
 
 I can't really estimate the effect of my changes in 
 StorageBackfillTestSchedulerThriftInterfaceTest, please have a closer look 
 at the changes I did there.
 
 Since this is also my first code submit, comments about codestyleother bad 
 habbits are very appreciated.
 
 
 Diffs
 -
 
   
 src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
  01b03508afac37b5a8f0ec5c3da1460695e1ef59 
   
 src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
  dc2cb37adf32df0a6e4c7ee2ba776ba9f1f3c2f8 
   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
 7eafe074b686d55ad96018006cf4acfa823513c3 
   
 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
  ad9126c32893080e128d086ea3bfd7ad23d27b89 
 
 Diff: https://reviews.apache.org/r/30010/diff/
 
 
 Testing
 ---
 
 Added test for ConfigurationManager.hasName 
 Added test testNoHostAndRackConstraintsAdded, that checks if the constraints 
 are present
 Tested on vagrant devcluster to see if constraints are also gone in real 
 life
 
 
 Thanks,
 
 Florian Pfeiffer
 




Re: Review Request 30010: [AURORA-184] Remove hardcoded 'host' and 'rack' limit constraints

2015-01-21 Thread Zameer Manji

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


I talked with Florian offline and the next step for this patch is to add CLI 
flags so this legacy behaviour can be enabled. This way we don't break 
backwards compatability and users who rely on this behaviour don't get a nasty 
surprise when they update to the next release.

- Zameer Manji


On Jan. 20, 2015, 1:46 p.m., Florian Pfeiffer wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/30010/
 ---
 
 (Updated Jan. 20, 2015, 1:46 p.m.)
 
 
 Review request for Aurora, Bill Farner and Zameer Manji.
 
 
 Bugs: AURORA-184
 https://issues.apache.org/jira/browse/AURORA-184
 
 
 Repository: aurora
 
 
 Description
 ---
 
 [AURORA-184] Remove hardcoded 'host' and 'rack' limit constraints
 
 This is the first step for AURORA-184, that removes the default hostrack 
 limit constraints.
 The second step that's still missing would be to add s.th. like 
 --default-constraints as start parameter to the scheduler. 
 
 AURORA-174 could probably be closed with this?(since the rack limit 
 constraint can be configured in the .aurora file)
 
 I can't really estimate the effect of my changes in 
 StorageBackfillTestSchedulerThriftInterfaceTest, please have a closer look 
 at the changes I did there.
 
 Since this is also my first code submit, comments about codestyleother bad 
 habbits are very appreciated.
 
 
 Diffs
 -
 
   
 src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
  01b03508afac37b5a8f0ec5c3da1460695e1ef59 
   
 src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
  dc2cb37adf32df0a6e4c7ee2ba776ba9f1f3c2f8 
   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
 7eafe074b686d55ad96018006cf4acfa823513c3 
   
 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
  ad9126c32893080e128d086ea3bfd7ad23d27b89 
 
 Diff: https://reviews.apache.org/r/30010/diff/
 
 
 Testing
 ---
 
 Added test for ConfigurationManager.hasName 
 Added test testNoHostAndRackConstraintsAdded, that checks if the constraints 
 are present
 Tested on vagrant devcluster to see if constraints are also gone in real 
 life
 
 
 Thanks,
 
 Florian Pfeiffer
 




Re: Review Request 28731: Implemented TaskScheduler benchmarks.

2015-01-21 Thread Bill Farner

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


Kevin - any chance you're available to take a pass at this soon?  My attempts 
to free up a chunk of time for a thoughtful review have so far failed, and this 
is blocking other work of Maxim's.

- Bill Farner


On Jan. 7, 2015, 1:34 a.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/28731/
 ---
 
 (Updated Jan. 7, 2015, 1:34 a.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added baseline benchmarks for a few static veto cases.
 
 
 Diffs
 -
 
   build.gradle f9f71a84493b782e9f6072e44e89a2c017cf2a09 
   src/jmh/java/org/apache/aurora/benchmark/Hosts.java PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/Offers.java PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/SchedulerBenchmark.java 
 5cecada93e4e04b689e826af49f691ed7e94ae49 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/Tasks.java PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeClusterState.java 
 PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java PRE-CREATION 
   
 src/jmh/java/org/apache/aurora/benchmark/fakes/FakeRescheduleCalculator.java 
 PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeStatsProvider.java 
 PRE-CREATION 
   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
 b6402ae42e3c7e4dca1c120fa6ef82d2d69e69d5 
   src/main/java/org/apache/aurora/scheduler/async/preemptor/ClusterState.java 
 f7e157c890b5627411acd4bd5c2559ef4829147c 
   
 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 
 0204d14b19ae412236f19ca274d81decb4eba12d 
 
 Diff: https://reviews.apache.org/r/28731/diff/
 
 
 Testing
 ---
 
 Sample run on a local box:
 ```
 # VM invoker: 
 /Library/Java/JavaVirtualMachines/jdk1.7.0_25.jdk/Contents/Home/jre/bin/java
 # VM options: -Dfile.encoding=UTF-8 -Duser.country=US -Duser.language=en 
 -Duser.variant
 # Warmup: 10 iterations, 1 s each
 # Measurement: 100 iterations, 1 s each
 # Timeout: 10 min per iteration
 # Threads: 1 thread, will synchronize iterations
 # Benchmark mode: Average time, time/op
 # Benchmark: 
 org.apache.aurora.benchmark.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark
 
 # Run progress: 0.00% complete, ETA 00:05:30
 # Fork: 1 of 1
 # Warmup Iteration   1: 278284250.000 ns/op
 # Warmup Iteration   2: 70294312.500 ns/op
 # Warmup Iteration   3: 19293379.310 ns/op
 # Warmup Iteration   4: 11945387.097 ns/op
 # Warmup Iteration   5: 10725388.350 ns/op
 # Warmup Iteration   6: 13043282.353 ns/op
 # Warmup Iteration   7: 9233083.333 ns/op
 # Warmup Iteration   8: 9521051.724 ns/op
 # Warmup Iteration   9: 10750854.369 ns/op
 # Warmup Iteration  10: 7460243.243 ns/op
 Iteration   1: 7885364.286 ns/op
 Iteration   2: 7735139.860 ns/op
 Iteration   3: 7660208.333 ns/op
 Iteration   4: 7761204.225 ns/op
 Iteration   5: 7868907.143 ns/op
 Iteration   6: 7567404.110 ns/op
 Iteration   7: 7611000.000 ns/op
 Iteration   8: 7766154.930 ns/op
 Iteration   9: 7669344.828 ns/op
 Iteration  10: 7707783.217 ns/op
 Iteration  11: 7435651.007 ns/op
 Iteration  12: 7697631.944 ns/op
 Iteration  13: 7712531.469 ns/op
 Iteration  14: 7899407.143 ns/op
 Iteration  15: 7448472.973 ns/op
 Iteration  16: 7791521.127 ns/op
 Iteration  17: 7612213.793 ns/op
 Iteration  18: 7710867.133 ns/op
 Iteration  19: 7649296.552 ns/op
 Iteration  20: 7768309.859 ns/op
 Iteration  21: 7688666.667 ns/op
 Iteration  22: 7531557.823 ns/op
 Iteration  23: 7381193.333 ns/op
 Iteration  24: 7726006.993 ns/op
 Iteration  25: 7603358.621 ns/op
 Iteration  26: 7653631.944 ns/op
 Iteration  27: 7442275.168 ns/op
 Iteration  28: 7613186.207 ns/op
 Iteration  29: 7765823.944 ns/op
 Iteration  30: 7489687.075 ns/op
 Iteration  31: 7811443.662 ns/op
 Iteration  32: 8015007.246 ns/op
 Iteration  33: 8192392.593 ns/op
 Iteration  34: 8040335.766 ns/op
 Iteration  35: 7584212.329 ns/op
 Iteration  36: 8001934.783 ns/op
 Iteration  37: 9744815.789 ns/op
 Iteration  38: 11688284.211 ns/op
 Iteration  39: 8661406.250 ns/op
 Iteration  40: 7678413.793 ns/op
 Iteration  41: 8502223.077 ns/op
 Iteration  42: 7640820.690 ns/op
 Iteration  43: 7875624.113 ns/op
 Iteration  44: 7506809.524 ns/op
 Iteration  45: 8005431.655 ns/op
 Iteration  46: 8081664.234 ns/op
 Iteration  47: 7579438.356 ns/op
 Iteration  48: 7993405.797 ns/op
 Iteration  49: 7571958.904 ns/op
 Iteration  50: 8116463.235 ns/op
 Iteration  51: 7941330.935 ns/op
 Iteration  52: 

Re: Review Request 30010: [AURORA-184] Remove hardcoded 'host' and 'rack' limit constraints

2015-01-21 Thread Zameer Manji

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



src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
https://reviews.apache.org/r/30010/#comment113510

The test below is not needed so this can be private.



src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
https://reviews.apache.org/r/30010/#comment113511

This test is not neccessary.



src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
https://reviews.apache.org/r/30010/#comment113512

This test is not necessary either. We don't need to add tests to ensure we 
are not doing the old behaviour.


- Zameer Manji


On Jan. 20, 2015, 1:46 p.m., Florian Pfeiffer wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/30010/
 ---
 
 (Updated Jan. 20, 2015, 1:46 p.m.)
 
 
 Review request for Aurora, Bill Farner and Zameer Manji.
 
 
 Bugs: AURORA-184
 https://issues.apache.org/jira/browse/AURORA-184
 
 
 Repository: aurora
 
 
 Description
 ---
 
 [AURORA-184] Remove hardcoded 'host' and 'rack' limit constraints
 
 This is the first step for AURORA-184, that removes the default hostrack 
 limit constraints.
 The second step that's still missing would be to add s.th. like 
 --default-constraints as start parameter to the scheduler. 
 
 AURORA-174 could probably be closed with this?(since the rack limit 
 constraint can be configured in the .aurora file)
 
 I can't really estimate the effect of my changes in 
 StorageBackfillTestSchedulerThriftInterfaceTest, please have a closer look 
 at the changes I did there.
 
 Since this is also my first code submit, comments about codestyleother bad 
 habbits are very appreciated.
 
 
 Diffs
 -
 
   
 src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
  01b03508afac37b5a8f0ec5c3da1460695e1ef59 
   
 src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
  dc2cb37adf32df0a6e4c7ee2ba776ba9f1f3c2f8 
   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
 7eafe074b686d55ad96018006cf4acfa823513c3 
   
 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
  ad9126c32893080e128d086ea3bfd7ad23d27b89 
 
 Diff: https://reviews.apache.org/r/30010/diff/
 
 
 Testing
 ---
 
 Added test for ConfigurationManager.hasName 
 Added test testNoHostAndRackConstraintsAdded, that checks if the constraints 
 are present
 Tested on vagrant devcluster to see if constraints are also gone in real 
 life
 
 
 Thanks,
 
 Florian Pfeiffer
 




Re: Review Request 29984: Minor clean up to mock usage in resource manager integration tests.

2015-01-21 Thread Brian Wickman

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

Ship it!


Ship It!

- Brian Wickman


On Jan. 16, 2015, 9:45 p.m., Joshua Cohen wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29984/
 ---
 
 (Updated Jan. 16, 2015, 9:45 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Brian Wickman.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Minor clean up to mock usage in resource manager integration tests.
 
 We didn't need to wrap the mock disk collector in a lambda, just rather than 
 asserting on the mock class, we instead needed to assert on its return value 
 (the instance).
 
 
 Diffs
 -
 
   
 src/test/python/apache/aurora/executor/common/test_resource_manager_integration.py
  bb3045700c2870329e175db44873dcb990226c7a 
 
 Diff: https://reviews.apache.org/r/29984/diff/
 
 
 Testing
 ---
 
 ./pants build 
 src/test/python/apache/aurora/executor/common:resource_manager_integration
 
 
 Thanks,
 
 Joshua Cohen
 




Re: Review Request 28731: Implemented TaskScheduler benchmarks.

2015-01-21 Thread Bill Farner

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


Meta question - how do you envision us using this?  When a perf issue is 
discovered, should we generally push for a test case to be added here to 
validate the perf fix?

- Bill Farner


On Jan. 7, 2015, 1:34 a.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/28731/
 ---
 
 (Updated Jan. 7, 2015, 1:34 a.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added baseline benchmarks for a few static veto cases.
 
 
 Diffs
 -
 
   build.gradle f9f71a84493b782e9f6072e44e89a2c017cf2a09 
   src/jmh/java/org/apache/aurora/benchmark/Hosts.java PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/Offers.java PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/SchedulerBenchmark.java 
 5cecada93e4e04b689e826af49f691ed7e94ae49 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/Tasks.java PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeClusterState.java 
 PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java PRE-CREATION 
   
 src/jmh/java/org/apache/aurora/benchmark/fakes/FakeRescheduleCalculator.java 
 PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeStatsProvider.java 
 PRE-CREATION 
   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
 b6402ae42e3c7e4dca1c120fa6ef82d2d69e69d5 
   src/main/java/org/apache/aurora/scheduler/async/preemptor/ClusterState.java 
 f7e157c890b5627411acd4bd5c2559ef4829147c 
   
 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 
 0204d14b19ae412236f19ca274d81decb4eba12d 
 
 Diff: https://reviews.apache.org/r/28731/diff/
 
 
 Testing
 ---
 
 Sample run on a local box:
 ```
 # VM invoker: 
 /Library/Java/JavaVirtualMachines/jdk1.7.0_25.jdk/Contents/Home/jre/bin/java
 # VM options: -Dfile.encoding=UTF-8 -Duser.country=US -Duser.language=en 
 -Duser.variant
 # Warmup: 10 iterations, 1 s each
 # Measurement: 100 iterations, 1 s each
 # Timeout: 10 min per iteration
 # Threads: 1 thread, will synchronize iterations
 # Benchmark mode: Average time, time/op
 # Benchmark: 
 org.apache.aurora.benchmark.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark
 
 # Run progress: 0.00% complete, ETA 00:05:30
 # Fork: 1 of 1
 # Warmup Iteration   1: 278284250.000 ns/op
 # Warmup Iteration   2: 70294312.500 ns/op
 # Warmup Iteration   3: 19293379.310 ns/op
 # Warmup Iteration   4: 11945387.097 ns/op
 # Warmup Iteration   5: 10725388.350 ns/op
 # Warmup Iteration   6: 13043282.353 ns/op
 # Warmup Iteration   7: 9233083.333 ns/op
 # Warmup Iteration   8: 9521051.724 ns/op
 # Warmup Iteration   9: 10750854.369 ns/op
 # Warmup Iteration  10: 7460243.243 ns/op
 Iteration   1: 7885364.286 ns/op
 Iteration   2: 7735139.860 ns/op
 Iteration   3: 7660208.333 ns/op
 Iteration   4: 7761204.225 ns/op
 Iteration   5: 7868907.143 ns/op
 Iteration   6: 7567404.110 ns/op
 Iteration   7: 7611000.000 ns/op
 Iteration   8: 7766154.930 ns/op
 Iteration   9: 7669344.828 ns/op
 Iteration  10: 7707783.217 ns/op
 Iteration  11: 7435651.007 ns/op
 Iteration  12: 7697631.944 ns/op
 Iteration  13: 7712531.469 ns/op
 Iteration  14: 7899407.143 ns/op
 Iteration  15: 7448472.973 ns/op
 Iteration  16: 7791521.127 ns/op
 Iteration  17: 7612213.793 ns/op
 Iteration  18: 7710867.133 ns/op
 Iteration  19: 7649296.552 ns/op
 Iteration  20: 7768309.859 ns/op
 Iteration  21: 7688666.667 ns/op
 Iteration  22: 7531557.823 ns/op
 Iteration  23: 7381193.333 ns/op
 Iteration  24: 7726006.993 ns/op
 Iteration  25: 7603358.621 ns/op
 Iteration  26: 7653631.944 ns/op
 Iteration  27: 7442275.168 ns/op
 Iteration  28: 7613186.207 ns/op
 Iteration  29: 7765823.944 ns/op
 Iteration  30: 7489687.075 ns/op
 Iteration  31: 7811443.662 ns/op
 Iteration  32: 8015007.246 ns/op
 Iteration  33: 8192392.593 ns/op
 Iteration  34: 8040335.766 ns/op
 Iteration  35: 7584212.329 ns/op
 Iteration  36: 8001934.783 ns/op
 Iteration  37: 9744815.789 ns/op
 Iteration  38: 11688284.211 ns/op
 Iteration  39: 8661406.250 ns/op
 Iteration  40: 7678413.793 ns/op
 Iteration  41: 8502223.077 ns/op
 Iteration  42: 7640820.690 ns/op
 Iteration  43: 7875624.113 ns/op
 Iteration  44: 7506809.524 ns/op
 Iteration  45: 8005431.655 ns/op
 Iteration  46: 8081664.234 ns/op
 Iteration  47: 7579438.356 ns/op
 Iteration  48: 7993405.797 ns/op
 Iteration  49: 7571958.904 ns/op
 Iteration  50: 8116463.235 ns/op
 Iteration  51: 7941330.935 ns/op
 Iteration  52: 7687145.833 ns/op
 Iteration  

Re: Review Request 28731: Implemented TaskScheduler benchmarks.

2015-01-21 Thread Bill Farner

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



src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java
https://reviews.apache.org/r/28731/#comment113621

I think we should proceed one of two ways here: 

1. whittle this down significantly to focus on a minimal set of classes 
related to the critical path when scheduling

2.accept that we're setting up ~the whole app, and do something more like 
what `SchedulerIT` does with modules.

The concern i have is this class will drift from production code and rot, 
or just be an ongoing maintenance burden.

Do you think either of those paths are realistic?


- Bill Farner


On Jan. 7, 2015, 1:34 a.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/28731/
 ---
 
 (Updated Jan. 7, 2015, 1:34 a.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added baseline benchmarks for a few static veto cases.
 
 
 Diffs
 -
 
   build.gradle f9f71a84493b782e9f6072e44e89a2c017cf2a09 
   src/jmh/java/org/apache/aurora/benchmark/Hosts.java PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/Offers.java PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/SchedulerBenchmark.java 
 5cecada93e4e04b689e826af49f691ed7e94ae49 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/Tasks.java PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeClusterState.java 
 PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java PRE-CREATION 
   
 src/jmh/java/org/apache/aurora/benchmark/fakes/FakeRescheduleCalculator.java 
 PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeStatsProvider.java 
 PRE-CREATION 
   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
 b6402ae42e3c7e4dca1c120fa6ef82d2d69e69d5 
   src/main/java/org/apache/aurora/scheduler/async/preemptor/ClusterState.java 
 f7e157c890b5627411acd4bd5c2559ef4829147c 
   
 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 
 0204d14b19ae412236f19ca274d81decb4eba12d 
 
 Diff: https://reviews.apache.org/r/28731/diff/
 
 
 Testing
 ---
 
 Sample run on a local box:
 ```
 # VM invoker: 
 /Library/Java/JavaVirtualMachines/jdk1.7.0_25.jdk/Contents/Home/jre/bin/java
 # VM options: -Dfile.encoding=UTF-8 -Duser.country=US -Duser.language=en 
 -Duser.variant
 # Warmup: 10 iterations, 1 s each
 # Measurement: 100 iterations, 1 s each
 # Timeout: 10 min per iteration
 # Threads: 1 thread, will synchronize iterations
 # Benchmark mode: Average time, time/op
 # Benchmark: 
 org.apache.aurora.benchmark.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark
 
 # Run progress: 0.00% complete, ETA 00:05:30
 # Fork: 1 of 1
 # Warmup Iteration   1: 278284250.000 ns/op
 # Warmup Iteration   2: 70294312.500 ns/op
 # Warmup Iteration   3: 19293379.310 ns/op
 # Warmup Iteration   4: 11945387.097 ns/op
 # Warmup Iteration   5: 10725388.350 ns/op
 # Warmup Iteration   6: 13043282.353 ns/op
 # Warmup Iteration   7: 9233083.333 ns/op
 # Warmup Iteration   8: 9521051.724 ns/op
 # Warmup Iteration   9: 10750854.369 ns/op
 # Warmup Iteration  10: 7460243.243 ns/op
 Iteration   1: 7885364.286 ns/op
 Iteration   2: 7735139.860 ns/op
 Iteration   3: 7660208.333 ns/op
 Iteration   4: 7761204.225 ns/op
 Iteration   5: 7868907.143 ns/op
 Iteration   6: 7567404.110 ns/op
 Iteration   7: 7611000.000 ns/op
 Iteration   8: 7766154.930 ns/op
 Iteration   9: 7669344.828 ns/op
 Iteration  10: 7707783.217 ns/op
 Iteration  11: 7435651.007 ns/op
 Iteration  12: 7697631.944 ns/op
 Iteration  13: 7712531.469 ns/op
 Iteration  14: 7899407.143 ns/op
 Iteration  15: 7448472.973 ns/op
 Iteration  16: 7791521.127 ns/op
 Iteration  17: 7612213.793 ns/op
 Iteration  18: 7710867.133 ns/op
 Iteration  19: 7649296.552 ns/op
 Iteration  20: 7768309.859 ns/op
 Iteration  21: 7688666.667 ns/op
 Iteration  22: 7531557.823 ns/op
 Iteration  23: 7381193.333 ns/op
 Iteration  24: 7726006.993 ns/op
 Iteration  25: 7603358.621 ns/op
 Iteration  26: 7653631.944 ns/op
 Iteration  27: 7442275.168 ns/op
 Iteration  28: 7613186.207 ns/op
 Iteration  29: 7765823.944 ns/op
 Iteration  30: 7489687.075 ns/op
 Iteration  31: 7811443.662 ns/op
 Iteration  32: 8015007.246 ns/op
 Iteration  33: 8192392.593 ns/op
 Iteration  34: 8040335.766 ns/op
 Iteration  35: 7584212.329 ns/op
 Iteration  36: 8001934.783 ns/op
 Iteration  37: 9744815.789 ns/op
 Iteration  38: 11688284.211 ns/op
 Iteration  39: 8661406.250 ns/op
 Iteration  40: 

Re: Review Request 28731: Implemented TaskScheduler benchmarks.

2015-01-21 Thread Maxim Khutornenko


 On Jan. 21, 2015, 11:44 p.m., Bill Farner wrote:
  src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java, line 101
  https://reviews.apache.org/r/28731/diff/5/?file=808351#file808351line101
 
  I think we should proceed one of two ways here: 
  
  1. whittle this down significantly to focus on a minimal set of classes 
  related to the critical path when scheduling
  
  2.accept that we're setting up ~the whole app, and do something more 
  like what `SchedulerIT` does with modules.
  
  The concern i have is this class will drift from production code and 
  rot, or just be an ongoing maintenance burden.
  
  Do you think either of those paths are realistic?

The current approach is on the lines of #1. The setup is crafted with classes 
participating in the scheduling loop. Do you have observations of the contrary?

I think approach #2 would introduce too much variablity into the JVM perf pass 
due to multiple scheduler sub-systems attempting to do their housekeeping. I 
doubt it will be any better in terms of rot either as it will require even more 
substantial setup and will not fully eliminate fakes/mocks.

To address your code rot concern, I think we should add './gradlew jmh' into 
our jenkins script to make sure it's on the critical change path.


- Maxim


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


On Jan. 7, 2015, 1:34 a.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/28731/
 ---
 
 (Updated Jan. 7, 2015, 1:34 a.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added baseline benchmarks for a few static veto cases.
 
 
 Diffs
 -
 
   build.gradle f9f71a84493b782e9f6072e44e89a2c017cf2a09 
   src/jmh/java/org/apache/aurora/benchmark/Hosts.java PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/Offers.java PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/SchedulerBenchmark.java 
 5cecada93e4e04b689e826af49f691ed7e94ae49 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/Tasks.java PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeClusterState.java 
 PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java PRE-CREATION 
   
 src/jmh/java/org/apache/aurora/benchmark/fakes/FakeRescheduleCalculator.java 
 PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeStatsProvider.java 
 PRE-CREATION 
   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
 b6402ae42e3c7e4dca1c120fa6ef82d2d69e69d5 
   src/main/java/org/apache/aurora/scheduler/async/preemptor/ClusterState.java 
 f7e157c890b5627411acd4bd5c2559ef4829147c 
   
 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 
 0204d14b19ae412236f19ca274d81decb4eba12d 
 
 Diff: https://reviews.apache.org/r/28731/diff/
 
 
 Testing
 ---
 
 Sample run on a local box:
 ```
 # VM invoker: 
 /Library/Java/JavaVirtualMachines/jdk1.7.0_25.jdk/Contents/Home/jre/bin/java
 # VM options: -Dfile.encoding=UTF-8 -Duser.country=US -Duser.language=en 
 -Duser.variant
 # Warmup: 10 iterations, 1 s each
 # Measurement: 100 iterations, 1 s each
 # Timeout: 10 min per iteration
 # Threads: 1 thread, will synchronize iterations
 # Benchmark mode: Average time, time/op
 # Benchmark: 
 org.apache.aurora.benchmark.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark
 
 # Run progress: 0.00% complete, ETA 00:05:30
 # Fork: 1 of 1
 # Warmup Iteration   1: 278284250.000 ns/op
 # Warmup Iteration   2: 70294312.500 ns/op
 # Warmup Iteration   3: 19293379.310 ns/op
 # Warmup Iteration   4: 11945387.097 ns/op
 # Warmup Iteration   5: 10725388.350 ns/op
 # Warmup Iteration   6: 13043282.353 ns/op
 # Warmup Iteration   7: 9233083.333 ns/op
 # Warmup Iteration   8: 9521051.724 ns/op
 # Warmup Iteration   9: 10750854.369 ns/op
 # Warmup Iteration  10: 7460243.243 ns/op
 Iteration   1: 7885364.286 ns/op
 Iteration   2: 7735139.860 ns/op
 Iteration   3: 7660208.333 ns/op
 Iteration   4: 7761204.225 ns/op
 Iteration   5: 7868907.143 ns/op
 Iteration   6: 7567404.110 ns/op
 Iteration   7: 7611000.000 ns/op
 Iteration   8: 7766154.930 ns/op
 Iteration   9: 7669344.828 ns/op
 Iteration  10: 7707783.217 ns/op
 Iteration  11: 7435651.007 ns/op
 Iteration  12: 7697631.944 ns/op
 Iteration  13: 7712531.469 ns/op
 Iteration  14: 7899407.143 ns/op
 Iteration  15: 7448472.973 ns/op
 Iteration  16: 7791521.127 ns/op
 Iteration  17: 7612213.793 ns/op
 Iteration  18: 7710867.133 ns/op
 Iteration  19: 

Re: Review Request 29117: Adding thrift API changes document.

2015-01-21 Thread Aurora ReviewBot

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


This patch does not apply cleanly on master (116ee2d), do you need to rebase?

I will refresh this build result if you post a review containing @ReviewBot 
retry

- Aurora ReviewBot


On Jan. 21, 2015, 10:59 p.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29117/
 ---
 
 (Updated Jan. 21, 2015, 10:59 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Bill Farner.
 
 
 Bugs: AURORA-973
 https://issues.apache.org/jira/browse/AURORA-973
 
 
 Repository: aurora
 
 
 Description
 ---
 
 This is a first stab at documenting thrift deprecation. Any 
 suggestions/comments are welcome.
 
 
 Diffs
 -
 
   docs/developing-aurora-client.md b9912bce44d65ddd7f1e35f0ea9356a89d5fe767 
   docs/developing-aurora-scheduler.md 
 7f6cc2e6c8e01115a9b7a7dc7633bcd88ba02a0f 
   docs/thrift-deprecation.md PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/29117/diff/
 
 
 Testing
 ---
 
 https://github.com/maxim111333/incubator-aurora/blob/populated_deprecation/docs/thrift-deprecation.md
 
 
 Thanks,
 
 Maxim Khutornenko
 




Re: Review Request 29117: Adding thrift API changes document.

2015-01-21 Thread Bill Farner


 On Jan. 9, 2015, 6:44 p.m., Bill Farner wrote:
  docs/thrift-deprecation.md, line 31
  https://reviews.apache.org/r/29117/diff/2/?file=793344#file793344line31
 
  There should be an item about logging and signaling in API responses 
  when deprecated fields are used.
 
 Maxim Khutornenko wrote:
 We have not done this before and there is no standard way to accomplish 
 it across our codebases. Should it rather be done at the feature level rather 
 than the thrift schema level?

I've definitely pushed for server-side logging in the scheduler, client-side 
logging, and i definitely think the API should include a `ResponseDetail` entry 
when a deprecated feature is used in a request.


 On Jan. 9, 2015, 6:44 p.m., Bill Farner wrote:
  docs/thrift-deprecation.md, line 3
  https://reviews.apache.org/r/29117/diff/2/?file=793344#file793344line3
 
  First suggestion should be to go read this page: 
  http://diwakergupta.github.io/thrift-missing-guide/
  
  The page doesn't call out schema evolution, but fills in a bunch of 
  other context.
 
 Maxim Khutornenko wrote:
 I had it initially but decided to leave it out to avoid possible 
 confusion. Glad to add it back if you feel it will be useful.

I see little harm - so i think it should be here.


 On Jan. 9, 2015, 6:44 p.m., Bill Farner wrote:
  docs/thrift-deprecation.md, line 16
  https://reviews.apache.org/r/29117/diff/2/?file=793344#file793344line16
 
  I have mixed feelings about this.  It's fine on the wire _for specific 
  thrift encodings_.  This would, for example, break /apibeta.
 
 Maxim Khutornenko wrote:
 Do you mean backwards compatibility with he old way of consuming /apibeta 
 output? I don't think we are at the position to fully support /apibeta 
 compatibilty and given its rather experimental status we most likely would 
 not do it anyway.

I dunno, i think it's something we should not take so lightly as to encourage 
casual renames.  It's a practice we need to begin stepping up anyhow as we 
prepare to introduce a non-thrift API.


- Bill


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


On Jan. 21, 2015, 10:59 p.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29117/
 ---
 
 (Updated Jan. 21, 2015, 10:59 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Bill Farner.
 
 
 Bugs: AURORA-973
 https://issues.apache.org/jira/browse/AURORA-973
 
 
 Repository: aurora
 
 
 Description
 ---
 
 This is a first stab at documenting thrift deprecation. Any 
 suggestions/comments are welcome.
 
 
 Diffs
 -
 
   docs/developing-aurora-client.md b9912bce44d65ddd7f1e35f0ea9356a89d5fe767 
   docs/developing-aurora-scheduler.md 
 7f6cc2e6c8e01115a9b7a7dc7633bcd88ba02a0f 
   docs/thrift-deprecation.md PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/29117/diff/
 
 
 Testing
 ---
 
 https://github.com/maxim111333/incubator-aurora/blob/populated_deprecation/docs/thrift-deprecation.md
 
 
 Thanks,
 
 Maxim Khutornenko
 




Re: Review Request 28731: Implemented TaskScheduler benchmarks.

2015-01-21 Thread Bill Farner


 On Jan. 21, 2015, 10:27 p.m., Bill Farner wrote:
  Meta question - how do you envision us using this?  When a perf issue is 
  discovered, should we generally push for a test case to be added here to 
  validate the perf fix?
 
 Maxim Khutornenko wrote:
 I doubt we can build a reliable/automated way to catch regressions due to 
 perf. There is too much randomness coming from JVM and machine load for the 
 scattered test runs to be comparable. These bencmarks are most useful when 
 run back to back against different builds. The way I see it is more of a fix 
 validation testbed than a comprehensive perf regression framework. 
 
 To your question, new tests may be required if the fix validation 
 requires cluster/data setup significantly different from the existing tests. 
 Otherwise, running the existing tests before and after the fix should be 
 sufficient.

Agreed - absolute figures are not necessary useful such that we will institute 
thresholds for passage.


- Bill


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


On Jan. 7, 2015, 1:34 a.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/28731/
 ---
 
 (Updated Jan. 7, 2015, 1:34 a.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added baseline benchmarks for a few static veto cases.
 
 
 Diffs
 -
 
   build.gradle f9f71a84493b782e9f6072e44e89a2c017cf2a09 
   src/jmh/java/org/apache/aurora/benchmark/Hosts.java PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/Offers.java PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/SchedulerBenchmark.java 
 5cecada93e4e04b689e826af49f691ed7e94ae49 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/Tasks.java PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeClusterState.java 
 PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java PRE-CREATION 
   
 src/jmh/java/org/apache/aurora/benchmark/fakes/FakeRescheduleCalculator.java 
 PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeStatsProvider.java 
 PRE-CREATION 
   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
 b6402ae42e3c7e4dca1c120fa6ef82d2d69e69d5 
   src/main/java/org/apache/aurora/scheduler/async/preemptor/ClusterState.java 
 f7e157c890b5627411acd4bd5c2559ef4829147c 
   
 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 
 0204d14b19ae412236f19ca274d81decb4eba12d 
 
 Diff: https://reviews.apache.org/r/28731/diff/
 
 
 Testing
 ---
 
 Sample run on a local box:
 ```
 # VM invoker: 
 /Library/Java/JavaVirtualMachines/jdk1.7.0_25.jdk/Contents/Home/jre/bin/java
 # VM options: -Dfile.encoding=UTF-8 -Duser.country=US -Duser.language=en 
 -Duser.variant
 # Warmup: 10 iterations, 1 s each
 # Measurement: 100 iterations, 1 s each
 # Timeout: 10 min per iteration
 # Threads: 1 thread, will synchronize iterations
 # Benchmark mode: Average time, time/op
 # Benchmark: 
 org.apache.aurora.benchmark.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark
 
 # Run progress: 0.00% complete, ETA 00:05:30
 # Fork: 1 of 1
 # Warmup Iteration   1: 278284250.000 ns/op
 # Warmup Iteration   2: 70294312.500 ns/op
 # Warmup Iteration   3: 19293379.310 ns/op
 # Warmup Iteration   4: 11945387.097 ns/op
 # Warmup Iteration   5: 10725388.350 ns/op
 # Warmup Iteration   6: 13043282.353 ns/op
 # Warmup Iteration   7: 9233083.333 ns/op
 # Warmup Iteration   8: 9521051.724 ns/op
 # Warmup Iteration   9: 10750854.369 ns/op
 # Warmup Iteration  10: 7460243.243 ns/op
 Iteration   1: 7885364.286 ns/op
 Iteration   2: 7735139.860 ns/op
 Iteration   3: 7660208.333 ns/op
 Iteration   4: 7761204.225 ns/op
 Iteration   5: 7868907.143 ns/op
 Iteration   6: 7567404.110 ns/op
 Iteration   7: 7611000.000 ns/op
 Iteration   8: 7766154.930 ns/op
 Iteration   9: 7669344.828 ns/op
 Iteration  10: 7707783.217 ns/op
 Iteration  11: 7435651.007 ns/op
 Iteration  12: 7697631.944 ns/op
 Iteration  13: 7712531.469 ns/op
 Iteration  14: 7899407.143 ns/op
 Iteration  15: 7448472.973 ns/op
 Iteration  16: 7791521.127 ns/op
 Iteration  17: 7612213.793 ns/op
 Iteration  18: 7710867.133 ns/op
 Iteration  19: 7649296.552 ns/op
 Iteration  20: 7768309.859 ns/op
 Iteration  21: 7688666.667 ns/op
 Iteration  22: 7531557.823 ns/op
 Iteration  23: 7381193.333 ns/op
 Iteration  24: 7726006.993 ns/op
 Iteration  25: 7603358.621 ns/op
 Iteration  26: 7653631.944 ns/op
 Iteration  27: 7442275.168 ns/op
 Iteration  28: 7613186.207 ns/op
 Iteration  29: 

Re: Review Request 28731: Implemented TaskScheduler benchmarks.

2015-01-21 Thread Maxim Khutornenko


 On Jan. 5, 2015, 8:38 p.m., Bill Farner wrote:
  src/jmh/java/org/apache/aurora/benchmark/fakes/FakeClusterState.java, line 
  23
  https://reviews.apache.org/r/28731/diff/4/?file=792353#file792353line23
 
  Did you consider using `CachedClusterState` instead?  That might cause 
  some other things to fall into place, and alleviate the manual maintenance 
  of this mapping.  AFAICT there's a small amount of wiring necessary to make 
  this work (including a more real `EventSink`), but that might be a 
  worthwhile investment if it simplifies the benchmark implementations.
 
 Maxim Khutornenko wrote:
 I am hesitant to make CachedClusterState public as it's meaningless 
 outside of its package. There is not much happening in FakeClusterState to 
 press for a real object use here.
 
 Bill Farner wrote:
 Isn't this stance contradictory with this from above: I actually prefer 
 relying on real objects as much as possible to get more realistic perf 
 picture.  Especially given that `CachedClusterState` was introduced for 
 performance reasons, seems like it belongs here just as much if not more than 
 `StateManagerImpl`.
 
 Maxim Khutornenko wrote:
 I was referring to fakes/mocks that replace a substantial chunk of 
 functionality with some custom logic. This specific case does not quite fit 
 into that picture as all it does is key/value mapping. I decided to leave the 
 CachedClusterState out due to the reasons above. If you feel 
 CachedClusterState is important I can reconsider. Though it will likely going 
 to make wiring more complicated due to event bus support.

Tried to wire in CachedClusterState and ran into AURORA-1046.


- Maxim


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


On Jan. 7, 2015, 1:34 a.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/28731/
 ---
 
 (Updated Jan. 7, 2015, 1:34 a.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added baseline benchmarks for a few static veto cases.
 
 
 Diffs
 -
 
   build.gradle f9f71a84493b782e9f6072e44e89a2c017cf2a09 
   src/jmh/java/org/apache/aurora/benchmark/Hosts.java PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/Offers.java PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/SchedulerBenchmark.java 
 5cecada93e4e04b689e826af49f691ed7e94ae49 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/Tasks.java PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeClusterState.java 
 PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java PRE-CREATION 
   
 src/jmh/java/org/apache/aurora/benchmark/fakes/FakeRescheduleCalculator.java 
 PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeStatsProvider.java 
 PRE-CREATION 
   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
 b6402ae42e3c7e4dca1c120fa6ef82d2d69e69d5 
   src/main/java/org/apache/aurora/scheduler/async/preemptor/ClusterState.java 
 f7e157c890b5627411acd4bd5c2559ef4829147c 
   
 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 
 0204d14b19ae412236f19ca274d81decb4eba12d 
 
 Diff: https://reviews.apache.org/r/28731/diff/
 
 
 Testing
 ---
 
 Sample run on a local box:
 ```
 # VM invoker: 
 /Library/Java/JavaVirtualMachines/jdk1.7.0_25.jdk/Contents/Home/jre/bin/java
 # VM options: -Dfile.encoding=UTF-8 -Duser.country=US -Duser.language=en 
 -Duser.variant
 # Warmup: 10 iterations, 1 s each
 # Measurement: 100 iterations, 1 s each
 # Timeout: 10 min per iteration
 # Threads: 1 thread, will synchronize iterations
 # Benchmark mode: Average time, time/op
 # Benchmark: 
 org.apache.aurora.benchmark.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark
 
 # Run progress: 0.00% complete, ETA 00:05:30
 # Fork: 1 of 1
 # Warmup Iteration   1: 278284250.000 ns/op
 # Warmup Iteration   2: 70294312.500 ns/op
 # Warmup Iteration   3: 19293379.310 ns/op
 # Warmup Iteration   4: 11945387.097 ns/op
 # Warmup Iteration   5: 10725388.350 ns/op
 # Warmup Iteration   6: 13043282.353 ns/op
 # Warmup Iteration   7: 9233083.333 ns/op
 # Warmup Iteration   8: 9521051.724 ns/op
 # Warmup Iteration   9: 10750854.369 ns/op
 # Warmup Iteration  10: 7460243.243 ns/op
 Iteration   1: 7885364.286 ns/op
 Iteration   2: 7735139.860 ns/op
 Iteration   3: 7660208.333 ns/op
 Iteration   4: 7761204.225 ns/op
 Iteration   5: 7868907.143 ns/op
 Iteration   6: 7567404.110 ns/op
 Iteration   7: 7611000.000 ns/op
 Iteration   8: 7766154.930 ns/op
 Iteration   9: 

Re: Review Request 29117: Adding thrift API changes document.

2015-01-21 Thread Maxim Khutornenko


 On Jan. 9, 2015, 6:44 p.m., Bill Farner wrote:
  docs/developing-aurora-client.md, line 124
  https://reviews.apache.org/r/29117/diff/2/?file=793342#file793342line124
 
  s/thrift/Thrift/

Uppercased everywhere.


 On Jan. 9, 2015, 6:44 p.m., Bill Farner wrote:
  docs/thrift-deprecation.md, line 3
  https://reviews.apache.org/r/29117/diff/2/?file=793344#file793344line3
 
  First suggestion should be to go read this page: 
  http://diwakergupta.github.io/thrift-missing-guide/
  
  The page doesn't call out schema evolution, but fills in a bunch of 
  other context.

I had it initially but decided to leave it out to avoid possible confusion. 
Glad to add it back if you feel it will be useful.


 On Jan. 9, 2015, 6:44 p.m., Bill Farner wrote:
  docs/thrift-deprecation.md, line 5
  https://reviews.apache.org/r/29117/diff/2/?file=793344#file793344line5
 
  capable of correctly handling

done


 On Jan. 9, 2015, 6:44 p.m., Bill Farner wrote:
  docs/thrift-deprecation.md, line 16
  https://reviews.apache.org/r/29117/diff/2/?file=793344#file793344line16
 
  I have mixed feelings about this.  It's fine on the wire _for specific 
  thrift encodings_.  This would, for example, break /apibeta.

Do you mean backwards compatibility with he old way of consuming /apibeta 
output? I don't think we are at the position to fully support /apibeta 
compatibilty and given its rather experimental status we most likely would not 
do it anyway.


 On Jan. 9, 2015, 6:44 p.m., Bill Farner wrote:
  docs/thrift-deprecation.md, line 26
  https://reviews.apache.org/r/29117/diff/2/?file=793344#file793344line26
 
  Lost me at a field double.  Rephrase?

rephrased


 On Jan. 9, 2015, 6:44 p.m., Bill Farner wrote:
  docs/thrift-deprecation.md, line 31
  https://reviews.apache.org/r/29117/diff/2/?file=793344#file793344line31
 
  There should be an item about logging and signaling in API responses 
  when deprecated fields are used.

We have not done this before and there is no standard way to accomplish it 
across our codebases. Should it rather be done at the feature level rather than 
the thrift schema level?


- Maxim


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


On Jan. 6, 2015, 11:30 p.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/29117/
 ---
 
 (Updated Jan. 6, 2015, 11:30 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Bill Farner.
 
 
 Bugs: AURORA-973
 https://issues.apache.org/jira/browse/AURORA-973
 
 
 Repository: aurora
 
 
 Description
 ---
 
 This is a first stab at documenting thrift deprecation. Any 
 suggestions/comments are welcome.
 
 
 Diffs
 -
 
   docs/developing-aurora-client.md b9912bce44d65ddd7f1e35f0ea9356a89d5fe767 
   docs/developing-aurora-scheduler.md 
 7f6cc2e6c8e01115a9b7a7dc7633bcd88ba02a0f 
   docs/thrift-deprecation.md PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/29117/diff/
 
 
 Testing
 ---
 
 https://github.com/maxim111333/incubator-aurora/blob/populated_deprecation/docs/thrift-deprecation.md
 
 
 Thanks,
 
 Maxim Khutornenko
 




Re: Review Request 29117: Adding thrift API changes document.

2015-01-21 Thread Maxim Khutornenko

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

(Updated Jan. 21, 2015, 10:59 p.m.)


Review request for Aurora, Kevin Sweeney and Bill Farner.


Changes
---

Bill's comments.


Bugs: AURORA-973
https://issues.apache.org/jira/browse/AURORA-973


Repository: aurora


Description
---

This is a first stab at documenting thrift deprecation. Any 
suggestions/comments are welcome.


Diffs (updated)
-

  docs/developing-aurora-client.md b9912bce44d65ddd7f1e35f0ea9356a89d5fe767 
  docs/developing-aurora-scheduler.md 7f6cc2e6c8e01115a9b7a7dc7633bcd88ba02a0f 
  docs/thrift-deprecation.md PRE-CREATION 

Diff: https://reviews.apache.org/r/29117/diff/


Testing
---

https://github.com/maxim111333/incubator-aurora/blob/populated_deprecation/docs/thrift-deprecation.md


Thanks,

Maxim Khutornenko



Re: Review Request 28731: Implemented TaskScheduler benchmarks.

2015-01-21 Thread Maxim Khutornenko


 On Jan. 21, 2015, 10:27 p.m., Bill Farner wrote:
  Meta question - how do you envision us using this?  When a perf issue is 
  discovered, should we generally push for a test case to be added here to 
  validate the perf fix?

I doubt we can build a reliable/automated way to catch regressions due to perf. 
There is too much randomness coming from JVM and machine load for the scattered 
test runs to be comparable. These bencmarks are most useful when run back to 
back against different builds. The way I see it is more of a fix validation 
testbed than a comprehensive perf regression framework. 

To your question, new tests may be required if the fix validation requires 
cluster/data setup significantly different from the existing tests. Otherwise, 
running the existing tests before and after the fix should be sufficient.


- Maxim


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


On Jan. 7, 2015, 1:34 a.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/28731/
 ---
 
 (Updated Jan. 7, 2015, 1:34 a.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added baseline benchmarks for a few static veto cases.
 
 
 Diffs
 -
 
   build.gradle f9f71a84493b782e9f6072e44e89a2c017cf2a09 
   src/jmh/java/org/apache/aurora/benchmark/Hosts.java PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/Offers.java PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/SchedulerBenchmark.java 
 5cecada93e4e04b689e826af49f691ed7e94ae49 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/Tasks.java PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeClusterState.java 
 PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java PRE-CREATION 
   
 src/jmh/java/org/apache/aurora/benchmark/fakes/FakeRescheduleCalculator.java 
 PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeStatsProvider.java 
 PRE-CREATION 
   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
 b6402ae42e3c7e4dca1c120fa6ef82d2d69e69d5 
   src/main/java/org/apache/aurora/scheduler/async/preemptor/ClusterState.java 
 f7e157c890b5627411acd4bd5c2559ef4829147c 
   
 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 
 0204d14b19ae412236f19ca274d81decb4eba12d 
 
 Diff: https://reviews.apache.org/r/28731/diff/
 
 
 Testing
 ---
 
 Sample run on a local box:
 ```
 # VM invoker: 
 /Library/Java/JavaVirtualMachines/jdk1.7.0_25.jdk/Contents/Home/jre/bin/java
 # VM options: -Dfile.encoding=UTF-8 -Duser.country=US -Duser.language=en 
 -Duser.variant
 # Warmup: 10 iterations, 1 s each
 # Measurement: 100 iterations, 1 s each
 # Timeout: 10 min per iteration
 # Threads: 1 thread, will synchronize iterations
 # Benchmark mode: Average time, time/op
 # Benchmark: 
 org.apache.aurora.benchmark.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark
 
 # Run progress: 0.00% complete, ETA 00:05:30
 # Fork: 1 of 1
 # Warmup Iteration   1: 278284250.000 ns/op
 # Warmup Iteration   2: 70294312.500 ns/op
 # Warmup Iteration   3: 19293379.310 ns/op
 # Warmup Iteration   4: 11945387.097 ns/op
 # Warmup Iteration   5: 10725388.350 ns/op
 # Warmup Iteration   6: 13043282.353 ns/op
 # Warmup Iteration   7: 9233083.333 ns/op
 # Warmup Iteration   8: 9521051.724 ns/op
 # Warmup Iteration   9: 10750854.369 ns/op
 # Warmup Iteration  10: 7460243.243 ns/op
 Iteration   1: 7885364.286 ns/op
 Iteration   2: 7735139.860 ns/op
 Iteration   3: 7660208.333 ns/op
 Iteration   4: 7761204.225 ns/op
 Iteration   5: 7868907.143 ns/op
 Iteration   6: 7567404.110 ns/op
 Iteration   7: 7611000.000 ns/op
 Iteration   8: 7766154.930 ns/op
 Iteration   9: 7669344.828 ns/op
 Iteration  10: 7707783.217 ns/op
 Iteration  11: 7435651.007 ns/op
 Iteration  12: 7697631.944 ns/op
 Iteration  13: 7712531.469 ns/op
 Iteration  14: 7899407.143 ns/op
 Iteration  15: 7448472.973 ns/op
 Iteration  16: 7791521.127 ns/op
 Iteration  17: 7612213.793 ns/op
 Iteration  18: 7710867.133 ns/op
 Iteration  19: 7649296.552 ns/op
 Iteration  20: 7768309.859 ns/op
 Iteration  21: 7688666.667 ns/op
 Iteration  22: 7531557.823 ns/op
 Iteration  23: 7381193.333 ns/op
 Iteration  24: 7726006.993 ns/op
 Iteration  25: 7603358.621 ns/op
 Iteration  26: 7653631.944 ns/op
 Iteration  27: 7442275.168 ns/op
 Iteration  28: 7613186.207 ns/op
 Iteration  29: 7765823.944 ns/op
 Iteration  30: 7489687.075 ns/op
 Iteration  31: 7811443.662 ns/op
 Iteration  32: 8015007.246 ns/op
 Iteration  33: 8192392.593 ns/op
 

Re: Review Request 28731: Implemented TaskScheduler benchmarks.

2015-01-21 Thread Maxim Khutornenko


 On Jan. 5, 2015, 8:38 p.m., Bill Farner wrote:
  src/jmh/java/org/apache/aurora/benchmark/fakes/FakeClusterState.java, line 
  23
  https://reviews.apache.org/r/28731/diff/4/?file=792353#file792353line23
 
  Did you consider using `CachedClusterState` instead?  That might cause 
  some other things to fall into place, and alleviate the manual maintenance 
  of this mapping.  AFAICT there's a small amount of wiring necessary to make 
  this work (including a more real `EventSink`), but that might be a 
  worthwhile investment if it simplifies the benchmark implementations.
 
 Maxim Khutornenko wrote:
 I am hesitant to make CachedClusterState public as it's meaningless 
 outside of its package. There is not much happening in FakeClusterState to 
 press for a real object use here.
 
 Bill Farner wrote:
 Isn't this stance contradictory with this from above: I actually prefer 
 relying on real objects as much as possible to get more realistic perf 
 picture.  Especially given that `CachedClusterState` was introduced for 
 performance reasons, seems like it belongs here just as much if not more than 
 `StateManagerImpl`.

I was referring to fakes/mocks that replace a substantial chunk of 
functionality with some custom logic. This specific case does not quite fit 
into that picture as all it does is key/value mapping. I decided to leave the 
CachedClusterState out due to the reasons above. If you feel CachedClusterState 
is important I can reconsider. Though it will likely going to make wiring more 
complicated due to event bus support.


- Maxim


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


On Jan. 7, 2015, 1:34 a.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/28731/
 ---
 
 (Updated Jan. 7, 2015, 1:34 a.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added baseline benchmarks for a few static veto cases.
 
 
 Diffs
 -
 
   build.gradle f9f71a84493b782e9f6072e44e89a2c017cf2a09 
   src/jmh/java/org/apache/aurora/benchmark/Hosts.java PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/Offers.java PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/SchedulerBenchmark.java 
 5cecada93e4e04b689e826af49f691ed7e94ae49 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/Tasks.java PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeClusterState.java 
 PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java PRE-CREATION 
   
 src/jmh/java/org/apache/aurora/benchmark/fakes/FakeRescheduleCalculator.java 
 PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeStatsProvider.java 
 PRE-CREATION 
   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
 b6402ae42e3c7e4dca1c120fa6ef82d2d69e69d5 
   src/main/java/org/apache/aurora/scheduler/async/preemptor/ClusterState.java 
 f7e157c890b5627411acd4bd5c2559ef4829147c 
   
 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 
 0204d14b19ae412236f19ca274d81decb4eba12d 
 
 Diff: https://reviews.apache.org/r/28731/diff/
 
 
 Testing
 ---
 
 Sample run on a local box:
 ```
 # VM invoker: 
 /Library/Java/JavaVirtualMachines/jdk1.7.0_25.jdk/Contents/Home/jre/bin/java
 # VM options: -Dfile.encoding=UTF-8 -Duser.country=US -Duser.language=en 
 -Duser.variant
 # Warmup: 10 iterations, 1 s each
 # Measurement: 100 iterations, 1 s each
 # Timeout: 10 min per iteration
 # Threads: 1 thread, will synchronize iterations
 # Benchmark mode: Average time, time/op
 # Benchmark: 
 org.apache.aurora.benchmark.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark
 
 # Run progress: 0.00% complete, ETA 00:05:30
 # Fork: 1 of 1
 # Warmup Iteration   1: 278284250.000 ns/op
 # Warmup Iteration   2: 70294312.500 ns/op
 # Warmup Iteration   3: 19293379.310 ns/op
 # Warmup Iteration   4: 11945387.097 ns/op
 # Warmup Iteration   5: 10725388.350 ns/op
 # Warmup Iteration   6: 13043282.353 ns/op
 # Warmup Iteration   7: 9233083.333 ns/op
 # Warmup Iteration   8: 9521051.724 ns/op
 # Warmup Iteration   9: 10750854.369 ns/op
 # Warmup Iteration  10: 7460243.243 ns/op
 Iteration   1: 7885364.286 ns/op
 Iteration   2: 7735139.860 ns/op
 Iteration   3: 7660208.333 ns/op
 Iteration   4: 7761204.225 ns/op
 Iteration   5: 7868907.143 ns/op
 Iteration   6: 7567404.110 ns/op
 Iteration   7: 7611000.000 ns/op
 Iteration   8: 7766154.930 ns/op
 Iteration   9: 7669344.828 ns/op
 Iteration  10: 7707783.217 ns/op
 Iteration  11: 7435651.007 ns/op
 Iteration  

Re: Review Request 28920: Add support for docker containers to aurora

2015-01-21 Thread Kevin Sweeney

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


Sorry for the delay in reply. I've been thinking a lot about this patch and 
it's really looking good. My comments are focused mostly on 
backwards-compatibility as well as the desire to not favor Docker over other 
future containerizers Mesos may add.


docs/deploying-aurora-scheduler.md
https://reviews.apache.org/r/28920/#comment113341

Latest version doesn't require a wrapper script, update docs to reflect?



docs/deploying-aurora-scheduler.md
https://reviews.apache.org/r/28920/#comment113339

Philosophical question: if there's already a hard requirement that the 
container have Python 2.7 why not require that the executor be baked in as 
well? Maybe it's worth calling out as a TODO, but you don't have to answer it 
now.



examples/vagrant/provision-dev-cluster.sh
https://reviews.apache.org/r/28920/#comment113340

nit: sh -c indirection is unnecessary here.



src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java
https://reviews.apache.org/r/28920/#comment113726

I don't see a reason the executor can't do this itself, by reading the 
Container field in AssignedTask and environment variables. I'd prefer not to 
introduce a new channel to send configuration from the scheduler to the 
executor with this review.

@wickman might be better able to answer whether this is feasible.



src/main/python/apache/thermos/config/schema_base.py
https://reviews.apache.org/r/28920/#comment113732

Can we match the union-like behavior the IDL defines here,

something like:

```py
class Container(Struct):
  docker = Docker
  
class Docker(Struct):
  image = Required(String)

```

This will be more easily extensible to more container types without 
requiring backwards-incompatible changes.



src/main/python/apache/thermos/core/runner.py
https://reviews.apache.org/r/28920/#comment113733

Naive question: since we have this block here can we drop the preamble from 
the scheduler?


- Kevin Sweeney


On Jan. 15, 2015, 4:08 p.m., Steve Niemitz wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/28920/
 ---
 
 (Updated Jan. 15, 2015, 4:08 p.m.)
 
 
 Review request for Aurora, Jay Buffington, Kevin Sweeney, and Bill Farner.
 
 
 Bugs: AURORA-633
 https://issues.apache.org/jira/browse/AURORA-633
 
 
 Repository: aurora
 
 
 Description
 ---
 
 This change adds support for launching docker containers through aurora.  
 These changes are based off of the discussion in 
 https://issues.apache.org/jira/browse/AURORA-633
 
 As of now, a special thermos_executor.sh script is needed to launch the 
 executor inside docker containers.  A sample aurora file is in 
 examples/jobs/docker.
 
 In addition, mesos-slave must be run with `--containerizers=docker,mesos`, 
 the example upstart config in examples/vagrant/upstart has been updated to 
 reflect this.
 
 More information is in subsequent review request comments.
 
 
 Diffs
 -
 
   Vagrantfile f8b7db8eebdc6a10989de3bc9a2c3e89ce17f5fc 
   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
 08ba1cdf88b712de22c26c04443079282db59ef9 
   config/legacy_untested_classes.txt 33c1d6eb4ea02e01b7002c2c2bae5a3858c8b0e5 
   docs/configuration-reference.md f3cb257206a194b82fd2045dc20456ee832dbcea 
   docs/deploying-aurora-scheduler.md 711ae7eda07c2c1735601c265c06a88c1862cce7 
   examples/jobs/docker/hello_docker.aurora PRE-CREATION 
   examples/vagrant/aurorabuild.sh 1e31f21998d02fd69ce0db88e6adb3d32cff67fd 
   examples/vagrant/provision-dev-cluster.sh 
 7af4b52a6876268a97630279221bb98d9b04efad 
   examples/vagrant/upstart/aurora-scheduler.conf 
 788ec254270bca074dae91829c7f4fccdc8f8bb0 
   examples/vagrant/upstart/mesos-slave.conf 
 512ce7ecf34042ed68dda55efb2dd0415f8469db 
   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
 72c7545e7f16549f6a9ccb5fb74a06f154a7ea94 
   src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 
 5226e3d1b303b1773a057078f2911c5ec2aa97f5 
   src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java 
 d885b224ec5a1d529347d84e03ba98ab6734a126 
   
 src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
  01b03508afac37b5a8f0ec5c3da1460695e1ef59 
   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
 5bf283062c9d119ff91ed45da8b236e36d0fc9aa 
   src/main/python/apache/aurora/config/thrift.py 
 ba94ac3c0cbaf3c91eb1a1d86a244ed6fa3b649c 
   src/main/python/apache/aurora/executor/aurora_executor.py 
 636b23d30a897b557eb8c3f8733c90b23cb807ef 
   

Re: Review Request 28731: Implemented TaskScheduler benchmarks.

2015-01-21 Thread Maxim Khutornenko

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

(Updated Jan. 22, 2015, 1:54 a.m.)


Review request for Aurora, Kevin Sweeney and Bill Farner.


Changes
---

Replaced FakeClusterState with CachedClusterState.


Repository: aurora


Description
---

Added baseline benchmarks for a few static veto cases.


Diffs (updated)
-

  build.gradle f9f71a84493b782e9f6072e44e89a2c017cf2a09 
  src/jmh/java/org/apache/aurora/benchmark/Hosts.java PRE-CREATION 
  src/jmh/java/org/apache/aurora/benchmark/Offers.java PRE-CREATION 
  src/jmh/java/org/apache/aurora/benchmark/SchedulerBenchmark.java 
5cecada93e4e04b689e826af49f691ed7e94ae49 
  src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
PRE-CREATION 
  src/jmh/java/org/apache/aurora/benchmark/Tasks.java PRE-CREATION 
  src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java PRE-CREATION 
  src/jmh/java/org/apache/aurora/benchmark/fakes/FakeRescheduleCalculator.java 
PRE-CREATION 
  src/jmh/java/org/apache/aurora/benchmark/fakes/FakeStatsProvider.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
b6402ae42e3c7e4dca1c120fa6ef82d2d69e69d5 
  
src/main/java/org/apache/aurora/scheduler/async/preemptor/CachedClusterState.java
 03c2a8fde4a3fe5ac73f44da2cbe227995501c46 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/ClusterState.java 
f7e157c890b5627411acd4bd5c2559ef4829147c 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 
0204d14b19ae412236f19ca274d81decb4eba12d 

Diff: https://reviews.apache.org/r/28731/diff/


Testing
---

Sample run on a local box:
```
# VM invoker: 
/Library/Java/JavaVirtualMachines/jdk1.7.0_25.jdk/Contents/Home/jre/bin/java
# VM options: -Dfile.encoding=UTF-8 -Duser.country=US -Duser.language=en 
-Duser.variant
# Warmup: 10 iterations, 1 s each
# Measurement: 100 iterations, 1 s each
# Timeout: 10 min per iteration
# Threads: 1 thread, will synchronize iterations
# Benchmark mode: Average time, time/op
# Benchmark: 
org.apache.aurora.benchmark.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark

# Run progress: 0.00% complete, ETA 00:05:30
# Fork: 1 of 1
# Warmup Iteration   1: 278284250.000 ns/op
# Warmup Iteration   2: 70294312.500 ns/op
# Warmup Iteration   3: 19293379.310 ns/op
# Warmup Iteration   4: 11945387.097 ns/op
# Warmup Iteration   5: 10725388.350 ns/op
# Warmup Iteration   6: 13043282.353 ns/op
# Warmup Iteration   7: 9233083.333 ns/op
# Warmup Iteration   8: 9521051.724 ns/op
# Warmup Iteration   9: 10750854.369 ns/op
# Warmup Iteration  10: 7460243.243 ns/op
Iteration   1: 7885364.286 ns/op
Iteration   2: 7735139.860 ns/op
Iteration   3: 7660208.333 ns/op
Iteration   4: 7761204.225 ns/op
Iteration   5: 7868907.143 ns/op
Iteration   6: 7567404.110 ns/op
Iteration   7: 7611000.000 ns/op
Iteration   8: 7766154.930 ns/op
Iteration   9: 7669344.828 ns/op
Iteration  10: 7707783.217 ns/op
Iteration  11: 7435651.007 ns/op
Iteration  12: 7697631.944 ns/op
Iteration  13: 7712531.469 ns/op
Iteration  14: 7899407.143 ns/op
Iteration  15: 7448472.973 ns/op
Iteration  16: 7791521.127 ns/op
Iteration  17: 7612213.793 ns/op
Iteration  18: 7710867.133 ns/op
Iteration  19: 7649296.552 ns/op
Iteration  20: 7768309.859 ns/op
Iteration  21: 7688666.667 ns/op
Iteration  22: 7531557.823 ns/op
Iteration  23: 7381193.333 ns/op
Iteration  24: 7726006.993 ns/op
Iteration  25: 7603358.621 ns/op
Iteration  26: 7653631.944 ns/op
Iteration  27: 7442275.168 ns/op
Iteration  28: 7613186.207 ns/op
Iteration  29: 7765823.944 ns/op
Iteration  30: 7489687.075 ns/op
Iteration  31: 7811443.662 ns/op
Iteration  32: 8015007.246 ns/op
Iteration  33: 8192392.593 ns/op
Iteration  34: 8040335.766 ns/op
Iteration  35: 7584212.329 ns/op
Iteration  36: 8001934.783 ns/op
Iteration  37: 9744815.789 ns/op
Iteration  38: 11688284.211 ns/op
Iteration  39: 8661406.250 ns/op
Iteration  40: 7678413.793 ns/op
Iteration  41: 8502223.077 ns/op
Iteration  42: 7640820.690 ns/op
Iteration  43: 7875624.113 ns/op
Iteration  44: 7506809.524 ns/op
Iteration  45: 8005431.655 ns/op
Iteration  46: 8081664.234 ns/op
Iteration  47: 7579438.356 ns/op
Iteration  48: 7993405.797 ns/op
Iteration  49: 7571958.904 ns/op
Iteration  50: 8116463.235 ns/op
Iteration  51: 7941330.935 ns/op
Iteration  52: 7687145.833 ns/op
Iteration  53: 8082554.745 ns/op
Iteration  54: 7597889.655 ns/op
Iteration  55: 7299907.285 ns/op
Iteration  56: 7992789.855 ns/op
Iteration  57: 7648268.966 ns/op
Iteration  58: 7570863.014 ns/op
Iteration  59: 7885078.571 ns/op
Iteration  60: 7647158.621 ns/op
Iteration  61: 7830858.156 ns/op
Iteration  62: 7773690.141 ns/op
Iteration  63: 7905850.000 ns/op
Iteration  64: 7653800.000 ns/op
Iteration  65: 7408248.322 ns/op
Iteration  66: 7961352.518 ns/op
Iteration  67: 7879785.714 ns/op

Re: Review Request 28731: Implemented TaskScheduler benchmarks.

2015-01-21 Thread Maxim Khutornenko


 On Jan. 5, 2015, 8:38 p.m., Bill Farner wrote:
  src/jmh/java/org/apache/aurora/benchmark/fakes/FakeClusterState.java, line 
  23
  https://reviews.apache.org/r/28731/diff/4/?file=792353#file792353line23
 
  Did you consider using `CachedClusterState` instead?  That might cause 
  some other things to fall into place, and alleviate the manual maintenance 
  of this mapping.  AFAICT there's a small amount of wiring necessary to make 
  this work (including a more real `EventSink`), but that might be a 
  worthwhile investment if it simplifies the benchmark implementations.
 
 Maxim Khutornenko wrote:
 I am hesitant to make CachedClusterState public as it's meaningless 
 outside of its package. There is not much happening in FakeClusterState to 
 press for a real object use here.
 
 Bill Farner wrote:
 Isn't this stance contradictory with this from above: I actually prefer 
 relying on real objects as much as possible to get more realistic perf 
 picture.  Especially given that `CachedClusterState` was introduced for 
 performance reasons, seems like it belongs here just as much if not more than 
 `StateManagerImpl`.
 
 Maxim Khutornenko wrote:
 I was referring to fakes/mocks that replace a substantial chunk of 
 functionality with some custom logic. This specific case does not quite fit 
 into that picture as all it does is key/value mapping. I decided to leave the 
 CachedClusterState out due to the reasons above. If you feel 
 CachedClusterState is important I can reconsider. Though it will likely going 
 to make wiring more complicated due to event bus support.
 
 Maxim Khutornenko wrote:
 Tried to wire in CachedClusterState and ran into AURORA-1046.

Should rebase more often. This has apparently been fixed in AURORA-1016. Update 
is coming.


- Maxim


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


On Jan. 7, 2015, 1:34 a.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/28731/
 ---
 
 (Updated Jan. 7, 2015, 1:34 a.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added baseline benchmarks for a few static veto cases.
 
 
 Diffs
 -
 
   build.gradle f9f71a84493b782e9f6072e44e89a2c017cf2a09 
   src/jmh/java/org/apache/aurora/benchmark/Hosts.java PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/Offers.java PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/SchedulerBenchmark.java 
 5cecada93e4e04b689e826af49f691ed7e94ae49 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/Tasks.java PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeClusterState.java 
 PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java PRE-CREATION 
   
 src/jmh/java/org/apache/aurora/benchmark/fakes/FakeRescheduleCalculator.java 
 PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeStatsProvider.java 
 PRE-CREATION 
   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
 b6402ae42e3c7e4dca1c120fa6ef82d2d69e69d5 
   src/main/java/org/apache/aurora/scheduler/async/preemptor/ClusterState.java 
 f7e157c890b5627411acd4bd5c2559ef4829147c 
   
 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 
 0204d14b19ae412236f19ca274d81decb4eba12d 
 
 Diff: https://reviews.apache.org/r/28731/diff/
 
 
 Testing
 ---
 
 Sample run on a local box:
 ```
 # VM invoker: 
 /Library/Java/JavaVirtualMachines/jdk1.7.0_25.jdk/Contents/Home/jre/bin/java
 # VM options: -Dfile.encoding=UTF-8 -Duser.country=US -Duser.language=en 
 -Duser.variant
 # Warmup: 10 iterations, 1 s each
 # Measurement: 100 iterations, 1 s each
 # Timeout: 10 min per iteration
 # Threads: 1 thread, will synchronize iterations
 # Benchmark mode: Average time, time/op
 # Benchmark: 
 org.apache.aurora.benchmark.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark
 
 # Run progress: 0.00% complete, ETA 00:05:30
 # Fork: 1 of 1
 # Warmup Iteration   1: 278284250.000 ns/op
 # Warmup Iteration   2: 70294312.500 ns/op
 # Warmup Iteration   3: 19293379.310 ns/op
 # Warmup Iteration   4: 11945387.097 ns/op
 # Warmup Iteration   5: 10725388.350 ns/op
 # Warmup Iteration   6: 13043282.353 ns/op
 # Warmup Iteration   7: 9233083.333 ns/op
 # Warmup Iteration   8: 9521051.724 ns/op
 # Warmup Iteration   9: 10750854.369 ns/op
 # Warmup Iteration  10: 7460243.243 ns/op
 Iteration   1: 7885364.286 ns/op
 Iteration   2: 7735139.860 ns/op
 Iteration   3: 7660208.333 ns/op
 Iteration   4: 7761204.225 ns/op
 Iteration   5: 7868907.143 

Re: Review Request 28731: Implemented TaskScheduler benchmarks.

2015-01-21 Thread Aurora ReviewBot

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


Master (116ee2d) is green with this patch.
  ./build-support/jenkins/build.sh

However, it appears that it might lack test coverage.

I will refresh this build result if you post a review containing @ReviewBot 
retry

- Aurora ReviewBot


On Jan. 22, 2015, 1:54 a.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/28731/
 ---
 
 (Updated Jan. 22, 2015, 1:54 a.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Added baseline benchmarks for a few static veto cases.
 
 
 Diffs
 -
 
   build.gradle f9f71a84493b782e9f6072e44e89a2c017cf2a09 
   src/jmh/java/org/apache/aurora/benchmark/Hosts.java PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/Offers.java PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/SchedulerBenchmark.java 
 5cecada93e4e04b689e826af49f691ed7e94ae49 
   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
 PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/Tasks.java PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java PRE-CREATION 
   
 src/jmh/java/org/apache/aurora/benchmark/fakes/FakeRescheduleCalculator.java 
 PRE-CREATION 
   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeStatsProvider.java 
 PRE-CREATION 
   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
 b6402ae42e3c7e4dca1c120fa6ef82d2d69e69d5 
   
 src/main/java/org/apache/aurora/scheduler/async/preemptor/CachedClusterState.java
  03c2a8fde4a3fe5ac73f44da2cbe227995501c46 
   src/main/java/org/apache/aurora/scheduler/async/preemptor/ClusterState.java 
 f7e157c890b5627411acd4bd5c2559ef4829147c 
   
 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 
 0204d14b19ae412236f19ca274d81decb4eba12d 
 
 Diff: https://reviews.apache.org/r/28731/diff/
 
 
 Testing
 ---
 
 Sample run on a local box:
 ```
 # VM invoker: 
 /Library/Java/JavaVirtualMachines/jdk1.7.0_25.jdk/Contents/Home/jre/bin/java
 # VM options: -Dfile.encoding=UTF-8 -Duser.country=US -Duser.language=en 
 -Duser.variant
 # Warmup: 10 iterations, 1 s each
 # Measurement: 100 iterations, 1 s each
 # Timeout: 10 min per iteration
 # Threads: 1 thread, will synchronize iterations
 # Benchmark mode: Average time, time/op
 # Benchmark: 
 org.apache.aurora.benchmark.SchedulingBenchmarks.ConstraintMismatchsSchedulingBenchmark.runBenchmark
 
 # Run progress: 0.00% complete, ETA 00:05:30
 # Fork: 1 of 1
 # Warmup Iteration   1: 278284250.000 ns/op
 # Warmup Iteration   2: 70294312.500 ns/op
 # Warmup Iteration   3: 19293379.310 ns/op
 # Warmup Iteration   4: 11945387.097 ns/op
 # Warmup Iteration   5: 10725388.350 ns/op
 # Warmup Iteration   6: 13043282.353 ns/op
 # Warmup Iteration   7: 9233083.333 ns/op
 # Warmup Iteration   8: 9521051.724 ns/op
 # Warmup Iteration   9: 10750854.369 ns/op
 # Warmup Iteration  10: 7460243.243 ns/op
 Iteration   1: 7885364.286 ns/op
 Iteration   2: 7735139.860 ns/op
 Iteration   3: 7660208.333 ns/op
 Iteration   4: 7761204.225 ns/op
 Iteration   5: 7868907.143 ns/op
 Iteration   6: 7567404.110 ns/op
 Iteration   7: 7611000.000 ns/op
 Iteration   8: 7766154.930 ns/op
 Iteration   9: 7669344.828 ns/op
 Iteration  10: 7707783.217 ns/op
 Iteration  11: 7435651.007 ns/op
 Iteration  12: 7697631.944 ns/op
 Iteration  13: 7712531.469 ns/op
 Iteration  14: 7899407.143 ns/op
 Iteration  15: 7448472.973 ns/op
 Iteration  16: 7791521.127 ns/op
 Iteration  17: 7612213.793 ns/op
 Iteration  18: 7710867.133 ns/op
 Iteration  19: 7649296.552 ns/op
 Iteration  20: 7768309.859 ns/op
 Iteration  21: 7688666.667 ns/op
 Iteration  22: 7531557.823 ns/op
 Iteration  23: 7381193.333 ns/op
 Iteration  24: 7726006.993 ns/op
 Iteration  25: 7603358.621 ns/op
 Iteration  26: 7653631.944 ns/op
 Iteration  27: 7442275.168 ns/op
 Iteration  28: 7613186.207 ns/op
 Iteration  29: 7765823.944 ns/op
 Iteration  30: 7489687.075 ns/op
 Iteration  31: 7811443.662 ns/op
 Iteration  32: 8015007.246 ns/op
 Iteration  33: 8192392.593 ns/op
 Iteration  34: 8040335.766 ns/op
 Iteration  35: 7584212.329 ns/op
 Iteration  36: 8001934.783 ns/op
 Iteration  37: 9744815.789 ns/op
 Iteration  38: 11688284.211 ns/op
 Iteration  39: 8661406.250 ns/op
 Iteration  40: 7678413.793 ns/op
 Iteration  41: 8502223.077 ns/op
 Iteration  42: 7640820.690 ns/op
 Iteration  43: 7875624.113 ns/op
 Iteration  44: 7506809.524 ns/op
 Iteration  45: 8005431.655 ns/op
 Iteration  46: 8081664.234 ns/op
 Iteration  47: 7579438.356 ns/op
 Iteration  48: 7993405.797 ns/op
 Iteration  49: 7571958.904 ns/op
 Iteration  50: