You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Maxim Khutornenko <ma...@apache.org> on 2014/09/19 03:56:04 UTC
Review Request 25812: Implementing quota checking for async job
updates.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25812/
-----------------------------------------------------------
Review request for Aurora, David McLaughlin and Bill Farner.
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 779e925e4d9e7889e8cfd369cea9a8e5da3554d2
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 83ac034cff009530e5e16c0613b9d085f3b908d8
src/main/thrift/org/apache/aurora/gen/api.thrift 2376a5e530b12fbbebb4cfc7555656ae07795518
src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 49770e5f87f047502e4f5653b908657a40d8683f
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 336cada625b85618486660fc24f3e83a312609f8
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
Re: Review Request 25812: Implementing quota checking for async job
updates.
Posted by Maxim Khutornenko <ma...@apache.org>.
> 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 aggregate delta
> > - walk each job, add delta if positive
This diff is built assuming the AURORA-718. As for the algorithm suggestion, I don't see how it would work for in-flight updates. Once the update started, the baseline for the delta is gone and we may be double counting (or missing) instances that have already been worked on.
The current algorithm does not suffer from that flaw as it treats affected instances separately:
- convert all instance tasks unaffected by update into resource aggregate
- convert all instance tasks affected by update into resource aggregate
- add the two above to yield overall consumption
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25812/#review54000
-----------------------------------------------------------
On Sept. 19, 2014, 6:03 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> 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 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 779e925e4d9e7889e8cfd369cea9a8e5da3554d2
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 83ac034cff009530e5e16c0613b9d085f3b908d8
> src/main/thrift/org/apache/aurora/gen/api.thrift 2376a5e530b12fbbebb4cfc7555656ae07795518
> src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 49770e5f87f047502e4f5653b908657a40d8683f
> 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 336cada625b85618486660fc24f3e83a312609f8
> 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
>
>
Re: Review Request 25812: Implementing quota checking for async job
updates.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
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 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 aggregate delta
- walk each job, add delta if positive
- Bill Farner
On Sept. 19, 2014, 6:03 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> 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 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 779e925e4d9e7889e8cfd369cea9a8e5da3554d2
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 83ac034cff009530e5e16c0613b9d085f3b908d8
> src/main/thrift/org/apache/aurora/gen/api.thrift 2376a5e530b12fbbebb4cfc7555656ae07795518
> src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 49770e5f87f047502e4f5653b908657a40d8683f
> 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 336cada625b85618486660fc24f3e83a312609f8
> 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
>
>
Re: Review Request 25812: Implementing quota checking for async job
updates.
Posted by Bill Farner <wf...@apache.org>.
> 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
>
>
Re: Review Request 25812: Implementing quota checking for async job
updates.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On Sept. 23, 2014, 1:01 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java, line 268
> > <https://reviews.apache.org/r/25812/diff/1/?file=694286#file694286line268>
> >
> > Remember - javadoc is like html. These newlines are not preserved unless you do it explicitly. Drop a <p> here, ditto in other comments.
This was mostly for in-code readability but adding <p> would not hurt.
> On Sept. 23, 2014, 1:01 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java, line 274
> > <https://reviews.apache.org/r/25812/diff/1/?file=694286#file694286line274>
> >
> > how about s/prodFromTasks/getRequiredQuota/?
Well, quota here means one thing - pre-defined cap to compare against. I feel it would be too confusing to overload that term for consumption. Changed it to prodResourcesFromTasks instead.
> On Sept. 23, 2014, 1:01 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, line 354
> > <https://reviews.apache.org/r/25812/diff/1/?file=694289#file694289line354>
> >
> > Catching IllegalArgumentException is likely to be more broad than you intend. I suggest you throw a specific exception from the validate function to avoid incorrectly replying 400 for something that should be 500.
Agree, thanks for pushing it.
> 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.
How about validateTaskLimits?
> 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.
The only place it's used in is already wrapping it into EnumSet.
What specifically in JobUpdateControllerImpl? Could not find anything suitable to update.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25812/#review54168
-----------------------------------------------------------
On Sept. 23, 2014, 12:59 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> 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 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 779e925e4d9e7889e8cfd369cea9a8e5da3554d2
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 83ac034cff009530e5e16c0613b9d085f3b908d8
> src/main/thrift/org/apache/aurora/gen/api.thrift 2376a5e530b12fbbebb4cfc7555656ae07795518
> src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 49770e5f87f047502e4f5653b908657a40d8683f
> 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 336cada625b85618486660fc24f3e83a312609f8
> 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
>
>
Re: Review Request 25812: Implementing quota checking for async job
updates.
Posted by Maxim Khutornenko <ma...@apache.org>.
> 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.
>
> Bill Farner wrote:
> https://github.com/apache/incubator-aurora/blob/012aca3d2a5667f9cfd6a1b9b0b91c982d34bb94/src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java#L254-L258
Ah, I was searching the non-rebased branch. Done.
- Maxim
-----------------------------------------------------------
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
>
>
Re: Review Request 25812: Implementing quota checking for async job
updates.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
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
<https://reviews.apache.org/r/25812/#comment94165>
Remember - javadoc is like html. These newlines are not preserved unless you do it explicitly. Drop a <p> here, ditto in other comments.
src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java
<https://reviews.apache.org/r/25812/#comment94163>
how about s/prodFromTasks/getRequiredQuota/?
src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java
<https://reviews.apache.org/r/25812/#comment94164>
ditto - how about how about getRequiredQuota
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
<https://reviews.apache.org/r/25812/#comment94225>
Catching IllegalArgumentException is likely to be more broad than you intend. I suggest you throw a specific exception from the validate function to avoid incorrectly replying 400 for something that should be 500.
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
<https://reviews.apache.org/r/25812/#comment94226>
How about 'validateInstanceAddition'? I'm not tied to the name, but something more informative than 'validate' would be nice.
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
<https://reviews.apache.org/r/25812/#comment94227>
Ditto - avoid catching IllegalArgumentException.
src/main/thrift/org/apache/aurora/gen/api.thrift
<https://reviews.apache.org/r/25812/#comment94228>
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.
- Bill Farner
On Sept. 23, 2014, 12:59 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> 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 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 779e925e4d9e7889e8cfd369cea9a8e5da3554d2
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 83ac034cff009530e5e16c0613b9d085f3b908d8
> src/main/thrift/org/apache/aurora/gen/api.thrift 2376a5e530b12fbbebb4cfc7555656ae07795518
> src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 49770e5f87f047502e4f5653b908657a40d8683f
> 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 336cada625b85618486660fc24f3e83a312609f8
> 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
>
>
Re: Review Request 25812: Implementing quota checking for async job
updates.
Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
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 Sweeney
On Sept. 23, 2014, 9:48 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25812/
> -----------------------------------------------------------
>
> (Updated Sept. 23, 2014, 9:48 a.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/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 8f9e6d6fa00840d8ff94eb7cf1c4415eb5cc1269
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 348bdbfbb51800d31b67ea1b3f2632975e598306
> 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
>
>
Re: Review Request 25812: Implementing quota checking for async job
updates.
Posted by Kevin Sweeney <ke...@apache.org>.
> On Sept. 23, 2014, 1:07 p.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java, line 180
> > <https://reviews.apache.org/r/25812/diff/3/?file=702913#file702913line180>
> >
> > Unless I'm missing something here, this will throw when there are multiple updates for a single role? I think you want Multimaps.index here.
>
> Bill Farner wrote:
> Multiple _active_ updates are not allowed.
ah, my mistake
- Kevin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25812/#review54315
-----------------------------------------------------------
On Sept. 23, 2014, 9:48 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25812/
> -----------------------------------------------------------
>
> (Updated Sept. 23, 2014, 9:48 a.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/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 8f9e6d6fa00840d8ff94eb7cf1c4415eb5cc1269
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 348bdbfbb51800d31b67ea1b3f2632975e598306
> 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
>
>
Re: Review Request 25812: Implementing quota checking for async job
updates.
Posted by Bill Farner <wf...@apache.org>.
> On Sept. 23, 2014, 8:07 p.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java, line 180
> > <https://reviews.apache.org/r/25812/diff/3/?file=702913#file702913line180>
> >
> > Unless I'm missing something here, this will throw when there are multiple updates for a single role? I think you want Multimaps.index here.
Multiple _active_ updates are not allowed.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25812/#review54315
-----------------------------------------------------------
On Sept. 23, 2014, 4:48 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:48 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/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 8f9e6d6fa00840d8ff94eb7cf1c4415eb5cc1269
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 348bdbfbb51800d31b67ea1b3f2632975e598306
> 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
>
>
Re: Review Request 25812: Implementing quota checking for async job
updates.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On Sept. 23, 2014, 8:07 p.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, line 1469
> > <https://reviews.apache.org/r/25812/diff/3/?file=702916#file702916line1469>
> >
> > Refactor this to live outside the Impl class? SchedulerThriftInterface shouldn't need about implementation classes
Moved to QuotaUtil.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25812/#review54315
-----------------------------------------------------------
On Sept. 23, 2014, 4:48 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:48 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/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 8f9e6d6fa00840d8ff94eb7cf1c4415eb5cc1269
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 348bdbfbb51800d31b67ea1b3f2632975e598306
> 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
>
>
Re: Review Request 25812: Implementing quota checking for async job
updates.
Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
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
<https://reviews.apache.org/r/25812/#comment94426>
Unless I'm missing something here, this will throw when there are multiple updates for a single role? I think you want Multimaps.index here.
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
<https://reviews.apache.org/r/25812/#comment94419>
Refactor this to live outside the Impl class? SchedulerThriftInterface shouldn't need about implementation classes
- Kevin Sweeney
On Sept. 23, 2014, 9:48 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25812/
> -----------------------------------------------------------
>
> (Updated Sept. 23, 2014, 9:48 a.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/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 8f9e6d6fa00840d8ff94eb7cf1c4415eb5cc1269
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 348bdbfbb51800d31b67ea1b3f2632975e598306
> 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
>
>
Re: Review Request 25812: Implementing quota checking for async job
updates.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On Sept. 23, 2014, 8:10 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java, line 174
> > <https://reviews.apache.org/r/25812/diff/3/?file=702913#file702913line174>
> >
> > Accept JobUpdateStore here, that's all that is used.
Done.
> On Sept. 23, 2014, 8:10 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java, line 182
> > <https://reviews.apache.org/r/25812/diff/3/?file=702913#file702913line182>
> >
> > Can you leave a longer comment here? I understand what is going on, but i'm not sure other readers will have the context to follow the algorithm.
Reworded.
> On Sept. 23, 2014, 8:10 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java, line 186
> > <https://reviews.apache.org/r/25812/diff/3/?file=702913#file702913line186>
> >
> > Can you extract this predicate to a factory function? I think it might aid readability.
Done.
> On Sept. 23, 2014, 8:10 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java, line 267
> > <https://reviews.apache.org/r/25812/diff/3/?file=702913#file702913line267>
> >
> > space between * and <p>
Done.
> On Sept. 23, 2014, 8:10 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java, line 325
> > <https://reviews.apache.org/r/25812/diff/3/?file=702920#file702920line325>
> >
> > It would be great if these test cases validated the actual computed consumption here. Can you also call quotaManager.getQuotaInfo to validate the accounting?
Sure, added coverage for consumption.
> On Sept. 23, 2014, 8:10 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java, line 403
> > <https://reviews.apache.org/r/25812/diff/3/?file=702920#file702920line403>
> >
> > Can you add 2 cases:
> > - update adds instances
> > - update removes instances
Added.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25812/#review54316
-----------------------------------------------------------
On Sept. 23, 2014, 4:48 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:48 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/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 8f9e6d6fa00840d8ff94eb7cf1c4415eb5cc1269
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 348bdbfbb51800d31b67ea1b3f2632975e598306
> 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
>
>
Re: Review Request 25812: Implementing quota checking for async job
updates.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
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 think that will help make the desired output of the algorithm easier to understand in the future.
src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java
<https://reviews.apache.org/r/25812/#comment94420>
Accept JobUpdateStore here, that's all that is used.
src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java
<https://reviews.apache.org/r/25812/#comment94422>
Can you leave a longer comment here? I understand what is going on, but i'm not sure other readers will have the context to follow the algorithm.
src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java
<https://reviews.apache.org/r/25812/#comment94423>
Can you extract this predicate to a factory function? I think it might aid readability.
src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java
<https://reviews.apache.org/r/25812/#comment94424>
space between * and <p>
src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java
<https://reviews.apache.org/r/25812/#comment94425>
space between * and <p>
src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java
<https://reviews.apache.org/r/25812/#comment94428>
It would be great if these test cases validated the actual computed consumption here. Can you also call quotaManager.getQuotaInfo to validate the accounting?
src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java
<https://reviews.apache.org/r/25812/#comment94427>
Can you add 2 cases:
- update adds instances
- update removes instances
- Bill Farner
On Sept. 23, 2014, 4:48 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:48 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/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 8f9e6d6fa00840d8ff94eb7cf1c4415eb5cc1269
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 348bdbfbb51800d31b67ea1b3f2632975e598306
> 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
>
>
Re: Review Request 25812: Implementing quota checking for async job
updates.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
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 and Bill Farner.
Changes
-------
-davmclau
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/quota/QuotaUtil.java PRE-CREATION
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/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 8f9e6d6fa00840d8ff94eb7cf1c4415eb5cc1269
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 348bdbfbb51800d31b67ea1b3f2632975e598306
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
Re: Review Request 25812: Implementing quota checking for async job
updates.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
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., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> 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 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/quota/QuotaUtil.java PRE-CREATION
> 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/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 8f9e6d6fa00840d8ff94eb7cf1c4415eb5cc1269
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 348bdbfbb51800d31b67ea1b3f2632975e598306
> 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
>
>
Re: Review Request 25812: Implementing quota checking for async job
updates.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
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 McLaughlin, Kevin Sweeney, and Bill Farner.
Changes
-------
CR comments.
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 (updated)
-----
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/quota/QuotaUtil.java PRE-CREATION
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/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 8f9e6d6fa00840d8ff94eb7cf1c4415eb5cc1269
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 348bdbfbb51800d31b67ea1b3f2632975e598306
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
Re: Review Request 25812: Implementing quota checking for async job
updates.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
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, Kevin Sweeney, and Bill Farner.
Changes
-------
CR comments.
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 (updated)
-----
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/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 8f9e6d6fa00840d8ff94eb7cf1c4415eb5cc1269
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 348bdbfbb51800d31b67ea1b3f2632975e598306
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
Re: Review Request 25812: Implementing quota checking for async job
updates.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
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.
Changes
-------
CR comments.
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 (updated)
-----
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
Re: Review Request 25812: Implementing quota checking for async job
updates.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
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 McLaughlin, Kevin Sweeney, and Bill Farner.
Changes
-------
+kevints
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 779e925e4d9e7889e8cfd369cea9a8e5da3554d2
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 83ac034cff009530e5e16c0613b9d085f3b908d8
src/main/thrift/org/apache/aurora/gen/api.thrift 2376a5e530b12fbbebb4cfc7555656ae07795518
src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 49770e5f87f047502e4f5653b908657a40d8683f
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 336cada625b85618486660fc24f3e83a312609f8
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
Re: Review Request 25812: Implementing quota checking for async job
updates.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
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 and Bill Farner.
Changes
-------
Added ticket.
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 779e925e4d9e7889e8cfd369cea9a8e5da3554d2
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 83ac034cff009530e5e16c0613b9d085f3b908d8
src/main/thrift/org/apache/aurora/gen/api.thrift 2376a5e530b12fbbebb4cfc7555656ae07795518
src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 49770e5f87f047502e4f5653b908657a40d8683f
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 336cada625b85618486660fc24f3e83a312609f8
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