You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Maxim Khutornenko <ma...@apache.org> on 2016/03/08 03:21:20 UTC
Review Request 44493: Log and terminate scheduler on updater thread
failure.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44493/
-----------------------------------------------------------
Review request for Aurora, John Sirois and Bill Farner.
Bugs: AURORA-1630
https://issues.apache.org/jira/browse/AURORA-1630
Repository: aurora
Description
-------
Moved HistoryPruner Runnable handler into AsyncUtil to reuse in updater.
Diffs
-----
src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java 80dc35ece876d46c8b94540cd6ab19092854867e
src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 22753b4579dfdaa1fceea2e095ba7063143959c6
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 48d7e2afb795a743ca9ca3e125a02195e45f1f67
src/main/java/org/apache/aurora/scheduler/updater/UpdaterModule.java c0472d732874dba5cd3534b07301432fed04fcca
src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java b39e38883a0e57210596d5c71ec5c90234837296
Diff: https://reviews.apache.org/r/44493/diff/
Testing
-------
./gradlew -Pq build
Thanks,
Maxim Khutornenko
Re: Review Request 44493: Log and terminate scheduler on updater
thread failure.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44493/#review122467
-----------------------------------------------------------
Ship it!
Master (a91a759) 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 March 8, 2016, 2:21 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44493/
> -----------------------------------------------------------
>
> (Updated March 8, 2016, 2:21 a.m.)
>
>
> Review request for Aurora, John Sirois and Bill Farner.
>
>
> Bugs: AURORA-1630
> https://issues.apache.org/jira/browse/AURORA-1630
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Moved HistoryPruner Runnable handler into AsyncUtil to reuse in updater.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java 80dc35ece876d46c8b94540cd6ab19092854867e
> src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 22753b4579dfdaa1fceea2e095ba7063143959c6
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 48d7e2afb795a743ca9ca3e125a02195e45f1f67
> src/main/java/org/apache/aurora/scheduler/updater/UpdaterModule.java c0472d732874dba5cd3534b07301432fed04fcca
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java b39e38883a0e57210596d5c71ec5c90234837296
>
> Diff: https://reviews.apache.org/r/44493/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 44493: Log and terminate scheduler on updater
thread failure.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On March 10, 2016, 3:39 p.m., John Sirois wrote:
> > src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java, line 128
> > <https://reviews.apache.org/r/44493/diff/1/?file=1290930#file1290930line128>
> >
> > It looks like the existing usages have inexpensive message formatting, but changing the util signature to (Lifecycle, Runnable, Logger, String, Object...) may make sense.
I actually prefer the simplicity of the current approach. Also as a bonus having Runnable as the last parameter improves readability for inline lambdas at call sites.
> On March 10, 2016, 3:39 p.m., John Sirois wrote:
> > src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java, line 130
> > <https://reviews.apache.org/r/44493/diff/1/?file=1290930#file1290930line130>
> >
> > It's verbose, but to avoid failure at a distance it probably makes sense to ensure parameters are non-null before creating the runnable.
Added.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44493/#review122846
-----------------------------------------------------------
On March 8, 2016, 2:21 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44493/
> -----------------------------------------------------------
>
> (Updated March 8, 2016, 2:21 a.m.)
>
>
> Review request for Aurora, John Sirois and Bill Farner.
>
>
> Bugs: AURORA-1630
> https://issues.apache.org/jira/browse/AURORA-1630
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Moved HistoryPruner Runnable handler into AsyncUtil to reuse in updater.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java 80dc35ece876d46c8b94540cd6ab19092854867e
> src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 22753b4579dfdaa1fceea2e095ba7063143959c6
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 48d7e2afb795a743ca9ca3e125a02195e45f1f67
> src/main/java/org/apache/aurora/scheduler/updater/UpdaterModule.java c0472d732874dba5cd3534b07301432fed04fcca
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java b39e38883a0e57210596d5c71ec5c90234837296
>
> Diff: https://reviews.apache.org/r/44493/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 44493: Log and terminate scheduler on updater
thread failure.
Posted by John Sirois <js...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44493/#review122846
-----------------------------------------------------------
Ship it!
src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java (line 128)
<https://reviews.apache.org/r/44493/#comment185036>
It looks like the existing usages have inexpensive message formatting, but changing the util signature to (Lifecycle, Runnable, Logger, String, Object...) may make sense.
src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java (line 130)
<https://reviews.apache.org/r/44493/#comment184942>
It's verbose, but to avoid failure at a distance it probably makes sense to ensure parameters are non-null before creating the runnable.
- John Sirois
On March 7, 2016, 7:21 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44493/
> -----------------------------------------------------------
>
> (Updated March 7, 2016, 7:21 p.m.)
>
>
> Review request for Aurora, John Sirois and Bill Farner.
>
>
> Bugs: AURORA-1630
> https://issues.apache.org/jira/browse/AURORA-1630
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Moved HistoryPruner Runnable handler into AsyncUtil to reuse in updater.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java 80dc35ece876d46c8b94540cd6ab19092854867e
> src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 22753b4579dfdaa1fceea2e095ba7063143959c6
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 48d7e2afb795a743ca9ca3e125a02195e45f1f67
> src/main/java/org/apache/aurora/scheduler/updater/UpdaterModule.java c0472d732874dba5cd3534b07301432fed04fcca
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java b39e38883a0e57210596d5c71ec5c90234837296
>
> Diff: https://reviews.apache.org/r/44493/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 44493: Log and terminate scheduler on updater
thread failure.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44493/
-----------------------------------------------------------
(Updated March 10, 2016, 8:31 p.m.)
Review request for Aurora, John Sirois and Bill Farner.
Changes
-------
feedback
Bugs: AURORA-1630
https://issues.apache.org/jira/browse/AURORA-1630
Repository: aurora
Description
-------
Moved HistoryPruner Runnable handler into AsyncUtil to reuse in updater.
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java 80dc35ece876d46c8b94540cd6ab19092854867e
src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 22753b4579dfdaa1fceea2e095ba7063143959c6
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 48d7e2afb795a743ca9ca3e125a02195e45f1f67
src/main/java/org/apache/aurora/scheduler/updater/UpdaterModule.java c0472d732874dba5cd3534b07301432fed04fcca
src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java b39e38883a0e57210596d5c71ec5c90234837296
Diff: https://reviews.apache.org/r/44493/diff/
Testing
-------
./gradlew -Pq build
Thanks,
Maxim Khutornenko
Re: Review Request 44493: Log and terminate scheduler on updater
thread failure.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44493/#review122840
-----------------------------------------------------------
Ping Bill, John
- Maxim Khutornenko
On March 8, 2016, 2:21 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44493/
> -----------------------------------------------------------
>
> (Updated March 8, 2016, 2:21 a.m.)
>
>
> Review request for Aurora, John Sirois and Bill Farner.
>
>
> Bugs: AURORA-1630
> https://issues.apache.org/jira/browse/AURORA-1630
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Moved HistoryPruner Runnable handler into AsyncUtil to reuse in updater.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java 80dc35ece876d46c8b94540cd6ab19092854867e
> src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 22753b4579dfdaa1fceea2e095ba7063143959c6
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 48d7e2afb795a743ca9ca3e125a02195e45f1f67
> src/main/java/org/apache/aurora/scheduler/updater/UpdaterModule.java c0472d732874dba5cd3534b07301432fed04fcca
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java b39e38883a0e57210596d5c71ec5c90234837296
>
> Diff: https://reviews.apache.org/r/44493/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 44493: Log and terminate scheduler on updater
thread failure.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On March 10, 2016, 1:40 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java, line 174
> > <https://reviews.apache.org/r/44493/diff/1/?file=1290931#file1290931line174>
> >
> > no big deal, but it would be nice if the line break style was consistent with above.
Done.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44493/#review122855
-----------------------------------------------------------
On March 8, 2016, 2:21 a.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44493/
> -----------------------------------------------------------
>
> (Updated March 8, 2016, 2:21 a.m.)
>
>
> Review request for Aurora, John Sirois and Bill Farner.
>
>
> Bugs: AURORA-1630
> https://issues.apache.org/jira/browse/AURORA-1630
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Moved HistoryPruner Runnable handler into AsyncUtil to reuse in updater.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java 80dc35ece876d46c8b94540cd6ab19092854867e
> src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 22753b4579dfdaa1fceea2e095ba7063143959c6
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 48d7e2afb795a743ca9ca3e125a02195e45f1f67
> src/main/java/org/apache/aurora/scheduler/updater/UpdaterModule.java c0472d732874dba5cd3534b07301432fed04fcca
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java b39e38883a0e57210596d5c71ec5c90234837296
>
> Diff: https://reviews.apache.org/r/44493/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 44493: Log and terminate scheduler on updater
thread failure.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44493/#review122855
-----------------------------------------------------------
Ship it!
src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java (line 161)
<https://reviews.apache.org/r/44493/#comment184946>
no big deal, but it would be nice if the line break style was consistent with above.
- Bill Farner
On March 7, 2016, 6:21 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44493/
> -----------------------------------------------------------
>
> (Updated March 7, 2016, 6:21 p.m.)
>
>
> Review request for Aurora, John Sirois and Bill Farner.
>
>
> Bugs: AURORA-1630
> https://issues.apache.org/jira/browse/AURORA-1630
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Moved HistoryPruner Runnable handler into AsyncUtil to reuse in updater.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java 80dc35ece876d46c8b94540cd6ab19092854867e
> src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 22753b4579dfdaa1fceea2e095ba7063143959c6
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 48d7e2afb795a743ca9ca3e125a02195e45f1f67
> src/main/java/org/apache/aurora/scheduler/updater/UpdaterModule.java c0472d732874dba5cd3534b07301432fed04fcca
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java b39e38883a0e57210596d5c71ec5c90234837296
>
> Diff: https://reviews.apache.org/r/44493/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>