> On May 14, 2018, 10:58 a.m., Santhosh Kumar Shanmugham wrote: > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java > > Lines 826 (patched) > > <https://reviews.apache.org/r/66192/diff/2/?file=2018260#file2018260line826> > > > > Sums up to be exactly `mutableRequest.instanceCount`? > > Renan DelValle wrote: > This is something I wonderd myself. The current design follows the > philosophy that the sum of all update groups does not have to be equal to the > instance count. > > If the instance count is greater than the instanceCount, the update will > carry forward until we run out of instance to update. > > For example, if we have update groups 1,2,3 and our instance count is 5, > we will get the following steps in practice: 1, 2, 2 > > If the instance count is lesser than the instance count, the update will > forward repeating the value of the last group size until the update completes. > > For example, if we have update groups 1,2 and our instance count is 5, we > will get the following steps: 1, 2, 2 > > I will write thourough documentation on this so that users know what to > expect when this update strategy is used. > > > One benefit of implementing the variable group size update this way is > that it provides a path going forward to have a single batch strategy in the > code base. > > Since we repeat the last group size we have, having a list of group sizes > of length 1 is equivalent to a batch update. (This was done based on a > comment by Stephan during the first review round that resonated with me.)
That makes sense. I missed the earlier conversation. > On May 14, 2018, 10:58 a.m., Santhosh Kumar Shanmugham wrote: > > src/main/java/org/apache/aurora/scheduler/updater/strategy/VariableBatchStrategy.java > > Lines 65 (patched) > > <https://reviews.apache.org/r/66192/diff/2/?file=2018263#file2018263line65> > > > > Can you include an example for the rolling forward and backward cases? > > Renan DelValle wrote: > Can you expand on what you mean by including an example? Should I put it > in as a comment on the code? Yes. Please include that in the javadoc. - Santhosh Kumar ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66192/#review203048 ----------------------------------------------------------- On May 14, 2018, 7:19 p.m., Renan DelValle wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66192/ > ----------------------------------------------------------- > > (Updated May 14, 2018, 7:19 p.m.) > > > Review request for Aurora, David McLaughlin, Jordan Ly, Santhosh Kumar > Shanmugham, and Stephan Erb. > > > Repository: aurora > > > Description > ------- > > Adding support for variable group sizes when executing an update. > > Design doc for this change is here: > https://docs.google.com/document/d/1xGk4ueH8YlmJCk6hQJh85u4to4M1VQD0l630IOchvgY/edit#heading=h.lg3hty82f5cz > > I opted for the path of least resistance with regards to the Thrift changes > as I didn't see any benefit in making the larger changes required to make the > interfaces a bit more flexible. > > Requesting feedback on these changes and the approach from the community > before I proceed. > > Tests will be added after the community approves of the direciton and > approach. > > Note to reviewers: Changes made in ActiveLimitedStrategy.java were made to > move towards getting rid of FluentIterable. I figured since I was touching > that code, it wouldn't hurt to test the Java 8 equivalent of it. I can get > rid of the change here and make it in a separate patch if desired. > > > Diffs > ----- > > api/src/main/thrift/org/apache/aurora/gen/api.thrift > ef754e32172e7490a47a13e7b526f243ffa3efeb > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java > 9fc0416086dd3eb2e2f4e8f659da59fcdea2b22b > src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java > 3992aa77fc305adc390a4aaeb1d3939d6241ddbd > > src/main/java/org/apache/aurora/scheduler/updater/strategy/ActiveLimitedStrategy.java > 855ea9c20788b51695b7eff5ac0970f0d52a9546 > > src/main/java/org/apache/aurora/scheduler/updater/strategy/VariableBatchStrategy.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/66192/diff/3/ > > > Testing > ------- > > > Thanks, > > Renan DelValle > >
