You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Bill Farner <wf...@apache.org> on 2014/09/30 02:59:28 UTC
Review Request 26158: Perform job update no-op detection in
SchedulerThriftInterface.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26158/
-----------------------------------------------------------
Review request for Aurora and Maxim Khutornenko.
Bugs: AURORA-769
https://issues.apache.org/jira/browse/AURORA-769
Repository: aurora
Description
-------
Perform job update no-op detection in SchedulerThriftInterface.
Diffs
-----
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2847bdbb773f4d5a7f31b52ceb806880e736a259
src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java 9d02474751ed22cb2dfd7903aa715d53b5ac805e
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java f37fbef4c50cfc2d98bc1fd89376a210f4fa8643
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 97bc1dfbbd83d30d6e94b70902f225f2c92acb77
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 12b19fd333a95d0d3d1864bac629d011c271ee52
src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 7254588b55df9a12217c8ec172abc5976892497e
Diff: https://reviews.apache.org/r/26158/diff/
Testing
-------
./gradlew build -Pq
Ran a no-op update in vagrant and saw expected behavior.
Thanks,
Bill Farner
Re: Review Request 26158: Perform job update no-op detection in
SchedulerThriftInterface.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26158/
-----------------------------------------------------------
(Updated Sept. 30, 2014, 10:08 p.m.)
Review request for Aurora and Maxim Khutornenko.
Changes
-------
Rebase after conflict on master.
Bugs: AURORA-769
https://issues.apache.org/jira/browse/AURORA-769
Repository: aurora
Description
-------
Perform job update no-op detection in SchedulerThriftInterface.
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2847bdbb773f4d5a7f31b52ceb806880e736a259
src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java 5daefff05edeaed236b76419ab65cbc3cddd8bac
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java f37fbef4c50cfc2d98bc1fd89376a210f4fa8643
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 97bc1dfbbd83d30d6e94b70902f225f2c92acb77
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 12b19fd333a95d0d3d1864bac629d011c271ee52
src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 7254588b55df9a12217c8ec172abc5976892497e
Diff: https://reviews.apache.org/r/26158/diff/
Testing
-------
./gradlew build -Pq
Ran a no-op update in vagrant and saw expected behavior.
Thanks,
Bill Farner
Re: Review Request 26158: Perform job update no-op detection in
SchedulerThriftInterface.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26158/
-----------------------------------------------------------
(Updated Sept. 30, 2014, 9:53 p.m.)
Review request for Aurora and Maxim Khutornenko.
Bugs: AURORA-769
https://issues.apache.org/jira/browse/AURORA-769
Repository: aurora
Description
-------
Perform job update no-op detection in SchedulerThriftInterface.
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2847bdbb773f4d5a7f31b52ceb806880e736a259
src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java 9d02474751ed22cb2dfd7903aa715d53b5ac805e
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java f37fbef4c50cfc2d98bc1fd89376a210f4fa8643
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 97bc1dfbbd83d30d6e94b70902f225f2c92acb77
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 12b19fd333a95d0d3d1864bac629d011c271ee52
src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 7254588b55df9a12217c8ec172abc5976892497e
Diff: https://reviews.apache.org/r/26158/diff/
Testing
-------
./gradlew build -Pq
Ran a no-op update in vagrant and saw expected behavior.
Thanks,
Bill Farner
Re: Review Request 26158: Perform job update no-op detection in
SchedulerThriftInterface.
Posted by Bill Farner <wf...@apache.org>.
> On Sept. 30, 2014, 4:51 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, line 1431
> > <https://reviews.apache.org/r/26158/diff/2/?file=709002#file709002line1431>
> >
> > This needs to be OK response.
Reverted.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26158/#review54984
-----------------------------------------------------------
On Sept. 30, 2014, 3:09 a.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26158/
> -----------------------------------------------------------
>
> (Updated Sept. 30, 2014, 3:09 a.m.)
>
>
> Review request for Aurora and Maxim Khutornenko.
>
>
> Bugs: AURORA-769
> https://issues.apache.org/jira/browse/AURORA-769
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Perform job update no-op detection in SchedulerThriftInterface.
>
>
> Diffs
> -----
>
> build.gradle 325a50e628ec0e2e21e812ef1daba98fc99a43d4
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2847bdbb773f4d5a7f31b52ceb806880e736a259
> src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java 9d02474751ed22cb2dfd7903aa715d53b5ac805e
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java f37fbef4c50cfc2d98bc1fd89376a210f4fa8643
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 97bc1dfbbd83d30d6e94b70902f225f2c92acb77
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 12b19fd333a95d0d3d1864bac629d011c271ee52
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 7254588b55df9a12217c8ec172abc5976892497e
>
> Diff: https://reviews.apache.org/r/26158/diff/
>
>
> Testing
> -------
>
> ./gradlew build -Pq
>
> Ran a no-op update in vagrant and saw expected behavior.
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 26158: Perform job update no-op detection in
SchedulerThriftInterface.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26158/#review54984
-----------------------------------------------------------
Ship it!
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
<https://reviews.apache.org/r/26158/#comment95312>
This needs to be OK response.
- Maxim Khutornenko
On Sept. 30, 2014, 3:09 a.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26158/
> -----------------------------------------------------------
>
> (Updated Sept. 30, 2014, 3:09 a.m.)
>
>
> Review request for Aurora and Maxim Khutornenko.
>
>
> Bugs: AURORA-769
> https://issues.apache.org/jira/browse/AURORA-769
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Perform job update no-op detection in SchedulerThriftInterface.
>
>
> Diffs
> -----
>
> build.gradle 325a50e628ec0e2e21e812ef1daba98fc99a43d4
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2847bdbb773f4d5a7f31b52ceb806880e736a259
> src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java 9d02474751ed22cb2dfd7903aa715d53b5ac805e
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java f37fbef4c50cfc2d98bc1fd89376a210f4fa8643
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 97bc1dfbbd83d30d6e94b70902f225f2c92acb77
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 12b19fd333a95d0d3d1864bac629d011c271ee52
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 7254588b55df9a12217c8ec172abc5976892497e
>
> Diff: https://reviews.apache.org/r/26158/diff/
>
>
> Testing
> -------
>
> ./gradlew build -Pq
>
> Ran a no-op update in vagrant and saw expected behavior.
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 26158: Perform job update no-op detection in
SchedulerThriftInterface.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26158/
-----------------------------------------------------------
(Updated Sept. 30, 2014, 3:09 a.m.)
Review request for Aurora and Maxim Khutornenko.
Bugs: AURORA-769
https://issues.apache.org/jira/browse/AURORA-769
Repository: aurora
Description
-------
Perform job update no-op detection in SchedulerThriftInterface.
Diffs (updated)
-----
build.gradle 325a50e628ec0e2e21e812ef1daba98fc99a43d4
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2847bdbb773f4d5a7f31b52ceb806880e736a259
src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java 9d02474751ed22cb2dfd7903aa715d53b5ac805e
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java f37fbef4c50cfc2d98bc1fd89376a210f4fa8643
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 97bc1dfbbd83d30d6e94b70902f225f2c92acb77
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 12b19fd333a95d0d3d1864bac629d011c271ee52
src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 7254588b55df9a12217c8ec172abc5976892497e
Diff: https://reviews.apache.org/r/26158/diff/
Testing
-------
./gradlew build -Pq
Ran a no-op update in vagrant and saw expected behavior.
Thanks,
Bill Farner
Re: Review Request 26158: Perform job update no-op detection in
SchedulerThriftInterface.
Posted by Bill Farner <wf...@apache.org>.
> On Sept. 30, 2014, 1:38 a.m., Maxim Khutornenko wrote:
> > Not quite obvious from the diff: why was not it enough to check the noop condition in the controller only?
`SchedulerThriftInterface` would get a `JobDiff`, and use that to build `IJobUpdateInstructions`. `JobUpdateControllerImpl` would take that, and calculate a diff against the state of the job. i.e. we were performing a diff against a diff, which gets weird when the instance count changes.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26158/#review54934
-----------------------------------------------------------
On Sept. 30, 2014, 12:59 a.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26158/
> -----------------------------------------------------------
>
> (Updated Sept. 30, 2014, 12:59 a.m.)
>
>
> Review request for Aurora and Maxim Khutornenko.
>
>
> Bugs: AURORA-769
> https://issues.apache.org/jira/browse/AURORA-769
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Perform job update no-op detection in SchedulerThriftInterface.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2847bdbb773f4d5a7f31b52ceb806880e736a259
> src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java 9d02474751ed22cb2dfd7903aa715d53b5ac805e
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java f37fbef4c50cfc2d98bc1fd89376a210f4fa8643
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 97bc1dfbbd83d30d6e94b70902f225f2c92acb77
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 12b19fd333a95d0d3d1864bac629d011c271ee52
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 7254588b55df9a12217c8ec172abc5976892497e
>
> Diff: https://reviews.apache.org/r/26158/diff/
>
>
> Testing
> -------
>
> ./gradlew build -Pq
>
> Ran a no-op update in vagrant and saw expected behavior.
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 26158: Perform job update no-op detection in
SchedulerThriftInterface.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26158/#review54934
-----------------------------------------------------------
Not quite obvious from the diff: why was not it enough to check the noop condition in the controller only?
- Maxim Khutornenko
On Sept. 30, 2014, 12:59 a.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26158/
> -----------------------------------------------------------
>
> (Updated Sept. 30, 2014, 12:59 a.m.)
>
>
> Review request for Aurora and Maxim Khutornenko.
>
>
> Bugs: AURORA-769
> https://issues.apache.org/jira/browse/AURORA-769
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Perform job update no-op detection in SchedulerThriftInterface.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2847bdbb773f4d5a7f31b52ceb806880e736a259
> src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java 9d02474751ed22cb2dfd7903aa715d53b5ac805e
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java f37fbef4c50cfc2d98bc1fd89376a210f4fa8643
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 97bc1dfbbd83d30d6e94b70902f225f2c92acb77
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 12b19fd333a95d0d3d1864bac629d011c271ee52
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 7254588b55df9a12217c8ec172abc5976892497e
>
> Diff: https://reviews.apache.org/r/26158/diff/
>
>
> Testing
> -------
>
> ./gradlew build -Pq
>
> Ran a no-op update in vagrant and saw expected behavior.
>
>
> Thanks,
>
> Bill Farner
>
>