Mailing-List: contact reviews-help@aurora.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: reviews@aurora.incubator.apache.org
Content-Type: multipart/alternative;
 boundary="===============8144164612952055665=="
MIME-Version: 1.0
Subject: Re: Review Request 25812: Implementing quota checking for async job
 updates.
From: "Bill Farner" <wfarner@apache.org>
To: "Bill Farner" <wfarner@apache.org>, "Kevin Sweeney" <kevints@apache.org>,
 "David McLaughlin" <david@dmclaughlin.com>
Cc: "Aurora" <reviews@aurora.incubator.apache.org>,
 "Maxim Khutornenko" <maxim@apache.org>
Date: Tue, 23 Sep 2014 16:37:22 -0000
Message-ID: <20140923163722.15000.45123@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Bill Farner" <noreply@reviews.apache.org>
References: <20140923010118.14999.53609@reviews.apache.org>
In-Reply-To: <20140923010118.14999.53609@reviews.apache.org>
Reply-To: "Bill Farner" <wfarner@apache.org>

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


> On Sept. 23, 2014, 1:01 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, line 1350
> > <https://reviews.apache.org/r/25812/diff/1/?file=694289#file694289line1350>
> >
> >     How about 'validateInstanceAddition'?  I'm not tied to the name, but something more informative than 'validate' would be nice.
> 
> Maxim Khutornenko wrote:
>     How about validateTaskLimits?

Sounds good.


> On Sept. 23, 2014, 1:01 a.m., Bill Farner wrote:
> > src/main/thrift/org/apache/aurora/gen/api.thrift, line 547
> > <https://reviews.apache.org/r/25812/diff/1/?file=694290#file694290line547>
> >
> >     Can you update JobUpdateControllerImpl to use this as well?
> >     
> >     Also, you should wrap this with a constant that we define.  Thrift produces a mutable static Set for this, which comes with risk.
> 
> Maxim Khutornenko wrote:
>     The only place it's used in is already wrapping it into EnumSet. 
>     
>     What specifically in JobUpdateControllerImpl? Could not find anything suitable to update.

https://github.com/apache/incubator-aurora/blob/012aca3d2a5667f9cfd6a1b9b0b91c982d34bb94/src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java#L254-L258


- Bill


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


On Sept. 23, 2014, 4:12 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25812/
> -----------------------------------------------------------
> 
> (Updated Sept. 23, 2014, 4:12 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Kevin Sweeney, and Bill Farner.
> 
> 
> Bugs: AURORA-686
>     https://issues.apache.org/jira/browse/AURORA-686
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Getting it right required a deep refactoring of the QuotaManager. The prod consumption is now calculated from:
> - production tasks not participating in job updates;
> - unaffected production tasks from a job being updated (i.e. those that are not covered by the update working set);
> - production tasks directly affected by the job update (max of old and new resources used).
> 
> Also, moved all logic back to SchedulerThriftInterface as quota checks are done only there now.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 3661f8487985f631e3ea437fe6430e0296376a9e 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 14b0dd8f86026840d0444c128f656a144eab017d 
>   src/main/java/org/apache/aurora/scheduler/state/StateModule.java 54b90127551c69509dbd41ee95c384dbbf1a7ee4 
>   src/main/java/org/apache/aurora/scheduler/state/TaskLimitValidator.java 70333737ee119ea083f0ac17a4205e7af9f4c753 
>   src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java ae0320b44b55a3630e255484ea7a881daab6a7bc 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 2376a5e530b12fbbebb4cfc7555656ae07795518 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 8381e2126c62c40d28a325c72686d01e82bb84cf 
>   src/test/java/org/apache/aurora/scheduler/state/TaskLimitValidatorTest.java 8f18617b2052201f87bb1464314c2ee45b279276 
>   src/test/java/org/apache/aurora/scheduler/storage/testing/StorageTestUtil.java 5aebbfbc691dfac4a066cb1425d18d3fccc77090 
>   src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 21e8a8ba5c47f549133ea339899bf0e5097ccbb9 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 40156c211a346664c0d2f174235efb2049cf3bb9 
> 
> Diff: https://reviews.apache.org/r/25812/diff/
> 
> 
> Testing
> -------
> 
> gradle -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


--===============8144164612952055665==--