Re: Review Request 25259: Add update information to the scheduler UI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/ --- (Updated Sept. 16, 2014, 6:06 p.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner. Changes --- Hide the latest updates content box entirely when there are no updates. Bugs: AURORA-614 https://issues.apache.org/jira/browse/AURORA-614 Repository: aurora Description --- Adds update history to the job page. Adds an update details page. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 0336a6e00a9a7e14125eaae271788faaae0bd371 src/main/resources/org/apache/aurora/scheduler/http/ui/breadcrumb.html c780b0fe98863b5421824a9652a7202da9f4302a src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css 2a752313cb8ae404605a9458b33237a911eec061 src/main/resources/org/apache/aurora/scheduler/http/ui/job.html e21dee015897eee62ade8f74e26f042b8e2be734 src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js fb3b5b12121a6e8a30dbf6fe069557f69a563408 src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 3477b7e667459665af9d9dc9d2456793822bc7f7 src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js 7f05a552f3786adb115ff9648f4fadce968230e9 src/main/resources/org/apache/aurora/scheduler/http/ui/js/filters.js 76f48f8aa2399c83fbafe8efcf6f4f3e47ab05a0 src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js e898f12a7b6c6c35b4bce5a8aa78f3fcaa83a6df src/main/resources/org/apache/aurora/scheduler/http/ui/timeDisplay.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/update.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html PRE-CREATION src/main/thrift/org/apache/aurora/gen/api.thrift a6dc548c804bfcb9166573496023bad80b2a2c91 Diff: https://reviews.apache.org/r/25259/diff/ Testing --- ./gradlew jsHint File Attachments job page https://reviews.apache.org/media/uploaded/files/2014/09/09/531eca81-a0ba-4438-8bd6-4b50d97b0270__job-progress-small-preview.png update page https://reviews.apache.org/media/uploaded/files/2014/09/09/8e3f2950-7d7e-404e-bca9-6c472b5218f7__update-page-finished.png Thanks, David McLaughlin
Re: Review Request 25255: Implement server-driven update commands.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25255/#review53554 --- Ship it! Mark informed me that he intends for this command to not be exposed until we have all the required code in place. With that assumption, and the assumption that we are not bound to these nouns/verbs long-term, i'm willing to skip the naming discussion. - Bill Farner On Sept. 11, 2014, 2:55 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25255/ --- (Updated Sept. 11, 2014, 2:55 p.m.) Review request for Aurora. Repository: aurora Description --- This change contains the basic commands needed to work with the scheduler-driven updater. (It does not yet cover querying for the status of the update; that will come in a subsequent change.) Diffs - src/main/python/apache/aurora/client/cli/BUILD ebe681a0d1735b7cc695dc3b7a14c4292d87ae32 src/main/python/apache/aurora/client/cli/context.py 51c7d24dca664e476e62f1864d095416dfab70e4 src/main/python/apache/aurora/client/cli/jobs.py ebc22aaa5a8aed311897b3ce9632b6f7175b6080 src/main/python/apache/aurora/client/cli/standalone_client.py b7c8de66d6e4664b536911f826e36a984e8d0fef src/main/python/apache/aurora/client/cli/update.py PRE-CREATION src/test/python/apache/aurora/client/cli/BUILD e1f9ebf96774b8f5c75de8570c6ba87d953ab649 src/test/python/apache/aurora/client/cli/test_restart.py a1e7a5a94a2d336239df98e2600658b97c546901 src/test/python/apache/aurora/client/cli/test_supdate.py PRE-CREATION src/test/python/apache/aurora/client/cli/util.py 95a2123e127c9811fd2305e71cfc5c7c4376f904 Diff: https://reviews.apache.org/r/25255/diff/ Testing --- New suite of tests for the new command; all unit tests pass. Thanks, Mark Chu-Carroll
Re: Review Request 25259: Add update information to the scheduler UI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/#review53555 --- Ship it! src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html https://reviews.apache.org/r/25259/#comment93245 ...forward roll. - this is no longer correct as we use this setting for both forward and back roll. Mind updating this and api.thrift? - Maxim Khutornenko On Sept. 16, 2014, 6:06 p.m., David McLaughlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/ --- (Updated Sept. 16, 2014, 6:06 p.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner. Bugs: AURORA-614 https://issues.apache.org/jira/browse/AURORA-614 Repository: aurora Description --- Adds update history to the job page. Adds an update details page. Diffs - src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 0336a6e00a9a7e14125eaae271788faaae0bd371 src/main/resources/org/apache/aurora/scheduler/http/ui/breadcrumb.html c780b0fe98863b5421824a9652a7202da9f4302a src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css 2a752313cb8ae404605a9458b33237a911eec061 src/main/resources/org/apache/aurora/scheduler/http/ui/job.html e21dee015897eee62ade8f74e26f042b8e2be734 src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js fb3b5b12121a6e8a30dbf6fe069557f69a563408 src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 3477b7e667459665af9d9dc9d2456793822bc7f7 src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js 7f05a552f3786adb115ff9648f4fadce968230e9 src/main/resources/org/apache/aurora/scheduler/http/ui/js/filters.js 76f48f8aa2399c83fbafe8efcf6f4f3e47ab05a0 src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js e898f12a7b6c6c35b4bce5a8aa78f3fcaa83a6df src/main/resources/org/apache/aurora/scheduler/http/ui/timeDisplay.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/update.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html PRE-CREATION src/main/thrift/org/apache/aurora/gen/api.thrift a6dc548c804bfcb9166573496023bad80b2a2c91 Diff: https://reviews.apache.org/r/25259/diff/ Testing --- ./gradlew jsHint File Attachments job page https://reviews.apache.org/media/uploaded/files/2014/09/09/531eca81-a0ba-4438-8bd6-4b50d97b0270__job-progress-small-preview.png update page https://reviews.apache.org/media/uploaded/files/2014/09/09/8e3f2950-7d7e-404e-bca9-6c472b5218f7__update-page-finished.png Thanks, David McLaughlin
Re: Review Request 25259: Add update information to the scheduler UI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/ --- (Updated Sept. 16, 2014, 6:29 p.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner. Changes --- Modified the maxFailedInstances description. Bugs: AURORA-614 https://issues.apache.org/jira/browse/AURORA-614 Repository: aurora Description --- Adds update history to the job page. Adds an update details page. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 0336a6e00a9a7e14125eaae271788faaae0bd371 src/main/resources/org/apache/aurora/scheduler/http/ui/breadcrumb.html c780b0fe98863b5421824a9652a7202da9f4302a src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css 2a752313cb8ae404605a9458b33237a911eec061 src/main/resources/org/apache/aurora/scheduler/http/ui/job.html e21dee015897eee62ade8f74e26f042b8e2be734 src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js fb3b5b12121a6e8a30dbf6fe069557f69a563408 src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 3477b7e667459665af9d9dc9d2456793822bc7f7 src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js 7f05a552f3786adb115ff9648f4fadce968230e9 src/main/resources/org/apache/aurora/scheduler/http/ui/js/filters.js 76f48f8aa2399c83fbafe8efcf6f4f3e47ab05a0 src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js e898f12a7b6c6c35b4bce5a8aa78f3fcaa83a6df src/main/resources/org/apache/aurora/scheduler/http/ui/timeDisplay.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/update.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html PRE-CREATION src/main/thrift/org/apache/aurora/gen/api.thrift a6dc548c804bfcb9166573496023bad80b2a2c91 Diff: https://reviews.apache.org/r/25259/diff/ Testing --- ./gradlew jsHint File Attachments job page https://reviews.apache.org/media/uploaded/files/2014/09/09/531eca81-a0ba-4438-8bd6-4b50d97b0270__job-progress-small-preview.png update page https://reviews.apache.org/media/uploaded/files/2014/09/09/8e3f2950-7d7e-404e-bca9-6c472b5218f7__update-page-finished.png Thanks, David McLaughlin
Re: Review Request 25259: Add update information to the scheduler UI
On Sept. 16, 2014, 6:12 p.m., Maxim Khutornenko wrote: src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html, line 41 https://reviews.apache.org/r/25259/diff/9/?file=690739#file690739line41 ...forward roll. - this is no longer correct as we use this setting for both forward and back roll. Mind updating this and api.thrift? Done. Can you confirm you're okay with the updated description? - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/#review53555 --- On Sept. 16, 2014, 6:29 p.m., David McLaughlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/ --- (Updated Sept. 16, 2014, 6:29 p.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner. Bugs: AURORA-614 https://issues.apache.org/jira/browse/AURORA-614 Repository: aurora Description --- Adds update history to the job page. Adds an update details page. Diffs - src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 0336a6e00a9a7e14125eaae271788faaae0bd371 src/main/resources/org/apache/aurora/scheduler/http/ui/breadcrumb.html c780b0fe98863b5421824a9652a7202da9f4302a src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css 2a752313cb8ae404605a9458b33237a911eec061 src/main/resources/org/apache/aurora/scheduler/http/ui/job.html e21dee015897eee62ade8f74e26f042b8e2be734 src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js fb3b5b12121a6e8a30dbf6fe069557f69a563408 src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 3477b7e667459665af9d9dc9d2456793822bc7f7 src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js 7f05a552f3786adb115ff9648f4fadce968230e9 src/main/resources/org/apache/aurora/scheduler/http/ui/js/filters.js 76f48f8aa2399c83fbafe8efcf6f4f3e47ab05a0 src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js e898f12a7b6c6c35b4bce5a8aa78f3fcaa83a6df src/main/resources/org/apache/aurora/scheduler/http/ui/timeDisplay.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/update.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html PRE-CREATION src/main/thrift/org/apache/aurora/gen/api.thrift a6dc548c804bfcb9166573496023bad80b2a2c91 Diff: https://reviews.apache.org/r/25259/diff/ Testing --- ./gradlew jsHint File Attachments job page https://reviews.apache.org/media/uploaded/files/2014/09/09/531eca81-a0ba-4438-8bd6-4b50d97b0270__job-progress-small-preview.png update page https://reviews.apache.org/media/uploaded/files/2014/09/09/8e3f2950-7d7e-404e-bca9-6c472b5218f7__update-page-finished.png Thanks, David McLaughlin
Re: Review Request 25259: Add update information to the scheduler UI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/#review53570 --- Ship it! src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js https://reviews.apache.org/r/25259/#comment93272 I think you can use .constant here and drop the top-level function () - Kevin Sweeney On Sept. 16, 2014, 11:29 a.m., David McLaughlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/ --- (Updated Sept. 16, 2014, 11:29 a.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner. Bugs: AURORA-614 https://issues.apache.org/jira/browse/AURORA-614 Repository: aurora Description --- Adds update history to the job page. Adds an update details page. Diffs - src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 0336a6e00a9a7e14125eaae271788faaae0bd371 src/main/resources/org/apache/aurora/scheduler/http/ui/breadcrumb.html c780b0fe98863b5421824a9652a7202da9f4302a src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css 2a752313cb8ae404605a9458b33237a911eec061 src/main/resources/org/apache/aurora/scheduler/http/ui/job.html e21dee015897eee62ade8f74e26f042b8e2be734 src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js fb3b5b12121a6e8a30dbf6fe069557f69a563408 src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 3477b7e667459665af9d9dc9d2456793822bc7f7 src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js 7f05a552f3786adb115ff9648f4fadce968230e9 src/main/resources/org/apache/aurora/scheduler/http/ui/js/filters.js 76f48f8aa2399c83fbafe8efcf6f4f3e47ab05a0 src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js e898f12a7b6c6c35b4bce5a8aa78f3fcaa83a6df src/main/resources/org/apache/aurora/scheduler/http/ui/timeDisplay.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/update.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html PRE-CREATION src/main/thrift/org/apache/aurora/gen/api.thrift a6dc548c804bfcb9166573496023bad80b2a2c91 Diff: https://reviews.apache.org/r/25259/diff/ Testing --- ./gradlew jsHint File Attachments job page https://reviews.apache.org/media/uploaded/files/2014/09/09/531eca81-a0ba-4438-8bd6-4b50d97b0270__job-progress-small-preview.png update page https://reviews.apache.org/media/uploaded/files/2014/09/09/8e3f2950-7d7e-404e-bca9-6c472b5218f7__update-page-finished.png Thanks, David McLaughlin
Re: Review Request 25259: Add update information to the scheduler UI
On Sept. 16, 2014, 6:12 p.m., Maxim Khutornenko wrote: src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html, line 41 https://reviews.apache.org/r/25259/diff/9/?file=690739#file690739line41 ...forward roll. - this is no longer correct as we use this setting for both forward and back roll. Mind updating this and api.thrift? David McLaughlin wrote: Done. Can you confirm you're okay with the updated description? lgtm - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/#review53555 --- On Sept. 16, 2014, 6:29 p.m., David McLaughlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/ --- (Updated Sept. 16, 2014, 6:29 p.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner. Bugs: AURORA-614 https://issues.apache.org/jira/browse/AURORA-614 Repository: aurora Description --- Adds update history to the job page. Adds an update details page. Diffs - src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 0336a6e00a9a7e14125eaae271788faaae0bd371 src/main/resources/org/apache/aurora/scheduler/http/ui/breadcrumb.html c780b0fe98863b5421824a9652a7202da9f4302a src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css 2a752313cb8ae404605a9458b33237a911eec061 src/main/resources/org/apache/aurora/scheduler/http/ui/job.html e21dee015897eee62ade8f74e26f042b8e2be734 src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js fb3b5b12121a6e8a30dbf6fe069557f69a563408 src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 3477b7e667459665af9d9dc9d2456793822bc7f7 src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js 7f05a552f3786adb115ff9648f4fadce968230e9 src/main/resources/org/apache/aurora/scheduler/http/ui/js/filters.js 76f48f8aa2399c83fbafe8efcf6f4f3e47ab05a0 src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js e898f12a7b6c6c35b4bce5a8aa78f3fcaa83a6df src/main/resources/org/apache/aurora/scheduler/http/ui/timeDisplay.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/update.html PRE-CREATION src/main/resources/org/apache/aurora/scheduler/http/ui/updateSettings.html PRE-CREATION src/main/thrift/org/apache/aurora/gen/api.thrift a6dc548c804bfcb9166573496023bad80b2a2c91 Diff: https://reviews.apache.org/r/25259/diff/ Testing --- ./gradlew jsHint File Attachments job page https://reviews.apache.org/media/uploaded/files/2014/09/09/531eca81-a0ba-4438-8bd6-4b50d97b0270__job-progress-small-preview.png update page https://reviews.apache.org/media/uploaded/files/2014/09/09/8e3f2950-7d7e-404e-bca9-6c472b5218f7__update-page-finished.png Thanks, David McLaughlin
Review Request 25710: Remove the JobUpdateAction.INSTANCE_SKIPPED.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25710/ --- Review request for Aurora and David McLaughlin. Repository: aurora Description --- Didn't see any reference in the UI code yet, so looks like this is a thrift-only change. Diffs - src/main/thrift/org/apache/aurora/gen/api.thrift a6dc548c804bfcb9166573496023bad80b2a2c91 Diff: https://reviews.apache.org/r/25710/diff/ Testing --- ./gradlew build -Pq Thanks, Bill Farner
Re: Review Request 25529: Add a controller for job updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25529/#review53581 --- Ship it! src/main/java/org/apache/aurora/scheduler/base/InstanceKeys.java https://reviews.apache.org/r/25529/#comment93295 Missing javadoc for this class and its public methods? src/main/java/org/apache/aurora/scheduler/base/InstanceKeys.java https://reviews.apache.org/r/25529/#comment93296 checkArgument(instanceId = 0)? - Kevin Sweeney On Sept. 15, 2014, 5:39 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25529/ --- (Updated Sept. 15, 2014, 5:39 p.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, Maxim Khutornenko, and Zameer Manji. Bugs: AURORA-613 https://issues.apache.org/jira/browse/AURORA-613 Repository: aurora Description --- There's a lot going on here, i'm happy to break this up if you'd perfer, but the bulk of the change (JobUpdateControllerImpl+Test) would remain. The remaining required piece here is to record JobInstanceUpdateEvents when actions are taken. As you try to follow JobUpdateControllerImplTest, take some care to understand how FakeScheduledExecutor works. Ultimately, it accepts work added to a mock, and executes that work when FakeClock is advanced past the scheduled time. It may not be obvious at first, but be glad it's there - this kind of async test would be a nightmare without it. Diffs - build.gradle 3237f8dfa3e7d4249a388042dba840a939d513b3 src/main/java/org/apache/aurora/scheduler/async/RescheduleCalculator.java aaa3513ae99c70ffa51fec0a241d67fb815b6d3f src/main/java/org/apache/aurora/scheduler/base/InstanceKeys.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/base/Jobs.java 7b10cf876d4424fab06113aa3e2989a6bef4d346 src/main/java/org/apache/aurora/scheduler/base/Query.java d8572bb21a92025e7a51cf18d5bdf00fc1281078 src/main/java/org/apache/aurora/scheduler/base/Tasks.java 1be16374aeebaee59063aec982690ffa1fbdcf0f src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java f9521f9599e2d26cf594d62fc6b6f6ca3d5fb108 src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 6a97f36a69c7723ec1c3730085d0df70f94dcac8 src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java c520e1378c4b7947bb866011027b79b4ab65547c src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 87b773ba80784e4af8cd78790c373c08e26e src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java fedba15792ff2ecf55922ce5c38b85ada2a9edf4 src/main/java/org/apache/aurora/scheduler/updater/InstanceAction.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/updater/InstanceActionHandler.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java d725bc356178f51464b4d8ea896f1bf76815e1b2 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 39bdca02295706714cf720545ffc291f9042702a src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 3f542ce3e45ec4561238736f4ce84b69f7e41219 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateEventSubscriber.java 7be792f0bc9c5ec14c7001cb243a17d643f9607f src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java 037602d6aabe6dda978cad008075c053e2c042eb src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java d6a1e4b2c916b1c4dbcc73fe79bd672fca9b3189 src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterFactory.java 80baa7f9360cc8b2bf7910c26d119d443d6cbbf9 src/main/java/org/apache/aurora/scheduler/updater/UpdateConfigurationException.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/updater/UpdaterModule.java 028cb079316ca48bb93a35913ae25ace78088fb4 src/main/java/org/apache/aurora/scheduler/updater/strategy/ActiveLimitedStrategy.java 8298ea2283298daa71de4d958ca6bb0898d47531 src/main/java/org/apache/aurora/scheduler/updater/strategy/BatchStrategy.java a3e666e85993b833a4765ce0ad5f4825dc9253e8 src/main/java/org/apache/aurora/scheduler/updater/strategy/QueueStrategy.java 0cf36839dbf64ad755383bef829e14fd94a97e30 src/main/thrift/org/apache/aurora/gen/api.thrift a6dc548c804bfcb9166573496023bad80b2a2c91 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java af644a967983b8da66231390d29839a034697852 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java cf9805155f0e17b75db9ef8edd4b805826107868
Re: Review Request 25710: Remove the JobUpdateAction.INSTANCE_SKIPPED.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25710/#review53585 --- services.js in the commit I shipped today has two references to INSTANCES_SKIPPED. - David McLaughlin On Sept. 16, 2014, 8:37 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25710/ --- (Updated Sept. 16, 2014, 8:37 p.m.) Review request for Aurora and David McLaughlin. Repository: aurora Description --- Didn't see any reference in the UI code yet, so looks like this is a thrift-only change. Diffs - src/main/thrift/org/apache/aurora/gen/api.thrift a6dc548c804bfcb9166573496023bad80b2a2c91 Diff: https://reviews.apache.org/r/25710/diff/ Testing --- ./gradlew build -Pq Thanks, Bill Farner
Re: Review Request 25710: Remove the JobUpdateAction.INSTANCE_SKIPPED.
On Sept. 16, 2014, 8:49 p.m., David McLaughlin wrote: services.js in the commit I shipped today has two references to INSTANCES_SKIPPED. Aha, you saw the writing on the wall and snuck in :-P - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25710/#review53585 --- On Sept. 16, 2014, 8:37 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25710/ --- (Updated Sept. 16, 2014, 8:37 p.m.) Review request for Aurora and David McLaughlin. Repository: aurora Description --- Didn't see any reference in the UI code yet, so looks like this is a thrift-only change. Diffs - src/main/thrift/org/apache/aurora/gen/api.thrift a6dc548c804bfcb9166573496023bad80b2a2c91 Diff: https://reviews.apache.org/r/25710/diff/ Testing --- ./gradlew build -Pq Thanks, Bill Farner
Re: Review Request 25710: Remove the JobUpdateAction.INSTANCE_SKIPPED.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25710/ --- (Updated Sept. 16, 2014, 8:55 p.m.) Review request for Aurora and David McLaughlin. Repository: aurora Description --- Didn't see any reference in the UI code yet, so looks like this is a thrift-only change. Diffs (updated) - src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js c80146aa3829e3c3102645a1864dbeaf5e2e56bc src/main/thrift/org/apache/aurora/gen/api.thrift e7770e651596a58b39138aadc240c45aaeb4230a Diff: https://reviews.apache.org/r/25710/diff/ Testing --- ./gradlew build -Pq Thanks, Bill Farner
Re: Review Request 25710: Remove the JobUpdateAction.INSTANCE_SKIPPED.
On Sept. 16, 2014, 8:49 p.m., David McLaughlin wrote: services.js in the commit I shipped today has two references to INSTANCES_SKIPPED. Bill Farner wrote: Aha, you saw the writing on the wall and snuck in :-P :) I'm fine with you just punting on changing them and leaving them for later when we add the second part of this... but they are also really easy references to remove. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25710/#review53585 --- On Sept. 16, 2014, 8:55 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25710/ --- (Updated Sept. 16, 2014, 8:55 p.m.) Review request for Aurora and David McLaughlin. Repository: aurora Description --- Didn't see any reference in the UI code yet, so looks like this is a thrift-only change. Diffs - src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js c80146aa3829e3c3102645a1864dbeaf5e2e56bc src/main/thrift/org/apache/aurora/gen/api.thrift e7770e651596a58b39138aadc240c45aaeb4230a Diff: https://reviews.apache.org/r/25710/diff/ Testing --- ./gradlew build -Pq Thanks, Bill Farner
Re: Review Request 25710: Remove the JobUpdateAction.INSTANCE_SKIPPED.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25710/#review53592 --- Ship it! Ship It! - David McLaughlin On Sept. 16, 2014, 8:55 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25710/ --- (Updated Sept. 16, 2014, 8:55 p.m.) Review request for Aurora and David McLaughlin. Repository: aurora Description --- Didn't see any reference in the UI code yet, so looks like this is a thrift-only change. Diffs - src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js c80146aa3829e3c3102645a1864dbeaf5e2e56bc src/main/thrift/org/apache/aurora/gen/api.thrift e7770e651596a58b39138aadc240c45aaeb4230a Diff: https://reviews.apache.org/r/25710/diff/ Testing --- ./gradlew build -Pq Thanks, Bill Farner
Re: Review Request 25529: Add a controller for job updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25529/#review53598 --- I'm tapping out of this review. I don't have enough knowledge to give a ship to this change. - Zameer Manji On Sept. 15, 2014, 5:39 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25529/ --- (Updated Sept. 15, 2014, 5:39 p.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, Maxim Khutornenko, and Zameer Manji. Bugs: AURORA-613 https://issues.apache.org/jira/browse/AURORA-613 Repository: aurora Description --- There's a lot going on here, i'm happy to break this up if you'd perfer, but the bulk of the change (JobUpdateControllerImpl+Test) would remain. The remaining required piece here is to record JobInstanceUpdateEvents when actions are taken. As you try to follow JobUpdateControllerImplTest, take some care to understand how FakeScheduledExecutor works. Ultimately, it accepts work added to a mock, and executes that work when FakeClock is advanced past the scheduled time. It may not be obvious at first, but be glad it's there - this kind of async test would be a nightmare without it. Diffs - build.gradle 3237f8dfa3e7d4249a388042dba840a939d513b3 src/main/java/org/apache/aurora/scheduler/async/RescheduleCalculator.java aaa3513ae99c70ffa51fec0a241d67fb815b6d3f src/main/java/org/apache/aurora/scheduler/base/InstanceKeys.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/base/Jobs.java 7b10cf876d4424fab06113aa3e2989a6bef4d346 src/main/java/org/apache/aurora/scheduler/base/Query.java d8572bb21a92025e7a51cf18d5bdf00fc1281078 src/main/java/org/apache/aurora/scheduler/base/Tasks.java 1be16374aeebaee59063aec982690ffa1fbdcf0f src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java f9521f9599e2d26cf594d62fc6b6f6ca3d5fb108 src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 6a97f36a69c7723ec1c3730085d0df70f94dcac8 src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java c520e1378c4b7947bb866011027b79b4ab65547c src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 87b773ba80784e4af8cd78790c373c08e26e src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java fedba15792ff2ecf55922ce5c38b85ada2a9edf4 src/main/java/org/apache/aurora/scheduler/updater/InstanceAction.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/updater/InstanceActionHandler.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java d725bc356178f51464b4d8ea896f1bf76815e1b2 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 39bdca02295706714cf720545ffc291f9042702a src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 3f542ce3e45ec4561238736f4ce84b69f7e41219 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateEventSubscriber.java 7be792f0bc9c5ec14c7001cb243a17d643f9607f src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java 037602d6aabe6dda978cad008075c053e2c042eb src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java d6a1e4b2c916b1c4dbcc73fe79bd672fca9b3189 src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterFactory.java 80baa7f9360cc8b2bf7910c26d119d443d6cbbf9 src/main/java/org/apache/aurora/scheduler/updater/UpdateConfigurationException.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/updater/UpdaterModule.java 028cb079316ca48bb93a35913ae25ace78088fb4 src/main/java/org/apache/aurora/scheduler/updater/strategy/ActiveLimitedStrategy.java 8298ea2283298daa71de4d958ca6bb0898d47531 src/main/java/org/apache/aurora/scheduler/updater/strategy/BatchStrategy.java a3e666e85993b833a4765ce0ad5f4825dc9253e8 src/main/java/org/apache/aurora/scheduler/updater/strategy/QueueStrategy.java 0cf36839dbf64ad755383bef829e14fd94a97e30 src/main/thrift/org/apache/aurora/gen/api.thrift a6dc548c804bfcb9166573496023bad80b2a2c91 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java af644a967983b8da66231390d29839a034697852 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java cf9805155f0e17b75db9ef8edd4b805826107868 src/test/java/org/apache/aurora/scheduler/updater/AddTaskTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/updater/EnumsTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java PRE-CREATION
Re: Review Request 25529: Add a controller for job updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25529/ --- (Updated Sept. 16, 2014, 9:27 p.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Zameer Manji. Bugs: AURORA-613 https://issues.apache.org/jira/browse/AURORA-613 Repository: aurora Description --- There's a lot going on here, i'm happy to break this up if you'd perfer, but the bulk of the change (JobUpdateControllerImpl+Test) would remain. The remaining required piece here is to record JobInstanceUpdateEvents when actions are taken. As you try to follow JobUpdateControllerImplTest, take some care to understand how FakeScheduledExecutor works. Ultimately, it accepts work added to a mock, and executes that work when FakeClock is advanced past the scheduled time. It may not be obvious at first, but be glad it's there - this kind of async test would be a nightmare without it. Diffs (updated) - build.gradle e92b27d734fb6dc9c81c39ec8cb680ef179c3695 src/main/java/org/apache/aurora/scheduler/async/RescheduleCalculator.java aaa3513ae99c70ffa51fec0a241d67fb815b6d3f src/main/java/org/apache/aurora/scheduler/base/InstanceKeys.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/base/Jobs.java 744d2b694dbcf329e67e44db206b5f7381056dbe src/main/java/org/apache/aurora/scheduler/base/Query.java d8572bb21a92025e7a51cf18d5bdf00fc1281078 src/main/java/org/apache/aurora/scheduler/base/Tasks.java 1be16374aeebaee59063aec982690ffa1fbdcf0f src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java f9521f9599e2d26cf594d62fc6b6f6ca3d5fb108 src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 6a97f36a69c7723ec1c3730085d0df70f94dcac8 src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java c520e1378c4b7947bb866011027b79b4ab65547c src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 87b773ba80784e4af8cd78790c373c08e26e src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java dc0169ac727190028fd87c640af0100d4e3ce694 src/main/java/org/apache/aurora/scheduler/updater/InstanceAction.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/updater/InstanceActionHandler.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java d725bc356178f51464b4d8ea896f1bf76815e1b2 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 39bdca02295706714cf720545ffc291f9042702a src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 3f542ce3e45ec4561238736f4ce84b69f7e41219 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateEventSubscriber.java 7be792f0bc9c5ec14c7001cb243a17d643f9607f src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java 037602d6aabe6dda978cad008075c053e2c042eb src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java d6a1e4b2c916b1c4dbcc73fe79bd672fca9b3189 src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterFactory.java 80baa7f9360cc8b2bf7910c26d119d443d6cbbf9 src/main/java/org/apache/aurora/scheduler/updater/UpdateConfigurationException.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/updater/UpdaterModule.java 028cb079316ca48bb93a35913ae25ace78088fb4 src/main/java/org/apache/aurora/scheduler/updater/strategy/ActiveLimitedStrategy.java 8298ea2283298daa71de4d958ca6bb0898d47531 src/main/java/org/apache/aurora/scheduler/updater/strategy/BatchStrategy.java a3e666e85993b833a4765ce0ad5f4825dc9253e8 src/main/java/org/apache/aurora/scheduler/updater/strategy/QueueStrategy.java 0cf36839dbf64ad755383bef829e14fd94a97e30 src/main/thrift/org/apache/aurora/gen/api.thrift 8396e3d247b82a205b599474ee02819b695bf3d8 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java af644a967983b8da66231390d29839a034697852 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 820a968672e33b18b7f00d7346c9c7d7963fce0e src/test/java/org/apache/aurora/scheduler/updater/AddTaskTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/updater/EnumsTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java f38e6a3e6a798b1c78d73c6d19404156eb6d8c91 src/test/java/org/apache/aurora/scheduler/updater/JobUpdateEventSubscriberTest.java 41274421aaae30b43abc3da15a63276a941094f9 src/test/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachineTest.java 6eed641895123d21ed760fa755ce7b30cec3fd44 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java PRE-CREATION
Re: Review Request 25529: Add a controller for job updates.
On Sept. 11, 2014, 11:41 p.m., Maxim Khutornenko wrote: Maxim Khutornenko wrote: Non-blocking but I was still waiting on my comments/questions addressed. Wow, sorry about that - i truly thought Kevin was the only remaining ship-it needed :-( Now that we have a straw-man put together, let's iterate on any warts (and feel free to call me out on things that still deserve attention). - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25529/#review53051 --- On Sept. 16, 2014, 9:27 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25529/ --- (Updated Sept. 16, 2014, 9:27 p.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Zameer Manji. Bugs: AURORA-613 https://issues.apache.org/jira/browse/AURORA-613 Repository: aurora Description --- There's a lot going on here, i'm happy to break this up if you'd perfer, but the bulk of the change (JobUpdateControllerImpl+Test) would remain. The remaining required piece here is to record JobInstanceUpdateEvents when actions are taken. As you try to follow JobUpdateControllerImplTest, take some care to understand how FakeScheduledExecutor works. Ultimately, it accepts work added to a mock, and executes that work when FakeClock is advanced past the scheduled time. It may not be obvious at first, but be glad it's there - this kind of async test would be a nightmare without it. Diffs - build.gradle e92b27d734fb6dc9c81c39ec8cb680ef179c3695 src/main/java/org/apache/aurora/scheduler/async/RescheduleCalculator.java aaa3513ae99c70ffa51fec0a241d67fb815b6d3f src/main/java/org/apache/aurora/scheduler/base/InstanceKeys.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/base/Jobs.java 744d2b694dbcf329e67e44db206b5f7381056dbe src/main/java/org/apache/aurora/scheduler/base/Query.java d8572bb21a92025e7a51cf18d5bdf00fc1281078 src/main/java/org/apache/aurora/scheduler/base/Tasks.java 1be16374aeebaee59063aec982690ffa1fbdcf0f src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java f9521f9599e2d26cf594d62fc6b6f6ca3d5fb108 src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 6a97f36a69c7723ec1c3730085d0df70f94dcac8 src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java c520e1378c4b7947bb866011027b79b4ab65547c src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 87b773ba80784e4af8cd78790c373c08e26e src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java dc0169ac727190028fd87c640af0100d4e3ce694 src/main/java/org/apache/aurora/scheduler/updater/InstanceAction.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/updater/InstanceActionHandler.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java d725bc356178f51464b4d8ea896f1bf76815e1b2 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 39bdca02295706714cf720545ffc291f9042702a src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 3f542ce3e45ec4561238736f4ce84b69f7e41219 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateEventSubscriber.java 7be792f0bc9c5ec14c7001cb243a17d643f9607f src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java 037602d6aabe6dda978cad008075c053e2c042eb src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java d6a1e4b2c916b1c4dbcc73fe79bd672fca9b3189 src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterFactory.java 80baa7f9360cc8b2bf7910c26d119d443d6cbbf9 src/main/java/org/apache/aurora/scheduler/updater/UpdateConfigurationException.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/updater/UpdaterModule.java 028cb079316ca48bb93a35913ae25ace78088fb4 src/main/java/org/apache/aurora/scheduler/updater/strategy/ActiveLimitedStrategy.java 8298ea2283298daa71de4d958ca6bb0898d47531 src/main/java/org/apache/aurora/scheduler/updater/strategy/BatchStrategy.java a3e666e85993b833a4765ce0ad5f4825dc9253e8 src/main/java/org/apache/aurora/scheduler/updater/strategy/QueueStrategy.java 0cf36839dbf64ad755383bef829e14fd94a97e30 src/main/thrift/org/apache/aurora/gen/api.thrift 8396e3d247b82a205b599474ee02819b695bf3d8 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java af644a967983b8da66231390d29839a034697852 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 820a968672e33b18b7f00d7346c9c7d7963fce0e
Re: Review Request 25721: Asynchronous JS for Scheduler UI
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25721/#review53630 --- This is because the getJobSummary requests is 10KB compared to 1KB in the sync version. I didn't grok this part. The _request_ is 10 KB? Nonetheless, why does sync/async change data representation in either direction? - Bill Farner On Sept. 17, 2014, 12:48 a.m., David McLaughlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25721/ --- (Updated Sept. 17, 2014, 12:48 a.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner. Bugs: AURORA-700 https://issues.apache.org/jira/browse/AURORA-700 Repository: aurora Description --- Asynchronous JS for Scheduler UI. I have tried to change the minimum amount of JavaScript to keep this review small, even though doing this made me really want to tear everything up and start again :-) I attached two screenshots to show the sync vs async behaviour in the browser - note that the async version is 2x the latency of the first. This is because the getJobSummary requests is 10KB compared to 1KB in the sync version. The point is how work is done in parallel. Diffs - src/main/resources/org/apache/aurora/scheduler/http/ui/job.html 14dce65158eab83906c68f9afabf49e39283287d src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 0884cc8f0504a953ef694dae0e6b05ba6e2bff61 src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js c80146aa3829e3c3102645a1864dbeaf5e2e56bc Diff: https://reviews.apache.org/r/25721/diff/ Testing --- ./gradlew jsHint I did manual testing to verify I didn't accidentally introduce any regressions. I have around 80% confidence there are no regressions here, mainly because I wasted an hour on totally unintuitive behaviour from SmartTable. So I'm going to do a bunch more testing, which will involve mocks for updates and crons. File Attachments Before: Synchronous, serial evaluation of network requests. https://reviews.apache.org/media/uploaded/files/2014/09/17/ce60917a-5c25-4600-8c1f-cc816aa96a5e__before-sync.png AFTER: Asynchronous, parallel network requests. https://reviews.apache.org/media/uploaded/files/2014/09/17/dc703579-7072-4aa0-b51a-6df99521fcd5__after-async.png Thanks, David McLaughlin
Re: Review Request 25721: Asynchronous JS for Scheduler UI
On Sept. 17, 2014, 12:52 a.m., Bill Farner wrote: This is because the getJobSummary requests is 10KB compared to 1KB in the sync version. I didn't grok this part. The _request_ is 10 KB? Nonetheless, why does sync/async change data representation in either direction? I took the screenshot on master before I started working on this and my hello_word job had 1 active task and 0 complete tasks. I left that flapping task running while I worked on async, which generated an extra 100 bad completed tasks which obviously increased the size of the response. - David --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25721/#review53630 --- On Sept. 17, 2014, 12:48 a.m., David McLaughlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25721/ --- (Updated Sept. 17, 2014, 12:48 a.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner. Bugs: AURORA-700 https://issues.apache.org/jira/browse/AURORA-700 Repository: aurora Description --- Asynchronous JS for Scheduler UI. I have tried to change the minimum amount of JavaScript to keep this review small, even though doing this made me really want to tear everything up and start again :-) I attached two screenshots to show the sync vs async behaviour in the browser - note that the async version is 2x the latency of the first. This is because the getJobSummary requests is 10KB compared to 1KB in the sync version. The point is how work is done in parallel. Diffs - src/main/resources/org/apache/aurora/scheduler/http/ui/job.html 14dce65158eab83906c68f9afabf49e39283287d src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 0884cc8f0504a953ef694dae0e6b05ba6e2bff61 src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js c80146aa3829e3c3102645a1864dbeaf5e2e56bc Diff: https://reviews.apache.org/r/25721/diff/ Testing --- ./gradlew jsHint I did manual testing to verify I didn't accidentally introduce any regressions. I have around 80% confidence there are no regressions here, mainly because I wasted an hour on totally unintuitive behaviour from SmartTable. So I'm going to do a bunch more testing, which will involve mocks for updates and crons. File Attachments Before: Synchronous, serial evaluation of network requests. https://reviews.apache.org/media/uploaded/files/2014/09/17/ce60917a-5c25-4600-8c1f-cc816aa96a5e__before-sync.png AFTER: Asynchronous, parallel network requests. https://reviews.apache.org/media/uploaded/files/2014/09/17/dc703579-7072-4aa0-b51a-6df99521fcd5__after-async.png Thanks, David McLaughlin
Re: Review Request 25721: Asynchronous JS for Scheduler UI
On Sept. 17, 2014, 12:52 a.m., Bill Farner wrote: This is because the getJobSummary requests is 10KB compared to 1KB in the sync version. I didn't grok this part. The _request_ is 10 KB? Nonetheless, why does sync/async change data representation in either direction? David McLaughlin wrote: I took the screenshot on master before I started working on this and my hello_word job had 1 active task and 0 complete tasks. I left that flapping task running while I worked on async, which generated an extra 100 bad completed tasks which obviously increased the size of the response. Oh i see, these requests/responses were just plain different. Thanks! - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25721/#review53630 --- On Sept. 17, 2014, 12:48 a.m., David McLaughlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25721/ --- (Updated Sept. 17, 2014, 12:48 a.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Bill Farner. Bugs: AURORA-700 https://issues.apache.org/jira/browse/AURORA-700 Repository: aurora Description --- Asynchronous JS for Scheduler UI. I have tried to change the minimum amount of JavaScript to keep this review small, even though doing this made me really want to tear everything up and start again :-) I attached two screenshots to show the sync vs async behaviour in the browser - note that the async version is 2x the latency of the first. This is because the getJobSummary requests is 10KB compared to 1KB in the sync version. The point is how work is done in parallel. Diffs - src/main/resources/org/apache/aurora/scheduler/http/ui/job.html 14dce65158eab83906c68f9afabf49e39283287d src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 0884cc8f0504a953ef694dae0e6b05ba6e2bff61 src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js c80146aa3829e3c3102645a1864dbeaf5e2e56bc Diff: https://reviews.apache.org/r/25721/diff/ Testing --- ./gradlew jsHint I did manual testing to verify I didn't accidentally introduce any regressions. I have around 80% confidence there are no regressions here, mainly because I wasted an hour on totally unintuitive behaviour from SmartTable. So I'm going to do a bunch more testing, which will involve mocks for updates and crons. File Attachments Before: Synchronous, serial evaluation of network requests. https://reviews.apache.org/media/uploaded/files/2014/09/17/ce60917a-5c25-4600-8c1f-cc816aa96a5e__before-sync.png AFTER: Asynchronous, parallel network requests. https://reviews.apache.org/media/uploaded/files/2014/09/17/dc703579-7072-4aa0-b51a-6df99521fcd5__after-async.png Thanks, David McLaughlin