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