Re: Review Request 25309: Fix output formatting error in job status.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25309/ --- (Updated Sept. 4, 2014, 9:24 a.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Changes --- Poking around, I noticed several more glitches in the output formatting, and corrected them. Take another quick look please? Bugs: aurora-672 https://issues.apache.org/jira/browse/aurora-672 Repository: aurora Description --- Fix output formatting error in job status. Diffs (updated) - src/main/python/apache/aurora/client/cli/jobs.py ebc22aaa5a8aed311897b3ce9632b6f7175b6080 src/test/python/apache/aurora/client/cli/test_status.py 311fac02af32e0ed687489a2352164effb4dba96 Diff: https://reviews.apache.org/r/25309/diff/ Testing --- Ran unit tests; added new test cases. Thanks, Mark Chu-Carroll
Re: Review Request 25309: Fix output formatting error in job status.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25309/#review52308 --- src/main/python/apache/aurora/client/cli/jobs.py https://reviews.apache.org/r/25309/#comment91078 Curious, why not using multiple '\t' instead of spacing? - Maxim Khutornenko On Sept. 4, 2014, 1:24 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25309/ --- (Updated Sept. 4, 2014, 1:24 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Bugs: aurora-672 https://issues.apache.org/jira/browse/aurora-672 Repository: aurora Description --- Fix output formatting error in job status. Diffs - src/main/python/apache/aurora/client/cli/jobs.py ebc22aaa5a8aed311897b3ce9632b6f7175b6080 src/test/python/apache/aurora/client/cli/test_status.py 311fac02af32e0ed687489a2352164effb4dba96 Diff: https://reviews.apache.org/r/25309/diff/ Testing --- Ran unit tests; added new test cases. Thanks, Mark Chu-Carroll
Re: Review Request 25309: Fix output formatting error in job status.
On Sept. 4, 2014, 11:42 a.m., Maxim Khutornenko wrote: src/main/python/apache/aurora/client/cli/jobs.py, line 543 https://reviews.apache.org/r/25309/diff/1-2/?file=675766#file675766line543 Curious, why not using multiple '\t' instead of spacing? Looking at the output with standard terminal tab settings, it was ugly. The use of multiple tabs just ate up too much horizontal space. - Mark --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25309/#review52308 --- On Sept. 4, 2014, 9:24 a.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25309/ --- (Updated Sept. 4, 2014, 9:24 a.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Bugs: aurora-672 https://issues.apache.org/jira/browse/aurora-672 Repository: aurora Description --- Fix output formatting error in job status. Diffs - src/main/python/apache/aurora/client/cli/jobs.py ebc22aaa5a8aed311897b3ce9632b6f7175b6080 src/test/python/apache/aurora/client/cli/test_status.py 311fac02af32e0ed687489a2352164effb4dba96 Diff: https://reviews.apache.org/r/25309/diff/ Testing --- Ran unit tests; added new test cases. Thanks, Mark Chu-Carroll
Re: Review Request 25309: Fix output formatting error in job status.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25309/#review52311 --- Ship it! Ship It! - Maxim Khutornenko On Sept. 4, 2014, 1:24 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25309/ --- (Updated Sept. 4, 2014, 1:24 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Bugs: aurora-672 https://issues.apache.org/jira/browse/aurora-672 Repository: aurora Description --- Fix output formatting error in job status. Diffs - src/main/python/apache/aurora/client/cli/jobs.py ebc22aaa5a8aed311897b3ce9632b6f7175b6080 src/test/python/apache/aurora/client/cli/test_status.py 311fac02af32e0ed687489a2352164effb4dba96 Diff: https://reviews.apache.org/r/25309/diff/ Testing --- Ran unit tests; added new test cases. Thanks, Mark Chu-Carroll
Re: Review Request 24915: Adding initial GC task delay on scheduler restart.
On Sept. 2, 2014, 4:43 p.m., Bill Farner wrote: I'm concerned that the problem we're solving is underspecified. An immediate issue i have with this patch is that it introduces a memory leak (in practice, this is hidden with default settings due to failover failover hides this). I'll reply on the ticket to try to reach consensus on what the problem is and what it means to fix it. Maxim Khutornenko wrote: I don't think we are trying to fix a Mesos problem here. Regardless of the underlying Mesos resolution (MESOS-1646), I do think Aurora should be a good Mesos citizen here and avoid abnormal offer activity spikes driven by failovers. In order for that memory leak to expose itself in any reasonable manner, there must be an intensive and constant churn of hosts in the cluster. I don't think it's a likely scenario but if you are concerned it should be easy to converge on a hybrid solution with expiring cache where the EXECUTOR_GC_RESTART_INTERVAL kicks in any time an item is not found in the cache. I do think Aurora should be a good Mesos citizen here and avoid abnormal offer activity spikes driven by failovers Can you offer more detail here - why is this bad behavior? - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24915/#review52037 --- On Aug. 20, 2014, 11:35 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24915/ --- (Updated Aug. 20, 2014, 11:35 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-608 https://issues.apache.org/jira/browse/AURORA-608 Repository: aurora Description --- Adding initial GC task delay on scheduler restart. Diffs - src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 5a38a1f69ac5dbe68af3bfe175899ddee392880b src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 65f404915bc60ffe11a7a57d9861ac5b37fa646a src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 04137072891b2a1f0ad663182629dd469b09324f Diff: https://reviews.apache.org/r/24915/diff/ Testing --- gradle -Pq build Thanks, Maxim Khutornenko
Re: Review Request 24915: Adding initial GC task delay on scheduler restart.
On Sept. 2, 2014, 4:43 p.m., Bill Farner wrote: I'm concerned that the problem we're solving is underspecified. An immediate issue i have with this patch is that it introduces a memory leak (in practice, this is hidden with default settings due to failover failover hides this). I'll reply on the ticket to try to reach consensus on what the problem is and what it means to fix it. Maxim Khutornenko wrote: I don't think we are trying to fix a Mesos problem here. Regardless of the underlying Mesos resolution (MESOS-1646), I do think Aurora should be a good Mesos citizen here and avoid abnormal offer activity spikes driven by failovers. In order for that memory leak to expose itself in any reasonable manner, there must be an intensive and constant churn of hosts in the cluster. I don't think it's a likely scenario but if you are concerned it should be easy to converge on a hybrid solution with expiring cache where the EXECUTOR_GC_RESTART_INTERVAL kicks in any time an item is not found in the cache. Bill Farner wrote: I do think Aurora should be a good Mesos citizen here and avoid abnormal offer activity spikes driven by failovers Can you offer more detail here - why is this bad behavior? We spread GC activity randomly to avoid hourly spikes (https://reviews.apache.org/r/16247/). Failovers, however, break that pattern and manifest in unusual GC activity spikes. All Mesos matters aside, I don't think scheduler interruptions should result in what appears externally as a catch-up behavior. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24915/#review52037 --- On Aug. 20, 2014, 11:35 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24915/ --- (Updated Aug. 20, 2014, 11:35 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-608 https://issues.apache.org/jira/browse/AURORA-608 Repository: aurora Description --- Adding initial GC task delay on scheduler restart. Diffs - src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 5a38a1f69ac5dbe68af3bfe175899ddee392880b src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 65f404915bc60ffe11a7a57d9861ac5b37fa646a src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 04137072891b2a1f0ad663182629dd469b09324f Diff: https://reviews.apache.org/r/24915/diff/ Testing --- gradle -Pq build Thanks, Maxim Khutornenko
Re: Review Request 25337: Preserve executor HealthCheckerThread name
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25337/#review52318 --- Ship it! Ship It! - Zameer Manji On Sept. 4, 2014, 3:50 a.m., Joe Smith wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25337/ --- (Updated Sept. 4, 2014, 3:50 a.m.) Review request for Aurora, David McLaughlin, Maxim Khutornenko, and Brian Wickman. Bugs: AURORA-682 https://issues.apache.org/jira/browse/AURORA-682 Repository: aurora Description --- Preserve executor HealthCheckerThread name Diffs - src/main/python/apache/aurora/executor/common/health_checker.py 603fff35b839c6f53d9379ec047d7d8135a1c65b src/test/python/apache/aurora/executor/common/test_health_checker.py 490d4c8b5c434f9d6f032d931e35c483b3a5b676 Diff: https://reviews.apache.org/r/25337/diff/ Testing --- [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./pants ./src/test/python/apache/aurora/executor/common:health_checker Build operating on top level addresses: set([BuildFileAddress(/Users/jsmith/workspace/aurora/src/test/python/apache/aurora/executor/common/BUILD, health_checker)]) test session starts = platform darwin -- Python 2.7.8 -- py-1.4.23 -- pytest-2.6.1 plugins: cov, timeout collected 4 items src/test/python/apache/aurora/executor/common/test_health_checker.py == 4 passed in 1.54 seconds == src.test.python.apache.aurora.executor.common.health_checker . SUCCESS Tests in vagrant image Thanks, Joe Smith
Re: Review Request 24915: Adding initial GC task delay on scheduler restart.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24915/#review52328 --- Per offline discussion, i've realized my memory leak concern is ~moot since we already have the unbounded HostAttributes store. I liked your suggestion of augmenting the way this soft state is used: - use a Map, no expiry - when receiving an offer, don't kick off a GC task if a map entry is not present, store a future timestamp after which we should launch a GC task - Bill Farner On Aug. 20, 2014, 11:35 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24915/ --- (Updated Aug. 20, 2014, 11:35 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-608 https://issues.apache.org/jira/browse/AURORA-608 Repository: aurora Description --- Adding initial GC task delay on scheduler restart. Diffs - src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 5a38a1f69ac5dbe68af3bfe175899ddee392880b src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 65f404915bc60ffe11a7a57d9861ac5b37fa646a src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 04137072891b2a1f0ad663182629dd469b09324f Diff: https://reviews.apache.org/r/24915/diff/ Testing --- gradle -Pq build Thanks, Maxim Khutornenko
Re: Review Request 25285: Upgrade to latest in jetty 7.x series.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25285/ --- (Updated Sept. 4, 2014, 5:48 p.m.) Review request for Aurora, David McLaughlin and Kevin Sweeney. Bugs: AURORA-679 https://issues.apache.org/jira/browse/AURORA-679 Repository: aurora Description --- Long story short: this change adds code, but enables us to remove code in a follow-up. This change brings us to Jetty 7, which is the newest jetty that still uses servlet-api 2.5. Jetty 8/9 will require a larger yak shave since it pulls a dependency thread all the way to guice (servlet-api - jersey-guice - guice-servlet - guice). This is also a small step towards insulating ourselves from transitive dependencies pulled in from twitter commons. [1] http://wiki.eclipse.org/Jetty/Starting/Porting_to_Jetty_7 Diffs (updated) - build.gradle 494c54421f716412d20d4591c2d39e24f647c071 src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java de49a1c5497e32ee4db944412e5c87807c742d3c src/main/java/org/apache/aurora/scheduler/http/RequestLogger.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/RequestLoggerTest.java PRE-CREATION Diff: https://reviews.apache.org/r/25285/diff/ Testing --- ./gradlew build -Pq Ran the scheduler in vagrant, verified index page works, request logging works, and all pages linked from the index page work. Thanks, Bill Farner
Re: Review Request 25337: Preserve executor HealthCheckerThread name
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25337/#review52332 --- Discussed offline but it probably makes sense to not do any status checkers subclassed directly from threading.Thread. instead, the threads should be attributes on the health checkers and started when .start() is called. - Brian Wickman On Sept. 4, 2014, 10:50 a.m., Joe Smith wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25337/ --- (Updated Sept. 4, 2014, 10:50 a.m.) Review request for Aurora, David McLaughlin, Maxim Khutornenko, and Brian Wickman. Bugs: AURORA-682 https://issues.apache.org/jira/browse/AURORA-682 Repository: aurora Description --- Preserve executor HealthCheckerThread name Diffs - src/main/python/apache/aurora/executor/common/health_checker.py 603fff35b839c6f53d9379ec047d7d8135a1c65b src/test/python/apache/aurora/executor/common/test_health_checker.py 490d4c8b5c434f9d6f032d931e35c483b3a5b676 Diff: https://reviews.apache.org/r/25337/diff/ Testing --- [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./pants ./src/test/python/apache/aurora/executor/common:health_checker Build operating on top level addresses: set([BuildFileAddress(/Users/jsmith/workspace/aurora/src/test/python/apache/aurora/executor/common/BUILD, health_checker)]) test session starts = platform darwin -- Python 2.7.8 -- py-1.4.23 -- pytest-2.6.1 plugins: cov, timeout collected 4 items src/test/python/apache/aurora/executor/common/test_health_checker.py == 4 passed in 1.54 seconds == src.test.python.apache.aurora.executor.common.health_checker . SUCCESS Tests in vagrant image Thanks, Joe Smith
Review Request 25346: Specify required vagrant version in Vagrantfile.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25346/ --- Review request for Aurora and Kevin Sweeney. Bugs: AURORA-683 https://issues.apache.org/jira/browse/AURORA-683 Repository: aurora Description --- Specify required vagrant version in Vagrantfile. Diffs - Vagrantfile ea0b25294f0efeec745dc8612e4d7d02979079cc Diff: https://reviews.apache.org/r/25346/diff/ Testing --- vagant up Thanks, Bill Farner
Re: Review Request 25346: Specify required vagrant version in Vagrantfile.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25346/#review52334 --- Ship it! thanks! - Joe Smith On Sept. 4, 2014, 11:48 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25346/ --- (Updated Sept. 4, 2014, 11:48 a.m.) Review request for Aurora and Kevin Sweeney. Bugs: AURORA-683 https://issues.apache.org/jira/browse/AURORA-683 Repository: aurora Description --- Specify required vagrant version in Vagrantfile. Diffs - Vagrantfile ea0b25294f0efeec745dc8612e4d7d02979079cc Diff: https://reviews.apache.org/r/25346/diff/ Testing --- vagant up Thanks, Bill Farner
Re: Review Request 25346: Specify required vagrant version in Vagrantfile.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25346/#review52335 --- Ship it! Ship It! - Kevin Sweeney On Sept. 4, 2014, 11:48 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25346/ --- (Updated Sept. 4, 2014, 11:48 a.m.) Review request for Aurora and Kevin Sweeney. Bugs: AURORA-683 https://issues.apache.org/jira/browse/AURORA-683 Repository: aurora Description --- Specify required vagrant version in Vagrantfile. Diffs - Vagrantfile ea0b25294f0efeec745dc8612e4d7d02979079cc Diff: https://reviews.apache.org/r/25346/diff/ Testing --- vagant up Thanks, Bill Farner
Review Request 25350: Fix build problem.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25350/ --- Review request for Aurora and Bill Farner. Bugs: aurora-684 https://issues.apache.org/jira/browse/aurora-684 Repository: aurora Description --- Timestamps in pretty-printed job status use the local timezone. Jenkins machines use GMT as their local time; so the time printed on jenkins differs from times printed elsewhere. This fix fuzzes out the time in the timestamps, so that it doesn't matter where the test is run. Diffs - src/test/python/apache/aurora/client/cli/test_status.py 2af24fbaa4971819636898df752fa886553d75a3 Diff: https://reviews.apache.org/r/25350/diff/ Testing --- Thanks, Mark Chu-Carroll
Re: Review Request 25350: Fix build problem.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25350/#review52341 --- Ship it! Can you change the subject of the review to be more specific? This isn't very friendly when scanning the commit log. src/test/python/apache/aurora/client/cli/test_status.py https://reviews.apache.org/r/25350/#comment91105 TODO inject a fake clock? - Bill Farner On Sept. 4, 2014, 7:45 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25350/ --- (Updated Sept. 4, 2014, 7:45 p.m.) Review request for Aurora and Bill Farner. Bugs: aurora-684 https://issues.apache.org/jira/browse/aurora-684 Repository: aurora Description --- Timestamps in pretty-printed job status use the local timezone. Jenkins machines use GMT as their local time; so the time printed on jenkins differs from times printed elsewhere. This fix fuzzes out the time in the timestamps, so that it doesn't matter where the test is run. Diffs - src/test/python/apache/aurora/client/cli/test_status.py 2af24fbaa4971819636898df752fa886553d75a3 Diff: https://reviews.apache.org/r/25350/diff/ Testing --- Thanks, Mark Chu-Carroll
Re: Review Request 24815: Refactoring SchedulerCore final part.
On Sept. 2, 2014, 10:36 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/state/TaskLimitValidator.java, line 35 https://reviews.apache.org/r/24815/diff/1/?file=662758#file662758line35 Is this interface useful? The implementation logic is trivial, and it's only used from SchedulerThriftInterface. I suggest putting it there. Maxim Khutornenko wrote: Right now that is correct. However, I'd expect JobUpdateController to accept it in order to check quota any time an instance about to be added. in order to check quota any time an instance about to be added Is there a situation i'm not thinking of where that is needed rather than performing the quota check only once when first initiating the update? Is this because QuotaManager doesn't account for in-progress job udpates? - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24815/#review52089 --- On Aug. 18, 2014, 8:04 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24815/ --- (Updated Aug. 18, 2014, 8:04 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-94 https://issues.apache.org/jira/browse/AURORA-94 Repository: aurora Description --- Moving the last bits of functionality out of SchedulerCore. Also, preparing the ground for task validation logic reuse in updater code. Diffs - src/main/java/org/apache/aurora/scheduler/base/ScheduleException.java e060e5ec88a0ab311415eaa638cf693c99c40049 src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java 62202810fd5829545fc77b91a20d1bb433a4916a src/main/java/org/apache/aurora/scheduler/state/SchedulerCore.java c636fd7fde8b592b167da8e5e9651ac772bc23de src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java 3dcb1c3e3d0138634b3d077c845ecc0d61d7fc0f src/main/java/org/apache/aurora/scheduler/state/StateManager.java 6e062b39175255264020bbb1cbd485de9a114a20 src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 6ad104b050aabecad1dadc975c27d9d3603659bf src/main/java/org/apache/aurora/scheduler/state/StateModule.java cc1eee4e19c092c0d401558ac01b54627f2d1290 src/main/java/org/apache/aurora/scheduler/state/TaskLimitValidator.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 7ef28858ad290c74248b89c49d2a684eb1c7127e src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java c7ae7db16e82c722fae1bccb9178f5ec203e91e5 src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 62dce07b42af02d25b788e763e4e2a026dd2d483 src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java fa611a913bad40a8c0515c578b394c460340e574 src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 1678411ad5c25e74f8b6d004bf491ea26ca0 src/test/java/org/apache/aurora/scheduler/state/TaskLimitValidatorTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/storage/mem/MemStorageSchedulerCoreImplTest.java 35bed104d838596abcbb5abd5cad29592b384dfa src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 649afa24b2cfc9a1d67d350473e439d209bd720c src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 43265fdab1ae900fb828374f6c69e562def2d682 Diff: https://reviews.apache.org/r/24815/diff/ Testing --- gradle -Pq build Thanks, Maxim Khutornenko
Re: Review Request 24815: Refactoring SchedulerCore final part.
On Sept. 2, 2014, 10:36 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/state/TaskLimitValidator.java, line 35 https://reviews.apache.org/r/24815/diff/1/?file=662758#file662758line35 Is this interface useful? The implementation logic is trivial, and it's only used from SchedulerThriftInterface. I suggest putting it there. Maxim Khutornenko wrote: Right now that is correct. However, I'd expect JobUpdateController to accept it in order to check quota any time an instance about to be added. Bill Farner wrote: in order to check quota any time an instance about to be added Is there a situation i'm not thinking of where that is needed rather than performing the quota check only once when first initiating the update? Is this because QuotaManager doesn't account for in-progress job udpates? Right. Since we are tracking quota at the role level but the update lock applied at the job level, there is always a possibility to exceed the allowed quota for long running updates. This is especially a problem with the server side-dirven process where a resumed update will restart in a potentially quite different quota environment (i.e. due to other jobs created while the update was paused). We could check quota whithin resumeUpdate() RPC but that would still leave room for a race. The only correct approach would be to check quota within the same transaction the addInstance action takes place. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24815/#review52089 --- On Aug. 18, 2014, 8:04 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24815/ --- (Updated Aug. 18, 2014, 8:04 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-94 https://issues.apache.org/jira/browse/AURORA-94 Repository: aurora Description --- Moving the last bits of functionality out of SchedulerCore. Also, preparing the ground for task validation logic reuse in updater code. Diffs - src/main/java/org/apache/aurora/scheduler/base/ScheduleException.java e060e5ec88a0ab311415eaa638cf693c99c40049 src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java 62202810fd5829545fc77b91a20d1bb433a4916a src/main/java/org/apache/aurora/scheduler/state/SchedulerCore.java c636fd7fde8b592b167da8e5e9651ac772bc23de src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java 3dcb1c3e3d0138634b3d077c845ecc0d61d7fc0f src/main/java/org/apache/aurora/scheduler/state/StateManager.java 6e062b39175255264020bbb1cbd485de9a114a20 src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 6ad104b050aabecad1dadc975c27d9d3603659bf src/main/java/org/apache/aurora/scheduler/state/StateModule.java cc1eee4e19c092c0d401558ac01b54627f2d1290 src/main/java/org/apache/aurora/scheduler/state/TaskLimitValidator.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 7ef28858ad290c74248b89c49d2a684eb1c7127e src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java c7ae7db16e82c722fae1bccb9178f5ec203e91e5 src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 62dce07b42af02d25b788e763e4e2a026dd2d483 src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java fa611a913bad40a8c0515c578b394c460340e574 src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 1678411ad5c25e74f8b6d004bf491ea26ca0 src/test/java/org/apache/aurora/scheduler/state/TaskLimitValidatorTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/storage/mem/MemStorageSchedulerCoreImplTest.java 35bed104d838596abcbb5abd5cad29592b384dfa src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 649afa24b2cfc9a1d67d350473e439d209bd720c src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 43265fdab1ae900fb828374f6c69e562def2d682 Diff: https://reviews.apache.org/r/24815/diff/ Testing --- gradle -Pq build Thanks, Maxim Khutornenko
Re: Review Request 24815: Refactoring SchedulerCore final part.
On Sept. 2, 2014, 10:36 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/state/TaskLimitValidator.java, line 35 https://reviews.apache.org/r/24815/diff/1/?file=662758#file662758line35 Is this interface useful? The implementation logic is trivial, and it's only used from SchedulerThriftInterface. I suggest putting it there. Maxim Khutornenko wrote: Right now that is correct. However, I'd expect JobUpdateController to accept it in order to check quota any time an instance about to be added. Bill Farner wrote: in order to check quota any time an instance about to be added Is there a situation i'm not thinking of where that is needed rather than performing the quota check only once when first initiating the update? Is this because QuotaManager doesn't account for in-progress job udpates? Maxim Khutornenko wrote: Right. Since we are tracking quota at the role level but the update lock applied at the job level, there is always a possibility to exceed the allowed quota for long running updates. This is especially a problem with the server side-dirven process where a resumed update will restart in a potentially quite different quota environment (i.e. due to other jobs created while the update was paused). We could check quota whithin resumeUpdate() RPC but that would still leave room for a race. The only correct approach would be to check quota within the same transaction the addInstance action takes place. That makes for a pretty crummy user experience, though, since there's nothing preventing creation of a new job and blowing up updates. That apparently is the case today, but i think it warrants a bug - can you file? - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24815/#review52089 --- On Aug. 18, 2014, 8:04 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24815/ --- (Updated Aug. 18, 2014, 8:04 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-94 https://issues.apache.org/jira/browse/AURORA-94 Repository: aurora Description --- Moving the last bits of functionality out of SchedulerCore. Also, preparing the ground for task validation logic reuse in updater code. Diffs - src/main/java/org/apache/aurora/scheduler/base/ScheduleException.java e060e5ec88a0ab311415eaa638cf693c99c40049 src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java 62202810fd5829545fc77b91a20d1bb433a4916a src/main/java/org/apache/aurora/scheduler/state/SchedulerCore.java c636fd7fde8b592b167da8e5e9651ac772bc23de src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java 3dcb1c3e3d0138634b3d077c845ecc0d61d7fc0f src/main/java/org/apache/aurora/scheduler/state/StateManager.java 6e062b39175255264020bbb1cbd485de9a114a20 src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 6ad104b050aabecad1dadc975c27d9d3603659bf src/main/java/org/apache/aurora/scheduler/state/StateModule.java cc1eee4e19c092c0d401558ac01b54627f2d1290 src/main/java/org/apache/aurora/scheduler/state/TaskLimitValidator.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 7ef28858ad290c74248b89c49d2a684eb1c7127e src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java c7ae7db16e82c722fae1bccb9178f5ec203e91e5 src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 62dce07b42af02d25b788e763e4e2a026dd2d483 src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java fa611a913bad40a8c0515c578b394c460340e574 src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 1678411ad5c25e74f8b6d004bf491ea26ca0 src/test/java/org/apache/aurora/scheduler/state/TaskLimitValidatorTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/storage/mem/MemStorageSchedulerCoreImplTest.java 35bed104d838596abcbb5abd5cad29592b384dfa src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 649afa24b2cfc9a1d67d350473e439d209bd720c src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 43265fdab1ae900fb828374f6c69e562def2d682 Diff: https://reviews.apache.org/r/24815/diff/ Testing --- gradle -Pq build Thanks, Maxim Khutornenko
Re: Review Request 24815: Refactoring SchedulerCore final part.
On Sept. 2, 2014, 10:36 p.m., Bill Farner wrote:
src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java, line
120
https://reviews.apache.org/r/24815/diff/1/?file=662756#file662756line120
At first glance, this is odd since ITaskConfig contains the components
of a JobKey. Is that argument necessary?
Maxim Khutornenko wrote:
I tried and really hated that approach. Adding a bit of redunancy helped
avoiding clumsy job key extraction and edge case error conditions (e.g. empty
collection, ITaskConfigs from different jobs and etc.)
avoiding clumsy job key extraction and edge case error conditions
Those cases are still there though, right? Except now there's an additional
case currently not handled where the keys don't match. Perhaps the better
approach is to change the signature to:
`insertPendingTasks(ITaskConfig config, SetInteger instanceIds);`
Looking at the call sites, this is how it's used in practice.
In fact, there's a relevant TODO in SanitizedConfiguration:
`// TODO(William Farner): Rework this API now that all configs are identical.`
Even the late SchedulerCore#addInstance took this approach:
```java
public void addInstances(
final IJobKey jobKey,
final ImmutableSetInteger instanceIds,
final ITaskConfig config) throws ScheduleException {
```
(though unfortunately it also duplicated the job key)
- Bill
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24815/#review52089
---
On Aug. 18, 2014, 8:04 p.m., Maxim Khutornenko wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24815/
---
(Updated Aug. 18, 2014, 8:04 p.m.)
Review request for Aurora and Bill Farner.
Bugs: AURORA-94
https://issues.apache.org/jira/browse/AURORA-94
Repository: aurora
Description
---
Moving the last bits of functionality out of SchedulerCore.
Also, preparing the ground for task validation logic reuse in updater code.
Diffs
-
src/main/java/org/apache/aurora/scheduler/base/ScheduleException.java
e060e5ec88a0ab311415eaa638cf693c99c40049
src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java
62202810fd5829545fc77b91a20d1bb433a4916a
src/main/java/org/apache/aurora/scheduler/state/SchedulerCore.java
c636fd7fde8b592b167da8e5e9651ac772bc23de
src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java
3dcb1c3e3d0138634b3d077c845ecc0d61d7fc0f
src/main/java/org/apache/aurora/scheduler/state/StateManager.java
6e062b39175255264020bbb1cbd485de9a114a20
src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java
6ad104b050aabecad1dadc975c27d9d3603659bf
src/main/java/org/apache/aurora/scheduler/state/StateModule.java
cc1eee4e19c092c0d401558ac01b54627f2d1290
src/main/java/org/apache/aurora/scheduler/state/TaskLimitValidator.java
PRE-CREATION
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
7ef28858ad290c74248b89c49d2a684eb1c7127e
src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java
c7ae7db16e82c722fae1bccb9178f5ec203e91e5
src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java
62dce07b42af02d25b788e763e4e2a026dd2d483
src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java
fa611a913bad40a8c0515c578b394c460340e574
src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java
1678411ad5c25e74f8b6d004bf491ea26ca0
src/test/java/org/apache/aurora/scheduler/state/TaskLimitValidatorTest.java
PRE-CREATION
src/test/java/org/apache/aurora/scheduler/storage/mem/MemStorageSchedulerCoreImplTest.java
35bed104d838596abcbb5abd5cad29592b384dfa
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
649afa24b2cfc9a1d67d350473e439d209bd720c
src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java
43265fdab1ae900fb828374f6c69e562def2d682
Diff: https://reviews.apache.org/r/24815/diff/
Testing
---
gradle -Pq build
Thanks,
Maxim Khutornenko
Re: Review Request 24815: Refactoring SchedulerCore final part.
On Sept. 2, 2014, 10:36 p.m., Bill Farner wrote:
src/main/java/org/apache/aurora/scheduler/state/TaskLimitValidator.java,
line 35
https://reviews.apache.org/r/24815/diff/1/?file=662758#file662758line35
Is this interface useful? The implementation logic is trivial, and
it's only used from SchedulerThriftInterface. I suggest putting it there.
Maxim Khutornenko wrote:
Right now that is correct. However, I'd expect JobUpdateController to
accept it in order to check quota any time an instance about to be added.
Bill Farner wrote:
in order to check quota any time an instance about to be added
Is there a situation i'm not thinking of where that is needed rather than
performing the quota check only once when first initiating the update? Is
this because QuotaManager doesn't account for in-progress job udpates?
Maxim Khutornenko wrote:
Right. Since we are tracking quota at the role level but the update lock
applied at the job level, there is always a possibility to exceed the allowed
quota for long running updates. This is especially a problem with the server
side-dirven process where a resumed update will restart in a potentially
quite different quota environment (i.e. due to other jobs created while the
update was paused). We could check quota whithin resumeUpdate() RPC but that
would still leave room for a race. The only correct approach would be to
check quota within the same transaction the addInstance action takes place.
Bill Farner wrote:
That makes for a pretty crummy user experience, though, since there's
nothing preventing creation of a new job and blowing up updates. That
apparently is the case today, but i think it warrants a bug - can you file?
https://issues.apache.org/jira/browse/AURORA-686
On Sept. 2, 2014, 10:36 p.m., Bill Farner wrote:
src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java, line
120
https://reviews.apache.org/r/24815/diff/1/?file=662756#file662756line120
At first glance, this is odd since ITaskConfig contains the components
of a JobKey. Is that argument necessary?
Maxim Khutornenko wrote:
I tried and really hated that approach. Adding a bit of redunancy helped
avoiding clumsy job key extraction and edge case error conditions (e.g. empty
collection, ITaskConfigs from different jobs and etc.)
Bill Farner wrote:
avoiding clumsy job key extraction and edge case error conditions
Those cases are still there though, right? Except now there's an
additional case currently not handled where the keys don't match. Perhaps
the better approach is to change the signature to:
`insertPendingTasks(ITaskConfig config, SetInteger instanceIds);`
Looking at the call sites, this is how it's used in practice.
In fact, there's a relevant TODO in SanitizedConfiguration:
`// TODO(William Farner): Rework this API now that all configs are
identical.`
Even the late SchedulerCore#addInstance took this approach:
```java
public void addInstances(
final IJobKey jobKey,
final ImmutableSetInteger instanceIds,
final ITaskConfig config) throws ScheduleException {
```
(though unfortunately it also duplicated the job key)
Sounds like a reasonable tradeoff. Will give it a shot.
| (though unfortunately it also duplicated the job key)
I am less concerned about that as it can be viewed as a convenience feature.
- Maxim
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24815/#review52089
---
On Aug. 18, 2014, 8:04 p.m., Maxim Khutornenko wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24815/
---
(Updated Aug. 18, 2014, 8:04 p.m.)
Review request for Aurora and Bill Farner.
Bugs: AURORA-94
https://issues.apache.org/jira/browse/AURORA-94
Repository: aurora
Description
---
Moving the last bits of functionality out of SchedulerCore.
Also, preparing the ground for task validation logic reuse in updater code.
Diffs
-
src/main/java/org/apache/aurora/scheduler/base/ScheduleException.java
e060e5ec88a0ab311415eaa638cf693c99c40049
src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java
62202810fd5829545fc77b91a20d1bb433a4916a
src/main/java/org/apache/aurora/scheduler/state/SchedulerCore.java
c636fd7fde8b592b167da8e5e9651ac772bc23de
src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java
3dcb1c3e3d0138634b3d077c845ecc0d61d7fc0f
src/main/java/org/apache/aurora/scheduler/state/StateManager.java
Review Request 25356: Add a factory to produce OneWayUpdate instances based on job state and update settings.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25356/ --- Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-613 https://issues.apache.org/jira/browse/AURORA-613 Repository: aurora Description --- This is the bridge between the top-level updater logic and the OneWayJobUpdater - taking a user request and the direction we've decided to move the job (forward or back), produce the appropriate OneWayJobUpdater. Diffs - src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 85196af38b6615e5aac7de30a4a601350e935c72 src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java aa8b5f2ee81d42db003f27e682f79e94b6625d87 src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterFactory.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java dca55ef5612105c9dc8fd57e789fdf1df28ba447 src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java d7baaa679289998a9ea80c0934990a49e5c173d7 src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterFactoryImplTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java e3e50d7d6526e8796dc5cd82b459190b09ceb72f Diff: https://reviews.apache.org/r/25356/diff/ Testing --- ./gradlew build -Pq Thanks, Bill Farner
Review Request 25357: Adding support for per-job task status metrics.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25357/ --- Review request for Aurora, David McLaughlin and Bill Farner. Bugs: AURORA-685 https://issues.apache.org/jira/browse/AURORA-685 Repository: aurora Description --- Currently tracking LOST and FAILED states. Diffs - src/main/java/org/apache/aurora/scheduler/TaskVars.java 6654c1675ac9f5f7d481e115cea7c224fb212467 src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java d02714c846a521ff9ac3e53d991731314e714ae2 Diff: https://reviews.apache.org/r/25357/diff/ Testing --- gradle -Pq build Thanks, Maxim Khutornenko
Re: Review Request 25356: Add a factory to produce OneWayUpdate instances based on job state and update settings.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25356/#review52361 --- Ship it! Ship It! - Zameer Manji On Sept. 4, 2014, 2:19 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25356/ --- (Updated Sept. 4, 2014, 2:19 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-613 https://issues.apache.org/jira/browse/AURORA-613 Repository: aurora Description --- This is the bridge between the top-level updater logic and the OneWayJobUpdater - taking a user request and the direction we've decided to move the job (forward or back), produce the appropriate OneWayJobUpdater. Diffs - src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 85196af38b6615e5aac7de30a4a601350e935c72 src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java aa8b5f2ee81d42db003f27e682f79e94b6625d87 src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterFactory.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java dca55ef5612105c9dc8fd57e789fdf1df28ba447 src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java d7baaa679289998a9ea80c0934990a49e5c173d7 src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterFactoryImplTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java e3e50d7d6526e8796dc5cd82b459190b09ceb72f Diff: https://reviews.apache.org/r/25356/diff/ Testing --- ./gradlew build -Pq Thanks, Bill Farner
Re: Review Request 25357: Adding support for per-job task status metrics.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25357/#review52362 --- Ship it! lgtm(y still scheduler-naive eyes). src/main/java/org/apache/aurora/scheduler/TaskVars.java https://reviews.apache.org/r/25357/#comment91130 use host (declared above) here in place of task.getAssignedTask().getSlaveHost()? - Joshua Cohen On Sept. 4, 2014, 9:42 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25357/ --- (Updated Sept. 4, 2014, 9:42 p.m.) Review request for Aurora, David McLaughlin and Bill Farner. Bugs: AURORA-685 https://issues.apache.org/jira/browse/AURORA-685 Repository: aurora Description --- Currently tracking LOST and FAILED states. Diffs - src/main/java/org/apache/aurora/scheduler/TaskVars.java 6654c1675ac9f5f7d481e115cea7c224fb212467 src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java d02714c846a521ff9ac3e53d991731314e714ae2 Diff: https://reviews.apache.org/r/25357/diff/ Testing --- gradle -Pq build Thanks, Maxim Khutornenko
Re: Review Request 25356: Add a factory to produce OneWayUpdate instances based on job state and update settings.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25356/#review52363 --- src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java https://reviews.apache.org/r/25356/#comment91131 Does it have to be synchronized? Could not find the reason based on its call chain. src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterFactory.java https://reviews.apache.org/r/25356/#comment91134 If I am reading it right the update will still proceed in case updateOnlyTheseInstances specifies an unrecognized range (i.e. not intersecting with the full working set). The InstanceUpdater appears to handle the optional desiredState as no-op. This is the departure from the current implementation where a disjoint --shards value fails the update. - Maxim Khutornenko On Sept. 4, 2014, 9:19 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25356/ --- (Updated Sept. 4, 2014, 9:19 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-613 https://issues.apache.org/jira/browse/AURORA-613 Repository: aurora Description --- This is the bridge between the top-level updater logic and the OneWayJobUpdater - taking a user request and the direction we've decided to move the job (forward or back), produce the appropriate OneWayJobUpdater. Diffs - src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 85196af38b6615e5aac7de30a4a601350e935c72 src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java aa8b5f2ee81d42db003f27e682f79e94b6625d87 src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterFactory.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java dca55ef5612105c9dc8fd57e789fdf1df28ba447 src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java d7baaa679289998a9ea80c0934990a49e5c173d7 src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterFactoryImplTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java e3e50d7d6526e8796dc5cd82b459190b09ceb72f Diff: https://reviews.apache.org/r/25356/diff/ Testing --- ./gradlew build -Pq Thanks, Bill Farner
Re: Review Request 24915: Adding initial GC task delay on scheduler restart.
On Sept. 4, 2014, 5:44 p.m., Bill Farner wrote: Per offline discussion, i've realized my memory leak concern is ~moot since we already have the unbounded HostAttributes store. I liked your suggestion of augmenting the way this soft state is used: - use a Map, no expiry - when receiving an offer, don't kick off a GC task if a map entry is not present, store a future timestamp after which we should launch a GC task Thanks. Just to make sure we are on the same page, the bullets above describe exactly how it's implemented by the current RB. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24915/#review52328 --- On Aug. 20, 2014, 11:35 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24915/ --- (Updated Aug. 20, 2014, 11:35 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-608 https://issues.apache.org/jira/browse/AURORA-608 Repository: aurora Description --- Adding initial GC task delay on scheduler restart. Diffs - src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 5a38a1f69ac5dbe68af3bfe175899ddee392880b src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 65f404915bc60ffe11a7a57d9861ac5b37fa646a src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 04137072891b2a1f0ad663182629dd469b09324f Diff: https://reviews.apache.org/r/24915/diff/ Testing --- gradle -Pq build Thanks, Maxim Khutornenko
Review Request 25361: Update vagrant provisioning and aurorabuild scripts to remove the need to sudo to run build commands in the vagrant image.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25361/ --- Review request for Aurora and Bill Farner. Bugs: AURORA-688 https://issues.apache.org/jira/browse/AURORA-688 Repository: aurora Description --- Update vagrant provisioning and aurorabuild scripts to remove the need to sudo to run build commands in the vagrant image. Diffs - docs/vagrant.md b66ff67973c2a7893230aac6164b368e8bccd596 examples/vagrant/aurorabuild.sh 8b39a167e23ad2569c57dd0eb29ef6025c1b7fad examples/vagrant/provision-dev-cluster.sh 4883df244a157371b4046b394bdf77f03e2e2470 Diff: https://reviews.apache.org/r/25361/diff/ Testing --- vagrant destroy vagrant up bash src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Joshua Cohen
Review Request 25365: Update slave url to use the thermos port.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25365/ --- Review request for Aurora and Bill Farner. Bugs: AURORA-644 https://issues.apache.org/jira/browse/AURORA-644 Repository: aurora Description --- Update slave url to use the thermos port. It was already hardcoded, so I just updated it, but it seems less than ideal. Should it be a constant or a command line arg? Diffs - src/main/resources/org/apache/aurora/scheduler/http/slaves.st defe1c5ececc8d742c78e52b9c43bebebba401ea Diff: https://reviews.apache.org/r/25365/diff/ Testing --- Ran scheduler, verified updated url. Thanks, Joshua Cohen
Review Request 25366: Set principal field in FrameworkInfo struct.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25366/ --- Review request for Aurora, Joshua Cohen, Kevin Sweeney, Maxim Khutornenko, and Bill Farner. Bugs: AURORA-687 https://issues.apache.org/jira/browse/AURORA-687 Repository: aurora Description --- Set principal field in FrameworkInfo struct. Diffs - src/main/java/org/apache/aurora/scheduler/DriverFactory.java 9cc04a84a37374ffca418e2ff767992ee23b9f3e Diff: https://reviews.apache.org/r/25366/diff/ Testing --- ./gradlew build -Pq Thanks, Zameer Manji
Re: Review Request 25366: Set principal field in FrameworkInfo struct.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25366/#review52371 --- src/main/java/org/apache/aurora/scheduler/DriverFactory.java https://reviews.apache.org/r/25366/#comment91136 properties.getProperty() rather than properties.get(...).toString() Also, given that we're getting this multiple times (above for the log, below for the Credential), might make sense to just get it once and reuse (though the above is just a get call, not sure if there's any downside there to the implicit string conversion done by getProperty). - Joshua Cohen On Sept. 4, 2014, 11:16 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25366/ --- (Updated Sept. 4, 2014, 11:16 p.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, Maxim Khutornenko, and Bill Farner. Bugs: AURORA-687 https://issues.apache.org/jira/browse/AURORA-687 Repository: aurora Description --- Set principal field in FrameworkInfo struct. Diffs - src/main/java/org/apache/aurora/scheduler/DriverFactory.java 9cc04a84a37374ffca418e2ff767992ee23b9f3e Diff: https://reviews.apache.org/r/25366/diff/ Testing --- ./gradlew build -Pq Thanks, Zameer Manji
Re: Review Request 24915: Adding initial GC task delay on scheduler restart.
On Sept. 4, 2014, 5:44 p.m., Bill Farner wrote: Per offline discussion, i've realized my memory leak concern is ~moot since we already have the unbounded HostAttributes store. I liked your suggestion of augmenting the way this soft state is used: - use a Map, no expiry - when receiving an offer, don't kick off a GC task if a map entry is not present, store a future timestamp after which we should launch a GC task Maxim Khutornenko wrote: Thanks. Just to make sure we are on the same page, the bullets above describe exactly how it's implemented by the current RB. Hah, i was indeed on a different page - taking another look. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24915/#review52328 --- On Aug. 20, 2014, 11:35 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24915/ --- (Updated Aug. 20, 2014, 11:35 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-608 https://issues.apache.org/jira/browse/AURORA-608 Repository: aurora Description --- Adding initial GC task delay on scheduler restart. Diffs - src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 5a38a1f69ac5dbe68af3bfe175899ddee392880b src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 65f404915bc60ffe11a7a57d9861ac5b37fa646a src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 04137072891b2a1f0ad663182629dd469b09324f Diff: https://reviews.apache.org/r/24915/diff/ Testing --- gradle -Pq build Thanks, Maxim Khutornenko
Re: Review Request 25366: Set principal field in FrameworkInfo struct.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25366/ --- (Updated Sept. 4, 2014, 4:36 p.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, Maxim Khutornenko, and Bill Farner. Changes --- Use .getProperty Bugs: AURORA-687 https://issues.apache.org/jira/browse/AURORA-687 Repository: aurora Description --- Set principal field in FrameworkInfo struct. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/DriverFactory.java 9cc04a84a37374ffca418e2ff767992ee23b9f3e Diff: https://reviews.apache.org/r/25366/diff/ Testing --- ./gradlew build -Pq Thanks, Zameer Manji
Re: Review Request 25366: Set principal field in FrameworkInfo struct.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25366/#review52377 --- Ship it! Ship It! - Joshua Cohen On Sept. 4, 2014, 11:36 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25366/ --- (Updated Sept. 4, 2014, 11:36 p.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, Maxim Khutornenko, and Bill Farner. Bugs: AURORA-687 https://issues.apache.org/jira/browse/AURORA-687 Repository: aurora Description --- Set principal field in FrameworkInfo struct. Diffs - src/main/java/org/apache/aurora/scheduler/DriverFactory.java 9cc04a84a37374ffca418e2ff767992ee23b9f3e Diff: https://reviews.apache.org/r/25366/diff/ Testing --- ./gradlew build -Pq Thanks, Zameer Manji
Re: Review Request 24915: Adding initial GC task delay on scheduler restart.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24915/#review52376 --- Ship it! LGTM overall. src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java https://reviews.apache.org/r/24915/#comment91149 Shouldn't this be == EXECUTOR_GC_INTERVAL to minimize the side-effects of a failover? As-is, you'll have a 15 minute period with 4x elevated GC runs. If so, this arg could collapse, and the new GcExecutorSettings method can go away. src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java https://reviews.apache.org/r/24915/#comment91148 Please wrap this with Collections.synchronizedMap to preserve thread safety. - Bill Farner On Aug. 20, 2014, 11:35 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24915/ --- (Updated Aug. 20, 2014, 11:35 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-608 https://issues.apache.org/jira/browse/AURORA-608 Repository: aurora Description --- Adding initial GC task delay on scheduler restart. Diffs - src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 5a38a1f69ac5dbe68af3bfe175899ddee392880b src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 65f404915bc60ffe11a7a57d9861ac5b37fa646a src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 04137072891b2a1f0ad663182629dd469b09324f Diff: https://reviews.apache.org/r/24915/diff/ Testing --- gradle -Pq build Thanks, Maxim Khutornenko
Re: Review Request 25357: Adding support for per-job task status metrics.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25357/#review52379 --- Ship it! Ship It! - David McLaughlin On Sept. 4, 2014, 9:42 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25357/ --- (Updated Sept. 4, 2014, 9:42 p.m.) Review request for Aurora, David McLaughlin and Bill Farner. Bugs: AURORA-685 https://issues.apache.org/jira/browse/AURORA-685 Repository: aurora Description --- Currently tracking LOST and FAILED states. Diffs - src/main/java/org/apache/aurora/scheduler/TaskVars.java 6654c1675ac9f5f7d481e115cea7c224fb212467 src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java d02714c846a521ff9ac3e53d991731314e714ae2 Diff: https://reviews.apache.org/r/25357/diff/ Testing --- gradle -Pq build Thanks, Maxim Khutornenko
Re: Review Request 25357: Adding support for per-job task status metrics.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25357/#review52382 --- src/main/java/org/apache/aurora/scheduler/TaskVars.java https://reviews.apache.org/r/25357/#comment91162 To mitigate the risk of OOM in the scheduler's internal TSDB, we should use untracked stats, similar to what's done in MetricCalculator.java. - Bill Farner On Sept. 4, 2014, 9:42 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25357/ --- (Updated Sept. 4, 2014, 9:42 p.m.) Review request for Aurora, David McLaughlin and Bill Farner. Bugs: AURORA-685 https://issues.apache.org/jira/browse/AURORA-685 Repository: aurora Description --- Currently tracking LOST and FAILED states. Diffs - src/main/java/org/apache/aurora/scheduler/TaskVars.java 6654c1675ac9f5f7d481e115cea7c224fb212467 src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java d02714c846a521ff9ac3e53d991731314e714ae2 Diff: https://reviews.apache.org/r/25357/diff/ Testing --- gradle -Pq build Thanks, Maxim Khutornenko
Re: Review Request 24815: Refactoring SchedulerCore final part.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24815/ --- (Updated Sept. 5, 2014, 12:09 a.m.) Review request for Aurora and Bill Farner. Changes --- Refactored StateManager.insertPendingTasks() and rebased. Bugs: AURORA-94 https://issues.apache.org/jira/browse/AURORA-94 Repository: aurora Description --- Moving the last bits of functionality out of SchedulerCore. Also, preparing the ground for task validation logic reuse in updater code. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/base/ScheduleException.java e060e5ec88a0ab311415eaa638cf693c99c40049 src/main/java/org/apache/aurora/scheduler/configuration/SanitizedConfiguration.java d511ec0054e380fd28b0c70bae7d9803b81abdf1 src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java 62202810fd5829545fc77b91a20d1bb433a4916a src/main/java/org/apache/aurora/scheduler/state/SchedulerCore.java c636fd7fde8b592b167da8e5e9651ac772bc23de src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java 1e60fcad82b05e3fe477a31e653b8c5e63c78050 src/main/java/org/apache/aurora/scheduler/state/StateManager.java 6e062b39175255264020bbb1cbd485de9a114a20 src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 6ad104b050aabecad1dadc975c27d9d3603659bf src/main/java/org/apache/aurora/scheduler/state/StateModule.java 2c712eff097c3334bfcf2559a52214367748d08a src/main/java/org/apache/aurora/scheduler/state/TaskLimitValidator.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 9171179ff482e0b56faf4073b2dc2a6f2f0def46 src/main/thrift/org/apache/aurora/gen/api.thrift a78a4d849e545000725a39fae49f406422c39da0 src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java c7ae7db16e82c722fae1bccb9178f5ec203e91e5 src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 62dce07b42af02d25b788e763e4e2a026dd2d483 src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java b22b390368e08d3790480ab5505852f398b2c69c src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 1678411ad5c25e74f8b6d004bf491ea26ca0 src/test/java/org/apache/aurora/scheduler/state/TaskLimitValidatorTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java ad2548c53d86b89efe6129702b8a5df259af3d4e src/test/java/org/apache/aurora/scheduler/storage/mem/MemStorageSchedulerCoreImplTest.java 35bed104d838596abcbb5abd5cad29592b384dfa src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 1686d4b158057d87180af3a211a4237f1213efa6 src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 055c177bc34cc306faaf7593e6c5156ad9636f6c src/test/resources/org/apache/aurora/gen/api.thrift.md5 81d1734a22a8744d6002aadb7fb446d132d10bd9 Diff: https://reviews.apache.org/r/24815/diff/ Testing --- gradle -Pq build Thanks, Maxim Khutornenko
Re: Review Request 25357: Adding support for per-job task status metrics.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25357/ --- (Updated Sept. 5, 2014, 12:23 a.m.) Review request for Aurora, David McLaughlin and Bill Farner. Changes --- CR comments. Bugs: AURORA-685 https://issues.apache.org/jira/browse/AURORA-685 Repository: aurora Description --- Currently tracking LOST and FAILED states. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/TaskVars.java 6654c1675ac9f5f7d481e115cea7c224fb212467 src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java d02714c846a521ff9ac3e53d991731314e714ae2 Diff: https://reviews.apache.org/r/25357/diff/ Testing --- gradle -Pq build Thanks, Maxim Khutornenko
Re: Review Request 25357: Adding support for per-job task status metrics.
On Sept. 4, 2014, 11:59 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/TaskVars.java, line 168 https://reviews.apache.org/r/25357/diff/1/?file=679026#file679026line168 To mitigate the risk of OOM in the scheduler's internal TSDB, we should use untracked stats, similar to what's done in MetricCalculator.java. Great suggestion, done. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25357/#review52382 --- On Sept. 4, 2014, 9:42 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25357/ --- (Updated Sept. 4, 2014, 9:42 p.m.) Review request for Aurora, David McLaughlin and Bill Farner. Bugs: AURORA-685 https://issues.apache.org/jira/browse/AURORA-685 Repository: aurora Description --- Currently tracking LOST and FAILED states. Diffs - src/main/java/org/apache/aurora/scheduler/TaskVars.java 6654c1675ac9f5f7d481e115cea7c224fb212467 src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java d02714c846a521ff9ac3e53d991731314e714ae2 Diff: https://reviews.apache.org/r/25357/diff/ Testing --- gradle -Pq build Thanks, Maxim Khutornenko
Re: Review Request 25337: Preserve executor HealthCheckerThread name
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25337/ --- (Updated Sept. 4, 2014, 5:24 p.m.) Review request for Aurora, David McLaughlin, Maxim Khutornenko, and Brian Wickman. Bugs: AURORA-682 https://issues.apache.org/jira/browse/AURORA-682 Repository: aurora Description --- Preserve executor HealthCheckerThread name Diffs (updated) - src/main/python/apache/aurora/executor/common/health_checker.py 603fff35b839c6f53d9379ec047d7d8135a1c65b src/test/python/apache/aurora/executor/common/BUILD 3229facf40070929adabb57fef667ab11bf3d1ec src/test/python/apache/aurora/executor/common/fixtures.py PRE-CREATION src/test/python/apache/aurora/executor/common/test_health_checker.py 490d4c8b5c434f9d6f032d931e35c483b3a5b676 src/test/python/apache/aurora/executor/common/test_task_info.py 344fd675c9a1ef9c88e39c16ac0f1dd50a9c1632 src/test/python/apache/aurora/executor/test_thermos_executor.py f6ca4dfd0fd262361709361c48c93799837e0a54 Diff: https://reviews.apache.org/r/25337/diff/ Testing (updated) --- ###STILL RUNNING E2E TESTS [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./build-support/python/isort-run [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./build-support/python/checkstyle-check [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./pants ./src/test/python/apache/aurora/executor:all src.test.python.apache.aurora.executor.common.announcer . SUCCESS src.test.python.apache.aurora.executor.common.directory_sandbox . SUCCESS src.test.python.apache.aurora.executor.common.executor_timeout . SUCCESS src.test.python.apache.aurora.executor.common.health_checker . SUCCESS src.test.python.apache.aurora.executor.common.status_checker . SUCCESS src.test.python.apache.aurora.executor.common.task_info . SUCCESS src.test.python.apache.aurora.executor.executor_base . SUCCESS src.test.python.apache.aurora.executor.executor_detector . SUCCESS src.test.python.apache.aurora.executor.executor_vars . SUCCESS src.test.python.apache.aurora.executor.gc_executor . SUCCESS src.test.python.apache.aurora.executor.status_manager . SUCCESS src.test.python.apache.aurora.executor.thermos_executor . SUCCESS src.test.python.apache.aurora.executor.thermos_task_runner . SUCCESS Thanks, Joe Smith
Re: Review Request 25337: Preserve executor HealthCheckerThread name
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25337/#review52387 --- src/test/python/apache/aurora/executor/test_thermos_executor.py https://reviews.apache.org/r/25337/#comment91167 I don't think this is supposed to be here. - Zameer Manji On Sept. 4, 2014, 5:24 p.m., Joe Smith wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25337/ --- (Updated Sept. 4, 2014, 5:24 p.m.) Review request for Aurora, David McLaughlin, Maxim Khutornenko, and Brian Wickman. Bugs: AURORA-682 https://issues.apache.org/jira/browse/AURORA-682 Repository: aurora Description --- Preserve executor HealthCheckerThread name Diffs - src/main/python/apache/aurora/executor/common/health_checker.py 603fff35b839c6f53d9379ec047d7d8135a1c65b src/test/python/apache/aurora/executor/common/BUILD 3229facf40070929adabb57fef667ab11bf3d1ec src/test/python/apache/aurora/executor/common/fixtures.py PRE-CREATION src/test/python/apache/aurora/executor/common/test_health_checker.py 490d4c8b5c434f9d6f032d931e35c483b3a5b676 src/test/python/apache/aurora/executor/common/test_task_info.py 344fd675c9a1ef9c88e39c16ac0f1dd50a9c1632 src/test/python/apache/aurora/executor/test_thermos_executor.py f6ca4dfd0fd262361709361c48c93799837e0a54 Diff: https://reviews.apache.org/r/25337/diff/ Testing --- ###STILL RUNNING E2E TESTS [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./build-support/python/isort-run [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./build-support/python/checkstyle-check [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./pants ./src/test/python/apache/aurora/executor:all src.test.python.apache.aurora.executor.common.announcer . SUCCESS src.test.python.apache.aurora.executor.common.directory_sandbox . SUCCESS src.test.python.apache.aurora.executor.common.executor_timeout . SUCCESS src.test.python.apache.aurora.executor.common.health_checker . SUCCESS src.test.python.apache.aurora.executor.common.status_checker . SUCCESS src.test.python.apache.aurora.executor.common.task_info . SUCCESS src.test.python.apache.aurora.executor.executor_base . SUCCESS src.test.python.apache.aurora.executor.executor_detector . SUCCESS src.test.python.apache.aurora.executor.executor_vars . SUCCESS src.test.python.apache.aurora.executor.gc_executor . SUCCESS src.test.python.apache.aurora.executor.status_manager . SUCCESS src.test.python.apache.aurora.executor.thermos_executor . SUCCESS src.test.python.apache.aurora.executor.thermos_task_runner . SUCCESS Thanks, Joe Smith
Re: Review Request 25337: Preserve executor HealthCheckerThread name
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25337/ --- (Updated Sept. 4, 2014, 5:43 p.m.) Review request for Aurora, David McLaughlin, Maxim Khutornenko, and Brian Wickman. Changes --- Great catch Zameer! [tw-172-25-20-192 aurora (yasumoto/fix_health_checker_name)]$ ./pants ./src/test/python/apache/aurora/executor:thermos_executor Build operating on top level addresses: set([BuildFileAddress(/Users/jsmith/workspace/aurora/src/test/python/apache/aurora/executor/BUILD, thermos_executor)]) test session starts = platform darwin -- Python 2.7.8 -- py-1.4.23 -- pytest-2.6.1 plugins: cov, timeout collected 15 items src/test/python/apache/aurora/executor/test_thermos_executor.py ... = 15 passed in 38.03 seconds = src.test.python.apache.aurora.executor.thermos_executor . SUCCESS Bugs: AURORA-682 https://issues.apache.org/jira/browse/AURORA-682 Repository: aurora Description --- Preserve executor HealthCheckerThread name Diffs (updated) - src/main/python/apache/aurora/executor/common/health_checker.py 603fff35b839c6f53d9379ec047d7d8135a1c65b src/test/python/apache/aurora/executor/common/BUILD 3229facf40070929adabb57fef667ab11bf3d1ec src/test/python/apache/aurora/executor/common/fixtures.py PRE-CREATION src/test/python/apache/aurora/executor/common/test_health_checker.py 490d4c8b5c434f9d6f032d931e35c483b3a5b676 src/test/python/apache/aurora/executor/common/test_task_info.py 344fd675c9a1ef9c88e39c16ac0f1dd50a9c1632 Diff: https://reviews.apache.org/r/25337/diff/ Testing --- ###STILL RUNNING E2E TESTS [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./build-support/python/isort-run [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./build-support/python/checkstyle-check [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./pants ./src/test/python/apache/aurora/executor:all src.test.python.apache.aurora.executor.common.announcer . SUCCESS src.test.python.apache.aurora.executor.common.directory_sandbox . SUCCESS src.test.python.apache.aurora.executor.common.executor_timeout . SUCCESS src.test.python.apache.aurora.executor.common.health_checker . SUCCESS src.test.python.apache.aurora.executor.common.status_checker . SUCCESS src.test.python.apache.aurora.executor.common.task_info . SUCCESS src.test.python.apache.aurora.executor.executor_base . SUCCESS src.test.python.apache.aurora.executor.executor_detector . SUCCESS src.test.python.apache.aurora.executor.executor_vars . SUCCESS src.test.python.apache.aurora.executor.gc_executor . SUCCESS src.test.python.apache.aurora.executor.status_manager . SUCCESS src.test.python.apache.aurora.executor.thermos_executor . SUCCESS src.test.python.apache.aurora.executor.thermos_task_runner . SUCCESS Thanks, Joe Smith
Re: Review Request 25337: Preserve executor HealthCheckerThread name
On Sept. 4, 2014, 5:39 p.m., Zameer Manji wrote: src/test/python/apache/aurora/executor/test_thermos_executor.py, line 395 https://reviews.apache.org/r/25337/diff/2/?file=679158#file679158line395 I don't think this is supposed to be here. whew, still pass. - Joe --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25337/#review52387 --- On Sept. 4, 2014, 5:43 p.m., Joe Smith wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25337/ --- (Updated Sept. 4, 2014, 5:43 p.m.) Review request for Aurora, David McLaughlin, Maxim Khutornenko, and Brian Wickman. Bugs: AURORA-682 https://issues.apache.org/jira/browse/AURORA-682 Repository: aurora Description --- Preserve executor HealthCheckerThread name Diffs - src/main/python/apache/aurora/executor/common/health_checker.py 603fff35b839c6f53d9379ec047d7d8135a1c65b src/test/python/apache/aurora/executor/common/BUILD 3229facf40070929adabb57fef667ab11bf3d1ec src/test/python/apache/aurora/executor/common/fixtures.py PRE-CREATION src/test/python/apache/aurora/executor/common/test_health_checker.py 490d4c8b5c434f9d6f032d931e35c483b3a5b676 src/test/python/apache/aurora/executor/common/test_task_info.py 344fd675c9a1ef9c88e39c16ac0f1dd50a9c1632 Diff: https://reviews.apache.org/r/25337/diff/ Testing --- ###STILL RUNNING E2E TESTS [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./build-support/python/isort-run [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./build-support/python/checkstyle-check [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./pants ./src/test/python/apache/aurora/executor:all src.test.python.apache.aurora.executor.common.announcer . SUCCESS src.test.python.apache.aurora.executor.common.directory_sandbox . SUCCESS src.test.python.apache.aurora.executor.common.executor_timeout . SUCCESS src.test.python.apache.aurora.executor.common.health_checker . SUCCESS src.test.python.apache.aurora.executor.common.status_checker . SUCCESS src.test.python.apache.aurora.executor.common.task_info . SUCCESS src.test.python.apache.aurora.executor.executor_base . SUCCESS src.test.python.apache.aurora.executor.executor_detector . SUCCESS src.test.python.apache.aurora.executor.executor_vars . SUCCESS src.test.python.apache.aurora.executor.gc_executor . SUCCESS src.test.python.apache.aurora.executor.status_manager . SUCCESS src.test.python.apache.aurora.executor.thermos_executor . SUCCESS src.test.python.apache.aurora.executor.thermos_task_runner . SUCCESS Thanks, Joe Smith
Re: Review Request 25337: Preserve executor HealthCheckerThread name
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25337/#review52389 --- \m/ src/main/python/apache/aurora/executor/common/health_checker.py https://reviews.apache.org/r/25337/#comment91170 make _healthy and _reason non-private src/main/python/apache/aurora/executor/common/health_checker.py https://reviews.apache.org/r/25337/#comment91171 super(HealthChecker, self).start() src/main/python/apache/aurora/executor/common/health_checker.py https://reviews.apache.org/r/25337/#comment91172 think you'll need an extra newline here for checkstyle to not complain src/test/python/apache/aurora/executor/common/test_health_checker.py https://reviews.apache.org/r/25337/#comment91174 please obtain an ephemeral port here instead. who is to say that port 9001 is not running a service that accepts the health check protocol? or at the very least, create a mock HttpSignaler src/test/python/apache/aurora/executor/common/test_health_checker.py https://reviews.apache.org/r/25337/#comment91175 all of these attributes should likely be public or read-only properties of private attributes src/test/python/apache/aurora/executor/common/test_health_checker.py https://reviews.apache.org/r/25337/#comment91173 the + is superfluous src/test/python/apache/aurora/executor/test_thermos_executor.py https://reviews.apache.org/r/25337/#comment91176 derp? - Brian Wickman On Sept. 5, 2014, 12:43 a.m., Joe Smith wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25337/ --- (Updated Sept. 5, 2014, 12:43 a.m.) Review request for Aurora, David McLaughlin, Maxim Khutornenko, and Brian Wickman. Bugs: AURORA-682 https://issues.apache.org/jira/browse/AURORA-682 Repository: aurora Description --- Preserve executor HealthCheckerThread name Diffs - src/main/python/apache/aurora/executor/common/health_checker.py 603fff35b839c6f53d9379ec047d7d8135a1c65b src/test/python/apache/aurora/executor/common/BUILD 3229facf40070929adabb57fef667ab11bf3d1ec src/test/python/apache/aurora/executor/common/fixtures.py PRE-CREATION src/test/python/apache/aurora/executor/common/test_health_checker.py 490d4c8b5c434f9d6f032d931e35c483b3a5b676 src/test/python/apache/aurora/executor/common/test_task_info.py 344fd675c9a1ef9c88e39c16ac0f1dd50a9c1632 Diff: https://reviews.apache.org/r/25337/diff/ Testing --- ###STILL RUNNING E2E TESTS [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./build-support/python/isort-run [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./build-support/python/checkstyle-check [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./pants ./src/test/python/apache/aurora/executor:all src.test.python.apache.aurora.executor.common.announcer . SUCCESS src.test.python.apache.aurora.executor.common.directory_sandbox . SUCCESS src.test.python.apache.aurora.executor.common.executor_timeout . SUCCESS src.test.python.apache.aurora.executor.common.health_checker . SUCCESS src.test.python.apache.aurora.executor.common.status_checker . SUCCESS src.test.python.apache.aurora.executor.common.task_info . SUCCESS src.test.python.apache.aurora.executor.executor_base . SUCCESS src.test.python.apache.aurora.executor.executor_detector . SUCCESS src.test.python.apache.aurora.executor.executor_vars . SUCCESS src.test.python.apache.aurora.executor.gc_executor . SUCCESS src.test.python.apache.aurora.executor.status_manager . SUCCESS src.test.python.apache.aurora.executor.thermos_executor . SUCCESS src.test.python.apache.aurora.executor.thermos_task_runner . SUCCESS Thanks, Joe Smith
Re: Review Request 25337: Preserve executor HealthCheckerThread name
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25337/#review52393 --- Ship it! LGTM mod HttpSignaler mocking. - Maxim Khutornenko On Sept. 5, 2014, 12:43 a.m., Joe Smith wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25337/ --- (Updated Sept. 5, 2014, 12:43 a.m.) Review request for Aurora, David McLaughlin, Maxim Khutornenko, and Brian Wickman. Bugs: AURORA-682 https://issues.apache.org/jira/browse/AURORA-682 Repository: aurora Description --- Preserve executor HealthCheckerThread name Diffs - src/main/python/apache/aurora/executor/common/health_checker.py 603fff35b839c6f53d9379ec047d7d8135a1c65b src/test/python/apache/aurora/executor/common/BUILD 3229facf40070929adabb57fef667ab11bf3d1ec src/test/python/apache/aurora/executor/common/fixtures.py PRE-CREATION src/test/python/apache/aurora/executor/common/test_health_checker.py 490d4c8b5c434f9d6f032d931e35c483b3a5b676 src/test/python/apache/aurora/executor/common/test_task_info.py 344fd675c9a1ef9c88e39c16ac0f1dd50a9c1632 Diff: https://reviews.apache.org/r/25337/diff/ Testing --- ###STILL RUNNING E2E TESTS [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./build-support/python/isort-run [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./build-support/python/checkstyle-check [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./pants ./src/test/python/apache/aurora/executor:all src.test.python.apache.aurora.executor.common.announcer . SUCCESS src.test.python.apache.aurora.executor.common.directory_sandbox . SUCCESS src.test.python.apache.aurora.executor.common.executor_timeout . SUCCESS src.test.python.apache.aurora.executor.common.health_checker . SUCCESS src.test.python.apache.aurora.executor.common.status_checker . SUCCESS src.test.python.apache.aurora.executor.common.task_info . SUCCESS src.test.python.apache.aurora.executor.executor_base . SUCCESS src.test.python.apache.aurora.executor.executor_detector . SUCCESS src.test.python.apache.aurora.executor.executor_vars . SUCCESS src.test.python.apache.aurora.executor.gc_executor . SUCCESS src.test.python.apache.aurora.executor.status_manager . SUCCESS src.test.python.apache.aurora.executor.thermos_executor . SUCCESS src.test.python.apache.aurora.executor.thermos_task_runner . SUCCESS Thanks, Joe Smith
Re: Review Request 25337: Preserve executor HealthCheckerThread name
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25337/ --- (Updated Sept. 4, 2014, 6:20 p.m.) Review request for Aurora, David McLaughlin, Maxim Khutornenko, and Brian Wickman. Changes --- Wickman's suggestions Bugs: AURORA-682 https://issues.apache.org/jira/browse/AURORA-682 Repository: aurora Description --- Preserve executor HealthCheckerThread name Diffs (updated) - src/main/python/apache/aurora/executor/common/health_checker.py 603fff35b839c6f53d9379ec047d7d8135a1c65b src/test/python/apache/aurora/executor/common/BUILD 3229facf40070929adabb57fef667ab11bf3d1ec src/test/python/apache/aurora/executor/common/fixtures.py PRE-CREATION src/test/python/apache/aurora/executor/common/test_health_checker.py 490d4c8b5c434f9d6f032d931e35c483b3a5b676 src/test/python/apache/aurora/executor/common/test_task_info.py 344fd675c9a1ef9c88e39c16ac0f1dd50a9c1632 Diff: https://reviews.apache.org/r/25337/diff/ Testing --- ###STILL RUNNING E2E TESTS [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./build-support/python/isort-run [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./build-support/python/checkstyle-check [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./pants ./src/test/python/apache/aurora/executor:all src.test.python.apache.aurora.executor.common.announcer . SUCCESS src.test.python.apache.aurora.executor.common.directory_sandbox . SUCCESS src.test.python.apache.aurora.executor.common.executor_timeout . SUCCESS src.test.python.apache.aurora.executor.common.health_checker . SUCCESS src.test.python.apache.aurora.executor.common.status_checker . SUCCESS src.test.python.apache.aurora.executor.common.task_info . SUCCESS src.test.python.apache.aurora.executor.executor_base . SUCCESS src.test.python.apache.aurora.executor.executor_detector . SUCCESS src.test.python.apache.aurora.executor.executor_vars . SUCCESS src.test.python.apache.aurora.executor.gc_executor . SUCCESS src.test.python.apache.aurora.executor.status_manager . SUCCESS src.test.python.apache.aurora.executor.thermos_executor . SUCCESS src.test.python.apache.aurora.executor.thermos_task_runner . SUCCESS Thanks, Joe Smith
Re: Review Request 25337: Preserve executor HealthCheckerThread name
On Sept. 4, 2014, 5:50 p.m., Brian Wickman wrote: src/main/python/apache/aurora/executor/common/health_checker.py, lines 131-132 https://reviews.apache.org/r/25337/diff/2/?file=679153#file679153line131 make _healthy and _reason non-private done On Sept. 4, 2014, 5:50 p.m., Brian Wickman wrote: src/main/python/apache/aurora/executor/common/health_checker.py, line 136 https://reviews.apache.org/r/25337/diff/2/?file=679153#file679153line136 super(HealthChecker, self).start() done On Sept. 4, 2014, 5:50 p.m., Brian Wickman wrote: src/main/python/apache/aurora/executor/common/health_checker.py, line 141 https://reviews.apache.org/r/25337/diff/2/?file=679153#file679153line141 think you'll need an extra newline here for checkstyle to not complain done On Sept. 4, 2014, 5:50 p.m., Brian Wickman wrote: src/test/python/apache/aurora/executor/common/test_health_checker.py, line 188 https://reviews.apache.org/r/25337/diff/2/?file=679156#file679156line188 the + is superfluous done On Sept. 4, 2014, 5:50 p.m., Brian Wickman wrote: src/test/python/apache/aurora/executor/test_thermos_executor.py, line 395 https://reviews.apache.org/r/25337/diff/2/?file=679158#file679158line395 derp? should be updated already, thanks On Sept. 4, 2014, 5:50 p.m., Brian Wickman wrote: src/test/python/apache/aurora/executor/common/test_health_checker.py, line 171 https://reviews.apache.org/r/25337/diff/2/?file=679156#file679156line171 all of these attributes should likely be public or read-only properties of private attributes done On Sept. 4, 2014, 5:50 p.m., Brian Wickman wrote: src/test/python/apache/aurora/executor/common/test_health_checker.py, line 158 https://reviews.apache.org/r/25337/diff/2/?file=679156#file679156line158 please obtain an ephemeral port here instead. who is to say that port 9001 is not running a service that accepts the health check protocol? or at the very least, create a mock HttpSignaler mocked - Joe --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25337/#review52389 --- On Sept. 4, 2014, 6:20 p.m., Joe Smith wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25337/ --- (Updated Sept. 4, 2014, 6:20 p.m.) Review request for Aurora, David McLaughlin, Maxim Khutornenko, and Brian Wickman. Bugs: AURORA-682 https://issues.apache.org/jira/browse/AURORA-682 Repository: aurora Description --- Preserve executor HealthCheckerThread name Diffs - src/main/python/apache/aurora/executor/common/health_checker.py 603fff35b839c6f53d9379ec047d7d8135a1c65b src/test/python/apache/aurora/executor/common/BUILD 3229facf40070929adabb57fef667ab11bf3d1ec src/test/python/apache/aurora/executor/common/fixtures.py PRE-CREATION src/test/python/apache/aurora/executor/common/test_health_checker.py 490d4c8b5c434f9d6f032d931e35c483b3a5b676 src/test/python/apache/aurora/executor/common/test_task_info.py 344fd675c9a1ef9c88e39c16ac0f1dd50a9c1632 Diff: https://reviews.apache.org/r/25337/diff/ Testing --- ###STILL RUNNING E2E TESTS [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./build-support/python/isort-run [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./build-support/python/checkstyle-check [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./pants ./src/test/python/apache/aurora/executor:all src.test.python.apache.aurora.executor.common.announcer . SUCCESS src.test.python.apache.aurora.executor.common.directory_sandbox . SUCCESS src.test.python.apache.aurora.executor.common.executor_timeout . SUCCESS src.test.python.apache.aurora.executor.common.health_checker . SUCCESS src.test.python.apache.aurora.executor.common.status_checker . SUCCESS src.test.python.apache.aurora.executor.common.task_info . SUCCESS src.test.python.apache.aurora.executor.executor_base . SUCCESS src.test.python.apache.aurora.executor.executor_detector . SUCCESS src.test.python.apache.aurora.executor.executor_vars . SUCCESS src.test.python.apache.aurora.executor.gc_executor . SUCCESS src.test.python.apache.aurora.executor.status_manager . SUCCESS src.test.python.apache.aurora.executor.thermos_executor .
Re: Review Request 25337: Preserve executor HealthCheckerThread name
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25337/#review52395 --- Ship it! src/main/python/apache/aurora/executor/common/health_checker.py https://reviews.apache.org/r/25337/#comment91188 Docstring, one variable per line so the style is consistent, and perhaps change the order so it's consistent with HealthChecker()? eg, s/ initial_interval_secs, interval_secs/ interval_secs, initial_interval_secs/ and update the caller to match. src/test/python/apache/aurora/executor/common/test_health_checker.py https://reviews.apache.org/r/25337/#comment91190 Why do you need to cast num_calls? - David Robinson On Sept. 5, 2014, 1:20 a.m., Joe Smith wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25337/ --- (Updated Sept. 5, 2014, 1:20 a.m.) Review request for Aurora, David McLaughlin, Maxim Khutornenko, and Brian Wickman. Bugs: AURORA-682 https://issues.apache.org/jira/browse/AURORA-682 Repository: aurora Description --- Preserve executor HealthCheckerThread name Diffs - src/main/python/apache/aurora/executor/common/health_checker.py 603fff35b839c6f53d9379ec047d7d8135a1c65b src/test/python/apache/aurora/executor/common/BUILD 3229facf40070929adabb57fef667ab11bf3d1ec src/test/python/apache/aurora/executor/common/fixtures.py PRE-CREATION src/test/python/apache/aurora/executor/common/test_health_checker.py 490d4c8b5c434f9d6f032d931e35c483b3a5b676 src/test/python/apache/aurora/executor/common/test_task_info.py 344fd675c9a1ef9c88e39c16ac0f1dd50a9c1632 Diff: https://reviews.apache.org/r/25337/diff/ Testing --- ###STILL RUNNING E2E TESTS [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./build-support/python/isort-run [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./build-support/python/checkstyle-check [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./pants ./src/test/python/apache/aurora/executor:all src.test.python.apache.aurora.executor.common.announcer . SUCCESS src.test.python.apache.aurora.executor.common.directory_sandbox . SUCCESS src.test.python.apache.aurora.executor.common.executor_timeout . SUCCESS src.test.python.apache.aurora.executor.common.health_checker . SUCCESS src.test.python.apache.aurora.executor.common.status_checker . SUCCESS src.test.python.apache.aurora.executor.common.task_info . SUCCESS src.test.python.apache.aurora.executor.executor_base . SUCCESS src.test.python.apache.aurora.executor.executor_detector . SUCCESS src.test.python.apache.aurora.executor.executor_vars . SUCCESS src.test.python.apache.aurora.executor.gc_executor . SUCCESS src.test.python.apache.aurora.executor.status_manager . SUCCESS src.test.python.apache.aurora.executor.thermos_executor . SUCCESS src.test.python.apache.aurora.executor.thermos_task_runner . SUCCESS Thanks, Joe Smith
Re: Review Request 25337: Preserve executor HealthCheckerThread name
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25337/#review52397 --- Ship it! lgtm. - David McLaughlin On Sept. 5, 2014, 1:20 a.m., Joe Smith wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25337/ --- (Updated Sept. 5, 2014, 1:20 a.m.) Review request for Aurora, David McLaughlin, Maxim Khutornenko, and Brian Wickman. Bugs: AURORA-682 https://issues.apache.org/jira/browse/AURORA-682 Repository: aurora Description --- Preserve executor HealthCheckerThread name Diffs - src/main/python/apache/aurora/executor/common/health_checker.py 603fff35b839c6f53d9379ec047d7d8135a1c65b src/test/python/apache/aurora/executor/common/BUILD 3229facf40070929adabb57fef667ab11bf3d1ec src/test/python/apache/aurora/executor/common/fixtures.py PRE-CREATION src/test/python/apache/aurora/executor/common/test_health_checker.py 490d4c8b5c434f9d6f032d931e35c483b3a5b676 src/test/python/apache/aurora/executor/common/test_task_info.py 344fd675c9a1ef9c88e39c16ac0f1dd50a9c1632 Diff: https://reviews.apache.org/r/25337/diff/ Testing --- ###STILL RUNNING E2E TESTS [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./build-support/python/isort-run [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./build-support/python/checkstyle-check [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./pants ./src/test/python/apache/aurora/executor:all src.test.python.apache.aurora.executor.common.announcer . SUCCESS src.test.python.apache.aurora.executor.common.directory_sandbox . SUCCESS src.test.python.apache.aurora.executor.common.executor_timeout . SUCCESS src.test.python.apache.aurora.executor.common.health_checker . SUCCESS src.test.python.apache.aurora.executor.common.status_checker . SUCCESS src.test.python.apache.aurora.executor.common.task_info . SUCCESS src.test.python.apache.aurora.executor.executor_base . SUCCESS src.test.python.apache.aurora.executor.executor_detector . SUCCESS src.test.python.apache.aurora.executor.executor_vars . SUCCESS src.test.python.apache.aurora.executor.gc_executor . SUCCESS src.test.python.apache.aurora.executor.status_manager . SUCCESS src.test.python.apache.aurora.executor.thermos_executor . SUCCESS src.test.python.apache.aurora.executor.thermos_task_runner . SUCCESS Thanks, Joe Smith
Re: Review Request 25337: Preserve executor HealthCheckerThread name
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25337/ --- (Updated Sept. 4, 2014, 8 p.m.) Review request for Aurora, David McLaughlin, Maxim Khutornenko, and Brian Wickman. Changes --- dRob's suggestions Bugs: AURORA-682 https://issues.apache.org/jira/browse/AURORA-682 Repository: aurora Description --- Preserve executor HealthCheckerThread name Diffs (updated) - src/main/python/apache/aurora/executor/common/health_checker.py 603fff35b839c6f53d9379ec047d7d8135a1c65b src/test/python/apache/aurora/executor/common/BUILD 3229facf40070929adabb57fef667ab11bf3d1ec src/test/python/apache/aurora/executor/common/fixtures.py PRE-CREATION src/test/python/apache/aurora/executor/common/test_health_checker.py 490d4c8b5c434f9d6f032d931e35c483b3a5b676 src/test/python/apache/aurora/executor/common/test_task_info.py 344fd675c9a1ef9c88e39c16ac0f1dd50a9c1632 Diff: https://reviews.apache.org/r/25337/diff/ Testing --- ###STILL RUNNING E2E TESTS [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./build-support/python/isort-run [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./build-support/python/checkstyle-check [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./pants ./src/test/python/apache/aurora/executor:all src.test.python.apache.aurora.executor.common.announcer . SUCCESS src.test.python.apache.aurora.executor.common.directory_sandbox . SUCCESS src.test.python.apache.aurora.executor.common.executor_timeout . SUCCESS src.test.python.apache.aurora.executor.common.health_checker . SUCCESS src.test.python.apache.aurora.executor.common.status_checker . SUCCESS src.test.python.apache.aurora.executor.common.task_info . SUCCESS src.test.python.apache.aurora.executor.executor_base . SUCCESS src.test.python.apache.aurora.executor.executor_detector . SUCCESS src.test.python.apache.aurora.executor.executor_vars . SUCCESS src.test.python.apache.aurora.executor.gc_executor . SUCCESS src.test.python.apache.aurora.executor.status_manager . SUCCESS src.test.python.apache.aurora.executor.thermos_executor . SUCCESS src.test.python.apache.aurora.executor.thermos_task_runner . SUCCESS Thanks, Joe Smith
Re: Review Request 25337: Preserve executor HealthCheckerThread name
On Sept. 4, 2014, 6:23 p.m., David Robinson wrote: src/test/python/apache/aurora/executor/common/test_health_checker.py, line 60 https://reviews.apache.org/r/25337/diff/3/?file=679164#file679164line60 Why do you need to cast num_calls? leftover from the previous test (I believe), thanks On Sept. 4, 2014, 6:23 p.m., David Robinson wrote: src/main/python/apache/aurora/executor/common/health_checker.py, lines 28-30 https://reviews.apache.org/r/25337/diff/3/?file=679161#file679161line28 Docstring, one variable per line so the style is consistent, and perhaps change the order so it's consistent with HealthChecker()? eg, s/ initial_interval_secs, interval_secs/ interval_secs, initial_interval_secs/ and update the caller to match. Let me know if this is what you had in mind! - Joe --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25337/#review52395 --- On Sept. 4, 2014, 8 p.m., Joe Smith wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25337/ --- (Updated Sept. 4, 2014, 8 p.m.) Review request for Aurora, David McLaughlin, Maxim Khutornenko, and Brian Wickman. Bugs: AURORA-682 https://issues.apache.org/jira/browse/AURORA-682 Repository: aurora Description --- Preserve executor HealthCheckerThread name Diffs - src/main/python/apache/aurora/executor/common/health_checker.py 603fff35b839c6f53d9379ec047d7d8135a1c65b src/test/python/apache/aurora/executor/common/BUILD 3229facf40070929adabb57fef667ab11bf3d1ec src/test/python/apache/aurora/executor/common/fixtures.py PRE-CREATION src/test/python/apache/aurora/executor/common/test_health_checker.py 490d4c8b5c434f9d6f032d931e35c483b3a5b676 src/test/python/apache/aurora/executor/common/test_task_info.py 344fd675c9a1ef9c88e39c16ac0f1dd50a9c1632 Diff: https://reviews.apache.org/r/25337/diff/ Testing --- ###STILL RUNNING E2E TESTS [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./build-support/python/isort-run [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./build-support/python/checkstyle-check [tw-mbp13-jsmith aurora (yasumoto/fix_health_checker_name)]$ ./pants ./src/test/python/apache/aurora/executor:all src.test.python.apache.aurora.executor.common.announcer . SUCCESS src.test.python.apache.aurora.executor.common.directory_sandbox . SUCCESS src.test.python.apache.aurora.executor.common.executor_timeout . SUCCESS src.test.python.apache.aurora.executor.common.health_checker . SUCCESS src.test.python.apache.aurora.executor.common.status_checker . SUCCESS src.test.python.apache.aurora.executor.common.task_info . SUCCESS src.test.python.apache.aurora.executor.executor_base . SUCCESS src.test.python.apache.aurora.executor.executor_detector . SUCCESS src.test.python.apache.aurora.executor.executor_vars . SUCCESS src.test.python.apache.aurora.executor.gc_executor . SUCCESS src.test.python.apache.aurora.executor.status_manager . SUCCESS src.test.python.apache.aurora.executor.thermos_executor . SUCCESS src.test.python.apache.aurora.executor.thermos_task_runner . SUCCESS Thanks, Joe Smith
