Re: Review Request 41717: Very Very WIP: Add jittering as an option to BackoffStrategy.

[Tony Dong <to...@gmail.com>]

> On Dec. 27, 2015, 10:11 p.m., Stephan Erb wrote:
> > Thanks for your effort.
> > 
> > I believe `ExpBackoffEqualJitter.java` is the most usefull strategy. It is the closest to the current design while still resolving the problem depicted in the AWS article http://d3j06wsuecq21p.cloudfront.net/assets/backoff/backoff_expo_ts.png. Providing different strategies and allowing the enduser/operator to select one might be overkill. There is no obvious tradeoff that we have to make configurable.
> 
> Bill Farner wrote:
>     I agree.  This is an interesting experiment, but i think it's best to choose an approximation for a universal solution and avoid adding knobs.

Sounds reasonable, how would you guys like to enable jitter? I feel like a config that flips it on/off is not the most flexible thing, but that isn't a big issue if we never add more strategies.


> On Dec. 27, 2015, 10:11 p.m., Stephan Erb wrote:
> > commons/src/main/java/org/apache/aurora/common/util/Random.java, line 62
> > <https://reviews.apache.org/r/41717/diff/1/?file=1176266#file1176266line62>
> >
> >     Having this implementation in Aurora doesn't feel right. It is rather low-level compared to all the other code. Is it really necessary?

It's copied and pasted from an implementation that's hidden in java.util.Random#internalNextLong.
I think it's useful to have a random that let's you get a bounded random long value, unfortunately java.util.random doesn't expose it.


- Tony


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


On Dec. 25, 2015, 10:18 a.m., Tony Dong wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41717/
> -----------------------------------------------------------
> 
> (Updated Dec. 25, 2015, 10:18 a.m.)
> 
> 
> Review request for Aurora, Stephan Erb and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The general idea is that we should be able to specify different
> types of BackoffStrategies, with varying configurations and all
> the legwork will be handled via injection. This change not only
> introduces different BackoffStrategies which can be selected via
> command line arguments, but also introduces a BackoffOptions
> class which makes configuring these different strategies more
> standardized.
> 
> - TODO: Update everywhere that uses BackoffStrategy to use Injection.
> - TODO: Add tests for all the different backoff strategies.
> - TODO: Add tests for nextLong().
> - TODO: Make BackoffHelper injectable and configurable with the BackoffStrategy Types.
> - TODO: Update Docs everywhere.
> 
> 
> Diffs
> -----
> 
>   commons/src/main/java/org/apache/aurora/common/args/parsers/EnumParser.java 9f6a3ff977ef6a0e2d472a8e2cf90e7f9a63a985 
>   commons/src/main/java/org/apache/aurora/common/util/BackoffHelper.java b251e9b84aa33c7c1e1366aaf8872a367864e408 
>   commons/src/main/java/org/apache/aurora/common/util/BackoffStrategy.java d954762496a2cbc0a098964bc2686fdaf6213e06 
>   commons/src/main/java/org/apache/aurora/common/util/Random.java 90d111e357c7aded81c3c79c4b814adf55424802 
>   commons/src/main/java/org/apache/aurora/common/util/TruncatedBinaryBackoff.java fd74b9f37c6cc24c7ea1cb239ba6354661d931e2 
>   commons/src/main/java/org/apache/aurora/common/util/backoff/BackoffStrategyType.java PRE-CREATION 
>   commons/src/main/java/org/apache/aurora/common/util/backoff/ExpBackoffDecorrelatedJitter.java PRE-CREATION 
>   commons/src/main/java/org/apache/aurora/common/util/backoff/ExpBackoffEqualJitter.java PRE-CREATION 
>   commons/src/main/java/org/apache/aurora/common/util/backoff/ExpBackoffFullJitter.java PRE-CREATION 
>   commons/src/main/java/org/apache/aurora/common/util/backoff/TruncatedBinaryBackoff.java PRE-CREATION 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Group.java d3681700e7f993da711f3b1ac87dc0335075cf71 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSetImpl.java 6a0a314ffc27e73024ac8ac7c9e3a7ba83567c3a 
>   commons/src/test/java/org/apache/aurora/common/util/BackoffHelperTest.java 78ba8fe7e0640b7c4b04b78384e15a33e1b81b2f 
>   commons/src/test/java/org/apache/aurora/common/util/SystemRandomTest.java PRE-CREATION 
>   commons/src/test/java/org/apache/aurora/common/util/TruncatedBinaryBackoffTest.java 127a60331f8560d98733bd16654087f840abef72 
>   commons/src/test/java/org/apache/aurora/common/util/backoff/BackoffOptionsTest.java PRE-CREATION 
>   commons/src/test/java/org/apache/aurora/common/util/backoff/ExpBackoffDecorrelatedJitterTest.java PRE-CREATION 
>   commons/src/test/java/org/apache/aurora/common/util/backoff/ExpBackoffEqualJitterTest.java PRE-CREATION 
>   commons/src/test/java/org/apache/aurora/common/util/backoff/ExpBackoffFullJitterTest.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java f355bc101252fb433c7437a791e6e92f94462fa6 
>   src/main/java/org/apache/aurora/scheduler/cron/quartz/CronModule.java 264537180da91f59173301bf20b549ea01c0d5cb 
>   src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java fbc589e9a7592cce6d92c4e987cde2e056406c3a 
>   src/main/java/org/apache/aurora/scheduler/reconciliation/KillRetry.java 119ef71ab2993f1ac74bab25bec3ec5cd4a67896 
>   src/main/java/org/apache/aurora/scheduler/reconciliation/ReconciliationModule.java 7dae70c4c9cb2efbf66e1d269f96676ff450eeaf 
>   src/main/java/org/apache/aurora/scheduler/scheduling/RescheduleCalculator.java 291bf5f0baefef6dd10d19ec89e173ce495e6380 
>   src/main/java/org/apache/aurora/scheduler/scheduling/SchedulingModule.java 577edcbf362493d577e2f12c876f1dbb9387ad79 
>   src/main/java/org/apache/aurora/scheduler/scheduling/TaskGroups.java c044ebe6f72183a67462bbd8e5be983eb592c3e9 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java 5dd4aba92b2627b646087fce8118d5ebfeb75f49 
>   src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java 19c8a1fe06a333324022da11f74d7c96b2942587 
>   src/test/java/org/apache/aurora/scheduler/reconciliation/KillRetryTest.java a561d0909cef27b24334165f0d40cfd734b2c9a6 
>   src/test/java/org/apache/aurora/scheduler/scheduling/RescheduleCalculatorImplTest.java b380f21ac169b4991158f39dc70526e11fca54f0 
>   src/test/java/org/apache/aurora/scheduler/scheduling/TaskGroupsTest.java 95cf25eda0a5bfc0cc4c46d1439ebe9d5359ce79 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 2024b2c50d5d1e44f3f95b915c8bcd58e39379cb 
> 
> Diff: https://reviews.apache.org/r/41717/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Tony Dong
> 
>