aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Zameer Manji" <zma...@apache.org>
Subject Re: Review Request 34440: Implementing task reconciler.
Date Wed, 20 May 2015 01:59:15 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message