aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Maxim Khutornenko" <ma...@apache.org>
Subject Re: Review Request 37560: Adding TierManager initial implementation.
Date Tue, 18 Aug 2015 18:46:31 GMT


> On Aug. 18, 2015, 6:08 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/TierInfo.java, line 27
> > <https://reviews.apache.org/r/37560/diff/2/?file=1043194#file1043194line27>
> >
> >     For better readability, how about using an enum?
> 
> Maxim Khutornenko wrote:
>     Not sure I undestand. Are you proposing converting TierInfo into an enum or the arguments
of TierInfo to be an EnumSet? I am afraid neither would make sense as TierInfo will eventually
wrap free-form task attributes defined in an external config file. Given the config file example
in https://docs.google.com/document/d/1r1WCHgmPJp5wbrqSZLsgtxPNj3sULfHrSFmxp2GyPTo, a possible
TierInfo constructor could look similar to this:
>     ```
>     TierInfo(String name, boolean revocable, boolean preemtible, boolean quotaRequired,
Amount<Long, TimeUnit> uptime, double percentage ...)
>     ```
> 
> Stephan Erb wrote:
>     I was referring to the boolean values which IMHO make constructor calls (for example
in tests) very hard to read. For example: `TierInfo('mytier', false, false, true, ...)` vs
`TierInfo('mytier', Offers.UNREVOCABLE, Jobs.UNPREEMPTIBLE, Quotas.REQUIRED, ...)`.
>     
>     But you are right that this adds an aditional indirection layer when passing values
from the config.

Got it. Yeah, I'd avoid adding indirection like that, given that the TierInfo constructor
is only intended to be called from TierManager.


- Maxim


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


On Aug. 18, 2015, 6:23 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37560/
> -----------------------------------------------------------
> 
> (Updated Aug. 18, 2015, 6:23 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Zameer Manji.
> 
> 
> Bugs: AURORA-1437
>     https://issues.apache.org/jira/browse/AURORA-1437
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Introducing `TierManager` implementation. This diff addresses basic needs within the
Oversubscription feature scope.
> 
> Only TaskAssigner is modified to use TierManager. Other consumers will be addressed separately.
> 
> More about the TierManager concept here: https://docs.google.com/document/d/1r1WCHgmPJp5wbrqSZLsgtxPNj3sULfHrSFmxp2GyPTo
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/Resources.java 40df26234ad9959227d8a56edbe563f252d47560

>   src/main/java/org/apache/aurora/scheduler/SchedulerModule.java 45265ea6cef5e26a3d7303a9653cfca110528d4b

>   src/main/java/org/apache/aurora/scheduler/TierInfo.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java ca4b5b0b01c7b347fc90f40d77f96cf9d4d94163

>   src/test/java/org/apache/aurora/scheduler/ResourcesTest.java a5878a4efada5b5872be77ad887a0ffe8695d6b8

>   src/test/java/org/apache/aurora/scheduler/TierManagerTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 88958d1510cf9402a7d1e422de9f7918004e188b

> 
> Diff: https://reviews.apache.org/r/37560/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


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