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




api/src/main/thrift/org/apache/aurora/gen/api.thrift (lines 944 - 950)
<https://reviews.apache.org/r/51662/#comment215425>

    I don't think there is much value in having this struct and the related 
`reconcileStatus` RPC. Cluster operators are normally relying on scheduler 
stats to monitor activities like this.



api/src/main/thrift/org/apache/aurora/gen/api.thrift (line 1223)
<https://reviews.apache.org/r/51662/#comment215428>

    How about `triggerExplicitTaskReconciliation` and 
`triggerImplicitTaskReconciliation`? This would be self-explanatory to any API 
consumer.
    
    nit: please end all comments with '.'



src/main/java/org/apache/aurora/scheduler/reconciliation/TaskReconciler.java 
(line 135)
<https://reviews.apache.org/r/51662/#comment215429>

    Suggest defaulting to the configured `settings.explicitBatchSize` in case 
the provided one is 0. This will allow the correspondent admin command to make 
batch size completely optional.



src/main/java/org/apache/aurora/scheduler/reconciliation/TaskReconciler.java 
(lines 135 - 143)
<https://reviews.apache.org/r/51662/#comment215435>

    This only prevents overlapping explicit or implicit runs but does not 
prevent explicit and implicit overlapping runs. Also, this does not cancel any 
calls already waiting for a lock.
    
    Given that we deal with admin-triggered events (who are expected to know 
what they do), I suggest we simplify the logic here by simply scheduling an out 
of band run and not try to synchronize anything around reconciliation runs. 
This would let you cut the bulk of the changes below.



src/main/python/apache/aurora/admin/admin.py (line 355)
<https://reviews.apache.org/r/51662/#comment215437>

    The default here would be 0 if you accept my suggestion above.


- Maxim Khutornenko


On Sept. 6, 2016, 11:34 p.m., Karthik Anantha Padmanabhan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51662/
> -----------------------------------------------------------
> 
> (Updated Sept. 6, 2016, 11:34 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/admin/admin_util.py 
> 394deb57af9ad8832a02ceab15f33b3c1e5c902b 
>   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
> 
>

Reply via email to