Re: Review Request 45222: Introduce "preemptible" flag in TierInfo with backward compatible support for "production" flag in TaskConfig.

2016-03-30 Thread Aurora ReviewBot

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



Master (55a2422) is red with this patch.
  ./build-support/jenkins/build.sh

   proxy_driver = ProxyDriver()
   with temporary_dir() as checkpoint_root:
 te = AuroraExecutor(
 >   
runner_provider=make_provider(checkpoint_root),
 
sandbox_provider=DefaultTestSandboxProvider())
 
 
src/test/python/apache/aurora/executor/test_thermos_executor.py:580: 
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 
src/test/python/apache/aurora/executor/test_thermos_executor.py:193: in 
make_provider
 pex_location=thermos_runner_path(),
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 
 build = True
 
 def thermos_runner_path(build=True):
   if not build:
 return getattr(thermos_runner_path, 'value', 
None)
 
   if not hasattr(thermos_runner_path, 'value'):
 pex_dir = safe_mkdtemp()
 >   assert subprocess.call(["./pants", 
"--pants-distdir=%s" % pex_dir, "binary",
   
"src/main/python/apache/thermos/runner:thermos_runner"]) == 0
 E   assert 1 == 0
 E+  where 1 = (['./pants', '--pants-distdir=/tmp/user/10021/tmpjUDijX', 
'binary', 'src/main/python/apache/thermos/runner:thermos_runner'])
 E+where  = subprocess.call
 
 
src/test/python/apache/aurora/executor/test_thermos_executor.py:185: 
AssertionError
 -- Captured stderr call --
 Traceback (most recent call last):
   File 
"/home/jenkins/.cache/pants/setup/bootstrap-Linux-x86_64/0.0.75/bin/pants", 
line 7, in 
 from pants.bin.pants_exe import main
 ImportError: No module named pants.bin.pants_exe
  generated xml file: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/415337499eb72578eab327a6487c1f5c9452b3d6.xml
 
  17 failed, 644 passed, 5 skipped, 1 warnings, 8 
error in 252.03 seconds 
 
FAILURE


23:21:11 05:37   [complete]
   FAILURE


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

- Aurora ReviewBot


On March 30, 2016, 11 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45222/
> ---
> 
> (Updated March 30, 2016, 11 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1616
> https://issues.apache.org/jira/browse/AURORA-1616
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Introduce "preemptible" flag in TierInfo with backward compatible support for 
> "production" flag in TaskConfig.
> 
> # Summary of changes
> - TierInfo extended to manage a new property: "preemptible"
> - If TaskConfig does not specify a tier, ``TierManager.getTier(..)`` falls 
> back to selecting a non-revocable tier matching ``TaskConfig.isProduction()``.
> - Removed ``TierInfo.DEFAULT`` and replaced its in test/benchmark code by 
> references to ``TaskTestUtil.DEV_TIER``.
> - Eager validation of tier specified in TaskConfig in 
> ``ConfigurationManager.validateAndPopulate(..)``
> 
> # Note on backward incompatible change
> TaskConfigs with a tier name not matching any tiers defined in ``tiers.json`` 
> were silently assigned a default tier. With this change, attempting to 
> schedule tasks that specify non-existent tier names will result in an error 
> (see ``ConfigurationManager.validateAndPopulate(ITaskConfig config)``).
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 4eebc9df16c7222fe2c40df2237f77c5d4e76e3a 
>   docs/reference/configuration.md c9459fe9d655ac0668af87b62dd5e8258bdb0f35 
>   src/jmh/java/org/apache/aurora/benchmark/Offers.java 
> 055a2ffcb13c643a3086343e3fbf71545c5fb0a6 
>   src/main/java/org/apache/aurora/scheduler/TierInfo.java 
> b4611b9cf99ca236cd1ea4610d01c0d293bf2e87 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java 
> 

Re: Review Request 45222: Introduce "preemptible" flag in TierInfo with backward compatible support for "production" flag in TaskConfig.

2016-03-30 Thread Amol Deshmukh

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

(Updated March 30, 2016, 4 p.m.)


Review request for Aurora, Joshua Cohen and Maxim Khutornenko.


Changes
---

- Merged remote HEAD and resolved conflict.


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


Repository: aurora


Description
---

Introduce "preemptible" flag in TierInfo with backward compatible support for 
"production" flag in TaskConfig.

# Summary of changes
- TierInfo extended to manage a new property: "preemptible"
- If TaskConfig does not specify a tier, ``TierManager.getTier(..)`` falls back 
to selecting a non-revocable tier matching ``TaskConfig.isProduction()``.
- Removed ``TierInfo.DEFAULT`` and replaced its in test/benchmark code by 
references to ``TaskTestUtil.DEV_TIER``.
- Eager validation of tier specified in TaskConfig in 
``ConfigurationManager.validateAndPopulate(..)``

# Note on backward incompatible change
TaskConfigs with a tier name not matching any tiers defined in ``tiers.json`` 
were silently assigned a default tier. With this change, attempting to schedule 
tasks that specify non-existent tier names will result in an error (see 
``ConfigurationManager.validateAndPopulate(ITaskConfig config)``).


Diffs (updated)
-

  RELEASE-NOTES.md 4eebc9df16c7222fe2c40df2237f77c5d4e76e3a 
  docs/reference/configuration.md c9459fe9d655ac0668af87b62dd5e8258bdb0f35 
  src/jmh/java/org/apache/aurora/benchmark/Offers.java 
055a2ffcb13c643a3086343e3fbf71545c5fb0a6 
  src/main/java/org/apache/aurora/scheduler/TierInfo.java 
b4611b9cf99ca236cd1ea4610d01c0d293bf2e87 
  src/main/java/org/apache/aurora/scheduler/TierManager.java 
480c4853a6c87dd63a9810ae013e5cfacb29336d 
  src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
97d87ff1b789625f2c07baf7a74a076f07742596 
  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
9cf8002e932d562e93fb8d17b4c56f564eb54ed5 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 9d2bc825edef7fabeccd2334db48acc1bf622eb0 
  src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
16fa2d35cdf7ecdf82dd1ec7739e685ec1ff1aa2 
  
src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java 
7f84e90774193b0d31adb7dafcab0a249167cdba 
  src/main/resources/org/apache/aurora/scheduler/tiers.json 
18701058076bedc5d1b667e2b97ad09ce84a9bb9 
  src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java 
39096af816864ada32a9c07958975740e805f6b0 
  src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
52113b80d91ecaf0cc2aeaad77e5fbc0ea4d1216 
  src/test/java/org/apache/aurora/scheduler/ResourcesTest.java 
4fe8c518c580418078275b8056a5c195a765681e 
  src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 
a662100ba726cff0e47b4f9650753db9cecdef51 
  src/test/java/org/apache/aurora/scheduler/TierModuleTest.java 
e0601c83486671596e412f022ffae78b01c81c9d 
  
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
 1a520b3cb035a9afc25406d2f313c9f861eee4d6 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
5103bd0f43d53079976b0f1596e299f2d91aa860 
  
src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
 b6f5e4632ac1e028fdf93da1735463373e2d2788 
  src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
b00add0b2fd4277e196505fffba4440e2e94207e 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
b1c71a6f1847d205c378d0b5a7269ea9d1165be5 

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


Testing
---

# End to end tests
```
$ ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
...
*** OK (All tests passed) ***
```

# Jenkins build.sh
```
$ ./build-support/jenkins/build.sh
...
   SUCCESS
```

# Local Scheduler
```
$ ./gradlew run
...
I0325 16:00:55.374 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
suppressing offer cycle.
I0325 16:01:00.371 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
suppressing offer cycle.
```

# Attempting to schedule job with invalid tier-name
```
vagrant@aurora:~/workspace$ aurora job create devcluster/vagrant/devel/test 
job.aurora
 INFO] Creating job test
Job creation failed due to error:
Invalid tier 'badtier' in TaskConfig.
```


Thanks,

Amol Deshmukh



Re: Review Request 45222: Introduce "preemptible" flag in TierInfo with backward compatible support for "production" flag in TaskConfig.

2016-03-30 Thread Joshua Cohen


> On March 30, 2016, 8:47 p.m., Joshua Cohen wrote:
> > Ship It!

Can you rebase and fix the conflicts in RELEASE-NOTES.md? Then I'll commit this.


- Joshua


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


On March 29, 2016, 5:51 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45222/
> ---
> 
> (Updated March 29, 2016, 5:51 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1616
> https://issues.apache.org/jira/browse/AURORA-1616
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Introduce "preemptible" flag in TierInfo with backward compatible support for 
> "production" flag in TaskConfig.
> 
> # Summary of changes
> - TierInfo extended to manage a new property: "preemptible"
> - If TaskConfig does not specify a tier, ``TierManager.getTier(..)`` falls 
> back to selecting a non-revocable tier matching ``TaskConfig.isProduction()``.
> - Removed ``TierInfo.DEFAULT`` and replaced its in test/benchmark code by 
> references to ``TaskTestUtil.DEV_TIER``.
> - Eager validation of tier specified in TaskConfig in 
> ``ConfigurationManager.validateAndPopulate(..)``
> 
> # Note on backward incompatible change
> TaskConfigs with a tier name not matching any tiers defined in ``tiers.json`` 
> were silently assigned a default tier. With this change, attempting to 
> schedule tasks that specify non-existent tier names will result in an error 
> (see ``ConfigurationManager.validateAndPopulate(ITaskConfig config)``).
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 3a83961f5133fc0021b7c61f1117488703a9849d 
>   docs/reference/configuration.md fa09fd47b7637c035a5a23aaf6e3bc1c2c7fc143 
>   src/jmh/java/org/apache/aurora/benchmark/Offers.java 
> 055a2ffcb13c643a3086343e3fbf71545c5fb0a6 
>   src/main/java/org/apache/aurora/scheduler/TierInfo.java 
> b4611b9cf99ca236cd1ea4610d01c0d293bf2e87 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java 
> 480c4853a6c87dd63a9810ae013e5cfacb29336d 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 97d87ff1b789625f2c07baf7a74a076f07742596 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 9cf8002e932d562e93fb8d17b4c56f564eb54ed5 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  9d2bc825edef7fabeccd2334db48acc1bf622eb0 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 16fa2d35cdf7ecdf82dd1ec7739e685ec1ff1aa2 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
>  7f84e90774193b0d31adb7dafcab0a249167cdba 
>   src/main/resources/org/apache/aurora/scheduler/tiers.json 
> 18701058076bedc5d1b667e2b97ad09ce84a9bb9 
>   src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java 
> 39096af816864ada32a9c07958975740e805f6b0 
>   src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
> 52113b80d91ecaf0cc2aeaad77e5fbc0ea4d1216 
>   src/test/java/org/apache/aurora/scheduler/ResourcesTest.java 
> 4fe8c518c580418078275b8056a5c195a765681e 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 
> a662100ba726cff0e47b4f9650753db9cecdef51 
>   src/test/java/org/apache/aurora/scheduler/TierModuleTest.java 
> e0601c83486671596e412f022ffae78b01c81c9d 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  1a520b3cb035a9afc25406d2f313c9f861eee4d6 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 5103bd0f43d53079976b0f1596e299f2d91aa860 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
>  b6f5e4632ac1e028fdf93da1735463373e2d2788 
>   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
> b00add0b2fd4277e196505fffba4440e2e94207e 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> b1c71a6f1847d205c378d0b5a7269ea9d1165be5 
> 
> Diff: https://reviews.apache.org/r/45222/diff/
> 
> 
> Testing
> ---
> 
> # End to end tests
> ```
> $ ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ...
> *** OK (All tests passed) ***
> ```
> 
> # Jenkins build.sh
> ```
> $ ./build-support/jenkins/build.sh
> ...
>SUCCESS
> ```
> 
> # Local Scheduler
> ```
> $ ./gradlew run
> ...
> I0325 16:00:55.374 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
> suppressing offer cycle.
> I0325 16:01:00.371 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
> suppressing offer cycle.
> ```
> 
> # Attempting to schedule job with invalid tier-name
> ```
> vagrant@aurora:~/workspace$ aurora job create devcluster/vagrant/devel/test 
> 

Re: Review Request 45222: Introduce "preemptible" flag in TierInfo with backward compatible support for "production" flag in TaskConfig.

2016-03-30 Thread Joshua Cohen

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


Ship it!




Ship It!

- Joshua Cohen


On March 29, 2016, 5:51 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45222/
> ---
> 
> (Updated March 29, 2016, 5:51 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1616
> https://issues.apache.org/jira/browse/AURORA-1616
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Introduce "preemptible" flag in TierInfo with backward compatible support for 
> "production" flag in TaskConfig.
> 
> # Summary of changes
> - TierInfo extended to manage a new property: "preemptible"
> - If TaskConfig does not specify a tier, ``TierManager.getTier(..)`` falls 
> back to selecting a non-revocable tier matching ``TaskConfig.isProduction()``.
> - Removed ``TierInfo.DEFAULT`` and replaced its in test/benchmark code by 
> references to ``TaskTestUtil.DEV_TIER``.
> - Eager validation of tier specified in TaskConfig in 
> ``ConfigurationManager.validateAndPopulate(..)``
> 
> # Note on backward incompatible change
> TaskConfigs with a tier name not matching any tiers defined in ``tiers.json`` 
> were silently assigned a default tier. With this change, attempting to 
> schedule tasks that specify non-existent tier names will result in an error 
> (see ``ConfigurationManager.validateAndPopulate(ITaskConfig config)``).
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 3a83961f5133fc0021b7c61f1117488703a9849d 
>   docs/reference/configuration.md fa09fd47b7637c035a5a23aaf6e3bc1c2c7fc143 
>   src/jmh/java/org/apache/aurora/benchmark/Offers.java 
> 055a2ffcb13c643a3086343e3fbf71545c5fb0a6 
>   src/main/java/org/apache/aurora/scheduler/TierInfo.java 
> b4611b9cf99ca236cd1ea4610d01c0d293bf2e87 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java 
> 480c4853a6c87dd63a9810ae013e5cfacb29336d 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 97d87ff1b789625f2c07baf7a74a076f07742596 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 9cf8002e932d562e93fb8d17b4c56f564eb54ed5 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  9d2bc825edef7fabeccd2334db48acc1bf622eb0 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 16fa2d35cdf7ecdf82dd1ec7739e685ec1ff1aa2 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
>  7f84e90774193b0d31adb7dafcab0a249167cdba 
>   src/main/resources/org/apache/aurora/scheduler/tiers.json 
> 18701058076bedc5d1b667e2b97ad09ce84a9bb9 
>   src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java 
> 39096af816864ada32a9c07958975740e805f6b0 
>   src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
> 52113b80d91ecaf0cc2aeaad77e5fbc0ea4d1216 
>   src/test/java/org/apache/aurora/scheduler/ResourcesTest.java 
> 4fe8c518c580418078275b8056a5c195a765681e 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 
> a662100ba726cff0e47b4f9650753db9cecdef51 
>   src/test/java/org/apache/aurora/scheduler/TierModuleTest.java 
> e0601c83486671596e412f022ffae78b01c81c9d 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  1a520b3cb035a9afc25406d2f313c9f861eee4d6 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 5103bd0f43d53079976b0f1596e299f2d91aa860 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
>  b6f5e4632ac1e028fdf93da1735463373e2d2788 
>   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
> b00add0b2fd4277e196505fffba4440e2e94207e 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> b1c71a6f1847d205c378d0b5a7269ea9d1165be5 
> 
> Diff: https://reviews.apache.org/r/45222/diff/
> 
> 
> Testing
> ---
> 
> # End to end tests
> ```
> $ ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ...
> *** OK (All tests passed) ***
> ```
> 
> # Jenkins build.sh
> ```
> $ ./build-support/jenkins/build.sh
> ...
>SUCCESS
> ```
> 
> # Local Scheduler
> ```
> $ ./gradlew run
> ...
> I0325 16:00:55.374 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
> suppressing offer cycle.
> I0325 16:01:00.371 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
> suppressing offer cycle.
> ```
> 
> # Attempting to schedule job with invalid tier-name
> ```
> vagrant@aurora:~/workspace$ aurora job create devcluster/vagrant/devel/test 
> job.aurora
>  INFO] Creating job test
> Job creation failed due to error:
>   Invalid tier 'badtier' in TaskConfig.
> ```

Re: Review Request 45222: Introduce "preemptible" flag in TierInfo with backward compatible support for "production" flag in TaskConfig.

2016-03-30 Thread Bill Farner


> On March 30, 2016, 9:49 a.m., Joshua Cohen wrote:
> > Code-wise this looks fine to me, however, I have some reservations about 
> > making tier required. I think that throwing a `TaskDescriptionError` when 
> > tier is defined, but not valid is fine, but can we/should we continue to 
> > default to some known value if tier is undefined?
> 
> Bill Farner wrote:
> Emphatic +1 - tier should never be required.
> 
> Amol Deshmukh wrote:
> # Scope of this change
> To clarify, this change does not require specifying a tier in the job 
> configuration. With this change, the **default out-of-box behavior** w.r.t. 
> tiers will remain largely unchanged:
> 1. If job configuration specifies a valid tier, that tier will be used.
> 2. If job configuration does not specify any tier, a tier will be 
> selected as follows:
> ```
> TaskConfig.production == true => "preferred"
> TaskConfig.production == false => "preemptible"
> ```
> 3. [Backward Incompatibility] If a job configuration specifies an invalid 
> tier, the scheduling attempt will fail (rather than select a default tier).
> 
> If the cluster administrator chooses to customize the tier configuration 
> so that:
> 1. **One of the ``preemptible`` or ``preferred`` tiers is removed** 
> (expected likelihood: low), attempts to schedule jobs with ``production = 
> false`` or ``production = true`` respectively, will fail.
> 2. **The ``revocable`` tier is removed** (expected likelihood: medium), 
> only attempts to schedule jobs that explicitly request ``tier = "revocable"`` 
> will fail.
> 3. **The provided tiers are renamed** (expected likelihood: very low), 
> client configurations that reference the tiers explicitly by the old names 
> will fail to schedule.
> 
> # Looking ahead
> The plan going forward as described in the proposal at 
> https://docs.google.com/document/d/1erszT-HsWf1zCIfhbqHlsotHxWUvDyI2xUwNQQQxLgs/edit#heading=h.5htko8wan7dm,
>  was indeed to make tiers required in client configuration. The immediate 
> driver for this was to deprecate the ``production`` flag in TaskConfig.
> 
> As an alternative to making tiers mandatory, I'd like to propose a change 
> to the tier configuration file to allow specifying a ``default`` tier. This 
> will yield control to the cluster administrator in determining the default 
> tier rather than bake the default into the code as a constant. If this 
> approach seems reasonable:
> 1. I will update the proposal accordingly.
> 2. Implement the support for default tier in tier configuration.

| To clarify, this change does not require specifying a tier in the job 
configuration.

Understood, i'm stating a preference on the direction; and was tipped off by a 
comment i spotted in this review.  I will continue this line of thought in 
[AURORA-1624](https://issues.apache.org/jira/browse/AURORA-1624) instead.


- Bill


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


On March 29, 2016, 10:51 a.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45222/
> ---
> 
> (Updated March 29, 2016, 10:51 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1616
> https://issues.apache.org/jira/browse/AURORA-1616
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Introduce "preemptible" flag in TierInfo with backward compatible support for 
> "production" flag in TaskConfig.
> 
> # Summary of changes
> - TierInfo extended to manage a new property: "preemptible"
> - If TaskConfig does not specify a tier, ``TierManager.getTier(..)`` falls 
> back to selecting a non-revocable tier matching ``TaskConfig.isProduction()``.
> - Removed ``TierInfo.DEFAULT`` and replaced its in test/benchmark code by 
> references to ``TaskTestUtil.DEV_TIER``.
> - Eager validation of tier specified in TaskConfig in 
> ``ConfigurationManager.validateAndPopulate(..)``
> 
> # Note on backward incompatible change
> TaskConfigs with a tier name not matching any tiers defined in ``tiers.json`` 
> were silently assigned a default tier. With this change, attempting to 
> schedule tasks that specify non-existent tier names will result in an error 
> (see ``ConfigurationManager.validateAndPopulate(ITaskConfig config)``).
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 3a83961f5133fc0021b7c61f1117488703a9849d 
>   docs/reference/configuration.md fa09fd47b7637c035a5a23aaf6e3bc1c2c7fc143 
>   src/jmh/java/org/apache/aurora/benchmark/Offers.java 
> 055a2ffcb13c643a3086343e3fbf71545c5fb0a6 
>   src/main/java/org/apache/aurora/scheduler/TierInfo.java 
> 

Re: Review Request 45222: Introduce "preemptible" flag in TierInfo with backward compatible support for "production" flag in TaskConfig.

2016-03-30 Thread Amol Deshmukh


> On March 30, 2016, 9:49 a.m., Joshua Cohen wrote:
> > Code-wise this looks fine to me, however, I have some reservations about 
> > making tier required. I think that throwing a `TaskDescriptionError` when 
> > tier is defined, but not valid is fine, but can we/should we continue to 
> > default to some known value if tier is undefined?
> 
> Bill Farner wrote:
> Emphatic +1 - tier should never be required.

# Scope of this change
To clarify, this change does not require specifying a tier in the job 
configuration. With this change, the **default out-of-box behavior** w.r.t. 
tiers will remain largely unchanged:
1. If job configuration specifies a valid tier, that tier will be used.
2. If job configuration does not specify any tier, a tier will be selected as 
follows:
```
TaskConfig.production == true => "preferred"
TaskConfig.production == false => "preemptible"
```
3. [Backward Incompatibility] If a job configuration specifies an invalid tier, 
the scheduling attempt will fail (rather than select a default tier).

If the cluster administrator chooses to customize the tier configuration so 
that:
1. **One of the ``preemptible`` or ``preferred`` tiers is removed** (expected 
likelihood: low), attempts to schedule jobs with ``production = false`` or 
``production = true`` respectively, will fail.
2. **The ``revocable`` tier is removed** (expected likelihood: medium), only 
attempts to schedule jobs that explicitly request ``tier = "revocable"`` will 
fail.
3. **The provided tiers are renamed** (expected likelihood: very low), client 
configurations that reference the tiers explicitly by the old names will fail 
to schedule.

# Looking ahead
The plan going forward as described in the proposal at 
https://docs.google.com/document/d/1erszT-HsWf1zCIfhbqHlsotHxWUvDyI2xUwNQQQxLgs/edit#heading=h.5htko8wan7dm,
 was indeed to make tiers required in client configuration. The immediate 
driver for this was to deprecate the ``production`` flag in TaskConfig.

As an alternative to making tiers mandatory, I'd like to propose a change to 
the tier configuration file to allow specifying a ``default`` tier. This will 
yield control to the cluster administrator in determining the default tier 
rather than bake the default into the code as a constant. If this approach 
seems reasonable:
1. I will update the proposal accordingly.
2. Implement the support for default tier in tier configuration.


- Amol


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


On March 29, 2016, 10:51 a.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45222/
> ---
> 
> (Updated March 29, 2016, 10:51 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1616
> https://issues.apache.org/jira/browse/AURORA-1616
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Introduce "preemptible" flag in TierInfo with backward compatible support for 
> "production" flag in TaskConfig.
> 
> # Summary of changes
> - TierInfo extended to manage a new property: "preemptible"
> - If TaskConfig does not specify a tier, ``TierManager.getTier(..)`` falls 
> back to selecting a non-revocable tier matching ``TaskConfig.isProduction()``.
> - Removed ``TierInfo.DEFAULT`` and replaced its in test/benchmark code by 
> references to ``TaskTestUtil.DEV_TIER``.
> - Eager validation of tier specified in TaskConfig in 
> ``ConfigurationManager.validateAndPopulate(..)``
> 
> # Note on backward incompatible change
> TaskConfigs with a tier name not matching any tiers defined in ``tiers.json`` 
> were silently assigned a default tier. With this change, attempting to 
> schedule tasks that specify non-existent tier names will result in an error 
> (see ``ConfigurationManager.validateAndPopulate(ITaskConfig config)``).
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 3a83961f5133fc0021b7c61f1117488703a9849d 
>   docs/reference/configuration.md fa09fd47b7637c035a5a23aaf6e3bc1c2c7fc143 
>   src/jmh/java/org/apache/aurora/benchmark/Offers.java 
> 055a2ffcb13c643a3086343e3fbf71545c5fb0a6 
>   src/main/java/org/apache/aurora/scheduler/TierInfo.java 
> b4611b9cf99ca236cd1ea4610d01c0d293bf2e87 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java 
> 480c4853a6c87dd63a9810ae013e5cfacb29336d 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 97d87ff1b789625f2c07baf7a74a076f07742596 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 9cf8002e932d562e93fb8d17b4c56f564eb54ed5 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  9d2bc825edef7fabeccd2334db48acc1bf622eb0 
>   

Re: Review Request 45222: Introduce "preemptible" flag in TierInfo with backward compatible support for "production" flag in TaskConfig.

2016-03-30 Thread Bill Farner


> On March 30, 2016, 9:49 a.m., Joshua Cohen wrote:
> > Code-wise this looks fine to me, however, I have some reservations about 
> > making tier required. I think that throwing a `TaskDescriptionError` when 
> > tier is defined, but not valid is fine, but can we/should we continue to 
> > default to some known value if tier is undefined?

Emphatic +1 - tier should never be required.


- Bill


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


On March 29, 2016, 10:51 a.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45222/
> ---
> 
> (Updated March 29, 2016, 10:51 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1616
> https://issues.apache.org/jira/browse/AURORA-1616
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Introduce "preemptible" flag in TierInfo with backward compatible support for 
> "production" flag in TaskConfig.
> 
> # Summary of changes
> - TierInfo extended to manage a new property: "preemptible"
> - If TaskConfig does not specify a tier, ``TierManager.getTier(..)`` falls 
> back to selecting a non-revocable tier matching ``TaskConfig.isProduction()``.
> - Removed ``TierInfo.DEFAULT`` and replaced its in test/benchmark code by 
> references to ``TaskTestUtil.DEV_TIER``.
> - Eager validation of tier specified in TaskConfig in 
> ``ConfigurationManager.validateAndPopulate(..)``
> 
> # Note on backward incompatible change
> TaskConfigs with a tier name not matching any tiers defined in ``tiers.json`` 
> were silently assigned a default tier. With this change, attempting to 
> schedule tasks that specify non-existent tier names will result in an error 
> (see ``ConfigurationManager.validateAndPopulate(ITaskConfig config)``).
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 3a83961f5133fc0021b7c61f1117488703a9849d 
>   docs/reference/configuration.md fa09fd47b7637c035a5a23aaf6e3bc1c2c7fc143 
>   src/jmh/java/org/apache/aurora/benchmark/Offers.java 
> 055a2ffcb13c643a3086343e3fbf71545c5fb0a6 
>   src/main/java/org/apache/aurora/scheduler/TierInfo.java 
> b4611b9cf99ca236cd1ea4610d01c0d293bf2e87 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java 
> 480c4853a6c87dd63a9810ae013e5cfacb29336d 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 97d87ff1b789625f2c07baf7a74a076f07742596 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 9cf8002e932d562e93fb8d17b4c56f564eb54ed5 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  9d2bc825edef7fabeccd2334db48acc1bf622eb0 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 16fa2d35cdf7ecdf82dd1ec7739e685ec1ff1aa2 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
>  7f84e90774193b0d31adb7dafcab0a249167cdba 
>   src/main/resources/org/apache/aurora/scheduler/tiers.json 
> 18701058076bedc5d1b667e2b97ad09ce84a9bb9 
>   src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java 
> 39096af816864ada32a9c07958975740e805f6b0 
>   src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
> 52113b80d91ecaf0cc2aeaad77e5fbc0ea4d1216 
>   src/test/java/org/apache/aurora/scheduler/ResourcesTest.java 
> 4fe8c518c580418078275b8056a5c195a765681e 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 
> a662100ba726cff0e47b4f9650753db9cecdef51 
>   src/test/java/org/apache/aurora/scheduler/TierModuleTest.java 
> e0601c83486671596e412f022ffae78b01c81c9d 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  1a520b3cb035a9afc25406d2f313c9f861eee4d6 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 5103bd0f43d53079976b0f1596e299f2d91aa860 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
>  b6f5e4632ac1e028fdf93da1735463373e2d2788 
>   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
> b00add0b2fd4277e196505fffba4440e2e94207e 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> b1c71a6f1847d205c378d0b5a7269ea9d1165be5 
> 
> Diff: https://reviews.apache.org/r/45222/diff/
> 
> 
> Testing
> ---
> 
> # End to end tests
> ```
> $ ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ...
> *** OK (All tests passed) ***
> ```
> 
> # Jenkins build.sh
> ```
> $ ./build-support/jenkins/build.sh
> ...
>SUCCESS
> ```
> 
> # Local Scheduler
> ```
> $ ./gradlew run
> ...
> I0325 16:00:55.374 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
> suppressing offer cycle.
> I0325 

Re: Review Request 45222: Introduce "preemptible" flag in TierInfo with backward compatible support for "production" flag in TaskConfig.

2016-03-30 Thread Joshua Cohen

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



Code-wise this looks fine to me, however, I have some reservations about making 
tier required. I think that throwing a `TaskDescriptionError` when tier is 
defined, but not valid is fine, but can we/should we continue to default to 
some known value if tier is undefined?

- Joshua Cohen


On March 29, 2016, 5:51 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45222/
> ---
> 
> (Updated March 29, 2016, 5:51 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1616
> https://issues.apache.org/jira/browse/AURORA-1616
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Introduce "preemptible" flag in TierInfo with backward compatible support for 
> "production" flag in TaskConfig.
> 
> # Summary of changes
> - TierInfo extended to manage a new property: "preemptible"
> - If TaskConfig does not specify a tier, ``TierManager.getTier(..)`` falls 
> back to selecting a non-revocable tier matching ``TaskConfig.isProduction()``.
> - Removed ``TierInfo.DEFAULT`` and replaced its in test/benchmark code by 
> references to ``TaskTestUtil.DEV_TIER``.
> - Eager validation of tier specified in TaskConfig in 
> ``ConfigurationManager.validateAndPopulate(..)``
> 
> # Note on backward incompatible change
> TaskConfigs with a tier name not matching any tiers defined in ``tiers.json`` 
> were silently assigned a default tier. With this change, attempting to 
> schedule tasks that specify non-existent tier names will result in an error 
> (see ``ConfigurationManager.validateAndPopulate(ITaskConfig config)``).
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 3a83961f5133fc0021b7c61f1117488703a9849d 
>   docs/reference/configuration.md fa09fd47b7637c035a5a23aaf6e3bc1c2c7fc143 
>   src/jmh/java/org/apache/aurora/benchmark/Offers.java 
> 055a2ffcb13c643a3086343e3fbf71545c5fb0a6 
>   src/main/java/org/apache/aurora/scheduler/TierInfo.java 
> b4611b9cf99ca236cd1ea4610d01c0d293bf2e87 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java 
> 480c4853a6c87dd63a9810ae013e5cfacb29336d 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 97d87ff1b789625f2c07baf7a74a076f07742596 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 9cf8002e932d562e93fb8d17b4c56f564eb54ed5 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  9d2bc825edef7fabeccd2334db48acc1bf622eb0 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 16fa2d35cdf7ecdf82dd1ec7739e685ec1ff1aa2 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
>  7f84e90774193b0d31adb7dafcab0a249167cdba 
>   src/main/resources/org/apache/aurora/scheduler/tiers.json 
> 18701058076bedc5d1b667e2b97ad09ce84a9bb9 
>   src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java 
> 39096af816864ada32a9c07958975740e805f6b0 
>   src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
> 52113b80d91ecaf0cc2aeaad77e5fbc0ea4d1216 
>   src/test/java/org/apache/aurora/scheduler/ResourcesTest.java 
> 4fe8c518c580418078275b8056a5c195a765681e 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 
> a662100ba726cff0e47b4f9650753db9cecdef51 
>   src/test/java/org/apache/aurora/scheduler/TierModuleTest.java 
> e0601c83486671596e412f022ffae78b01c81c9d 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  1a520b3cb035a9afc25406d2f313c9f861eee4d6 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 5103bd0f43d53079976b0f1596e299f2d91aa860 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
>  b6f5e4632ac1e028fdf93da1735463373e2d2788 
>   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
> b00add0b2fd4277e196505fffba4440e2e94207e 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> b1c71a6f1847d205c378d0b5a7269ea9d1165be5 
> 
> Diff: https://reviews.apache.org/r/45222/diff/
> 
> 
> Testing
> ---
> 
> # End to end tests
> ```
> $ ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ...
> *** OK (All tests passed) ***
> ```
> 
> # Jenkins build.sh
> ```
> $ ./build-support/jenkins/build.sh
> ...
>SUCCESS
> ```
> 
> # Local Scheduler
> ```
> $ ./gradlew run
> ...
> I0325 16:00:55.374 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
> suppressing offer cycle.
> I0325 16:01:00.371 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
> suppressing offer cycle.
> ```
> 
> # Attempting 

Re: Review Request 45222: Introduce "preemptible" flag in TierInfo with backward compatible support for "production" flag in TaskConfig.

2016-03-29 Thread Aurora ReviewBot

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


Ship it!




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

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

- Aurora ReviewBot


On March 29, 2016, 5:51 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45222/
> ---
> 
> (Updated March 29, 2016, 5:51 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1616
> https://issues.apache.org/jira/browse/AURORA-1616
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Introduce "preemptible" flag in TierInfo with backward compatible support for 
> "production" flag in TaskConfig.
> 
> # Summary of changes
> - TierInfo extended to manage a new property: "preemptible"
> - If TaskConfig does not specify a tier, ``TierManager.getTier(..)`` falls 
> back to selecting a non-revocable tier matching ``TaskConfig.isProduction()``.
> - Removed ``TierInfo.DEFAULT`` and replaced its in test/benchmark code by 
> references to ``TaskTestUtil.DEV_TIER``.
> - Eager validation of tier specified in TaskConfig in 
> ``ConfigurationManager.validateAndPopulate(..)``
> 
> # Note on backward incompatible change
> TaskConfigs with a tier name not matching any tiers defined in ``tiers.json`` 
> were silently assigned a default tier. With this change, attempting to 
> schedule tasks that specify non-existent tier names will result in an error 
> (see ``ConfigurationManager.validateAndPopulate(ITaskConfig config)``).
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md 3a83961f5133fc0021b7c61f1117488703a9849d 
>   docs/reference/configuration.md fa09fd47b7637c035a5a23aaf6e3bc1c2c7fc143 
>   src/jmh/java/org/apache/aurora/benchmark/Offers.java 
> 055a2ffcb13c643a3086343e3fbf71545c5fb0a6 
>   src/main/java/org/apache/aurora/scheduler/TierInfo.java 
> b4611b9cf99ca236cd1ea4610d01c0d293bf2e87 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java 
> 480c4853a6c87dd63a9810ae013e5cfacb29336d 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 97d87ff1b789625f2c07baf7a74a076f07742596 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 9cf8002e932d562e93fb8d17b4c56f564eb54ed5 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  9d2bc825edef7fabeccd2334db48acc1bf622eb0 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 16fa2d35cdf7ecdf82dd1ec7739e685ec1ff1aa2 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
>  7f84e90774193b0d31adb7dafcab0a249167cdba 
>   src/main/resources/org/apache/aurora/scheduler/tiers.json 
> 18701058076bedc5d1b667e2b97ad09ce84a9bb9 
>   src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java 
> 39096af816864ada32a9c07958975740e805f6b0 
>   src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
> 52113b80d91ecaf0cc2aeaad77e5fbc0ea4d1216 
>   src/test/java/org/apache/aurora/scheduler/ResourcesTest.java 
> 4fe8c518c580418078275b8056a5c195a765681e 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 
> a662100ba726cff0e47b4f9650753db9cecdef51 
>   src/test/java/org/apache/aurora/scheduler/TierModuleTest.java 
> e0601c83486671596e412f022ffae78b01c81c9d 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  1a520b3cb035a9afc25406d2f313c9f861eee4d6 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 5103bd0f43d53079976b0f1596e299f2d91aa860 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
>  b6f5e4632ac1e028fdf93da1735463373e2d2788 
>   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
> b00add0b2fd4277e196505fffba4440e2e94207e 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> b1c71a6f1847d205c378d0b5a7269ea9d1165be5 
> 
> Diff: https://reviews.apache.org/r/45222/diff/
> 
> 
> Testing
> ---
> 
> # End to end tests
> ```
> $ ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ...
> *** OK (All tests passed) ***
> ```
> 
> # Jenkins build.sh
> ```
> $ ./build-support/jenkins/build.sh
> ...
>SUCCESS
> ```
> 
> # Local Scheduler
> ```
> $ ./gradlew run
> ...
> I0325 16:00:55.374 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
> suppressing offer cycle.
> I0325 16:01:00.371 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
> suppressing offer cycle.
> ```
> 
> # Attempting to schedule job with invalid tier-name
> ```
> vagrant@aurora:~/workspace$ aurora job create 

Re: Review Request 45222: Introduce "preemptible" flag in TierInfo with backward compatible support for "production" flag in TaskConfig.

2016-03-29 Thread Amol Deshmukh

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

(Updated March 29, 2016, 10:51 a.m.)


Review request for Aurora, Joshua Cohen and Maxim Khutornenko.


Changes
---

- \-wfarner
- Doc changes to account for this change.
- RELEASE notes entry to clarify the backward incompatibility wrt bad tier name 
in job configuration.


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


Repository: aurora


Description
---

Introduce "preemptible" flag in TierInfo with backward compatible support for 
"production" flag in TaskConfig.

# Summary of changes
- TierInfo extended to manage a new property: "preemptible"
- If TaskConfig does not specify a tier, ``TierManager.getTier(..)`` falls back 
to selecting a non-revocable tier matching ``TaskConfig.isProduction()``.
- Removed ``TierInfo.DEFAULT`` and replaced its in test/benchmark code by 
references to ``TaskTestUtil.DEV_TIER``.
- Eager validation of tier specified in TaskConfig in 
``ConfigurationManager.validateAndPopulate(..)``

# Note on backward incompatible change
TaskConfigs with a tier name not matching any tiers defined in ``tiers.json`` 
were silently assigned a default tier. With this change, attempting to schedule 
tasks that specify non-existent tier names will result in an error (see 
``ConfigurationManager.validateAndPopulate(ITaskConfig config)``).


Diffs (updated)
-

  RELEASE-NOTES.md 3a83961f5133fc0021b7c61f1117488703a9849d 
  docs/reference/configuration.md fa09fd47b7637c035a5a23aaf6e3bc1c2c7fc143 
  src/jmh/java/org/apache/aurora/benchmark/Offers.java 
055a2ffcb13c643a3086343e3fbf71545c5fb0a6 
  src/main/java/org/apache/aurora/scheduler/TierInfo.java 
b4611b9cf99ca236cd1ea4610d01c0d293bf2e87 
  src/main/java/org/apache/aurora/scheduler/TierManager.java 
480c4853a6c87dd63a9810ae013e5cfacb29336d 
  src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
97d87ff1b789625f2c07baf7a74a076f07742596 
  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
9cf8002e932d562e93fb8d17b4c56f564eb54ed5 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 9d2bc825edef7fabeccd2334db48acc1bf622eb0 
  src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
16fa2d35cdf7ecdf82dd1ec7739e685ec1ff1aa2 
  
src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java 
7f84e90774193b0d31adb7dafcab0a249167cdba 
  src/main/resources/org/apache/aurora/scheduler/tiers.json 
18701058076bedc5d1b667e2b97ad09ce84a9bb9 
  src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java 
39096af816864ada32a9c07958975740e805f6b0 
  src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
52113b80d91ecaf0cc2aeaad77e5fbc0ea4d1216 
  src/test/java/org/apache/aurora/scheduler/ResourcesTest.java 
4fe8c518c580418078275b8056a5c195a765681e 
  src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 
a662100ba726cff0e47b4f9650753db9cecdef51 
  src/test/java/org/apache/aurora/scheduler/TierModuleTest.java 
e0601c83486671596e412f022ffae78b01c81c9d 
  
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
 1a520b3cb035a9afc25406d2f313c9f861eee4d6 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
5103bd0f43d53079976b0f1596e299f2d91aa860 
  
src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
 b6f5e4632ac1e028fdf93da1735463373e2d2788 
  src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
b00add0b2fd4277e196505fffba4440e2e94207e 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
b1c71a6f1847d205c378d0b5a7269ea9d1165be5 

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


Testing
---

# End to end tests
```
$ ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
...
*** OK (All tests passed) ***
```

# Jenkins build.sh
```
$ ./build-support/jenkins/build.sh
...
   SUCCESS
```

# Local Scheduler
```
$ ./gradlew run
...
I0325 16:00:55.374 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
suppressing offer cycle.
I0325 16:01:00.371 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
suppressing offer cycle.
```

# Attempting to schedule job with invalid tier-name
```
vagrant@aurora:~/workspace$ aurora job create devcluster/vagrant/devel/test 
job.aurora
 INFO] Creating job test
Job creation failed due to error:
Invalid tier 'badtier' in TaskConfig.
```


Thanks,

Amol Deshmukh



Re: Review Request 45222: Introduce "preemptible" flag in TierInfo with backward compatible support for "production" flag in TaskConfig.

2016-03-28 Thread Amol Deshmukh


> On March 28, 2016, 1:23 p.m., Bill Farner wrote:
> > I won't have time to review this, so i'd like to stand down.
> > 
> > However, please make sure to add appropriate release notes, specifically 
> > for the stated backwards incompatibility.

Will do. I will post an updated review request with the release notes entry.


- Amol


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


On March 28, 2016, 12:43 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45222/
> ---
> 
> (Updated March 28, 2016, 12:43 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1616
> https://issues.apache.org/jira/browse/AURORA-1616
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Introduce "preemptible" flag in TierInfo with backward compatible support for 
> "production" flag in TaskConfig.
> 
> # Summary of changes
> - TierInfo extended to manage a new property: "preemptible"
> - If TaskConfig does not specify a tier, ``TierManager.getTier(..)`` falls 
> back to selecting a non-revocable tier matching ``TaskConfig.isProduction()``.
> - Removed ``TierInfo.DEFAULT`` and replaced its in test/benchmark code by 
> references to ``TaskTestUtil.DEV_TIER``.
> - Eager validation of tier specified in TaskConfig in 
> ``ConfigurationManager.validateAndPopulate(..)``
> 
> # Note on backward incompatible change
> TaskConfigs with a tier name not matching any tiers defined in ``tiers.json`` 
> were silently assigned a default tier. With this change, attempting to 
> schedule tasks that specify non-existent tier names will result in an error 
> (see ``ConfigurationManager.validateAndPopulate(ITaskConfig config)``).
> 
> 
> Diffs
> -
> 
>   src/jmh/java/org/apache/aurora/benchmark/Offers.java 
> 055a2ffcb13c643a3086343e3fbf71545c5fb0a6 
>   src/main/java/org/apache/aurora/scheduler/TierInfo.java 
> b4611b9cf99ca236cd1ea4610d01c0d293bf2e87 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java 
> 480c4853a6c87dd63a9810ae013e5cfacb29336d 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 97d87ff1b789625f2c07baf7a74a076f07742596 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 9cf8002e932d562e93fb8d17b4c56f564eb54ed5 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  b3b8ccf868c2a2f18f720a837e90d763072dd3eb 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 16fa2d35cdf7ecdf82dd1ec7739e685ec1ff1aa2 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
>  7f84e90774193b0d31adb7dafcab0a249167cdba 
>   src/main/resources/org/apache/aurora/scheduler/tiers.json 
> 18701058076bedc5d1b667e2b97ad09ce84a9bb9 
>   src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java 
> 39096af816864ada32a9c07958975740e805f6b0 
>   src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
> 52113b80d91ecaf0cc2aeaad77e5fbc0ea4d1216 
>   src/test/java/org/apache/aurora/scheduler/ResourcesTest.java 
> 4fe8c518c580418078275b8056a5c195a765681e 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 
> a662100ba726cff0e47b4f9650753db9cecdef51 
>   src/test/java/org/apache/aurora/scheduler/TierModuleTest.java 
> e0601c83486671596e412f022ffae78b01c81c9d 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  1a520b3cb035a9afc25406d2f313c9f861eee4d6 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 5103bd0f43d53079976b0f1596e299f2d91aa860 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
>  b6f5e4632ac1e028fdf93da1735463373e2d2788 
>   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
> b00add0b2fd4277e196505fffba4440e2e94207e 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> b1c71a6f1847d205c378d0b5a7269ea9d1165be5 
> 
> Diff: https://reviews.apache.org/r/45222/diff/
> 
> 
> Testing
> ---
> 
> # End to end tests
> ```
> $ ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ...
> *** OK (All tests passed) ***
> ```
> 
> # Jenkins build.sh
> ```
> $ ./build-support/jenkins/build.sh
> ...
>SUCCESS
> ```
> 
> # Local Scheduler
> ```
> $ ./gradlew run
> ...
> I0325 16:00:55.374 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
> suppressing offer cycle.
> I0325 16:01:00.371 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
> suppressing offer cycle.
> ```
> 
> # Attempting to schedule job with invalid tier-name
> ```
> vagrant@aurora:~/workspace$ aurora 

Re: Review Request 45222: Introduce "preemptible" flag in TierInfo with backward compatible support for "production" flag in TaskConfig.

2016-03-28 Thread Bill Farner

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



I won't have time to review this, so i'd like to stand down.

However, please make sure to add appropriate release notes, specifically for 
the stated backwards incompatibility.

- Bill Farner


On March 28, 2016, 12:43 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45222/
> ---
> 
> (Updated March 28, 2016, 12:43 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1616
> https://issues.apache.org/jira/browse/AURORA-1616
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Introduce "preemptible" flag in TierInfo with backward compatible support for 
> "production" flag in TaskConfig.
> 
> # Summary of changes
> - TierInfo extended to manage a new property: "preemptible"
> - If TaskConfig does not specify a tier, ``TierManager.getTier(..)`` falls 
> back to selecting a non-revocable tier matching ``TaskConfig.isProduction()``.
> - Removed ``TierInfo.DEFAULT`` and replaced its in test/benchmark code by 
> references to ``TaskTestUtil.DEV_TIER``.
> - Eager validation of tier specified in TaskConfig in 
> ``ConfigurationManager.validateAndPopulate(..)``
> 
> # Note on backward incompatible change
> TaskConfigs with a tier name not matching any tiers defined in ``tiers.json`` 
> were silently assigned a default tier. With this change, attempting to 
> schedule tasks that specify non-existent tier names will result in an error 
> (see ``ConfigurationManager.validateAndPopulate(ITaskConfig config)``).
> 
> 
> Diffs
> -
> 
>   src/jmh/java/org/apache/aurora/benchmark/Offers.java 
> 055a2ffcb13c643a3086343e3fbf71545c5fb0a6 
>   src/main/java/org/apache/aurora/scheduler/TierInfo.java 
> b4611b9cf99ca236cd1ea4610d01c0d293bf2e87 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java 
> 480c4853a6c87dd63a9810ae013e5cfacb29336d 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 97d87ff1b789625f2c07baf7a74a076f07742596 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 9cf8002e932d562e93fb8d17b4c56f564eb54ed5 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  b3b8ccf868c2a2f18f720a837e90d763072dd3eb 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 16fa2d35cdf7ecdf82dd1ec7739e685ec1ff1aa2 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
>  7f84e90774193b0d31adb7dafcab0a249167cdba 
>   src/main/resources/org/apache/aurora/scheduler/tiers.json 
> 18701058076bedc5d1b667e2b97ad09ce84a9bb9 
>   src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java 
> 39096af816864ada32a9c07958975740e805f6b0 
>   src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
> 52113b80d91ecaf0cc2aeaad77e5fbc0ea4d1216 
>   src/test/java/org/apache/aurora/scheduler/ResourcesTest.java 
> 4fe8c518c580418078275b8056a5c195a765681e 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 
> a662100ba726cff0e47b4f9650753db9cecdef51 
>   src/test/java/org/apache/aurora/scheduler/TierModuleTest.java 
> e0601c83486671596e412f022ffae78b01c81c9d 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  1a520b3cb035a9afc25406d2f313c9f861eee4d6 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 5103bd0f43d53079976b0f1596e299f2d91aa860 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
>  b6f5e4632ac1e028fdf93da1735463373e2d2788 
>   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
> b00add0b2fd4277e196505fffba4440e2e94207e 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> b1c71a6f1847d205c378d0b5a7269ea9d1165be5 
> 
> Diff: https://reviews.apache.org/r/45222/diff/
> 
> 
> Testing
> ---
> 
> # End to end tests
> ```
> $ ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ...
> *** OK (All tests passed) ***
> ```
> 
> # Jenkins build.sh
> ```
> $ ./build-support/jenkins/build.sh
> ...
>SUCCESS
> ```
> 
> # Local Scheduler
> ```
> $ ./gradlew run
> ...
> I0325 16:00:55.374 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
> suppressing offer cycle.
> I0325 16:01:00.371 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
> suppressing offer cycle.
> ```
> 
> # Attempting to schedule job with invalid tier-name
> ```
> vagrant@aurora:~/workspace$ aurora job create devcluster/vagrant/devel/test 
> job.aurora
>  INFO] Creating job test
> Job creation failed due to error:
>   Invalid tier 

Re: Review Request 45222: Introduce "preemptible" flag in TierInfo with backward compatible support for "production" flag in TaskConfig.

2016-03-28 Thread Aurora ReviewBot

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



This patch does not apply cleanly against master (0950095), do you need to 
rebase?

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

- Aurora ReviewBot


On March 28, 2016, 7:43 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45222/
> ---
> 
> (Updated March 28, 2016, 7:43 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1616
> https://issues.apache.org/jira/browse/AURORA-1616
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Introduce "preemptible" flag in TierInfo with backward compatible support for 
> "production" flag in TaskConfig.
> 
> # Summary of changes
> - TierInfo extended to manage a new property: "preemptible"
> - If TaskConfig does not specify a tier, ``TierManager.getTier(..)`` falls 
> back to selecting a non-revocable tier matching ``TaskConfig.isProduction()``.
> - Removed ``TierInfo.DEFAULT`` and replaced its in test/benchmark code by 
> references to ``TaskTestUtil.DEV_TIER``.
> - Eager validation of tier specified in TaskConfig in 
> ``ConfigurationManager.validateAndPopulate(..)``
> 
> # Note on backward incompatible change
> TaskConfigs with a tier name not matching any tiers defined in ``tiers.json`` 
> were silently assigned a default tier. With this change, attempting to 
> schedule tasks that specify non-existent tier names will result in an error 
> (see ``ConfigurationManager.validateAndPopulate(ITaskConfig config)``).
> 
> 
> Diffs
> -
> 
>   src/jmh/java/org/apache/aurora/benchmark/Offers.java 
> 055a2ffcb13c643a3086343e3fbf71545c5fb0a6 
>   src/main/java/org/apache/aurora/scheduler/TierInfo.java 
> b4611b9cf99ca236cd1ea4610d01c0d293bf2e87 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java 
> 480c4853a6c87dd63a9810ae013e5cfacb29336d 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 97d87ff1b789625f2c07baf7a74a076f07742596 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 9cf8002e932d562e93fb8d17b4c56f564eb54ed5 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  b3b8ccf868c2a2f18f720a837e90d763072dd3eb 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 16fa2d35cdf7ecdf82dd1ec7739e685ec1ff1aa2 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
>  7f84e90774193b0d31adb7dafcab0a249167cdba 
>   src/main/resources/org/apache/aurora/scheduler/tiers.json 
> 18701058076bedc5d1b667e2b97ad09ce84a9bb9 
>   src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java 
> 39096af816864ada32a9c07958975740e805f6b0 
>   src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
> 52113b80d91ecaf0cc2aeaad77e5fbc0ea4d1216 
>   src/test/java/org/apache/aurora/scheduler/ResourcesTest.java 
> 4fe8c518c580418078275b8056a5c195a765681e 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 
> a662100ba726cff0e47b4f9650753db9cecdef51 
>   src/test/java/org/apache/aurora/scheduler/TierModuleTest.java 
> e0601c83486671596e412f022ffae78b01c81c9d 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  1a520b3cb035a9afc25406d2f313c9f861eee4d6 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 5103bd0f43d53079976b0f1596e299f2d91aa860 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
>  b6f5e4632ac1e028fdf93da1735463373e2d2788 
>   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
> b00add0b2fd4277e196505fffba4440e2e94207e 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> b1c71a6f1847d205c378d0b5a7269ea9d1165be5 
> 
> Diff: https://reviews.apache.org/r/45222/diff/
> 
> 
> Testing
> ---
> 
> # End to end tests
> ```
> $ ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ...
> *** OK (All tests passed) ***
> ```
> 
> # Jenkins build.sh
> ```
> $ ./build-support/jenkins/build.sh
> ...
>SUCCESS
> ```
> 
> # Local Scheduler
> ```
> $ ./gradlew run
> ...
> I0325 16:00:55.374 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
> suppressing offer cycle.
> I0325 16:01:00.371 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
> suppressing offer cycle.
> ```
> 
> # Attempting to schedule job with invalid tier-name
> ```
> vagrant@aurora:~/workspace$ aurora job create devcluster/vagrant/devel/test 
> job.aurora
>  INFO] Creating job test
> Job creation failed due to error:
>   Invalid tier 

Re: Review Request 45222: Introduce "preemptible" flag in TierInfo with backward compatible support for "production" flag in TaskConfig.

2016-03-28 Thread Amol Deshmukh

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

(Updated March 28, 2016, 12:43 p.m.)


Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner.


Changes
---

- Optimized the iteration of victim tasks.
- Updated expected frequency of the ``tierManager.getTier(..)`` mock in unit 
tests to exact value (with couple of exceptions for reasons mentioned in the 
previous comment).


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


Repository: aurora


Description
---

Introduce "preemptible" flag in TierInfo with backward compatible support for 
"production" flag in TaskConfig.

# Summary of changes
- TierInfo extended to manage a new property: "preemptible"
- If TaskConfig does not specify a tier, ``TierManager.getTier(..)`` falls back 
to selecting a non-revocable tier matching ``TaskConfig.isProduction()``.
- Removed ``TierInfo.DEFAULT`` and replaced its in test/benchmark code by 
references to ``TaskTestUtil.DEV_TIER``.
- Eager validation of tier specified in TaskConfig in 
``ConfigurationManager.validateAndPopulate(..)``

# Note on backward incompatible change
TaskConfigs with a tier name not matching any tiers defined in ``tiers.json`` 
were silently assigned a default tier. With this change, attempting to schedule 
tasks that specify non-existent tier names will result in an error (see 
``ConfigurationManager.validateAndPopulate(ITaskConfig config)``).


Diffs (updated)
-

  src/jmh/java/org/apache/aurora/benchmark/Offers.java 
055a2ffcb13c643a3086343e3fbf71545c5fb0a6 
  src/main/java/org/apache/aurora/scheduler/TierInfo.java 
b4611b9cf99ca236cd1ea4610d01c0d293bf2e87 
  src/main/java/org/apache/aurora/scheduler/TierManager.java 
480c4853a6c87dd63a9810ae013e5cfacb29336d 
  src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
97d87ff1b789625f2c07baf7a74a076f07742596 
  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
9cf8002e932d562e93fb8d17b4c56f564eb54ed5 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 b3b8ccf868c2a2f18f720a837e90d763072dd3eb 
  src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
16fa2d35cdf7ecdf82dd1ec7739e685ec1ff1aa2 
  
src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java 
7f84e90774193b0d31adb7dafcab0a249167cdba 
  src/main/resources/org/apache/aurora/scheduler/tiers.json 
18701058076bedc5d1b667e2b97ad09ce84a9bb9 
  src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java 
39096af816864ada32a9c07958975740e805f6b0 
  src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
52113b80d91ecaf0cc2aeaad77e5fbc0ea4d1216 
  src/test/java/org/apache/aurora/scheduler/ResourcesTest.java 
4fe8c518c580418078275b8056a5c195a765681e 
  src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 
a662100ba726cff0e47b4f9650753db9cecdef51 
  src/test/java/org/apache/aurora/scheduler/TierModuleTest.java 
e0601c83486671596e412f022ffae78b01c81c9d 
  
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
 1a520b3cb035a9afc25406d2f313c9f861eee4d6 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
5103bd0f43d53079976b0f1596e299f2d91aa860 
  
src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
 b6f5e4632ac1e028fdf93da1735463373e2d2788 
  src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
b00add0b2fd4277e196505fffba4440e2e94207e 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
b1c71a6f1847d205c378d0b5a7269ea9d1165be5 

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


Testing
---

# End to end tests
```
$ ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
...
*** OK (All tests passed) ***
```

# Jenkins build.sh
```
$ ./build-support/jenkins/build.sh
...
   SUCCESS
```

# Local Scheduler
```
$ ./gradlew run
...
I0325 16:00:55.374 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
suppressing offer cycle.
I0325 16:01:00.371 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
suppressing offer cycle.
```

# Attempting to schedule job with invalid tier-name
```
vagrant@aurora:~/workspace$ aurora job create devcluster/vagrant/devel/test 
job.aurora
 INFO] Creating job test
Job creation failed due to error:
Invalid tier 'badtier' in TaskConfig.
```


Thanks,

Amol Deshmukh



Re: Review Request 45222: Introduce "preemptible" flag in TierInfo with backward compatible support for "production" flag in TaskConfig.

2016-03-28 Thread Amol Deshmukh


> On March 28, 2016, 9:36 a.m., Maxim Khutornenko wrote:
> > src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java,
> >  line 131
> > 
> >
> > Any reason to allow more than "once" (default) here? Same in other 
> > tests.
> 
> Amol Deshmukh wrote:
> The actual cardinality of the call ``tierManager.getTier(lowPriority)`` 
> here is 3:
> 1. Once in the ``victimToResources`` filter (existing behavior)
> 2. Twice in 
> ``o.a.a.s.preemptor.PreemptionVictimFilter.PreemptionVictimFilterImpl#filterPreemptionVictims``,
>  due to the lazy evaluation of the ``FluentIterable preemptableTasks``:
>   a. Once via ``preemptableTasks.isEmpty()``
>   b. Once via ``resourceOrder.immutableSortedCopy(preemptableTasks)``
> 
> Now that I think about it, it is just as easily possible to have the 
> FluentIterable evaluated once by making the ``immutableSortedCopy`` before 
> checking for emptiness. I will repost an updated review.

Ok, so while that optimization does cut down the number of times the filter 
operation is performed, there is still some indeterminism introduced due to the 
use of ``victimToResources`` transform in ``resourceOrder`` for tests involving 
more than one eligible victim. Those cases, I believe, will be best served by 
the use of ``atLeastOnce()`` instead of the exact cardinality discovered 
through testing.


- Amol


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


On March 25, 2016, 5:08 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45222/
> ---
> 
> (Updated March 25, 2016, 5:08 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1616
> https://issues.apache.org/jira/browse/AURORA-1616
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Introduce "preemptible" flag in TierInfo with backward compatible support for 
> "production" flag in TaskConfig.
> 
> # Summary of changes
> - TierInfo extended to manage a new property: "preemptible"
> - If TaskConfig does not specify a tier, ``TierManager.getTier(..)`` falls 
> back to selecting a non-revocable tier matching ``TaskConfig.isProduction()``.
> - Removed ``TierInfo.DEFAULT`` and replaced its in test/benchmark code by 
> references to ``TaskTestUtil.DEV_TIER``.
> - Eager validation of tier specified in TaskConfig in 
> ``ConfigurationManager.validateAndPopulate(..)``
> 
> # Note on backward incompatible change
> TaskConfigs with a tier name not matching any tiers defined in ``tiers.json`` 
> were silently assigned a default tier. With this change, attempting to 
> schedule tasks that specify non-existent tier names will result in an error 
> (see ``ConfigurationManager.validateAndPopulate(ITaskConfig config)``).
> 
> 
> Diffs
> -
> 
>   src/jmh/java/org/apache/aurora/benchmark/Offers.java 
> 055a2ffcb13c643a3086343e3fbf71545c5fb0a6 
>   src/main/java/org/apache/aurora/scheduler/TierInfo.java 
> b4611b9cf99ca236cd1ea4610d01c0d293bf2e87 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java 
> 480c4853a6c87dd63a9810ae013e5cfacb29336d 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 97d87ff1b789625f2c07baf7a74a076f07742596 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 9cf8002e932d562e93fb8d17b4c56f564eb54ed5 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  b3b8ccf868c2a2f18f720a837e90d763072dd3eb 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 16fa2d35cdf7ecdf82dd1ec7739e685ec1ff1aa2 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
>  7f84e90774193b0d31adb7dafcab0a249167cdba 
>   src/main/resources/org/apache/aurora/scheduler/tiers.json 
> 18701058076bedc5d1b667e2b97ad09ce84a9bb9 
>   src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java 
> 39096af816864ada32a9c07958975740e805f6b0 
>   src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
> 52113b80d91ecaf0cc2aeaad77e5fbc0ea4d1216 
>   src/test/java/org/apache/aurora/scheduler/ResourcesTest.java 
> 4fe8c518c580418078275b8056a5c195a765681e 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 
> a662100ba726cff0e47b4f9650753db9cecdef51 
>   src/test/java/org/apache/aurora/scheduler/TierModuleTest.java 
> e0601c83486671596e412f022ffae78b01c81c9d 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  1a520b3cb035a9afc25406d2f313c9f861eee4d6 
>   
> 

Re: Review Request 45222: Introduce "preemptible" flag in TierInfo with backward compatible support for "production" flag in TaskConfig.

2016-03-28 Thread Amol Deshmukh


> On March 28, 2016, 9:36 a.m., Maxim Khutornenko wrote:
> > src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java,
> >  line 131
> > 
> >
> > Any reason to allow more than "once" (default) here? Same in other 
> > tests.

The actual cardinality of the call ``tierManager.getTier(lowPriority)`` here is 
3:
1. Once in the ``victimToResources`` filter (existing behavior)
2. Twice in 
``o.a.a.s.preemptor.PreemptionVictimFilter.PreemptionVictimFilterImpl#filterPreemptionVictims``,
 due to the lazy evaluation of the ``FluentIterable preemptableTasks``:
  a. Once via ``preemptableTasks.isEmpty()``
  b. Once via ``resourceOrder.immutableSortedCopy(preemptableTasks)``

Now that I think about it, it is just as easily possible to have the 
FluentIterable evaluated once by making the ``immutableSortedCopy`` before 
checking for emptiness. I will repost an updated review.


- Amol


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


On March 25, 2016, 5:08 p.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45222/
> ---
> 
> (Updated March 25, 2016, 5:08 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1616
> https://issues.apache.org/jira/browse/AURORA-1616
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Introduce "preemptible" flag in TierInfo with backward compatible support for 
> "production" flag in TaskConfig.
> 
> # Summary of changes
> - TierInfo extended to manage a new property: "preemptible"
> - If TaskConfig does not specify a tier, ``TierManager.getTier(..)`` falls 
> back to selecting a non-revocable tier matching ``TaskConfig.isProduction()``.
> - Removed ``TierInfo.DEFAULT`` and replaced its in test/benchmark code by 
> references to ``TaskTestUtil.DEV_TIER``.
> - Eager validation of tier specified in TaskConfig in 
> ``ConfigurationManager.validateAndPopulate(..)``
> 
> # Note on backward incompatible change
> TaskConfigs with a tier name not matching any tiers defined in ``tiers.json`` 
> were silently assigned a default tier. With this change, attempting to 
> schedule tasks that specify non-existent tier names will result in an error 
> (see ``ConfigurationManager.validateAndPopulate(ITaskConfig config)``).
> 
> 
> Diffs
> -
> 
>   src/jmh/java/org/apache/aurora/benchmark/Offers.java 
> 055a2ffcb13c643a3086343e3fbf71545c5fb0a6 
>   src/main/java/org/apache/aurora/scheduler/TierInfo.java 
> b4611b9cf99ca236cd1ea4610d01c0d293bf2e87 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java 
> 480c4853a6c87dd63a9810ae013e5cfacb29336d 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 97d87ff1b789625f2c07baf7a74a076f07742596 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 9cf8002e932d562e93fb8d17b4c56f564eb54ed5 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  b3b8ccf868c2a2f18f720a837e90d763072dd3eb 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 16fa2d35cdf7ecdf82dd1ec7739e685ec1ff1aa2 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
>  7f84e90774193b0d31adb7dafcab0a249167cdba 
>   src/main/resources/org/apache/aurora/scheduler/tiers.json 
> 18701058076bedc5d1b667e2b97ad09ce84a9bb9 
>   src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java 
> 39096af816864ada32a9c07958975740e805f6b0 
>   src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
> 52113b80d91ecaf0cc2aeaad77e5fbc0ea4d1216 
>   src/test/java/org/apache/aurora/scheduler/ResourcesTest.java 
> 4fe8c518c580418078275b8056a5c195a765681e 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 
> a662100ba726cff0e47b4f9650753db9cecdef51 
>   src/test/java/org/apache/aurora/scheduler/TierModuleTest.java 
> e0601c83486671596e412f022ffae78b01c81c9d 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  1a520b3cb035a9afc25406d2f313c9f861eee4d6 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 5103bd0f43d53079976b0f1596e299f2d91aa860 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
>  b6f5e4632ac1e028fdf93da1735463373e2d2788 
>   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
> b00add0b2fd4277e196505fffba4440e2e94207e 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> b1c71a6f1847d205c378d0b5a7269ea9d1165be5 
> 
> Diff: 

Re: Review Request 45222: Introduce "preemptible" flag in TierInfo with backward compatible support for "production" flag in TaskConfig.

2016-03-28 Thread Maxim Khutornenko

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


Ship it!





src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
 (line 130)


Any reason to allow more than "once" (default) here? Same in other tests.


- Maxim Khutornenko


On March 26, 2016, 12:08 a.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45222/
> ---
> 
> (Updated March 26, 2016, 12:08 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1616
> https://issues.apache.org/jira/browse/AURORA-1616
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Introduce "preemptible" flag in TierInfo with backward compatible support for 
> "production" flag in TaskConfig.
> 
> # Summary of changes
> - TierInfo extended to manage a new property: "preemptible"
> - If TaskConfig does not specify a tier, ``TierManager.getTier(..)`` falls 
> back to selecting a non-revocable tier matching ``TaskConfig.isProduction()``.
> - Removed ``TierInfo.DEFAULT`` and replaced its in test/benchmark code by 
> references to ``TaskTestUtil.DEV_TIER``.
> - Eager validation of tier specified in TaskConfig in 
> ``ConfigurationManager.validateAndPopulate(..)``
> 
> # Note on backward incompatible change
> TaskConfigs with a tier name not matching any tiers defined in ``tiers.json`` 
> were silently assigned a default tier. With this change, attempting to 
> schedule tasks that specify non-existent tier names will result in an error 
> (see ``ConfigurationManager.validateAndPopulate(ITaskConfig config)``).
> 
> 
> Diffs
> -
> 
>   src/jmh/java/org/apache/aurora/benchmark/Offers.java 
> 055a2ffcb13c643a3086343e3fbf71545c5fb0a6 
>   src/main/java/org/apache/aurora/scheduler/TierInfo.java 
> b4611b9cf99ca236cd1ea4610d01c0d293bf2e87 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java 
> 480c4853a6c87dd63a9810ae013e5cfacb29336d 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 97d87ff1b789625f2c07baf7a74a076f07742596 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 9cf8002e932d562e93fb8d17b4c56f564eb54ed5 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  b3b8ccf868c2a2f18f720a837e90d763072dd3eb 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 16fa2d35cdf7ecdf82dd1ec7739e685ec1ff1aa2 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
>  7f84e90774193b0d31adb7dafcab0a249167cdba 
>   src/main/resources/org/apache/aurora/scheduler/tiers.json 
> 18701058076bedc5d1b667e2b97ad09ce84a9bb9 
>   src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java 
> 39096af816864ada32a9c07958975740e805f6b0 
>   src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
> 52113b80d91ecaf0cc2aeaad77e5fbc0ea4d1216 
>   src/test/java/org/apache/aurora/scheduler/ResourcesTest.java 
> 4fe8c518c580418078275b8056a5c195a765681e 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 
> a662100ba726cff0e47b4f9650753db9cecdef51 
>   src/test/java/org/apache/aurora/scheduler/TierModuleTest.java 
> e0601c83486671596e412f022ffae78b01c81c9d 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  1a520b3cb035a9afc25406d2f313c9f861eee4d6 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 5103bd0f43d53079976b0f1596e299f2d91aa860 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
>  b6f5e4632ac1e028fdf93da1735463373e2d2788 
>   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
> b00add0b2fd4277e196505fffba4440e2e94207e 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> b1c71a6f1847d205c378d0b5a7269ea9d1165be5 
> 
> Diff: https://reviews.apache.org/r/45222/diff/
> 
> 
> Testing
> ---
> 
> # End to end tests
> ```
> $ ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ...
> *** OK (All tests passed) ***
> ```
> 
> # Jenkins build.sh
> ```
> $ ./build-support/jenkins/build.sh
> ...
>SUCCESS
> ```
> 
> # Local Scheduler
> ```
> $ ./gradlew run
> ...
> I0325 16:00:55.374 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
> suppressing offer cycle.
> I0325 16:01:00.371 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
> suppressing offer cycle.
> ```
> 
> # Attempting to schedule job with invalid tier-name
> ```
> vagrant@aurora:~/workspace$ aurora job create devcluster/vagrant/devel/test 
> job.aurora
>  INFO] 

Re: Review Request 45222: Introduce "preemptible" flag in TierInfo with backward compatible support for "production" flag in TaskConfig.

2016-03-25 Thread Aurora ReviewBot

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


Ship it!




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

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

- Aurora ReviewBot


On March 26, 2016, 12:08 a.m., Amol Deshmukh wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45222/
> ---
> 
> (Updated March 26, 2016, 12:08 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-1616
> https://issues.apache.org/jira/browse/AURORA-1616
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Introduce "preemptible" flag in TierInfo with backward compatible support for 
> "production" flag in TaskConfig.
> 
> # Summary of changes
> - TierInfo extended to manage a new property: "preemptible"
> - If TaskConfig does not specify a tier, ``TierManager.getTier(..)`` falls 
> back to selecting a non-revocable tier matching ``TaskConfig.isProduction()``.
> - Removed ``TierInfo.DEFAULT`` and replaced its in test/benchmark code by 
> references to ``TaskTestUtil.DEV_TIER``.
> - Eager validation of tier specified in TaskConfig in 
> ``ConfigurationManager.validateAndPopulate(..)``
> 
> # Note on backward incompatible change
> TaskConfigs with a tier name not matching any tiers defined in ``tiers.json`` 
> were silently assigned a default tier. With this change, attempting to 
> schedule tasks that specify non-existent tier names will result in an error 
> (see ``ConfigurationManager.validateAndPopulate(ITaskConfig config)``).
> 
> 
> Diffs
> -
> 
>   src/jmh/java/org/apache/aurora/benchmark/Offers.java 
> 055a2ffcb13c643a3086343e3fbf71545c5fb0a6 
>   src/main/java/org/apache/aurora/scheduler/TierInfo.java 
> b4611b9cf99ca236cd1ea4610d01c0d293bf2e87 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java 
> 480c4853a6c87dd63a9810ae013e5cfacb29336d 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 97d87ff1b789625f2c07baf7a74a076f07742596 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 9cf8002e932d562e93fb8d17b4c56f564eb54ed5 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  b3b8ccf868c2a2f18f720a837e90d763072dd3eb 
>   src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
> 16fa2d35cdf7ecdf82dd1ec7739e685ec1ff1aa2 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
>  7f84e90774193b0d31adb7dafcab0a249167cdba 
>   src/main/resources/org/apache/aurora/scheduler/tiers.json 
> 18701058076bedc5d1b667e2b97ad09ce84a9bb9 
>   src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java 
> 39096af816864ada32a9c07958975740e805f6b0 
>   src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
> 52113b80d91ecaf0cc2aeaad77e5fbc0ea4d1216 
>   src/test/java/org/apache/aurora/scheduler/ResourcesTest.java 
> 4fe8c518c580418078275b8056a5c195a765681e 
>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 
> a662100ba726cff0e47b4f9650753db9cecdef51 
>   src/test/java/org/apache/aurora/scheduler/TierModuleTest.java 
> e0601c83486671596e412f022ffae78b01c81c9d 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  1a520b3cb035a9afc25406d2f313c9f861eee4d6 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 5103bd0f43d53079976b0f1596e299f2d91aa860 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
>  b6f5e4632ac1e028fdf93da1735463373e2d2788 
>   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
> b00add0b2fd4277e196505fffba4440e2e94207e 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> b1c71a6f1847d205c378d0b5a7269ea9d1165be5 
> 
> Diff: https://reviews.apache.org/r/45222/diff/
> 
> 
> Testing
> ---
> 
> # End to end tests
> ```
> $ ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> ...
> *** OK (All tests passed) ***
> ```
> 
> # Jenkins build.sh
> ```
> $ ./build-support/jenkins/build.sh
> ...
>SUCCESS
> ```
> 
> # Local Scheduler
> ```
> $ ./gradlew run
> ...
> I0325 16:00:55.374 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
> suppressing offer cycle.
> I0325 16:01:00.371 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
> suppressing offer cycle.
> ```
> 
> # Attempting to schedule job with invalid tier-name
> ```
> vagrant@aurora:~/workspace$ aurora job create devcluster/vagrant/devel/test 
> job.aurora
>  INFO] Creating job test
> Job creation failed due to error:
>   Invalid 

Re: Review Request 45222: Introduce "preemptible" flag in TierInfo with backward compatible support for "production" flag in TaskConfig.

2016-03-25 Thread Amol Deshmukh

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

(Updated March 25, 2016, 5:08 p.m.)


Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner.


Changes
---

Linked AURORA-1616


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


Repository: aurora


Description
---

Introduce "preemptible" flag in TierInfo with backward compatible support for 
"production" flag in TaskConfig.

# Summary of changes
- TierInfo extended to manage a new property: "preemptible"
- If TaskConfig does not specify a tier, ``TierManager.getTier(..)`` falls back 
to selecting a non-revocable tier matching ``TaskConfig.isProduction()``.
- Removed ``TierInfo.DEFAULT`` and replaced its in test/benchmark code by 
references to ``TaskTestUtil.DEV_TIER``.
- Eager validation of tier specified in TaskConfig in 
``ConfigurationManager.validateAndPopulate(..)``

# Note on backward incompatible change
TaskConfigs with a tier name not matching any tiers defined in ``tiers.json`` 
were silently assigned a default tier. With this change, attempting to schedule 
tasks that specify non-existent tier names will result in an error (see 
``ConfigurationManager.validateAndPopulate(ITaskConfig config)``).


Diffs
-

  src/jmh/java/org/apache/aurora/benchmark/Offers.java 
055a2ffcb13c643a3086343e3fbf71545c5fb0a6 
  src/main/java/org/apache/aurora/scheduler/TierInfo.java 
b4611b9cf99ca236cd1ea4610d01c0d293bf2e87 
  src/main/java/org/apache/aurora/scheduler/TierManager.java 
480c4853a6c87dd63a9810ae013e5cfacb29336d 
  src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
97d87ff1b789625f2c07baf7a74a076f07742596 
  src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
9cf8002e932d562e93fb8d17b4c56f564eb54ed5 
  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 b3b8ccf868c2a2f18f720a837e90d763072dd3eb 
  src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 
16fa2d35cdf7ecdf82dd1ec7739e685ec1ff1aa2 
  
src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java 
7f84e90774193b0d31adb7dafcab0a249167cdba 
  src/main/resources/org/apache/aurora/scheduler/tiers.json 
18701058076bedc5d1b667e2b97ad09ce84a9bb9 
  src/test/java/org/apache/aurora/scheduler/AcceptedOfferTest.java 
39096af816864ada32a9c07958975740e805f6b0 
  src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
52113b80d91ecaf0cc2aeaad77e5fbc0ea4d1216 
  src/test/java/org/apache/aurora/scheduler/ResourcesTest.java 
4fe8c518c580418078275b8056a5c195a765681e 
  src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 
a662100ba726cff0e47b4f9650753db9cecdef51 
  src/test/java/org/apache/aurora/scheduler/TierModuleTest.java 
e0601c83486671596e412f022ffae78b01c81c9d 
  
src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
 1a520b3cb035a9afc25406d2f313c9f861eee4d6 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
5103bd0f43d53079976b0f1596e299f2d91aa860 
  
src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
 b6f5e4632ac1e028fdf93da1735463373e2d2788 
  src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
b00add0b2fd4277e196505fffba4440e2e94207e 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
b1c71a6f1847d205c378d0b5a7269ea9d1165be5 

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


Testing
---

# End to end tests
```
$ ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
...
*** OK (All tests passed) ***
```

# Jenkins build.sh
```
$ ./build-support/jenkins/build.sh
...
   SUCCESS
```

# Local Scheduler
```
$ ./gradlew run
...
I0325 16:00:55.374 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
suppressing offer cycle.
I0325 16:01:00.371 [pool-9-thread-1, FakeMaster:139] All offers consumed, 
suppressing offer cycle.
```

# Attempting to schedule job with invalid tier-name
```
vagrant@aurora:~/workspace$ aurora job create devcluster/vagrant/devel/test 
job.aurora
 INFO] Creating job test
Job creation failed due to error:
Invalid tier 'badtier' in TaskConfig.
```


Thanks,

Amol Deshmukh