> On March 20, 2018, 9:29 p.m., David McLaughlin wrote: > > src/main/java/org/apache/aurora/scheduler/updater/strategy/VariableBatchStrategy.java > > Lines 107-111 (patched) > > <https://reviews.apache.org/r/66192/diff/1/?file=1984357#file1984357line107> > > > > Is this idea of a step sate needed? Strategies writing to the database > > seems like a design smell. > > > > If I have 10 instances: > > > > [0,1,2,3,4,5,6,7,8,9] > > > > And I pass a VariableBatchStrategy of: [1, 4, 5] > > > > Then using the BatchStrategy it is *always* in the following order: > > > > [[0], [1,2,3,4], [5,6,7,8,9]] > > > > And I don't *think* I need to write the step to the database to figure > > out which step I'm in? I can use the number idle and active to figure out > > exactly how many have been processed. > > Jordan Ly wrote: > +1, removing the need for persistent state would greatly reduce the > surface area of this patch. > > Santhosh Kumar Shanmugham wrote: > +1
Thanks for the feedback! I thought about doing it this way originally, but I wasn't totally convinced it would work well given the only info I had to go on inside of the strategy was how many tasks were waiting to be updated and how many were in the progress of being updated. As David pointed out, the order will remain the same (unless of course changes are made to https://github.com/apache/aurora/blob/2e1ca42887bc8ea1e8c6cddebe9d1cf29268c714/src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java#L190 ) so the update order should be deterministic (at least within the same version of Aurora). The key points for this feature are: how this would survive leader elections/scheduler restarts, being able to roll back in a predictable manner at any given point of failure, and manainting the order in which tasks(shards) would get updated. I think I should be able to tackle all of these without writing to the database to persist with this new approach. - Renan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66192/#review199635 ----------------------------------------------------------- On March 20, 2018, 7:10 p.m., Renan DelValle wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66192/ > ----------------------------------------------------------- > > (Updated March 20, 2018, 7:10 p.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 > >
