Re: Review Request 27044: Make executor overhead configurable
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/ --- (Updated Oct. 27, 2014, 4:03 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Changes --- Create an interface for the factory and reuse an existing object for injecting RAM and CPU overhead. Bugs: AURORA-830 https://issues.apache.org/jira/browse/AURORA-830 Repository: aurora Description --- This patch changes the scheduler such that the executor overhead can be configured from the commandline. Diffs (updated) - config/legacy_untested_classes.txt PRE-CREATION src/main/java/org/apache/aurora/scheduler/ExecutorOptions.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java 83d0406a8bc7ccc1ae29804d2a4c8e8dfb90072c src/main/java/org/apache/aurora/scheduler/ResourceSlot.java ed60447c798a97daceda4a3bba6ee9bcdcaedd0f src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 40b652c679d8e340f585e28cbed066335d9d760d src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 4088559c39014befaddb0b29dad45fac9f4545c4 src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java da29428adaebcb27b20a10a8c6b7e380662fce4a src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java e96974764844b5d1a3a05f6996075fccee209594 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 899416fceae498353880012b8a93491cff461064 src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 4065629e9d488b122aa811b9802def0b51a21294 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3 Diff: https://reviews.apache.org/r/27044/diff/ Testing --- ./gradlew clean build Thanks, Zameer Manji
Re: Review Request 27044: Make executor overhead configurable
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/#review58714 --- I couldn't figure out how to use FactoryModuleBuilder in a clean way so I'm no longer persuing that option with the code. - Zameer Manji On Oct. 27, 2014, 4:03 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/ --- (Updated Oct. 27, 2014, 4:03 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-830 https://issues.apache.org/jira/browse/AURORA-830 Repository: aurora Description --- This patch changes the scheduler such that the executor overhead can be configured from the commandline. Diffs - config/legacy_untested_classes.txt PRE-CREATION src/main/java/org/apache/aurora/scheduler/ExecutorOptions.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java 83d0406a8bc7ccc1ae29804d2a4c8e8dfb90072c src/main/java/org/apache/aurora/scheduler/ResourceSlot.java ed60447c798a97daceda4a3bba6ee9bcdcaedd0f src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 40b652c679d8e340f585e28cbed066335d9d760d src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 4088559c39014befaddb0b29dad45fac9f4545c4 src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java da29428adaebcb27b20a10a8c6b7e380662fce4a src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java e96974764844b5d1a3a05f6996075fccee209594 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 899416fceae498353880012b8a93491cff461064 src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 4065629e9d488b122aa811b9802def0b51a21294 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3 Diff: https://reviews.apache.org/r/27044/diff/ Testing --- ./gradlew clean build Thanks, Zameer Manji
Re: Review Request 27044: Make executor overhead configurable
On Oct. 22, 2014, 10:07 p.m., Bill Farner wrote: config/legacy_untested_classes.txt, line 9 https://reviews.apache.org/r/27044/diff/1/?file=728771#file728771line9 In the interest of this file being delete only, can you bite the bullet and create a unit test to cover these in a test? Zameer Manji wrote: That invovles doing this TODO(wfarner): Try to accomplish all this by subclassing SchedulerMain and actually using AppLauncher. in SchedulerIT. I feel that deserves its own RB. Sorry for the extremely late reply to this, but you don't have to bite off that to cover the modules. You should be able to construct a `SchedulerMain`, call `getModules()` and test the bindings set up. If you want to precede this review with that one, that's fine :-) - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/#review57917 --- On Oct. 27, 2014, 11:03 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/ --- (Updated Oct. 27, 2014, 11:03 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-830 https://issues.apache.org/jira/browse/AURORA-830 Repository: aurora Description --- This patch changes the scheduler such that the executor overhead can be configured from the commandline. Diffs - config/legacy_untested_classes.txt PRE-CREATION src/main/java/org/apache/aurora/scheduler/ExecutorOptions.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java 83d0406a8bc7ccc1ae29804d2a4c8e8dfb90072c src/main/java/org/apache/aurora/scheduler/ResourceSlot.java ed60447c798a97daceda4a3bba6ee9bcdcaedd0f src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 40b652c679d8e340f585e28cbed066335d9d760d src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 4088559c39014befaddb0b29dad45fac9f4545c4 src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java da29428adaebcb27b20a10a8c6b7e380662fce4a src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java e96974764844b5d1a3a05f6996075fccee209594 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 899416fceae498353880012b8a93491cff461064 src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 4065629e9d488b122aa811b9802def0b51a21294 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3 Diff: https://reviews.apache.org/r/27044/diff/ Testing --- ./gradlew clean build Thanks, Zameer Manji
Re: Review Request 27044: Make executor overhead configurable
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/#review58742 --- src/main/java/org/apache/aurora/scheduler/ExecutorOptions.java https://reviews.apache.org/r/27044/#comment99874 blank line separating wrapped method signature from body, for better readability src/main/java/org/apache/aurora/scheduler/ExecutorOptions.java https://reviews.apache.org/r/27044/#comment99875 It's odd that you check that executorCPUs is positive, but not this. src/main/java/org/apache/aurora/scheduler/ExecutorOptions.java https://reviews.apache.org/r/27044/#comment99876 is this one intentionally package private while the others are public? i don't care, but it is confusing. src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java https://reviews.apache.org/r/27044/#comment99880 Is it aware of executor overhead? I don't see that anymore, in fact - the class seems silly now. src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java https://reviews.apache.org/r/27044/#comment99881 s/Executor/executor/ ditto elsewhere. src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java https://reviews.apache.org/r/27044/#comment99883 Does this summing have anything to do with executor resources? If not, i don't believe it belongs here (perhaps a static function on the Resources/ResourceSlot class). If it does, it should be documented. Ditto below. src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java https://reviews.apache.org/r/27044/#comment99887 remove newline src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java https://reviews.apache.org/r/27044/#comment99888 newline above src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java https://reviews.apache.org/r/27044/#comment99889 fits on one line - Bill Farner On Oct. 27, 2014, 11:03 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/ --- (Updated Oct. 27, 2014, 11:03 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-830 https://issues.apache.org/jira/browse/AURORA-830 Repository: aurora Description --- This patch changes the scheduler such that the executor overhead can be configured from the commandline. Diffs - config/legacy_untested_classes.txt PRE-CREATION src/main/java/org/apache/aurora/scheduler/ExecutorOptions.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java 83d0406a8bc7ccc1ae29804d2a4c8e8dfb90072c src/main/java/org/apache/aurora/scheduler/ResourceSlot.java ed60447c798a97daceda4a3bba6ee9bcdcaedd0f src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 40b652c679d8e340f585e28cbed066335d9d760d src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 4088559c39014befaddb0b29dad45fac9f4545c4 src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java da29428adaebcb27b20a10a8c6b7e380662fce4a src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java e96974764844b5d1a3a05f6996075fccee209594 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 899416fceae498353880012b8a93491cff461064 src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 4065629e9d488b122aa811b9802def0b51a21294 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3 Diff: https://reviews.apache.org/r/27044/diff/ Testing --- ./gradlew clean build Thanks, Zameer Manji
Re: Review Request 27044: Make executor overhead configurable
On Oct. 22, 2014, 10:32 a.m., Joshua Cohen wrote: src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java, line 67 https://reviews.apache.org/r/27044/diff/1/?file=728781#file728781line67 Do we actually need to use a ResourceSlotFactory here? The usage seems to be entirely internal to the test. We could just use constants? We need the slot factory to create the task factory in this test. - Zameer --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/#review57825 --- On Oct. 22, 2014, 9:57 a.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/ --- (Updated Oct. 22, 2014, 9:57 a.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-830 https://issues.apache.org/jira/browse/AURORA-830 Repository: aurora Description --- This patch changes the scheduler such that the executor overhead can be configured from the commandline. Diffs - config/legacy_untested_classes.txt 542ac31996d76fd4ab4e0583a737496c0e217a50 src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java 83d0406a8bc7ccc1ae29804d2a4c8e8dfb90072c src/main/java/org/apache/aurora/scheduler/ResourceSlot.java ed60447c798a97daceda4a3bba6ee9bcdcaedd0f src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 048740953629a950c136179c9b880b8dce8fa932 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java e9f251508257cd7287ff00773e0073a3cd130df8 src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 283f7e1e22decfe1053bd5640e8283b40eeac592 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java da29428adaebcb27b20a10a8c6b7e380662fce4a src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java e96974764844b5d1a3a05f6996075fccee209594 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java c405d4c8b127c2dd7054c9520064da0346690f02 src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 4065629e9d488b122aa811b9802def0b51a21294 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3 Diff: https://reviews.apache.org/r/27044/diff/ Testing --- ./gradlew clean build Thanks, Zameer Manji
Re: Review Request 27044: Make executor overhead configurable
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/#review58146 --- Driveby comment: Consider using Guice's FactoryModuleBuilder to minimize the factory boilerplate needed. IMO this should be the default anytime you want a factory that gets some of its dependencies from Guice, as the created objects will be instrumentable by Guice AOP. - Kevin Sweeney On Oct. 22, 2014, 9:57 a.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/ --- (Updated Oct. 22, 2014, 9:57 a.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-830 https://issues.apache.org/jira/browse/AURORA-830 Repository: aurora Description --- This patch changes the scheduler such that the executor overhead can be configured from the commandline. Diffs - config/legacy_untested_classes.txt 542ac31996d76fd4ab4e0583a737496c0e217a50 src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java 83d0406a8bc7ccc1ae29804d2a4c8e8dfb90072c src/main/java/org/apache/aurora/scheduler/ResourceSlot.java ed60447c798a97daceda4a3bba6ee9bcdcaedd0f src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 048740953629a950c136179c9b880b8dce8fa932 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java e9f251508257cd7287ff00773e0073a3cd130df8 src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 283f7e1e22decfe1053bd5640e8283b40eeac592 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java da29428adaebcb27b20a10a8c6b7e380662fce4a src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java e96974764844b5d1a3a05f6996075fccee209594 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java c405d4c8b127c2dd7054c9520064da0346690f02 src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 4065629e9d488b122aa811b9802def0b51a21294 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3 Diff: https://reviews.apache.org/r/27044/diff/ Testing --- ./gradlew clean build Thanks, Zameer Manji
Re: Review Request 27044: Make executor overhead configurable
On Oct. 22, 2014, 3:07 p.m., Bill Farner wrote: config/legacy_untested_classes.txt, line 9 https://reviews.apache.org/r/27044/diff/1/?file=728771#file728771line9 In the interest of this file being delete only, can you bite the bullet and create a unit test to cover these in a test? That invovles doing this TODO(wfarner): Try to accomplish all this by subclassing SchedulerMain and actually using AppLauncher. in SchedulerIT. I feel that deserves its own RB. - Zameer --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/#review57917 --- On Oct. 22, 2014, 9:57 a.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/ --- (Updated Oct. 22, 2014, 9:57 a.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-830 https://issues.apache.org/jira/browse/AURORA-830 Repository: aurora Description --- This patch changes the scheduler such that the executor overhead can be configured from the commandline. Diffs - config/legacy_untested_classes.txt 542ac31996d76fd4ab4e0583a737496c0e217a50 src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java 83d0406a8bc7ccc1ae29804d2a4c8e8dfb90072c src/main/java/org/apache/aurora/scheduler/ResourceSlot.java ed60447c798a97daceda4a3bba6ee9bcdcaedd0f src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 048740953629a950c136179c9b880b8dce8fa932 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java e9f251508257cd7287ff00773e0073a3cd130df8 src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 283f7e1e22decfe1053bd5640e8283b40eeac592 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java da29428adaebcb27b20a10a8c6b7e380662fce4a src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java e96974764844b5d1a3a05f6996075fccee209594 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java c405d4c8b127c2dd7054c9520064da0346690f02 src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 4065629e9d488b122aa811b9802def0b51a21294 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3 Diff: https://reviews.apache.org/r/27044/diff/ Testing --- ./gradlew clean build Thanks, Zameer Manji
Re: Review Request 27044: Make executor overhead configurable
On Oct. 22, 2014, 4:02 p.m., Maxim Khutornenko wrote: config/legacy_untested_classes.txt, line 1 https://reviews.apache.org/r/27044/diff/1/?file=728771#file728771line1 This seems to be out of order, revert? This is the output of the unix `sort` command. - Zameer --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/#review57819 --- On Oct. 22, 2014, 9:57 a.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/ --- (Updated Oct. 22, 2014, 9:57 a.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-830 https://issues.apache.org/jira/browse/AURORA-830 Repository: aurora Description --- This patch changes the scheduler such that the executor overhead can be configured from the commandline. Diffs - config/legacy_untested_classes.txt 542ac31996d76fd4ab4e0583a737496c0e217a50 src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java 83d0406a8bc7ccc1ae29804d2a4c8e8dfb90072c src/main/java/org/apache/aurora/scheduler/ResourceSlot.java ed60447c798a97daceda4a3bba6ee9bcdcaedd0f src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 048740953629a950c136179c9b880b8dce8fa932 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java e9f251508257cd7287ff00773e0073a3cd130df8 src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 283f7e1e22decfe1053bd5640e8283b40eeac592 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java da29428adaebcb27b20a10a8c6b7e380662fce4a src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java e96974764844b5d1a3a05f6996075fccee209594 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java c405d4c8b127c2dd7054c9520064da0346690f02 src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 4065629e9d488b122aa811b9802def0b51a21294 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3 Diff: https://reviews.apache.org/r/27044/diff/ Testing --- ./gradlew clean build Thanks, Zameer Manji
Re: Review Request 27044: Make executor overhead configurable
On Oct. 22, 2014, 5:32 p.m., Joshua Cohen wrote: src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java, line 67 https://reviews.apache.org/r/27044/diff/1/?file=728781#file728781line67 Do we actually need to use a ResourceSlotFactory here? The usage seems to be entirely internal to the test. We could just use constants? Zameer Manji wrote: We need the slot factory to create the task factory in this test. derp, missed that. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/#review57825 --- On Oct. 22, 2014, 4:57 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/ --- (Updated Oct. 22, 2014, 4:57 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-830 https://issues.apache.org/jira/browse/AURORA-830 Repository: aurora Description --- This patch changes the scheduler such that the executor overhead can be configured from the commandline. Diffs - config/legacy_untested_classes.txt 542ac31996d76fd4ab4e0583a737496c0e217a50 src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java 83d0406a8bc7ccc1ae29804d2a4c8e8dfb90072c src/main/java/org/apache/aurora/scheduler/ResourceSlot.java ed60447c798a97daceda4a3bba6ee9bcdcaedd0f src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 048740953629a950c136179c9b880b8dce8fa932 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java e9f251508257cd7287ff00773e0073a3cd130df8 src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 283f7e1e22decfe1053bd5640e8283b40eeac592 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java da29428adaebcb27b20a10a8c6b7e380662fce4a src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java e96974764844b5d1a3a05f6996075fccee209594 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java c405d4c8b127c2dd7054c9520064da0346690f02 src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 4065629e9d488b122aa811b9802def0b51a21294 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3 Diff: https://reviews.apache.org/r/27044/diff/ Testing --- ./gradlew clean build Thanks, Zameer Manji
Re: Review Request 27044: Make executor overhead configurable
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/ --- (Updated Oct. 23, 2014, 4:06 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Changes --- Most feedback. Bugs: AURORA-830 https://issues.apache.org/jira/browse/AURORA-830 Repository: aurora Description --- This patch changes the scheduler such that the executor overhead can be configured from the commandline. Diffs (updated) - config/legacy_untested_classes.txt PRE-CREATION src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java 83d0406a8bc7ccc1ae29804d2a4c8e8dfb90072c src/main/java/org/apache/aurora/scheduler/ResourceSlot.java ed60447c798a97daceda4a3bba6ee9bcdcaedd0f src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 40b652c679d8e340f585e28cbed066335d9d760d src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 4088559c39014befaddb0b29dad45fac9f4545c4 src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java da29428adaebcb27b20a10a8c6b7e380662fce4a src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java e96974764844b5d1a3a05f6996075fccee209594 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 899416fceae498353880012b8a93491cff461064 src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 4065629e9d488b122aa811b9802def0b51a21294 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3 Diff: https://reviews.apache.org/r/27044/diff/ Testing --- ./gradlew clean build Thanks, Zameer Manji
Re: Review Request 27044: Make executor overhead configurable
On Oct. 23, 2014, 3:32 p.m., Kevin Sweeney wrote: Driveby comment: Consider using Guice's FactoryModuleBuilder to minimize the factory boilerplate needed. IMO this should be the default anytime you want a factory that gets some of its dependencies from Guice, as the created objects will be instrumentable by Guice AOP. This is a great idea. I'm looking into it now. - Zameer --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/#review58146 --- On Oct. 23, 2014, 4:06 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/ --- (Updated Oct. 23, 2014, 4:06 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-830 https://issues.apache.org/jira/browse/AURORA-830 Repository: aurora Description --- This patch changes the scheduler such that the executor overhead can be configured from the commandline. Diffs - config/legacy_untested_classes.txt PRE-CREATION src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java 83d0406a8bc7ccc1ae29804d2a4c8e8dfb90072c src/main/java/org/apache/aurora/scheduler/ResourceSlot.java ed60447c798a97daceda4a3bba6ee9bcdcaedd0f src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 40b652c679d8e340f585e28cbed066335d9d760d src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 4088559c39014befaddb0b29dad45fac9f4545c4 src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java da29428adaebcb27b20a10a8c6b7e380662fce4a src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java e96974764844b5d1a3a05f6996075fccee209594 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 899416fceae498353880012b8a93491cff461064 src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 4065629e9d488b122aa811b9802def0b51a21294 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3 Diff: https://reviews.apache.org/r/27044/diff/ Testing --- ./gradlew clean build Thanks, Zameer Manji
Re: Review Request 27044: Make executor overhead configurable
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/#review58170 --- src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java https://reviews.apache.org/r/27044/#comment99095 private? Does not seem to be used outside. src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java https://reviews.apache.org/r/27044/#comment99096 newline src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java https://reviews.apache.org/r/27044/#comment99097 newline src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java https://reviews.apache.org/r/27044/#comment99098 missing param and return here and below src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java https://reviews.apache.org/r/27044/#comment99099 newline src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java https://reviews.apache.org/r/27044/#comment99101 missing javadoc from here down src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java https://reviews.apache.org/r/27044/#comment99102 newline - Maxim Khutornenko On Oct. 23, 2014, 11:06 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/ --- (Updated Oct. 23, 2014, 11:06 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-830 https://issues.apache.org/jira/browse/AURORA-830 Repository: aurora Description --- This patch changes the scheduler such that the executor overhead can be configured from the commandline. Diffs - config/legacy_untested_classes.txt PRE-CREATION src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java 83d0406a8bc7ccc1ae29804d2a4c8e8dfb90072c src/main/java/org/apache/aurora/scheduler/ResourceSlot.java ed60447c798a97daceda4a3bba6ee9bcdcaedd0f src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 40b652c679d8e340f585e28cbed066335d9d760d src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 4088559c39014befaddb0b29dad45fac9f4545c4 src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java da29428adaebcb27b20a10a8c6b7e380662fce4a src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java e96974764844b5d1a3a05f6996075fccee209594 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 899416fceae498353880012b8a93491cff461064 src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 4065629e9d488b122aa811b9802def0b51a21294 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3 Diff: https://reviews.apache.org/r/27044/diff/ Testing --- ./gradlew clean build Thanks, Zameer Manji
Re: Review Request 27044: Make executor overhead configurable
On Oct. 23, 2014, 4:47 p.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java, line 52 https://reviews.apache.org/r/27044/diff/2/?file=731618#file731618line52 private? Does not seem to be used outside. Making it private is a PMD violation: Avoid instantiation through private constructors from outside of the constructors class. - Zameer --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/#review58170 --- On Oct. 23, 2014, 4:06 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/ --- (Updated Oct. 23, 2014, 4:06 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-830 https://issues.apache.org/jira/browse/AURORA-830 Repository: aurora Description --- This patch changes the scheduler such that the executor overhead can be configured from the commandline. Diffs - config/legacy_untested_classes.txt PRE-CREATION src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java 83d0406a8bc7ccc1ae29804d2a4c8e8dfb90072c src/main/java/org/apache/aurora/scheduler/ResourceSlot.java ed60447c798a97daceda4a3bba6ee9bcdcaedd0f src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 40b652c679d8e340f585e28cbed066335d9d760d src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 4088559c39014befaddb0b29dad45fac9f4545c4 src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java da29428adaebcb27b20a10a8c6b7e380662fce4a src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java e96974764844b5d1a3a05f6996075fccee209594 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 899416fceae498353880012b8a93491cff461064 src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 4065629e9d488b122aa811b9802def0b51a21294 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3 Diff: https://reviews.apache.org/r/27044/diff/ Testing --- ./gradlew clean build Thanks, Zameer Manji
Re: Review Request 27044: Make executor overhead configurable
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/ --- (Updated Oct. 23, 2014, 5:24 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Changes --- Maxim's feedback. Bugs: AURORA-830 https://issues.apache.org/jira/browse/AURORA-830 Repository: aurora Description --- This patch changes the scheduler such that the executor overhead can be configured from the commandline. Diffs (updated) - config/legacy_untested_classes.txt PRE-CREATION src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java 83d0406a8bc7ccc1ae29804d2a4c8e8dfb90072c src/main/java/org/apache/aurora/scheduler/ResourceSlot.java ed60447c798a97daceda4a3bba6ee9bcdcaedd0f src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 40b652c679d8e340f585e28cbed066335d9d760d src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 4088559c39014befaddb0b29dad45fac9f4545c4 src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java da29428adaebcb27b20a10a8c6b7e380662fce4a src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java e96974764844b5d1a3a05f6996075fccee209594 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 899416fceae498353880012b8a93491cff461064 src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 4065629e9d488b122aa811b9802def0b51a21294 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3 Diff: https://reviews.apache.org/r/27044/diff/ Testing --- ./gradlew clean build Thanks, Zameer Manji
Re: Review Request 27044: Make executor overhead configurable
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/#review58204 --- Master (53f4e73) is red with this patch. ./build-support/jenkins/build.sh at org.gradle.util.ConfigureUtil.configure(ConfigureUtil.java:130) at org.gradle.util.ConfigureUtil.configure(ConfigureUtil.java:91) at org.gradle.util.ConfigureUtil$configure.call(Unknown Source) at org.gradle.api.internal.project.DefaultIsolatedAntBuilder.execute(DefaultIsolatedAntBuilder.groovy:116) at org.gradle.api.internal.project.IsolatedAntBuilder$execute$0.call(Unknown Source) at org.gradle.api.plugins.quality.Checkstyle.run(Checkstyle.groovy:120) at org.gradle.internal.reflect.JavaMethod.invoke(JavaMethod.java:63) at org.gradle.api.internal.project.taskfactory.AnnotationProcessingTaskFactory$StandardTaskAction.doExecute(AnnotationProcessingTaskFactory.java:218) at org.gradle.api.internal.project.taskfactory.AnnotationProcessingTaskFactory$StandardTaskAction.execute(AnnotationProcessingTaskFactory.java:211) at org.gradle.api.internal.project.taskfactory.AnnotationProcessingTaskFactory$StandardTaskAction.execute(AnnotationProcessingTaskFactory.java:200) at org.gradle.api.internal.AbstractTask$TaskActionWrapper.execute(AbstractTask.java:579) at org.gradle.api.internal.AbstractTask$TaskActionWrapper.execute(AbstractTask.java:562) at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.executeAction(ExecuteActionsTaskExecuter.java:80) at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.executeActions(ExecuteActionsTaskExecuter.java:61) ... 47 more BUILD FAILED Total time: 51.003 secs - Aurora ReviewBot On Oct. 24, 2014, 12:24 a.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/ --- (Updated Oct. 24, 2014, 12:24 a.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-830 https://issues.apache.org/jira/browse/AURORA-830 Repository: aurora Description --- This patch changes the scheduler such that the executor overhead can be configured from the commandline. Diffs - config/legacy_untested_classes.txt PRE-CREATION src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java 83d0406a8bc7ccc1ae29804d2a4c8e8dfb90072c src/main/java/org/apache/aurora/scheduler/ResourceSlot.java ed60447c798a97daceda4a3bba6ee9bcdcaedd0f src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 40b652c679d8e340f585e28cbed066335d9d760d src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 4088559c39014befaddb0b29dad45fac9f4545c4 src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 5cb0b2d15b1cd22de653946f4dfacac4cf3ab2e6 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java da29428adaebcb27b20a10a8c6b7e380662fce4a src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java e96974764844b5d1a3a05f6996075fccee209594 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 899416fceae498353880012b8a93491cff461064 src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 4065629e9d488b122aa811b9802def0b51a21294 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3 Diff: https://reviews.apache.org/r/27044/diff/ Testing --- ./gradlew clean build Thanks, Zameer Manji
Review Request 27044: Make executor overhead configurable
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/ --- Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-830 https://issues.apache.org/jira/browse/AURORA-830 Repository: aurora Description --- This patch changes the scheduler such that the executor overhead can be configured from the commandline. Diffs - config/legacy_untested_classes.txt 542ac31996d76fd4ab4e0583a737496c0e217a50 src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java 83d0406a8bc7ccc1ae29804d2a4c8e8dfb90072c src/main/java/org/apache/aurora/scheduler/ResourceSlot.java ed60447c798a97daceda4a3bba6ee9bcdcaedd0f src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 048740953629a950c136179c9b880b8dce8fa932 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java e9f251508257cd7287ff00773e0073a3cd130df8 src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 283f7e1e22decfe1053bd5640e8283b40eeac592 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java da29428adaebcb27b20a10a8c6b7e380662fce4a src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java e96974764844b5d1a3a05f6996075fccee209594 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java c405d4c8b127c2dd7054c9520064da0346690f02 src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 4065629e9d488b122aa811b9802def0b51a21294 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3 Diff: https://reviews.apache.org/r/27044/diff/ Testing --- ./gradlew clean build Thanks, Zameer Manji
Re: Review Request 27044: Make executor overhead configurable
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27044/#review57825
---
src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java
https://reviews.apache.org/r/27044/#comment98695
should this be a static import?
src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java
https://reviews.apache.org/r/27044/#comment98697
Continunation indent style is for these to be indented 4 spaces past the
parent, not lined up.
So:
public SchedulingFilterImpl(
Storage storage,
MaintenanceController maintenance,
...) {
// code
}
src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java
https://reviews.apache.org/r/27044/#comment98698
Do we actually need to use a ResourceSlotFactory here? The usage seems to
be entirely internal to the test. We could just use constants?
src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
https://reviews.apache.org/r/27044/#comment98699
Indentation looks off here?
- Joshua Cohen
On Oct. 22, 2014, 4:57 p.m., Zameer Manji wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27044/
---
(Updated Oct. 22, 2014, 4:57 p.m.)
Review request for Aurora, Maxim Khutornenko and Bill Farner.
Bugs: AURORA-830
https://issues.apache.org/jira/browse/AURORA-830
Repository: aurora
Description
---
This patch changes the scheduler such that the executor overhead can be
configured from the commandline.
Diffs
-
config/legacy_untested_classes.txt 542ac31996d76fd4ab4e0583a737496c0e217a50
src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java
83d0406a8bc7ccc1ae29804d2a4c8e8dfb90072c
src/main/java/org/apache/aurora/scheduler/ResourceSlot.java
ed60447c798a97daceda4a3bba6ee9bcdcaedd0f
src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java
PRE-CREATION
src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java
048740953629a950c136179c9b880b8dce8fa932
src/main/java/org/apache/aurora/scheduler/async/Preemptor.java
e9f251508257cd7287ff00773e0073a3cd130df8
src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java
283f7e1e22decfe1053bd5640e8283b40eeac592
src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java
1e3018e8c740ff322e0809ac2995121aa7d9b6d4
src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java
da29428adaebcb27b20a10a8c6b7e380662fce4a
src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java
4db9be86f2e7db08d12e0182914a7c5130301b13
src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java
e96974764844b5d1a3a05f6996075fccee209594
src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java
c405d4c8b127c2dd7054c9520064da0346690f02
src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java
8ee84cda8670d117e2efa2d1a114da6f0d8315d6
src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java
4065629e9d488b122aa811b9802def0b51a21294
src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
6a9c4ee278ed3ee8222404504e571f20991c2ae2
src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java
c48cbae4864127e7799917182439f7670285b0d3
Diff: https://reviews.apache.org/r/27044/diff/
Testing
---
./gradlew clean build
Thanks,
Zameer Manji
Re: Review Request 27044: Make executor overhead configurable
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/#review57917 --- config/legacy_untested_classes.txt https://reviews.apache.org/r/27044/#comment98818 In the interest of this file being delete only, can you bite the bullet and create a unit test to cover these in a test? - Bill Farner On Oct. 22, 2014, 4:57 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/ --- (Updated Oct. 22, 2014, 4:57 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-830 https://issues.apache.org/jira/browse/AURORA-830 Repository: aurora Description --- This patch changes the scheduler such that the executor overhead can be configured from the commandline. Diffs - config/legacy_untested_classes.txt 542ac31996d76fd4ab4e0583a737496c0e217a50 src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java 83d0406a8bc7ccc1ae29804d2a4c8e8dfb90072c src/main/java/org/apache/aurora/scheduler/ResourceSlot.java ed60447c798a97daceda4a3bba6ee9bcdcaedd0f src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 048740953629a950c136179c9b880b8dce8fa932 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java e9f251508257cd7287ff00773e0073a3cd130df8 src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 283f7e1e22decfe1053bd5640e8283b40eeac592 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java da29428adaebcb27b20a10a8c6b7e380662fce4a src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java e96974764844b5d1a3a05f6996075fccee209594 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java c405d4c8b127c2dd7054c9520064da0346690f02 src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 4065629e9d488b122aa811b9802def0b51a21294 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3 Diff: https://reviews.apache.org/r/27044/diff/ Testing --- ./gradlew clean build Thanks, Zameer Manji
Re: Review Request 27044: Make executor overhead configurable
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/#review57819 --- config/legacy_untested_classes.txt https://reviews.apache.org/r/27044/#comment98688 This seems to be out of order, revert? src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java https://reviews.apache.org/r/27044/#comment98819 Don't need a '\n' here. src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java https://reviews.apache.org/r/27044/#comment98820 This would be more compact: ``` .addResources(Resources.makeMesosResource( Resources.RAM_MB, slotFactory.getExecutorRAM().as(Data.MB))) ``` src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java https://reviews.apache.org/r/27044/#comment98821 Missing class javadoc. src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java https://reviews.apache.org/r/27044/#comment98822 requireNonNull src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java https://reviews.apache.org/r/27044/#comment98823 Move this param to the next line. src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java https://reviews.apache.org/r/27044/#comment98825 MorePreconditions.checkArgumentRange() src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java https://reviews.apache.org/r/27044/#comment98824 requireNonNull src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java https://reviews.apache.org/r/27044/#comment98826 Missing javadoc on non-getter public methods. src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java https://reviews.apache.org/r/27044/#comment98827 Formatting is off. - Maxim Khutornenko On Oct. 22, 2014, 4:57 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27044/ --- (Updated Oct. 22, 2014, 4:57 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-830 https://issues.apache.org/jira/browse/AURORA-830 Repository: aurora Description --- This patch changes the scheduler such that the executor overhead can be configured from the commandline. Diffs - config/legacy_untested_classes.txt 542ac31996d76fd4ab4e0583a737496c0e217a50 src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java 83d0406a8bc7ccc1ae29804d2a4c8e8dfb90072c src/main/java/org/apache/aurora/scheduler/ResourceSlot.java ed60447c798a97daceda4a3bba6ee9bcdcaedd0f src/main/java/org/apache/aurora/scheduler/ResourceSlotFactory.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 048740953629a950c136179c9b880b8dce8fa932 src/main/java/org/apache/aurora/scheduler/async/Preemptor.java e9f251508257cd7287ff00773e0073a3cd130df8 src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 283f7e1e22decfe1053bd5640e8283b40eeac592 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 1e3018e8c740ff322e0809ac2995121aa7d9b6d4 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java da29428adaebcb27b20a10a8c6b7e380662fce4a src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13 src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java e96974764844b5d1a3a05f6996075fccee209594 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java c405d4c8b127c2dd7054c9520064da0346690f02 src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 4065629e9d488b122aa811b9802def0b51a21294 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 6a9c4ee278ed3ee8222404504e571f20991c2ae2 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3 Diff: https://reviews.apache.org/r/27044/diff/ Testing --- ./gradlew clean build Thanks, Zameer Manji
