Mailing-List: contact reviews-help@aurora.apache.org; run by ezmlm
Precedence: bulk
Reply-To: reviews@aurora.apache.org
Content-Type: multipart/alternative;
 boundary="===============4458546775101862202=="
MIME-Version: 1.0
Subject: Re: Review Request 62956: When scheduling,
 skip offers with no CPU and no mem
From: Stephan Erb <serb@apache.org>
To: David McLaughlin <david@dmclaughlin.com>, Jordan Ly <jordan.ly8@gmail.com>
Cc: Bill Farner <wfarner@apache.org>, Aurora <reviews@aurora.apache.org>
Date: Wed, 18 Oct 2017 21:48:08 -0000
Message-ID: <20171018214808.63763.35148@reviews-vm2.apache.org>
Auto-Submitted: auto-generated
Sender: Stephan Erb <noreply@reviews.apache.org>
References: <20171018212503.63799.14433@reviews-vm2.apache.org>
In-Reply-To: <20171018212503.63799.14433@reviews-vm2.apache.org>
Reply-To: Stephan Erb <serb@apache.org>
archived-at: Wed, 18 Oct 2017 21:48:14 -0000

--===============4458546775101862202==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


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


Ship it!


src/main/java/org/apache/aurora/scheduler/HostOffer.java
Lines 44-47 (patched)
<https://reviews.apache.org/r/62956/#comment265579>

    Please extend this comment a bit and indicate that this checks for the absence of revocable and non-revocable resources.


src/main/java/org/apache/aurora/scheduler/HostOffer.java
Lines 62-66 (patched)
<https://reviews.apache.org/r/62956/#comment265583>

    I have had a short peek at our clusters and the number of offers matching this criteria would be very low. 
    
    It could be increased if we'd use a higher bound like 0.1 CPU and 64mb memory as those would already be needed as executor overhead.
    
    I suspect for Twitter even using 0 as abound is significant though? Correct?


src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java
Lines 295-296 (patched)
<https://reviews.apache.org/r/62956/#comment265580>

    For consistency with the other early aborts below, write this in form of an `if` with a `continue` statement?
    
    (I don't feel strongly about this though. It just feels a little bit weird that this is handled differently here)


- Stephan Erb


On Oct. 18, 2017, 11:25 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62956/
> -----------------------------------------------------------
> 
> (Updated Oct. 18, 2017, 11:25 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Jordan Ly.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> There's no reason for us to evaluate offers with no CPUs or memory, so reject them early in the offer lifecycle.
> 
> This is an incremental performance optimization, but it may net significant improvements based on observations in some very large clusters.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/HostOffer.java bc40d0798f40003cab5bf6efe607217e4d5de9f1 
>   src/main/java/org/apache/aurora/scheduler/http/Utilization.java 3c77e2983ce00f897f3d5ed106b779cd7f7f0940 
>   src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java 1b1239753f40d7d46d91724def6c25037eb79f1c 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceBag.java d5db81b88a0369d0b26c8fbf70efab3886ad7695 
>   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java e35720fca903cdcd27fff50f12ece9cc615f5d54 
>   src/main/java/org/apache/aurora/scheduler/stats/TaskStatCalculator.java b98aaaf48ae60afef19a368ee96abc897300f8fa 
>   src/test/java/org/apache/aurora/scheduler/HostOfferTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 2cfdc090ff75a63111ae146c9fe7b3542e7ac83f 
>   src/test/java/org/apache/aurora/scheduler/offers/Offers.java 129b4437315c6ad4ea47ca75d4ae6e28cadd7911 
>   src/test/java/org/apache/aurora/scheduler/resources/ResourceTestUtil.java 765a527acb96997989c920be8b69dfa1113dc302 
>   src/test/java/org/apache/aurora/scheduler/state/FirstFitTaskAssignerTest.java a74efbdc55383cd748f52cd55e1a94551d7d2ba1 
> 
> 
> Diff: https://reviews.apache.org/r/62956/diff/4/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bill Farner
> 
>


--===============4458546775101862202==--