aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Santhosh Kumar Shanmugham <santhoshkuma...@gmail.com>
Subject Re: Review Request 51662: AURORA-1602 Aurora admin commands for reconcilation
Date Wed, 07 Sep 2016 23:21:00 GMT


> On Sept. 7, 2016, 2:45 p.m., Santhosh Kumar Shanmugham wrote:
> > api/src/main/thrift/org/apache/aurora/gen/api.thrift, lines 1209-1214
> > <https://reviews.apache.org/r/51662/diff/4/?file=1493667#file1493667line1209>
> >
> >     Did we consider combining the "explicit" and "implicit" reconciliation RPCs
into a single "reconcile" RPC with an optional batchSize argument, so that it maps to the
mesos master API.
> >     
> >     http://mesos.apache.org/documentation/latest/reconciliation/
> 
> Karthik Anantha Padmanabhan wrote:
>     We have more than one option on the scheduler for the explicit reconcile. If at all
we want to expose those to the admin CLI in the future it may be better to have them as two
separate API's.
> 
> Santhosh Kumar Shanmugham wrote:
>     "We have more than one option on the scheduler for the explicit reconcile." - can
you given an example of what you mean by this.
> 
> Karthik Anantha Padmanabhan wrote:
>     There is another releavant parameter called `explicitBatchDelaySeconds` which dictates
how long we wait in between batches before we call reconcile on mesos. 
>     
>     But it is possible that we may add another option in the future ? In that case wouldn't
it be better to have two separate API's ?
> 
> Karthik Anantha Padmanabhan wrote:
>     Hmm actually if we are passing in a struct of `ExplicitReconcileSettings` , then
adding options will be easier. So I can check `isNull` on the settings object on the scheduler
to identify an implicit reconcile. 
>     
>     I can go either way. I just don't know if there is a strong reason to make it a single
API as opposed to two.
> 
> Maxim Khutornenko wrote:
>     Please, don't save a few lines by merging the RPCs or forking off of the presence
of the `batchSize`. Explicit and implicit reconciliations have very different scopes and cover
different failure scenarios. I'd advocate in favor of Karthik's original proposal with separate
call paths (RPCs and client commands) that clearly state their purpose in the admin API.

Since the Scheduler's reconcile is basically a reflection of the master's reconcile API, keeping
them alike will make it more easier to reason about.

+1 for introducing a struct as the parameter incase the operator wishes to override the settings.


- Santhosh Kumar


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51662/#review148089
-----------------------------------------------------------


On Sept. 7, 2016, 2:08 p.m., Karthik Anantha Padmanabhan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51662/
> -----------------------------------------------------------
> 
> (Updated Sept. 7, 2016, 2:08 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> AURORA-1602 Aurora admin commands for reconcilation
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift c5765b70501c101f0535b4eed94e9948c36808f9

>   src/main/java/org/apache/aurora/scheduler/reconciliation/TaskReconciler.java 3275d72a0a74909e635bce615e2036ec72aa38ee

>   src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 929d021a336c6a3438613c9340c84a1096dc9069

>   src/main/python/apache/aurora/admin/admin.py 76009b9c1c7a130c25abad544a176dc590dafb12

>   src/main/python/apache/aurora/client/api/__init__.py 9149c3018ae58d405f284fcbd4076d251ccc8192

>   src/test/java/org/apache/aurora/scheduler/reconciliation/TaskReconcilerTest.java b9317dc20456f90057ec2bf4d10619a5ae986187

>   src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
779dc302602ae8842084807ca868a491ea99b676 
>   src/test/python/apache/aurora/admin/test_admin.py eb193c4a33f5275f05a995338446bec0e19bc1cf

>   src/test/python/apache/aurora/client/api/test_scheduler_client.py afac2500551af2fce406bb906aa4e33f353e90a1

> 
> Diff: https://reviews.apache.org/r/51662/diff/
> 
> 
> Testing
> -------
> 
> * Manually tested on my local vagrant installation
> * ./build-support/jenkins/build.sh
> 
> 
> Thanks,
> 
> Karthik Anantha Padmanabhan
> 
>


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