> 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 
>   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
> 
>

Reply via email to