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 22:26:07 GMT


> On Sept. 7, 2016, 2:45 p.m., Santhosh Kumar Shanmugham wrote:
> > src/main/python/apache/aurora/admin/admin.py, line 342
> > <https://reviews.apache.org/r/51662/diff/4/?file=1493670#file1493670line342>
> >
> >     'default=0' here and the usage doc reads default as 1000.
> 
> Karthik Anantha Padmanabhan wrote:
>     We want the default value to match the one from settings.explicitBatchSize on the
scheduler. So on the scheduler we use the default value if the value here is 0. Although I
agree that the 0 here is a bit misleading. 
>     
>     I like the optional batchSize argument for the explicit reconcile RPC. 
>     
>     I am thinking there can be a 
>     
>     struct ExplicitReconcileSettings {
>      1. optional i32 batchSize;
>     }
>     
>     This can let us add more explicit reconcile settings in the future. 
>     
>     Also does thrift lets us declare optional parameters in a function (unless it is
wrapped in a struct) ?

AFAIK, Thrift does not support optional function parameters, due to the limitations of the
languages that it gets translated to. You are on the correct path, in making this a 'struct'.


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

"We have more than one option on the scheduler for the explicit reconcile." - can you given
an example of what you mean by this.


- 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