aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Amol Deshmukh <a...@apache.org>
Subject Re: Review Request 45222: Introduce "preemptible" flag in TierInfo with backward compatible support for "production" flag in TaskConfig.
Date Mon, 28 Mar 2016 19:41:05 GMT


> On March 28, 2016, 9:36 a.m., Maxim Khutornenko wrote:
> > src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java,
line 131
> > <https://reviews.apache.org/r/45222/diff/1/?file=1315322#file1315322line131>
> >
> >     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 
>   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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message