You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Zameer Manji <zm...@apache.org> on 2015/06/03 02:58:47 UTC
Review Request 34967: Use TaskStatus Reason to set memory limit
message.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34967/
-----------------------------------------------------------
Review request for Aurora and Maxim Khutornenko.
Bugs: AURORA-1341
https://issues.apache.org/jira/browse/AURORA-1341
Repository: aurora
Description
-------
Use TaskStatus Reason to set memory limit message instead of checking the contents of the message field. Future versions of Mesos can change the diagnostic information in the message field causing Aurora to display no information when a task fails.
Diffs
-----
src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 5af691d0ac959cf4b5d01752daf996803e91ed16
src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java fe2fc954350b42487151fc820ebad22a41aeb039
Diff: https://reviews.apache.org/r/34967/diff/
Testing
-------
./gradlew test -Pq
Thanks,
Zameer Manji
Re: Review Request 34967: Use TaskStatus Reason to set memory limit
message.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34967/#review86337
-----------------------------------------------------------
Master (54d561e) is red with this patch.
./build-support/jenkins/build.sh
:processResources
:classes
:jar
:startScripts
:distTar
:distZip
:assemble
:compileJmhJavaNote: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/jmh/java/org/apache/aurora/benchmark/fakes/FakeSchedulerDriver.java uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
:processJmhResources UP-TO-DATE
:jmhClasses
:checkstyleJmh
:jsHint
:checkstyleMain
:compileTestJavaNote: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java uses unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
:processTestResources
:testClasses
:checkstyleTest[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java:203: 'block' child have incorrect indentation level 6, expected level should be 8.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java:203: 'latch' have incorrect indentation level 6, expected level should be 8.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java:203: 'method call' child have incorrect indentation level 6, expected level should be 8.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java:204: 'block' child have incorrect indentation level 6, expected level should be 8.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java:204: 'return' have incorrect indentation level 6, expected level should be 8.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java:205: 'block rcurly' have incorrect indentation level 4, expected level should be 6.
FAILED
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':checkstyleTest'.
> Checkstyle rule violations were found. See the report at: file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/checkstyle/test.xml
* 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 53.047 secs
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On June 3, 2015, 12:58 a.m., Zameer Manji wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34967/
> -----------------------------------------------------------
>
> (Updated June 3, 2015, 12:58 a.m.)
>
>
> Review request for Aurora and Maxim Khutornenko.
>
>
> Bugs: AURORA-1341
> https://issues.apache.org/jira/browse/AURORA-1341
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Use TaskStatus Reason to set memory limit message instead of checking the contents of the message field. Future versions of Mesos can change the diagnostic information in the message field causing Aurora to display no information when a task fails.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 5af691d0ac959cf4b5d01752daf996803e91ed16
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java fe2fc954350b42487151fc820ebad22a41aeb039
>
> Diff: https://reviews.apache.org/r/34967/diff/
>
>
> Testing
> -------
>
> ./gradlew test -Pq
>
>
> Thanks,
>
> Zameer Manji
>
>
Re: Review Request 34967: Use TaskStatus Reason to set memory limit
message.
Posted by Zameer Manji <zm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34967/
-----------------------------------------------------------
(Updated June 3, 2015, 11:13 a.m.)
Review request for Aurora and Maxim Khutornenko.
Changes
-------
Maxim's feedback.
Bugs: AURORA-1341
https://issues.apache.org/jira/browse/AURORA-1341
Repository: aurora
Description
-------
Use TaskStatus Reason to set memory limit message instead of checking the contents of the message field. Future versions of Mesos can change the diagnostic information in the message field causing Aurora to display no information when a task fails.
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 5af691d0ac959cf4b5d01752daf996803e91ed16
src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java fe2fc954350b42487151fc820ebad22a41aeb039
Diff: https://reviews.apache.org/r/34967/diff/
Testing
-------
./gradlew test -Pq
Thanks,
Zameer Manji
Re: Review Request 34967: Use TaskStatus Reason to set memory limit
message.
Posted by Zameer Manji <zm...@apache.org>.
> On June 3, 2015, 9:01 a.m., Maxim Khutornenko wrote:
> > I understand it may be too much for this scope but we should really address AURORA-1193 to improve user experience for all status update reasons, not just OOM. Giving it a ship it as a stop gap solution.
Agreed, but that is a larger scope which I do not want to tackle. Fixing this TODO will ensure our behaviour does not regress in future mesos versions however.
> On June 3, 2015, 9:01 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java, line 187
> > <https://reviews.apache.org/r/34967/diff/2/?file=977048#file977048line187>
> >
> > Remove MEMORY_LIMIT_EXCEEDED as it's no longer referenced.
Done.
> On June 3, 2015, 9:01 a.m., Maxim Khutornenko wrote:
> > src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java, lines 226-227
> > <https://reviews.apache.org/r/34967/diff/2/?file=977049#file977049line226>
> >
> > any particular reason for this change?
No, reverted.
- Zameer
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34967/#review86343
-----------------------------------------------------------
On June 2, 2015, 6:11 p.m., Zameer Manji wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34967/
> -----------------------------------------------------------
>
> (Updated June 2, 2015, 6:11 p.m.)
>
>
> Review request for Aurora and Maxim Khutornenko.
>
>
> Bugs: AURORA-1341
> https://issues.apache.org/jira/browse/AURORA-1341
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Use TaskStatus Reason to set memory limit message instead of checking the contents of the message field. Future versions of Mesos can change the diagnostic information in the message field causing Aurora to display no information when a task fails.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 5af691d0ac959cf4b5d01752daf996803e91ed16
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java fe2fc954350b42487151fc820ebad22a41aeb039
>
> Diff: https://reviews.apache.org/r/34967/diff/
>
>
> Testing
> -------
>
> ./gradlew test -Pq
>
>
> Thanks,
>
> Zameer Manji
>
>
Re: Review Request 34967: Use TaskStatus Reason to set memory limit
message.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34967/#review86343
-----------------------------------------------------------
Ship it!
I understand it may be too much for this scope but we should really address AURORA-1193 to improve user experience for all status update reasons, not just OOM. Giving it a ship it as a stop gap solution.
src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java
<https://reviews.apache.org/r/34967/#comment138300>
Remove MEMORY_LIMIT_EXCEEDED as it's no longer referenced.
src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java
<https://reviews.apache.org/r/34967/#comment138397>
any particular reason for this change?
- Maxim Khutornenko
On June 3, 2015, 1:11 a.m., Zameer Manji wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34967/
> -----------------------------------------------------------
>
> (Updated June 3, 2015, 1:11 a.m.)
>
>
> Review request for Aurora and Maxim Khutornenko.
>
>
> Bugs: AURORA-1341
> https://issues.apache.org/jira/browse/AURORA-1341
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Use TaskStatus Reason to set memory limit message instead of checking the contents of the message field. Future versions of Mesos can change the diagnostic information in the message field causing Aurora to display no information when a task fails.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 5af691d0ac959cf4b5d01752daf996803e91ed16
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java fe2fc954350b42487151fc820ebad22a41aeb039
>
> Diff: https://reviews.apache.org/r/34967/diff/
>
>
> Testing
> -------
>
> ./gradlew test -Pq
>
>
> Thanks,
>
> Zameer Manji
>
>
Re: Review Request 34967: Use TaskStatus Reason to set memory limit
message.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34967/#review86345
-----------------------------------------------------------
Ship it!
Master (54d561e) is green with this patch.
./build-support/jenkins/build.sh
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On June 3, 2015, 1:11 a.m., Zameer Manji wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34967/
> -----------------------------------------------------------
>
> (Updated June 3, 2015, 1:11 a.m.)
>
>
> Review request for Aurora and Maxim Khutornenko.
>
>
> Bugs: AURORA-1341
> https://issues.apache.org/jira/browse/AURORA-1341
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Use TaskStatus Reason to set memory limit message instead of checking the contents of the message field. Future versions of Mesos can change the diagnostic information in the message field causing Aurora to display no information when a task fails.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 5af691d0ac959cf4b5d01752daf996803e91ed16
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java fe2fc954350b42487151fc820ebad22a41aeb039
>
> Diff: https://reviews.apache.org/r/34967/diff/
>
>
> Testing
> -------
>
> ./gradlew test -Pq
>
>
> Thanks,
>
> Zameer Manji
>
>
Re: Review Request 34967: Use TaskStatus Reason to set memory limit
message.
Posted by Zameer Manji <zm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34967/
-----------------------------------------------------------
(Updated June 2, 2015, 6:11 p.m.)
Review request for Aurora and Maxim Khutornenko.
Changes
-------
Fix indentation
Bugs: AURORA-1341
https://issues.apache.org/jira/browse/AURORA-1341
Repository: aurora
Description
-------
Use TaskStatus Reason to set memory limit message instead of checking the contents of the message field. Future versions of Mesos can change the diagnostic information in the message field causing Aurora to display no information when a task fails.
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 5af691d0ac959cf4b5d01752daf996803e91ed16
src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java fe2fc954350b42487151fc820ebad22a41aeb039
Diff: https://reviews.apache.org/r/34967/diff/
Testing
-------
./gradlew test -Pq
Thanks,
Zameer Manji