----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66192/#review199707 -----------------------------------------------------------
I am mostly concerned about the UX. Users will be able to specify both batch size and variable batch size and must know that variable batch sizing takes precedent over other strategies. Is it worth it to make a larger investment into the Thrift interface and avoid ambiguity? Or refactor the current batching strategy to use the new variable codepath (a single batch size specified to the variable strategy should behave the same as the current implementation). - Jordan Ly On March 21, 2018, 2:10 a.m., Renan DelValle wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66192/ > ----------------------------------------------------------- > > (Updated March 21, 2018, 2:10 a.m.) > > > Review request for Aurora, David McLaughlin, 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 > api/src/main/thrift/org/apache/aurora/gen/storage.thrift > b79e2045ccda05d5058565f81988dfe33feea8f1 > src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java > b25f3831cecc58c90375c90b16142421f8f09e38 > src/main/java/org/apache/aurora/scheduler/storage/durability/Loader.java > 10864f122eff5027c88d835baae6de483d960218 > > src/main/java/org/apache/aurora/scheduler/storage/durability/WriteRecorder.java > 8d70cae35289a9e36142bab288cf0c9398ebd2d4 > > src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java > 9e86b9e276ea90a249284a824705b5bbf19dcbce > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java > 9fc0416086dd3eb2e2f4e8f659da59fcdea2b22b > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java > f8be8058f3a80a18b999d2666e2adb33e1e55fef > 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/1/ > > > Testing > ------- > > > Thanks, > > Renan DelValle > >
