Re: Review Request 25812: Implementing quota checking for async job updates.

2014-09-24 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25812/ --- (Updated Sept. 24, 2014, 9:26 p.m.) Review request for Aurora, Kevin Sweeney an

Re: Review Request 25812: Implementing quota checking for async job updates.

2014-09-24 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25812/#review54426 --- Ship it! Ship It! - Bill Farner On Sept. 23, 2014, 11:08 p.m., M

Re: Review Request 25812: Implementing quota checking for async job updates.

2014-09-23 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25812/ --- (Updated Sept. 23, 2014, 11:08 p.m.) Review request for Aurora, David McLaughli

Re: Review Request 25812: Implementing quota checking for async job updates.

2014-09-23 Thread Maxim Khutornenko
> On Sept. 23, 2014, 8:10 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java, line 174 > > > > > > Accept JobUpdateStore here, that's all that is used. Done. > On Sept. 23

Re: Review Request 25812: Implementing quota checking for async job updates.

2014-09-23 Thread Maxim Khutornenko
> On Sept. 23, 2014, 8:07 p.m., Kevin Sweeney wrote: > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, > > line 1469 > > > > > > Refactor this to live outside the Impl class? Schedul

Re: Review Request 25812: Implementing quota checking for async job updates.

2014-09-23 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25812/#review54334 --- Ship it! LGTM mod Bill's comments and Impl suggestion - Kevin Swee

Re: Review Request 25812: Implementing quota checking for async job updates.

2014-09-23 Thread Kevin Sweeney
> On Sept. 23, 2014, 1:07 p.m., Kevin Sweeney wrote: > > src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java, line 180 > > > > > > Unless I'm missing something here, this will throw when there are > > mu

Re: Review Request 25812: Implementing quota checking for async job updates.

2014-09-23 Thread Bill Farner
> On Sept. 23, 2014, 8:07 p.m., Kevin Sweeney wrote: > > src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java, line 180 > > > > > > Unless I'm missing something here, this will throw when there are > > mu

Re: Review Request 25812: Implementing quota checking for async job updates.

2014-09-23 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25812/#review54316 --- LGTM with some additional test coverage for the new accounting. I t

Re: Review Request 25812: Implementing quota checking for async job updates.

2014-09-23 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25812/#review54315 --- src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java <

Re: Review Request 25812: Implementing quota checking for async job updates.

2014-09-23 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25812/ --- (Updated Sept. 23, 2014, 4:48 p.m.) Review request for Aurora, David McLaughlin

Re: Review Request 25812: Implementing quota checking for async job updates.

2014-09-23 Thread Maxim Khutornenko
> On Sept. 23, 2014, 1:01 a.m., Bill Farner wrote: > > src/main/thrift/org/apache/aurora/gen/api.thrift, line 547 > > > > > > Can you update JobUpdateControllerImpl to use this as well? > > > > Also, you shoul

Re: Review Request 25812: Implementing quota checking for async job updates.

2014-09-23 Thread Bill Farner
> On Sept. 23, 2014, 1:01 a.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, > > line 1350 > > > > > > How about 'validateInstanceAddition'? I'm not tied to

Re: Review Request 25812: Implementing quota checking for async job updates.

2014-09-23 Thread Maxim Khutornenko
--- 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

Re: Review Request 25812: Implementing quota checking for async job updates.

2014-09-23 Thread Maxim Khutornenko
> On Sept. 23, 2014, 1:01 a.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java, line 268 > > > > > > Remember - javadoc is like html. These newlines are not preserved > > unl

Re: Review Request 25812: Implementing quota checking for async job updates.

2014-09-22 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25812/#review54168 --- src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java <

Re: Review Request 25812: Implementing quota checking for async job updates.

2014-09-22 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25812/ --- (Updated Sept. 23, 2014, 12:59 a.m.) Review request for Aurora, David McLaughli

Re: Review Request 25812: Implementing quota checking for async job updates.

2014-09-19 Thread Maxim Khutornenko
> On Sept. 19, 2014, 7:38 p.m., Bill Farner wrote: > > Do you think it makes sense to wait for AURORA-718 before we land this? I > > think that will help simplify the algorithm: > > > > - convert each job to a resource aggregate > > - convert each job update to a possibly-negative resource agg

Re: Review Request 25812: Implementing quota checking for async job updates.

2014-09-19 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25812/#review54000 --- Do you think it makes sense to wait for AURORA-718 before we land th

Re: Review Request 25812: Implementing quota checking for async job updates.

2014-09-19 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25812/ --- (Updated Sept. 19, 2014, 6:03 p.m.) Review request for Aurora, David McLaughlin