Re: Review Request 40922: Thermos: Add ability to specify process outputs destination

2015-12-25 Thread Stephan Erb 

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



docs/configuration-reference.md (line 158)


This section should also explicitly list the `file` option.



docs/deploying-aurora-scheduler.md (line 177)


This section should link more explicitly to the configuration-reference 
section describing all options.



src/main/python/apache/aurora/executor/bin/thermos_executor_main.py (line 107)


I'd prefer if the default value is specified here . Makes it easier to see 
which values are used when looking at the scheduler command line of an existing 
binary, as it prints the default values when running with `--help`.



src/main/python/apache/thermos/config/schema_base.py (line 72)


`destination` and `mode` could both be tuned independently. I think it make 
sense to specify default values here, so that the user does not has to repeat 
the option he wants to keep at default.



src/main/python/apache/thermos/core/process.py (line 61)


I'd find `console` more obvious than `stream`. What do you think?



src/main/python/apache/thermos/core/process.py (line 62)


`mixed` does somewhat imply that some bits go there, some other there. 
`both` would be more precise. 

However, I can also envision that we might have something like `syslog` in 
the future. So, maybe specifying those as a list of applicable options would be 
easier to extend? The `none` case would then be an empty list.

What do you think?



src/main/python/apache/thermos/core/runner.py (line 731)


Given that you change the default argument, this would no longer be 
necessary.


- Stephan Erb


On Dec. 23, 2015, 6:39 p.m., Martin Hrabovcin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40922/
> ---
> 
> (Updated Dec. 23, 2015, 6:39 p.m.)
> 
> 
> Review request for Aurora and John Sirois.
> 
> 
> Bugs: AURORA-1548
> https://issues.apache.org/jira/browse/AURORA-1548
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch will provide way to **optionally** specify running process outputs 
> destination. Implementation was built on top of 
> https://reviews.apache.org/r/30695/
> 
> **What was changed:**
> 
> New `destination` parameter is available on global cluster level and also on 
> each `Process` level. Possible options are `file` (default), `stream` to 
> parent process stdout/stderr, `mixed` will split output to files and stream 
> and finally `none` to discard any logs produced by running process.
> 
> 
> Diffs
> -
> 
>   NEWS ebfc3a6 
>   docs/configuration-reference.md cf63cfa 
>   docs/deploying-aurora-scheduler.md 73b2e04 
>   src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 7b7ef4b 
>   src/main/python/apache/aurora/executor/thermos_task_runner.py 25fcca2 
>   src/main/python/apache/thermos/config/schema_base.py 5552108 
>   src/main/python/apache/thermos/core/process.py 8efdfdc 
>   src/main/python/apache/thermos/core/runner.py 11c06a8 
>   src/main/python/apache/thermos/runner/thermos_runner.py a36bd2a 
>   src/test/python/apache/thermos/core/test_process.py 261371d 
> 
> Diff: https://reviews.apache.org/r/40922/diff/
> 
> 
> Testing
> ---
> 
> Unit test coverage is provided for new functionality.
> 
> I did also manual testing with mesos/docker and I made sure that logs are 
> being written to expected files and also same output gets to docker daemon.
> 
> 
> Thanks,
> 
> Martin Hrabovcin
> 
>

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

2015-12-25 Thread Tony Dong 

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

Review request for Aurora.


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

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

2015-12-25 Thread Tony Dong 

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


I'm putting up this WIP review to get some general feedback.
I have a feeling I went way past what was originally intended for 
https://issues.apache.org/jira/browse/AURORA-1472
So I want to get a second opinion on my general approach before I go and polish 
this up.

- Tony Dong


On Dec. 25, 2015, 10:14 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:14 a.m.)
> 
> 
> Review request for Aurora.
> 
> 
> 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

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

2015-12-25 Thread Aurora ReviewBot 

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


Master (1ae77d5) is red with this patch.
  ./build-support/jenkins/build.sh

:api:processResources UP-TO-DATE
:api:classes
:api:jar
:commons:generateThriftJava
:commons:classesThriftNote: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.

:commons-args:compileJavaNote: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/commons-args/src/main/java/org/apache/aurora/common/args/apt/Configuration.java
 uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.

:commons-args:processResources
:commons-args:classes
:commons-args:jar
:commons:compileJavaNote: Writing 
file:/home/jenkins/jenkins-slave/workspace/AuroraBot/commons/dist/classes/main/org/apache/aurora/common/args/apt/cmdline.arg.info.txt.1
Note: Writing 
file:/home/jenkins/jenkins-slave/workspace/AuroraBot/commons/dist/classes/main/META-INF/compiler/resource-mappings/org.apache.aurora.common.args.apt.CmdLineProcessor
/home/jenkins/jenkins-slave/workspace/AuroraBot/commons/src/main/java/org/apache/aurora/common/util/backoff/BackoffHelper.java:58:
 error: constructor TruncatedBinaryBackoff in class TruncatedBinaryBackoff 
cannot be applied to given types;
this(new TruncatedBinaryBackoff(initialBackoff, maxBackoff));
 ^
  required: BackoffOptions
  found: Amount,Amount
  reason: actual and formal argument lists differ in length
Note: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/commons/src/main/java/org/apache/aurora/common/logging/Glog.java
 uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
1 error
 FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':commons:compileJava'.
> Compilation failed; see the compiler error output for details.

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.

BUILD FAILED

Total time: 1 mins 20.045 secs


I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


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