You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Maxim Khutornenko <ma...@apache.org> on 2015/05/20 00:50:30 UTC
Review Request 34440: Implementing task reconciler.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34440/
-----------------------------------------------------------
Review request for Aurora, Ben Mahler, Kevin Sweeney, and Zameer Manji.
Bugs: AURORA-1047
https://issues.apache.org/jira/browse/AURORA-1047
Repository: aurora
Description
-------
Adding a new service to request explict/implicit task reconciliations on a periodic basis. The reconciler is OFF by default until the GC executor code is removed.
Diffs
-----
src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 316ab1c06ce63c9a3f7232264d30a40f487fc45c
src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java e9d47fda3355786a4e68eac5c28490c04bc68cb3
src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/mesos/Driver.java 975ea02b45488608286e743888de25862cc77add
src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java 35cada6160af01c13362fa7c14b9ff8da0300004
src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 02e87989a17d95d36e61ffcef2e86c91774972bc
src/test/java/org/apache/aurora/scheduler/async/TaskReconcilerTest.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 2beea4fc1a24c0050898077ecdf6cac2b19fab31
Diff: https://reviews.apache.org/r/34440/diff/
Testing
-------
./gradlew -Pq build
Thanks,
Maxim Khutornenko
Re: Review Request 34440: Implementing task reconciler.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34440/#review84415
-----------------------------------------------------------
Ship it!
Master (920263b) is green with this patch.
./build-support/jenkins/build.sh
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On May 19, 2015, 10:50 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34440/
> -----------------------------------------------------------
>
> (Updated May 19, 2015, 10:50 p.m.)
>
>
> Review request for Aurora, Ben Mahler, Kevin Sweeney, and Zameer Manji.
>
>
> Bugs: AURORA-1047
> https://issues.apache.org/jira/browse/AURORA-1047
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adding a new service to request explict/implicit task reconciliations on a periodic basis. The reconciler is OFF by default until the GC executor code is removed.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 316ab1c06ce63c9a3f7232264d30a40f487fc45c
> src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java e9d47fda3355786a4e68eac5c28490c04bc68cb3
> src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java 975ea02b45488608286e743888de25862cc77add
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java 35cada6160af01c13362fa7c14b9ff8da0300004
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 02e87989a17d95d36e61ffcef2e86c91774972bc
> src/test/java/org/apache/aurora/scheduler/async/TaskReconcilerTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 2beea4fc1a24c0050898077ecdf6cac2b19fab31
>
> Diff: https://reviews.apache.org/r/34440/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 34440: Implementing task reconciler.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On May 20, 2015, 12:44 a.m., Ben Mahler wrote:
> > src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java, lines 185-207
> > <https://reviews.apache.org/r/34440/diff/1/?file=964437#file964437line185>
> >
> > Hm.. I thought the approach was going to be to have a flag that toggles between the gc executor and task reconciliation, so that if there are any issues, we can transition back to the GC executor without having to rollback the version of Aurora that is deployed?
Implementing a single flag switch would require more throwaway code. The switch is accomplished by individually setting the gc_executor_path and reconciliation_initial_delay values and will have to be done by the operator.
> On May 20, 2015, 12:44 a.m., Ben Mahler wrote:
> > src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java, lines 186-189
> > <https://reviews.apache.org/r/34440/diff/1/?file=964437#file964437line186>
> >
> > Something to consider is that the important time to reconcile is after any (re-)registration callbacks, but that is a little more complicated to implement here.
> >
> > It's not that important for now since you don't currently know when reconciliation is finished, and we reconcile forever here. Just wanted to mention it for when we decide to improve the reconciliation API!
We only (re-)register on scheduler startups. When the feature is on we can set the initial delay low enough to closely follow registration (e.g. 1 minute).
> On May 20, 2015, 12:44 a.m., Ben Mahler wrote:
> > src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java, lines 136-139
> > <https://reviews.apache.org/r/34440/diff/1/?file=964438#file964438line136>
> >
> > As we discussed, at some point we may want to improve the reconciliation API to support sending diffs. This would require setting the state (which is originally why the state was required here). So you may want to have the ability to know what the mesos state of a task is given the scheduler's state.
> >
> > Something to keep in mind.
Thanks. We currently don't have direct one-to-one mapping between TaskState and ScheduleStatus. Specifically, both TaskState.TASK_STARTING and TaskState.TASK_STAGING are mapped to ScheduleStatus.STARTING. This makes explicit reconciliation error prone wrt these states. I will file a ticket to investigate a solution here.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34440/#review84427
-----------------------------------------------------------
On May 19, 2015, 10:50 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34440/
> -----------------------------------------------------------
>
> (Updated May 19, 2015, 10:50 p.m.)
>
>
> Review request for Aurora, Ben Mahler, Kevin Sweeney, and Zameer Manji.
>
>
> Bugs: AURORA-1047
> https://issues.apache.org/jira/browse/AURORA-1047
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adding a new service to request explict/implicit task reconciliations on a periodic basis. The reconciler is OFF by default until the GC executor code is removed.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 316ab1c06ce63c9a3f7232264d30a40f487fc45c
> src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java e9d47fda3355786a4e68eac5c28490c04bc68cb3
> src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java 975ea02b45488608286e743888de25862cc77add
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java 35cada6160af01c13362fa7c14b9ff8da0300004
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 02e87989a17d95d36e61ffcef2e86c91774972bc
> src/test/java/org/apache/aurora/scheduler/async/TaskReconcilerTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 2beea4fc1a24c0050898077ecdf6cac2b19fab31
>
> Diff: https://reviews.apache.org/r/34440/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 34440: Implementing task reconciler.
Posted by Ben Mahler <be...@gmail.com>.
> On May 20, 2015, 12:44 a.m., Ben Mahler wrote:
> > src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java, lines 186-189
> > <https://reviews.apache.org/r/34440/diff/1/?file=964437#file964437line186>
> >
> > Something to consider is that the important time to reconcile is after any (re-)registration callbacks, but that is a little more complicated to implement here.
> >
> > It's not that important for now since you don't currently know when reconciliation is finished, and we reconcile forever here. Just wanted to mention it for when we decide to improve the reconciliation API!
>
> Maxim Khutornenko wrote:
> We only (re-)register on scheduler startups. When the feature is on we can set the initial delay low enough to closely follow registration (e.g. 1 minute).
There are two cases for re-registration: case 1 is scheduler failover (which you describe), and case 2 is master failover (the driver is going to re-register under the covers if the master fails). Case 2 is what I was referring to, where you will receive a re-registered callback during the steady state of the scheduler's lifecycle.
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34440/#review84427
-----------------------------------------------------------
On May 20, 2015, 1:34 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34440/
> -----------------------------------------------------------
>
> (Updated May 20, 2015, 1:34 a.m.)
>
>
> Review request for Aurora, Ben Mahler, Kevin Sweeney, and Zameer Manji.
>
>
> Bugs: AURORA-1047
> https://issues.apache.org/jira/browse/AURORA-1047
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adding a new service to request explict/implicit task reconciliations on a periodic basis. The reconciler is OFF by default until the GC executor code is removed.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 316ab1c06ce63c9a3f7232264d30a40f487fc45c
> src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java e9d47fda3355786a4e68eac5c28490c04bc68cb3
> src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java 975ea02b45488608286e743888de25862cc77add
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java 35cada6160af01c13362fa7c14b9ff8da0300004
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 02e87989a17d95d36e61ffcef2e86c91774972bc
> src/test/java/org/apache/aurora/scheduler/async/TaskReconcilerTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 2beea4fc1a24c0050898077ecdf6cac2b19fab31
>
> Diff: https://reviews.apache.org/r/34440/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 34440: Implementing task reconciler.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On May 20, 2015, 12:44 a.m., Ben Mahler wrote:
> > src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java, lines 186-189
> > <https://reviews.apache.org/r/34440/diff/1/?file=964437#file964437line186>
> >
> > Something to consider is that the important time to reconcile is after any (re-)registration callbacks, but that is a little more complicated to implement here.
> >
> > It's not that important for now since you don't currently know when reconciliation is finished, and we reconcile forever here. Just wanted to mention it for when we decide to improve the reconciliation API!
>
> Maxim Khutornenko wrote:
> We only (re-)register on scheduler startups. When the feature is on we can set the initial delay low enough to closely follow registration (e.g. 1 minute).
>
> Ben Mahler wrote:
> There are two cases for re-registration: case 1 is scheduler failover (which you describe), and case 2 is master failover (the driver is going to re-register under the covers if the master fails). Case 2 is what I was referring to, where you will receive a re-registered callback during the steady state of the scheduler's lifecycle.
I see what you meant now. Yes, that's possible. While it's easy to trigger an out of band reconciliation on DriverRegistered event, I'd be wary of compounding multiple reconciliation streams at this point. The regular scheduled run should pick up inconsistencies within an hour (or less).
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34440/#review84427
-----------------------------------------------------------
On May 20, 2015, 1:34 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34440/
> -----------------------------------------------------------
>
> (Updated May 20, 2015, 1:34 a.m.)
>
>
> Review request for Aurora, Ben Mahler, Kevin Sweeney, and Zameer Manji.
>
>
> Bugs: AURORA-1047
> https://issues.apache.org/jira/browse/AURORA-1047
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adding a new service to request explict/implicit task reconciliations on a periodic basis. The reconciler is OFF by default until the GC executor code is removed.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 316ab1c06ce63c9a3f7232264d30a40f487fc45c
> src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java e9d47fda3355786a4e68eac5c28490c04bc68cb3
> src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java 975ea02b45488608286e743888de25862cc77add
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java 35cada6160af01c13362fa7c14b9ff8da0300004
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 02e87989a17d95d36e61ffcef2e86c91774972bc
> src/test/java/org/apache/aurora/scheduler/async/TaskReconcilerTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 2beea4fc1a24c0050898077ecdf6cac2b19fab31
>
> Diff: https://reviews.apache.org/r/34440/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 34440: Implementing task reconciler.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34440/#review84427
-----------------------------------------------------------
Thanks Maxim!
src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java
<https://reviews.apache.org/r/34440/#comment135675>
Hm.. I thought the approach was going to be to have a flag that toggles between the gc executor and task reconciliation, so that if there are any issues, we can transition back to the GC executor without having to rollback the version of Aurora that is deployed?
src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java
<https://reviews.apache.org/r/34440/#comment135678>
Something to consider is that the important time to reconcile is after any (re-)registration callbacks, but that is a little more complicated to implement here.
It's not that important for now since you don't currently know when reconciliation is finished, and we reconcile forever here. Just wanted to mention it for when we decide to improve the reconciliation API!
src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java
<https://reviews.apache.org/r/34440/#comment135676>
As we discussed, at some point we may want to improve the reconciliation API to support sending diffs. This would require setting the state (which is originally why the state was required here). So you may want to have the ability to know what the mesos state of a task is given the scheduler's state.
Something to keep in mind.
- Ben Mahler
On May 19, 2015, 10:50 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34440/
> -----------------------------------------------------------
>
> (Updated May 19, 2015, 10:50 p.m.)
>
>
> Review request for Aurora, Ben Mahler, Kevin Sweeney, and Zameer Manji.
>
>
> Bugs: AURORA-1047
> https://issues.apache.org/jira/browse/AURORA-1047
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adding a new service to request explict/implicit task reconciliations on a periodic basis. The reconciler is OFF by default until the GC executor code is removed.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 316ab1c06ce63c9a3f7232264d30a40f487fc45c
> src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java e9d47fda3355786a4e68eac5c28490c04bc68cb3
> src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java 975ea02b45488608286e743888de25862cc77add
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java 35cada6160af01c13362fa7c14b9ff8da0300004
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 02e87989a17d95d36e61ffcef2e86c91774972bc
> src/test/java/org/apache/aurora/scheduler/async/TaskReconcilerTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 2beea4fc1a24c0050898077ecdf6cac2b19fab31
>
> Diff: https://reviews.apache.org/r/34440/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 34440: Implementing task reconciler.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On May 22, 2015, 8:33 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java, line 44
> > <https://reviews.apache.org/r/34440/diff/2/?file=964681#file964681line44>
> >
> > It's kind of odd to abstract this in this way. Do we expect the unit to change? If not, can we just static import Time.MINUTES and directly reference MINUTES everywhere so that it's clear what unit is in use?
That's a great idea. Changed.
> On May 22, 2015, 8:33 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java, line 206
> > <https://reviews.apache.org/r/34440/diff/2/?file=964680#file964680line206>
> >
> > Add @Positive (probably should add to all of these args?).
Spread can technically be 0. Added @Positive to intervals though.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34440/#review84977
-----------------------------------------------------------
On May 20, 2015, 1:34 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34440/
> -----------------------------------------------------------
>
> (Updated May 20, 2015, 1:34 a.m.)
>
>
> Review request for Aurora, Ben Mahler, Kevin Sweeney, and Zameer Manji.
>
>
> Bugs: AURORA-1047
> https://issues.apache.org/jira/browse/AURORA-1047
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adding a new service to request explict/implicit task reconciliations on a periodic basis. The reconciler is OFF by default until the GC executor code is removed.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 316ab1c06ce63c9a3f7232264d30a40f487fc45c
> src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java e9d47fda3355786a4e68eac5c28490c04bc68cb3
> src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java 975ea02b45488608286e743888de25862cc77add
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java 35cada6160af01c13362fa7c14b9ff8da0300004
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 02e87989a17d95d36e61ffcef2e86c91774972bc
> src/test/java/org/apache/aurora/scheduler/async/TaskReconcilerTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 2beea4fc1a24c0050898077ecdf6cac2b19fab31
>
> Diff: https://reviews.apache.org/r/34440/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 34440: Implementing task reconciler.
Posted by Joshua Cohen <jc...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34440/#review84977
-----------------------------------------------------------
Ship it!
src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java
<https://reviews.apache.org/r/34440/#comment136432>
Add @Positive (probably should add to all of these args?).
src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java
<https://reviews.apache.org/r/34440/#comment136429>
It's kind of odd to abstract this in this way. Do we expect the unit to change? If not, can we just static import Time.MINUTES and directly reference MINUTES everywhere so that it's clear what unit is in use?
- Joshua Cohen
On May 20, 2015, 1:34 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34440/
> -----------------------------------------------------------
>
> (Updated May 20, 2015, 1:34 a.m.)
>
>
> Review request for Aurora, Ben Mahler, Kevin Sweeney, and Zameer Manji.
>
>
> Bugs: AURORA-1047
> https://issues.apache.org/jira/browse/AURORA-1047
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adding a new service to request explict/implicit task reconciliations on a periodic basis. The reconciler is OFF by default until the GC executor code is removed.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 316ab1c06ce63c9a3f7232264d30a40f487fc45c
> src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java e9d47fda3355786a4e68eac5c28490c04bc68cb3
> src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java 975ea02b45488608286e743888de25862cc77add
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java 35cada6160af01c13362fa7c14b9ff8da0300004
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 02e87989a17d95d36e61ffcef2e86c91774972bc
> src/test/java/org/apache/aurora/scheduler/async/TaskReconcilerTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 2beea4fc1a24c0050898077ecdf6cac2b19fab31
>
> Diff: https://reviews.apache.org/r/34440/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 34440: Implementing task reconciler.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34440/#review84999
-----------------------------------------------------------
Ship it!
Master (998993d) is green with this patch.
./build-support/jenkins/build.sh
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On May 22, 2015, 8:56 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34440/
> -----------------------------------------------------------
>
> (Updated May 22, 2015, 8:56 p.m.)
>
>
> Review request for Aurora, Ben Mahler, Joshua Cohen, and Zameer Manji.
>
>
> Bugs: AURORA-1047
> https://issues.apache.org/jira/browse/AURORA-1047
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adding a new service to request explict/implicit task reconciliations on a periodic basis. The reconciler is OFF by default until the GC executor code is removed.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 316ab1c06ce63c9a3f7232264d30a40f487fc45c
> src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java e9d47fda3355786a4e68eac5c28490c04bc68cb3
> src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java 975ea02b45488608286e743888de25862cc77add
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java 35cada6160af01c13362fa7c14b9ff8da0300004
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 02e87989a17d95d36e61ffcef2e86c91774972bc
> src/test/java/org/apache/aurora/scheduler/async/TaskReconcilerTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 2beea4fc1a24c0050898077ecdf6cac2b19fab31
>
> Diff: https://reviews.apache.org/r/34440/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 34440: Implementing task reconciler.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34440/
-----------------------------------------------------------
(Updated May 22, 2015, 8:56 p.m.)
Review request for Aurora, Ben Mahler, Joshua Cohen, and Zameer Manji.
Changes
-------
Adding space between imports.
Bugs: AURORA-1047
https://issues.apache.org/jira/browse/AURORA-1047
Repository: aurora
Description
-------
Adding a new service to request explict/implicit task reconciliations on a periodic basis. The reconciler is OFF by default until the GC executor code is removed.
Diffs (updated)
-----
src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 316ab1c06ce63c9a3f7232264d30a40f487fc45c
src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java e9d47fda3355786a4e68eac5c28490c04bc68cb3
src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/mesos/Driver.java 975ea02b45488608286e743888de25862cc77add
src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java 35cada6160af01c13362fa7c14b9ff8da0300004
src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 02e87989a17d95d36e61ffcef2e86c91774972bc
src/test/java/org/apache/aurora/scheduler/async/TaskReconcilerTest.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 2beea4fc1a24c0050898077ecdf6cac2b19fab31
Diff: https://reviews.apache.org/r/34440/diff/
Testing
-------
./gradlew -Pq build
Thanks,
Maxim Khutornenko
Re: Review Request 34440: Implementing task reconciler.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34440/#review84990
-----------------------------------------------------------
Master (998993d) is red with this patch.
./build-support/jenkins/build.sh
:api:checkPython
:api:generateThriftEntitiesJava
:api:classesThriftEntities
:api:compileJava UP-TO-DATE
:api:generateThriftResources
:api:processResources UP-TO-DATE
:api:classes
:api:jar
:compileJavaNote: Writing file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/com/twitter/common/args/apt/cmdline.arg.info.txt.2
:processResources
:classes
:jar
:startScripts
:distTar
:distZip
:assemble
:compileJmhJavaNote: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/jmh/java/org/apache/aurora/benchmark/fakes/FakeSchedulerDriver.java uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
:processJmhResources UP-TO-DATE
:jmhClasses
:checkstyleJmh
:jsHint
:checkstyleMain[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java:37: 'com.twitter.common.quantity.Time.MINUTES' should be separated from previous imports.
FAILED
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':checkstyleMain'.
> Checkstyle rule violations were found. See the report at: file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/checkstyle/main.xml
* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output.
BUILD FAILED
Total time: 1 mins 53.619 secs
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On May 22, 2015, 8:47 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34440/
> -----------------------------------------------------------
>
> (Updated May 22, 2015, 8:47 p.m.)
>
>
> Review request for Aurora, Ben Mahler, Joshua Cohen, and Zameer Manji.
>
>
> Bugs: AURORA-1047
> https://issues.apache.org/jira/browse/AURORA-1047
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adding a new service to request explict/implicit task reconciliations on a periodic basis. The reconciler is OFF by default until the GC executor code is removed.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 316ab1c06ce63c9a3f7232264d30a40f487fc45c
> src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java e9d47fda3355786a4e68eac5c28490c04bc68cb3
> src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java 975ea02b45488608286e743888de25862cc77add
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java 35cada6160af01c13362fa7c14b9ff8da0300004
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 02e87989a17d95d36e61ffcef2e86c91774972bc
> src/test/java/org/apache/aurora/scheduler/async/TaskReconcilerTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 2beea4fc1a24c0050898077ecdf6cac2b19fab31
>
> Diff: https://reviews.apache.org/r/34440/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 34440: Implementing task reconciler.
Posted by Joshua Cohen <jc...@apache.org>.
> On May 22, 2015, 8:52 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java, line 186
> > <https://reviews.apache.org/r/34440/diff/2-3/?file=964680#file964680line186>
> >
> > Add @Positive here as well.
>
> Maxim Khutornenko wrote:
> Well, this can and should be 0 as well. We start reconciler service on scheduler active, so no reason to delay it further once the feature is active.
ack.
- Joshua
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34440/#review84992
-----------------------------------------------------------
On May 22, 2015, 8:56 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34440/
> -----------------------------------------------------------
>
> (Updated May 22, 2015, 8:56 p.m.)
>
>
> Review request for Aurora, Ben Mahler, Joshua Cohen, and Zameer Manji.
>
>
> Bugs: AURORA-1047
> https://issues.apache.org/jira/browse/AURORA-1047
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adding a new service to request explict/implicit task reconciliations on a periodic basis. The reconciler is OFF by default until the GC executor code is removed.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 316ab1c06ce63c9a3f7232264d30a40f487fc45c
> src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java e9d47fda3355786a4e68eac5c28490c04bc68cb3
> src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java 975ea02b45488608286e743888de25862cc77add
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java 35cada6160af01c13362fa7c14b9ff8da0300004
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 02e87989a17d95d36e61ffcef2e86c91774972bc
> src/test/java/org/apache/aurora/scheduler/async/TaskReconcilerTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 2beea4fc1a24c0050898077ecdf6cac2b19fab31
>
> Diff: https://reviews.apache.org/r/34440/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 34440: Implementing task reconciler.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On May 22, 2015, 8:52 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java, line 186
> > <https://reviews.apache.org/r/34440/diff/2-3/?file=964680#file964680line186>
> >
> > Add @Positive here as well.
Well, this can and should be 0 as well. We start reconciler service on scheduler active, so no reason to delay it further once the feature is active.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34440/#review84992
-----------------------------------------------------------
On May 22, 2015, 8:47 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34440/
> -----------------------------------------------------------
>
> (Updated May 22, 2015, 8:47 p.m.)
>
>
> Review request for Aurora, Ben Mahler, Joshua Cohen, and Zameer Manji.
>
>
> Bugs: AURORA-1047
> https://issues.apache.org/jira/browse/AURORA-1047
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adding a new service to request explict/implicit task reconciliations on a periodic basis. The reconciler is OFF by default until the GC executor code is removed.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 316ab1c06ce63c9a3f7232264d30a40f487fc45c
> src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java e9d47fda3355786a4e68eac5c28490c04bc68cb3
> src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java 975ea02b45488608286e743888de25862cc77add
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java 35cada6160af01c13362fa7c14b9ff8da0300004
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 02e87989a17d95d36e61ffcef2e86c91774972bc
> src/test/java/org/apache/aurora/scheduler/async/TaskReconcilerTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 2beea4fc1a24c0050898077ecdf6cac2b19fab31
>
> Diff: https://reviews.apache.org/r/34440/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 34440: Implementing task reconciler.
Posted by Joshua Cohen <jc...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34440/#review84992
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java
<https://reviews.apache.org/r/34440/#comment136454>
Add @Positive here as well.
- Joshua Cohen
On May 22, 2015, 8:47 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34440/
> -----------------------------------------------------------
>
> (Updated May 22, 2015, 8:47 p.m.)
>
>
> Review request for Aurora, Ben Mahler, Joshua Cohen, and Zameer Manji.
>
>
> Bugs: AURORA-1047
> https://issues.apache.org/jira/browse/AURORA-1047
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adding a new service to request explict/implicit task reconciliations on a periodic basis. The reconciler is OFF by default until the GC executor code is removed.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 316ab1c06ce63c9a3f7232264d30a40f487fc45c
> src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java e9d47fda3355786a4e68eac5c28490c04bc68cb3
> src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java 975ea02b45488608286e743888de25862cc77add
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java 35cada6160af01c13362fa7c14b9ff8da0300004
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 02e87989a17d95d36e61ffcef2e86c91774972bc
> src/test/java/org/apache/aurora/scheduler/async/TaskReconcilerTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 2beea4fc1a24c0050898077ecdf6cac2b19fab31
>
> Diff: https://reviews.apache.org/r/34440/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 34440: Implementing task reconciler.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34440/
-----------------------------------------------------------
(Updated May 22, 2015, 8:47 p.m.)
Review request for Aurora, Ben Mahler, Joshua Cohen, and Zameer Manji.
Changes
-------
-kevints
+jcohen
CR comments.
Bugs: AURORA-1047
https://issues.apache.org/jira/browse/AURORA-1047
Repository: aurora
Description
-------
Adding a new service to request explict/implicit task reconciliations on a periodic basis. The reconciler is OFF by default until the GC executor code is removed.
Diffs (updated)
-----
src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 316ab1c06ce63c9a3f7232264d30a40f487fc45c
src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java e9d47fda3355786a4e68eac5c28490c04bc68cb3
src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/mesos/Driver.java 975ea02b45488608286e743888de25862cc77add
src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java 35cada6160af01c13362fa7c14b9ff8da0300004
src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 02e87989a17d95d36e61ffcef2e86c91774972bc
src/test/java/org/apache/aurora/scheduler/async/TaskReconcilerTest.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 2beea4fc1a24c0050898077ecdf6cac2b19fab31
Diff: https://reviews.apache.org/r/34440/diff/
Testing
-------
./gradlew -Pq build
Thanks,
Maxim Khutornenko
Re: Review Request 34440: Implementing task reconciler.
Posted by Zameer Manji <zm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34440/#review84442
-----------------------------------------------------------
Ship it!
Ship It!
- Zameer Manji
On May 19, 2015, 6:34 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34440/
> -----------------------------------------------------------
>
> (Updated May 19, 2015, 6:34 p.m.)
>
>
> Review request for Aurora, Ben Mahler, Kevin Sweeney, and Zameer Manji.
>
>
> Bugs: AURORA-1047
> https://issues.apache.org/jira/browse/AURORA-1047
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adding a new service to request explict/implicit task reconciliations on a periodic basis. The reconciler is OFF by default until the GC executor code is removed.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 316ab1c06ce63c9a3f7232264d30a40f487fc45c
> src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java e9d47fda3355786a4e68eac5c28490c04bc68cb3
> src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java 975ea02b45488608286e743888de25862cc77add
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java 35cada6160af01c13362fa7c14b9ff8da0300004
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 02e87989a17d95d36e61ffcef2e86c91774972bc
> src/test/java/org/apache/aurora/scheduler/async/TaskReconcilerTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 2beea4fc1a24c0050898077ecdf6cac2b19fab31
>
> Diff: https://reviews.apache.org/r/34440/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 34440: Implementing task reconciler.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34440/#review84440
-----------------------------------------------------------
Ship it!
Master (920263b) is green with this patch.
./build-support/jenkins/build.sh
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On May 20, 2015, 1:34 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34440/
> -----------------------------------------------------------
>
> (Updated May 20, 2015, 1:34 a.m.)
>
>
> Review request for Aurora, Ben Mahler, Kevin Sweeney, and Zameer Manji.
>
>
> Bugs: AURORA-1047
> https://issues.apache.org/jira/browse/AURORA-1047
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adding a new service to request explict/implicit task reconciliations on a periodic basis. The reconciler is OFF by default until the GC executor code is removed.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 316ab1c06ce63c9a3f7232264d30a40f487fc45c
> src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java e9d47fda3355786a4e68eac5c28490c04bc68cb3
> src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java 975ea02b45488608286e743888de25862cc77add
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java 35cada6160af01c13362fa7c14b9ff8da0300004
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 02e87989a17d95d36e61ffcef2e86c91774972bc
> src/test/java/org/apache/aurora/scheduler/async/TaskReconcilerTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 2beea4fc1a24c0050898077ecdf6cac2b19fab31
>
> Diff: https://reviews.apache.org/r/34440/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 34440: Implementing task reconciler.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34440/
-----------------------------------------------------------
(Updated May 20, 2015, 1:34 a.m.)
Review request for Aurora, Ben Mahler, Kevin Sweeney, and Zameer Manji.
Changes
-------
Zameer's and Ben's comments.
Bugs: AURORA-1047
https://issues.apache.org/jira/browse/AURORA-1047
Repository: aurora
Description
-------
Adding a new service to request explict/implicit task reconciliations on a periodic basis. The reconciler is OFF by default until the GC executor code is removed.
Diffs (updated)
-----
src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 316ab1c06ce63c9a3f7232264d30a40f487fc45c
src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java e9d47fda3355786a4e68eac5c28490c04bc68cb3
src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java PRE-CREATION
src/main/java/org/apache/aurora/scheduler/mesos/Driver.java 975ea02b45488608286e743888de25862cc77add
src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java 35cada6160af01c13362fa7c14b9ff8da0300004
src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 02e87989a17d95d36e61ffcef2e86c91774972bc
src/test/java/org/apache/aurora/scheduler/async/TaskReconcilerTest.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 2beea4fc1a24c0050898077ecdf6cac2b19fab31
Diff: https://reviews.apache.org/r/34440/diff/
Testing
-------
./gradlew -Pq build
Thanks,
Maxim Khutornenko
Re: Review Request 34440: Implementing task reconciler.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On May 19, 2015, 11:48 p.m., Zameer Manji wrote:
> > Does it make sense for the reconciler to run in parallel with the GC executor mechanism? It seems fine to me, but I would like some re-assurance here.
GC executor is not adding anything when task reconciliation is ON. Is there a particular use case you have in mind?
> On May 19, 2015, 11:48 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java, line 186
> > <https://reviews.apache.org/r/34440/diff/1/?file=964437#file964437line186>
> >
> > This seems a little hacky, is there a reason why we cannot have an --enable_reconciliation flag? If disabled we can bind TaskReconciler to an implementation that does nothing.
I am hesitant adding yet another flag to accomplish what can already be done. This feature is not expected to be optional once GC executor is gone, so I don't see much motivation for extra complexity. Happy to reconsider though if others feel the same.
> On May 19, 2015, 11:48 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java, line 43
> > <https://reviews.apache.org/r/34440/diff/1/?file=964438#file964438line43>
> >
> > Please rename TIME to something that is more representitive of what it holds.
This holds what it tells (Time enum value). Any suggestions?
> On May 19, 2015, 11:48 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java, line 96
> > <https://reviews.apache.org/r/34440/diff/1/?file=964438#file964438line96>
> >
> > Linking to http://mesos.apache.org/documentation/latest/reconciliation/ or briefly explaining what is expected here would be nice.
Added a link to the class header.
> On May 19, 2015, 11:48 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java, line 106
> > <https://reviews.apache.org/r/34440/diff/1/?file=964438#file964438line106>
> >
> > Unrelated to this diff, but can you please explain how the StatusUpdates triggered from `reconcileTasks` are handled?
Status updates are handled via the existing MesosSchedulerImpl.statusUpdate() handling, which effectively routes the call to StateManager to attempt a task transition (or do nothing if states match).
> On May 19, 2015, 11:48 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java, line 129
> > <https://reviews.apache.org/r/34440/diff/1/?file=964438#file964438line129>
> >
> > perhaps we should call `executor.shutdownNow` here?
We never explicitly shutdown the executor but rather let it die along with the VM. Besides, it's a shared executor, which isn't created here.
> On May 19, 2015, 11:48 p.m., Zameer Manji wrote:
> > src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java, line 67
> > <https://reviews.apache.org/r/34440/diff/1/?file=964441#file964441line67>
> >
> > How is this related to this diff?
This is related to a bug I found in FakeScheduledExecutor. It was not honoring the initial delay the way normal scheduled executor does.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34440/#review84421
-----------------------------------------------------------
On May 19, 2015, 10:50 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34440/
> -----------------------------------------------------------
>
> (Updated May 19, 2015, 10:50 p.m.)
>
>
> Review request for Aurora, Ben Mahler, Kevin Sweeney, and Zameer Manji.
>
>
> Bugs: AURORA-1047
> https://issues.apache.org/jira/browse/AURORA-1047
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adding a new service to request explict/implicit task reconciliations on a periodic basis. The reconciler is OFF by default until the GC executor code is removed.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 316ab1c06ce63c9a3f7232264d30a40f487fc45c
> src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java e9d47fda3355786a4e68eac5c28490c04bc68cb3
> src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java 975ea02b45488608286e743888de25862cc77add
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java 35cada6160af01c13362fa7c14b9ff8da0300004
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 02e87989a17d95d36e61ffcef2e86c91774972bc
> src/test/java/org/apache/aurora/scheduler/async/TaskReconcilerTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 2beea4fc1a24c0050898077ecdf6cac2b19fab31
>
> Diff: https://reviews.apache.org/r/34440/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 34440: Implementing task reconciler.
Posted by Zameer Manji <zm...@apache.org>.
> On May 19, 2015, 4:48 p.m., Zameer Manji wrote:
> > Does it make sense for the reconciler to run in parallel with the GC executor mechanism? It seems fine to me, but I would like some re-assurance here.
>
> Maxim Khutornenko wrote:
> GC executor is not adding anything when task reconciliation is ON. Is there a particular use case you have in mind?
No, I just wanted confirmation that the GC executor causes no harm when reconciliation is ON.
> On May 19, 2015, 4:48 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java, line 186
> > <https://reviews.apache.org/r/34440/diff/1/?file=964437#file964437line186>
> >
> > This seems a little hacky, is there a reason why we cannot have an --enable_reconciliation flag? If disabled we can bind TaskReconciler to an implementation that does nothing.
>
> Maxim Khutornenko wrote:
> I am hesitant adding yet another flag to accomplish what can already be done. This feature is not expected to be optional once GC executor is gone, so I don't see much motivation for extra complexity. Happy to reconsider though if others feel the same.
Depending on the timeline for the GC executor removal, I see the benefit of a simple binary flag. If 0.9.0 will offer both mechanisms then I think a binary flag is required. If 0.9.0 will only offer reconciliation then I think this is fine.
> On May 19, 2015, 4:48 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java, line 43
> > <https://reviews.apache.org/r/34440/diff/1/?file=964438#file964438line43>
> >
> > Please rename TIME to something that is more representitive of what it holds.
>
> Maxim Khutornenko wrote:
> This holds what it tells (Time enum value). Any suggestions?
TIME_UNIT?
- Zameer
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34440/#review84421
-----------------------------------------------------------
On May 19, 2015, 6:34 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34440/
> -----------------------------------------------------------
>
> (Updated May 19, 2015, 6:34 p.m.)
>
>
> Review request for Aurora, Ben Mahler, Kevin Sweeney, and Zameer Manji.
>
>
> Bugs: AURORA-1047
> https://issues.apache.org/jira/browse/AURORA-1047
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adding a new service to request explict/implicit task reconciliations on a periodic basis. The reconciler is OFF by default until the GC executor code is removed.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 316ab1c06ce63c9a3f7232264d30a40f487fc45c
> src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java e9d47fda3355786a4e68eac5c28490c04bc68cb3
> src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java 975ea02b45488608286e743888de25862cc77add
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java 35cada6160af01c13362fa7c14b9ff8da0300004
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 02e87989a17d95d36e61ffcef2e86c91774972bc
> src/test/java/org/apache/aurora/scheduler/async/TaskReconcilerTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 2beea4fc1a24c0050898077ecdf6cac2b19fab31
>
> Diff: https://reviews.apache.org/r/34440/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 34440: Implementing task reconciler.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On May 19, 2015, 11:48 p.m., Zameer Manji wrote:
> > Does it make sense for the reconciler to run in parallel with the GC executor mechanism? It seems fine to me, but I would like some re-assurance here.
>
> Maxim Khutornenko wrote:
> GC executor is not adding anything when task reconciliation is ON. Is there a particular use case you have in mind?
>
> Zameer Manji wrote:
> No, I just wanted confirmation that the GC executor causes no harm when reconciliation is ON.
No harm if GC executor is ON except for more load on the scheduler.
> On May 19, 2015, 11:48 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java, line 43
> > <https://reviews.apache.org/r/34440/diff/1/?file=964438#file964438line43>
> >
> > Please rename TIME to something that is more representitive of what it holds.
>
> Maxim Khutornenko wrote:
> This holds what it tells (Time enum value). Any suggestions?
>
> Zameer Manji wrote:
> TIME_UNIT?
Changed to static import.
> On May 19, 2015, 11:48 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java, line 186
> > <https://reviews.apache.org/r/34440/diff/1/?file=964437#file964437line186>
> >
> > This seems a little hacky, is there a reason why we cannot have an --enable_reconciliation flag? If disabled we can bind TaskReconciler to an implementation that does nothing.
>
> Maxim Khutornenko wrote:
> I am hesitant adding yet another flag to accomplish what can already be done. This feature is not expected to be optional once GC executor is gone, so I don't see much motivation for extra complexity. Happy to reconsider though if others feel the same.
>
> Zameer Manji wrote:
> Depending on the timeline for the GC executor removal, I see the benefit of a simple binary flag. If 0.9.0 will offer both mechanisms then I think a binary flag is required. If 0.9.0 will only offer reconciliation then I think this is fine.
I fully expect 0.9.0 to not have GC executor in place.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34440/#review84421
-----------------------------------------------------------
On May 20, 2015, 1:34 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34440/
> -----------------------------------------------------------
>
> (Updated May 20, 2015, 1:34 a.m.)
>
>
> Review request for Aurora, Ben Mahler, Kevin Sweeney, and Zameer Manji.
>
>
> Bugs: AURORA-1047
> https://issues.apache.org/jira/browse/AURORA-1047
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adding a new service to request explict/implicit task reconciliations on a periodic basis. The reconciler is OFF by default until the GC executor code is removed.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 316ab1c06ce63c9a3f7232264d30a40f487fc45c
> src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java e9d47fda3355786a4e68eac5c28490c04bc68cb3
> src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java 975ea02b45488608286e743888de25862cc77add
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java 35cada6160af01c13362fa7c14b9ff8da0300004
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 02e87989a17d95d36e61ffcef2e86c91774972bc
> src/test/java/org/apache/aurora/scheduler/async/TaskReconcilerTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 2beea4fc1a24c0050898077ecdf6cac2b19fab31
>
> Diff: https://reviews.apache.org/r/34440/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 34440: Implementing task reconciler.
Posted by Zameer Manji <zm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34440/#review84421
-----------------------------------------------------------
Does it make sense for the reconciler to run in parallel with the GC executor mechanism? It seems fine to me, but I would like some re-assurance here.
src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java
<https://reviews.apache.org/r/34440/#comment135664>
This seems a little hacky, is there a reason why we cannot have an --enable_reconciliation flag? If disabled we can bind TaskReconciler to an implementation that does nothing.
src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java
<https://reviews.apache.org/r/34440/#comment135665>
Please rename TIME to something that is more representitive of what it holds.
src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java
<https://reviews.apache.org/r/34440/#comment135667>
Linking to http://mesos.apache.org/documentation/latest/reconciliation/ or briefly explaining what is expected here would be nice.
src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java
<https://reviews.apache.org/r/34440/#comment135671>
Unrelated to this diff, but can you please explain how the StatusUpdates triggered from `reconcileTasks` are handled?
src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java
<https://reviews.apache.org/r/34440/#comment135666>
perhaps we should call `executor.shutdownNow` here?
src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java
<https://reviews.apache.org/r/34440/#comment135670>
How is this related to this diff?
- Zameer Manji
On May 19, 2015, 3:50 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34440/
> -----------------------------------------------------------
>
> (Updated May 19, 2015, 3:50 p.m.)
>
>
> Review request for Aurora, Ben Mahler, Kevin Sweeney, and Zameer Manji.
>
>
> Bugs: AURORA-1047
> https://issues.apache.org/jira/browse/AURORA-1047
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Adding a new service to request explict/implicit task reconciliations on a periodic basis. The reconciler is OFF by default until the GC executor code is removed.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 316ab1c06ce63c9a3f7232264d30a40f487fc45c
> src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java e9d47fda3355786a4e68eac5c28490c04bc68cb3
> src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java 975ea02b45488608286e743888de25862cc77add
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java 35cada6160af01c13362fa7c14b9ff8da0300004
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 02e87989a17d95d36e61ffcef2e86c91774972bc
> src/test/java/org/apache/aurora/scheduler/async/TaskReconcilerTest.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/testing/FakeScheduledExecutor.java 2beea4fc1a24c0050898077ecdf6cac2b19fab31
>
> Diff: https://reviews.apache.org/r/34440/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>