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 32597: Improving async preemptor efficiency.
Date Tue, 21 Apr 2015 21:12:00 GMT


> On April 17, 2015, 5:59 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java,
lines 142-144
> > <https://reviews.apache.org/r/32597/diff/5/?file=931268#file931268line142>
> >
> >     ```
> >     Set<String> allSlaves = Sets.newHashSet(Iterables.concat(
> >         slavesToOffers.keySet(),
> >         slavesToActiveTasks.keySet()));
> >     ```
> 
> Maxim Khutornenko wrote:
>     Not opposed to the change but how is this better exactly? The way it's currently
written will iterate over all available slaves only once whereas the proposed change will
have to do it twice (at least the way I read it).
> 
> Bill Farner wrote:
>     Are you assuming that `Iterables.concat` eagerly iterates over the inputs?  If so,
that is false:
>     
>     http://docs.guava-libraries.googlecode.com/git-history/release/javadoc/com/google/common/collect/Iterables.html#concat(java.lang.Iterable...)
>     
>     I prefer the snippet i've posted since it uses a single statement for a single logical
entity.  Of course, if that has the side-effect of signifcantly worse performance, it is not
worth it.

Benchmarked both approaches and could not find any difference in perf. Taking your suggestion.


- Maxim


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


On April 21, 2015, 1:12 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32597/
> -----------------------------------------------------------
> 
> (Updated April 21, 2015, 1:12 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1219
>     https://issues.apache.org/jira/browse/AURORA-1219
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is addressing the preemption efficiency loss due to making preemptor async. Summary:
> - Implemented fair task processing by evaluating pending task groups in round-robin fashion
(e.g.: {G1:3, G2:2} -> {G1, G2, G1, G2, G1}) against all available slaves.
> - Moved relevant functionality from PreemptionSlotFinder (now PreemptionVictimFilter)
into PendingTaskProcessor.
> 
> The bulk of new functionality is in PendingTaskIterator and PendingTaskProcessor. The
rest is refactoring to adjust to the new traversal approach.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
00919b7910704c5025465e1071378a978e5e60a3 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionProposal.java PRE-CREATION

>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java
f16f964f56f0f9da523950891293083f1bd86780 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 5200811ec8fe6fedf42ae2712f29051a8c0bf4a9

>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java dc7eb4421ff305dca32f36c83605c2864fea8b11

>   src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7cea881a8c3c11142bd04b3c794cd86a310b15e7

>   src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 6af3949b85297043640edccc1a490906c0fcff6c

>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 7bb1e7a9f27088636d6549c089b1d079dfeaf2ee

>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java
8a9a3b7d9686e29632f4e267f591cdb19826e0e7 
>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 64283fab8c61b841007d7c0a02b083b3067bc78d

>   src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java
eed2de99a145dd2124b7f2b4d401214f1d8adf2e 
> 
> Diff: https://reviews.apache.org/r/32597/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> Manual testing in vagrant
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


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