You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Ashwin Murthy <as...@gmail.com> on 2016/03/31 01:19:57 UTC
Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/
-----------------------------------------------------------
Review request for Aurora and Bill Farner.
Repository: aurora
Description
-------
AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
Diffs
-----
src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/45511/diff/
Testing
-------
Added new unit test
Thanks,
Ashwin Murthy
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126221
-----------------------------------------------------------
Master (193f17e) is red with this patch.
./build-support/jenkins/build.sh
:checkstyleJmh
:jsHint
:checkstyleMain[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:22: error: 'org.apache.aurora.scheduler.http.LeaderRedirect.LeaderStatus' should be separated from previous imports.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:25: error: Line is longer than 100 characters (found 163).
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:30: error: 'member def modifier' have incorrect indentation level 4, expected level should be 2.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:32: error: 'ctor def modifier' have incorrect indentation level 4, expected level should be 2.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:34: error: 'ctor def' child have incorrect indentation level 8, expected level should be 4.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:35: error: 'ctor def rcurly' have incorrect indentation level 4, expected level should be 2.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:39: error: Line is longer than 100 characters (found 111).
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:44: error: 'method def modifier' have incorrect indentation level 4, expected level should be 2.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:47: error: 'member def type' have incorrect indentation level 8, expected level should be 4.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:47: error: 'method def' child have incorrect indentation level 8, expected level should be 4.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:49: error: 'if' have incorrect indentation level 8, expected level should be 4.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:49:11: error: 'if' is not followed by whitespace.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:50: error: 'if' child have incorrect indentation level 12, expected level should be 6.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:51: error: 'if rcurly' have incorrect indentation level 8, expected level should be 4.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:51:9: error: '}' at column 9 should be on the same line as the next part of a multi-block statement.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:52: error: 'else' have incorrect indentation level 8, expected level should be 4.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:52:16: error: 'if' is not followed by whitespace.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:53: error: 'if' child have incorrect indentation level 12, expected level should be 6.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:54: error: 'if rcurly' have incorrect indentation level 8, expected level should be 4.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:54:9: error: '}' at column 9 should be on the same line as the next part of a multi-block statement.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:55: error: 'else' have incorrect indentation level 8, expected level should be 4.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:56: error: 'else' child have incorrect indentation level 12, expected level should be 6.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:57: error: 'else rcurly' have incorrect indentation level 8, expected level should be 4.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:58: error: 'method def rcurly' have incorrect indentation level 4, expected level should be 2.
FAILED
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':checkstyleMain'.
> Checkstyle rule violations were found. See the report at: file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/checkstyle/main.html
* 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 35.849 secs
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On March 30, 2016, 11:19 p.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated March 30, 2016, 11:19 p.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Ashwin Murthy <as...@gmail.com>.
> On March 30, 2016, 11:30 p.m., Ashwin Murthy wrote:
> >
LeaderHealth
Element Missed Instructions Cov. Missed Branches Cov. Missed Cxty Missed Lines Missed Methods
Total 0 of 30 100% 0 of 4 100% 0 4 0 9 0 2
get() 22 100% 4 100% 0 3 0 6 0 1
LeaderHealth(LeaderRedirect) 8 100% n/a 0 1 0 3 0 1
Code coverage is 100% for the new code
- Ashwin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126223
-----------------------------------------------------------
On March 31, 2016, 2:35 a.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated March 31, 2016, 2:35 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Ashwin Murthy <as...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126223
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java (line 25)
<https://reviews.apache.org/r/45511/#comment189132>
Need to fix this comment. It should be 200, 500 or 503. Will udpate in next rev
- Ashwin Murthy
On March 30, 2016, 11:19 p.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated March 30, 2016, 11:19 p.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126535
-----------------------------------------------------------
Master (17ade11) is red with this patch.
./build-support/jenkins/build.sh
:commons:processResources
:commons:classes
:commons:jar
:compileJava/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java:74: Note: Wrote forwarder org.apache.aurora.scheduler.storage.log.WriteAheadStorageForwarder
@Forward({
^
Note: Writing file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/org/apache/aurora/common/args/apt/cmdline.arg.info.txt.2
Note: Writing file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/META-INF/compiler/resource-mappings/org.apache.aurora.common.args.apt.CmdLineProcessor
:generateBuildProperties
: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[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:65: error: Line is longer than 100 characters (found 117).
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:68: error: Line is longer than 100 characters (found 119).
FAILED
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':checkstyleMain'.
> Checkstyle rule violations were found. See the report at: file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/checkstyle/main.html
* 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: 5 mins 55.199 secs
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On April 1, 2016, 3:20 a.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated April 1, 2016, 3:20 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 6fc3afeb5a9e2f2c2ba944fbc6d611d3494eb779
> docs/operations/endpoints.md PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126545
-----------------------------------------------------------
Master (17ade11) 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
:compileTestJava/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java:38: Note: Wrote forwarder org.apache.aurora.scheduler.thrift.aop.MockDecoratedThriftForwarder
@Forward(AnnotatedAuroraAdmin.class)
^
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
:processTestResources
:testClasses
:checkstyleTest[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java:36: error: Wrong order for 'org.easymock.EasyMock.expect' import. Order should be: java, javax, scala, com, net, org. Each group should be separated by a single blank line.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java:37: error: Wrong order for 'org.easymock.EasyMock.capture' import. Order should be: java, javax, scala, com, net, org. Each group should be separated by a single blank line.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java:98: error: File contains a sequence of empty lines.
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.html
* 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: 3 mins 19.592 secs
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On April 1, 2016, 4:23 a.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated April 1, 2016, 4:23 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 6fc3afeb5a9e2f2c2ba944fbc6d611d3494eb779
> docs/operations/endpoints.md PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126557
-----------------------------------------------------------
Ship it!
Master (17ade11) 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 April 1, 2016, 6:08 a.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated April 1, 2016, 6:08 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 6fc3afeb5a9e2f2c2ba944fbc6d611d3494eb779
> docs/operations/endpoints.md PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126585
-----------------------------------------------------------
Ship it!
Master (17ade11) 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 April 1, 2016, 2:21 p.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated April 1, 2016, 2:21 p.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 6fc3afeb5a9e2f2c2ba944fbc6d611d3494eb779
> docs/reference/scheduler-endpoints.md PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126680
-----------------------------------------------------------
Ship it!
Master (17ade11) 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 April 2, 2016, 5:24 a.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated April 2, 2016, 5:24 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 6fc3afeb5a9e2f2c2ba944fbc6d611d3494eb779
> docs/reference/scheduler-endpoints.md PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java df649ff4ffdc741afdbc4850c2dbf98bd3e218f1
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/main/resources/scheduler/assets/index.html 861d5518890182737c6e545d6c409004c23cd18d
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh e1c12bbd4382c31e576439f6693d82d5661029b9
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126857
-----------------------------------------------------------
Fix it, then Ship it!
LGTM after some very minor tweaks. Thanks!
RELEASE-NOTES.md (line 8)
<https://reviews.apache.org/r/45511/#comment189944>
remove the extra hyphen, otherwise thie results in a separate bullet.
docs/reference/scheduler-endpoints.md (line 2)
<https://reviews.apache.org/r/45511/#comment189945>
Please remove the TODO. While not a lie, the ticket is the best place to track the work.
src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java (line 42)
<https://reviews.apache.org/r/45511/#comment189947>
Remove "This is added as part of AURORA-1493 and", as it becomes irrelevant ~immediately after commit.
- Bill Farner
On April 1, 2016, 10:24 p.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated April 1, 2016, 10:24 p.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 6fc3afeb5a9e2f2c2ba944fbc6d611d3494eb779
> docs/reference/scheduler-endpoints.md PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java df649ff4ffdc741afdbc4850c2dbf98bd3e218f1
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/main/resources/scheduler/assets/index.html 861d5518890182737c6e545d6c409004c23cd18d
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh e1c12bbd4382c31e576439f6693d82d5661029b9
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Ashwin Murthy <as...@gmail.com>.
> On April 4, 2016, 8:31 p.m., Aurora ReviewBot wrote:
> > Master (9847b02) is green with this patch.
> > ./build-support/jenkins/build.sh
> >
> > I will refresh this build result if you post a review containing "@ReviewBot retry"
Bill, will you be merging? Thanks!
- Ashwin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126933
-----------------------------------------------------------
On April 4, 2016, 7:43 p.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated April 4, 2016, 7:43 p.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 6fc3afeb5a9e2f2c2ba944fbc6d611d3494eb779
> docs/reference/scheduler-endpoints.md PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java df649ff4ffdc741afdbc4850c2dbf98bd3e218f1
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/main/resources/scheduler/assets/index.html 861d5518890182737c6e545d6c409004c23cd18d
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh e1c12bbd4382c31e576439f6693d82d5661029b9
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Bill Farner <wf...@apache.org>.
> On April 4, 2016, 1:31 p.m., Aurora ReviewBot wrote:
> > Master (9847b02) is green with this patch.
> > ./build-support/jenkins/build.sh
> >
> > I will refresh this build result if you post a review containing "@ReviewBot retry"
>
> Ashwin Murthy wrote:
> Bill, will you be merging? Thanks!
My mistake, for some reason i had convinced myself there was another reviewer. Thanks for the nudge! This will be on master shortly.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126933
-----------------------------------------------------------
On April 4, 2016, 12:43 p.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated April 4, 2016, 12:43 p.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 6fc3afeb5a9e2f2c2ba944fbc6d611d3494eb779
> docs/reference/scheduler-endpoints.md PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java df649ff4ffdc741afdbc4850c2dbf98bd3e218f1
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/main/resources/scheduler/assets/index.html 861d5518890182737c6e545d6c409004c23cd18d
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh e1c12bbd4382c31e576439f6693d82d5661029b9
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126933
-----------------------------------------------------------
Ship it!
Master (9847b02) 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 April 4, 2016, 7:43 p.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated April 4, 2016, 7:43 p.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 6fc3afeb5a9e2f2c2ba944fbc6d611d3494eb779
> docs/reference/scheduler-endpoints.md PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java df649ff4ffdc741afdbc4850c2dbf98bd3e218f1
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/main/resources/scheduler/assets/index.html 861d5518890182737c6e545d6c409004c23cd18d
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh e1c12bbd4382c31e576439f6693d82d5661029b9
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Ashwin Murthy <as...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/
-----------------------------------------------------------
(Updated April 5, 2016, 9:53 a.m.)
Review request for Aurora and Bill Farner.
Changes
-------
Adding bug.
-wfarner
Bugs: AURORA-1493
https://issues.apache.org/jira/browse/AURORA-1493
Repository: aurora
Description
-------
AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
Diffs
-----
RELEASE-NOTES.md 6fc3afeb5a9e2f2c2ba944fbc6d611d3494eb779
docs/reference/scheduler-endpoints.md PRE-CREATION
src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java df649ff4ffdc741afdbc4850c2dbf98bd3e218f1
src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
src/main/resources/scheduler/assets/index.html 861d5518890182737c6e545d6c409004c23cd18d
src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh e1c12bbd4382c31e576439f6693d82d5661029b9
Diff: https://reviews.apache.org/r/45511/diff/
Testing
-------
Added new unit test
Thanks,
Ashwin Murthy
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Ashwin Murthy <as...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/
-----------------------------------------------------------
(Updated April 4, 2016, 7:43 p.m.)
Review request for Aurora and Bill Farner.
Repository: aurora
Description
-------
AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
Diffs (updated)
-----
RELEASE-NOTES.md 6fc3afeb5a9e2f2c2ba944fbc6d611d3494eb779
docs/reference/scheduler-endpoints.md PRE-CREATION
src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java df649ff4ffdc741afdbc4850c2dbf98bd3e218f1
src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
src/main/resources/scheduler/assets/index.html 861d5518890182737c6e545d6c409004c23cd18d
src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh e1c12bbd4382c31e576439f6693d82d5661029b9
Diff: https://reviews.apache.org/r/45511/diff/
Testing
-------
Added new unit test
Thanks,
Ashwin Murthy
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Ashwin Murthy <as...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/
-----------------------------------------------------------
(Updated April 2, 2016, 5:24 a.m.)
Review request for Aurora and Bill Farner.
Repository: aurora
Description
-------
AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
Diffs (updated)
-----
RELEASE-NOTES.md 6fc3afeb5a9e2f2c2ba944fbc6d611d3494eb779
docs/reference/scheduler-endpoints.md PRE-CREATION
src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java df649ff4ffdc741afdbc4850c2dbf98bd3e218f1
src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
src/main/resources/scheduler/assets/index.html 861d5518890182737c6e545d6c409004c23cd18d
src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh e1c12bbd4382c31e576439f6693d82d5661029b9
Diff: https://reviews.apache.org/r/45511/diff/
Testing
-------
Added new unit test
Thanks,
Ashwin Murthy
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Ashwin Murthy <as...@gmail.com>.
> On April 1, 2016, 7:48 p.m., Bill Farner wrote:
> > Biggest issue - the servlet isn't actually wired in and therefore the scheduler does not respond to `/leaderhealth`. I suggest we add a check in the end-to-end tests to verify this behavior and make sure wiring is correct.
> >
> > As for behavior, i'm still not fond of returning 500 for an expected phase of the scheduler lifecycle (e.g. while leadership is changing). Counters for 500 responses should be reserved for things that people can alert on and debug. How about 502 (bad gateway) instead?
> >
> > I've gone ahead and addressed my outstanding comments [here](https://github.com/wfarner/aurora/commit/a769a9de3361611ef75398475f511ccb96a9d2d3), feel free to pull that commit and apply the changes you agree with.
>
> Ashwin Murthy wrote:
> Oh the jettyservermodule update is a big miss. I knew i had to make this earlier but forgot to get to it. Thanks for the catch and I will add the e2e test for the same reason.
>
> Ashwin Murthy wrote:
> I am fine with bad gateway instead of 500.
Bill, I basically took all your changes since it all made sense. Updated the scheduler-endpoints.md which was missing to include the new status code of 502 instead of 500.
- Ashwin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126623
-----------------------------------------------------------
On April 2, 2016, 5:24 a.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated April 2, 2016, 5:24 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 6fc3afeb5a9e2f2c2ba944fbc6d611d3494eb779
> docs/reference/scheduler-endpoints.md PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java df649ff4ffdc741afdbc4850c2dbf98bd3e218f1
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/main/resources/scheduler/assets/index.html 861d5518890182737c6e545d6c409004c23cd18d
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh e1c12bbd4382c31e576439f6693d82d5661029b9
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Ashwin Murthy <as...@gmail.com>.
> On April 1, 2016, 7:48 p.m., Bill Farner wrote:
> > Biggest issue - the servlet isn't actually wired in and therefore the scheduler does not respond to `/leaderhealth`. I suggest we add a check in the end-to-end tests to verify this behavior and make sure wiring is correct.
> >
> > As for behavior, i'm still not fond of returning 500 for an expected phase of the scheduler lifecycle (e.g. while leadership is changing). Counters for 500 responses should be reserved for things that people can alert on and debug. How about 502 (bad gateway) instead?
> >
> > I've gone ahead and addressed my outstanding comments [here](https://github.com/wfarner/aurora/commit/a769a9de3361611ef75398475f511ccb96a9d2d3), feel free to pull that commit and apply the changes you agree with.
Oh the jettyservermodule update is a big miss. I knew i had to make this earlier but forgot to get to it. Thanks for the catch and I will add the e2e test for the same reason.
- Ashwin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126623
-----------------------------------------------------------
On April 1, 2016, 2:21 p.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated April 1, 2016, 2:21 p.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 6fc3afeb5a9e2f2c2ba944fbc6d611d3494eb779
> docs/reference/scheduler-endpoints.md PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Ashwin Murthy <as...@gmail.com>.
> On April 1, 2016, 7:48 p.m., Bill Farner wrote:
> > Biggest issue - the servlet isn't actually wired in and therefore the scheduler does not respond to `/leaderhealth`. I suggest we add a check in the end-to-end tests to verify this behavior and make sure wiring is correct.
> >
> > As for behavior, i'm still not fond of returning 500 for an expected phase of the scheduler lifecycle (e.g. while leadership is changing). Counters for 500 responses should be reserved for things that people can alert on and debug. How about 502 (bad gateway) instead?
> >
> > I've gone ahead and addressed my outstanding comments [here](https://github.com/wfarner/aurora/commit/a769a9de3361611ef75398475f511ccb96a9d2d3), feel free to pull that commit and apply the changes you agree with.
>
> Ashwin Murthy wrote:
> Oh the jettyservermodule update is a big miss. I knew i had to make this earlier but forgot to get to it. Thanks for the catch and I will add the e2e test for the same reason.
I am fine with bad gateway instead of 500.
- Ashwin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126623
-----------------------------------------------------------
On April 1, 2016, 2:21 p.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated April 1, 2016, 2:21 p.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 6fc3afeb5a9e2f2c2ba944fbc6d611d3494eb779
> docs/reference/scheduler-endpoints.md PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126623
-----------------------------------------------------------
Biggest issue - the servlet isn't actually wired in and therefore the scheduler does not respond to `/leaderhealth`. I suggest we add a check in the end-to-end tests to verify this behavior and make sure wiring is correct.
As for behavior, i'm still not fond of returning 500 for an expected phase of the scheduler lifecycle (e.g. while leadership is changing). Counters for 500 responses should be reserved for things that people can alert on and debug. How about 502 (bad gateway) instead?
I've gone ahead and addressed my outstanding comments [here](https://github.com/wfarner/aurora/commit/a769a9de3361611ef75398475f511ccb96a9d2d3), feel free to pull that commit and apply the changes you agree with.
src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java (line 36)
<https://reviews.apache.org/r/45511/#comment189633>
src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java (line 36)
<https://reviews.apache.org/r/45511/#comment189634>
I suggest stripping the HTML tags.
As the code stands, the response `Content-Type` is guessed based on the request, and for `curl` the result is raw HTML tags. I think `text/plain` is fine here, and we should just not use HTML.
src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java (line 58)
<https://reviews.apache.org/r/45511/#comment189635>
To be even more human-friendly the response body can also includes the current status. Not a huge deal, but could save somebody a few seconds of clicking.
- Bill Farner
On April 1, 2016, 7:21 a.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated April 1, 2016, 7:21 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 6fc3afeb5a9e2f2c2ba944fbc6d611d3494eb779
> docs/reference/scheduler-endpoints.md PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Ashwin Murthy <as...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/
-----------------------------------------------------------
(Updated April 1, 2016, 2:21 p.m.)
Review request for Aurora and Bill Farner.
Repository: aurora
Description
-------
AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
Diffs (updated)
-----
RELEASE-NOTES.md 6fc3afeb5a9e2f2c2ba944fbc6d611d3494eb779
docs/reference/scheduler-endpoints.md PRE-CREATION
src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/45511/diff/
Testing
-------
Added new unit test
Thanks,
Ashwin Murthy
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Ashwin Murthy <as...@gmail.com>.
> On April 1, 2016, 11:31 a.m., Stephan Erb wrote:
> > Looks good. Minor nitpicks below.
Addressed all your comments. Thanks Stephan!
Biil, Stephan, Joshua:
Any more comments to address or will one of you commit?
- Ashwin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126573
-----------------------------------------------------------
On April 1, 2016, 2:21 p.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated April 1, 2016, 2:21 p.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 6fc3afeb5a9e2f2c2ba944fbc6d611d3494eb779
> docs/reference/scheduler-endpoints.md PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Stephan Erb <se...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126573
-----------------------------------------------------------
Fix it, then Ship it!
Looks good. Minor nitpicks below.
RELEASE-NOTES.md (line 6)
<https://reviews.apache.org/r/45511/#comment189595>
Please stick to max 100 cols.
RELEASE-NOTES.md (line 8)
<https://reviews.apache.org/r/45511/#comment189594>
Please leave this line at the top of our changelog, as it tends to be the most important entry.
docs/operations/endpoints.md (line 1)
<https://reviews.apache.org/r/45511/#comment189596>
Seeing this spelled out, it will probably be more like a (compelete) reference rather than a guide for operators.
Would you mind moving this to `reference/scheduler-endpoints.md`?
src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java (line 36)
<https://reviews.apache.org/r/45511/#comment189593>
That's cool. Thanks :-)
- Stephan Erb
On April 1, 2016, 8:08 a.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated April 1, 2016, 8:08 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 6fc3afeb5a9e2f2c2ba944fbc6d611d3494eb779
> docs/operations/endpoints.md PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Ashwin Murthy <as...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/
-----------------------------------------------------------
(Updated April 1, 2016, 6:08 a.m.)
Review request for Aurora and Bill Farner.
Repository: aurora
Description
-------
AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
Diffs (updated)
-----
RELEASE-NOTES.md 6fc3afeb5a9e2f2c2ba944fbc6d611d3494eb779
docs/operations/endpoints.md PRE-CREATION
src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/45511/diff/
Testing
-------
Added new unit test
Thanks,
Ashwin Murthy
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126553
-----------------------------------------------------------
Master (17ade11) is red with this patch.
./build-support/jenkins/build.sh
:generateBuildProperties
: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
:compileTestJava/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java:38: Note: Wrote forwarder org.apache.aurora.scheduler.thrift.aop.MockDecoratedThriftForwarder
@Forward(AnnotatedAuroraAdmin.class)
^
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
:processTestResources
:testClasses
:checkstyleTest[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java:36: error: Wrong order for 'org.easymock.EasyMock.capture' import. Order should be: java, javax, scala, com, net, org. Each group should be separated by a single blank line.
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.html
* 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: 5 mins 49.756 secs
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On April 1, 2016, 5:49 a.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated April 1, 2016, 5:49 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 6fc3afeb5a9e2f2c2ba944fbc6d611d3494eb779
> docs/operations/endpoints.md PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Ashwin Murthy <as...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/
-----------------------------------------------------------
(Updated April 1, 2016, 5:49 a.m.)
Review request for Aurora and Bill Farner.
Repository: aurora
Description
-------
AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
Diffs (updated)
-----
RELEASE-NOTES.md 6fc3afeb5a9e2f2c2ba944fbc6d611d3494eb779
docs/operations/endpoints.md PRE-CREATION
src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/45511/diff/
Testing
-------
Added new unit test
Thanks,
Ashwin Murthy
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Ashwin Murthy <as...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/
-----------------------------------------------------------
(Updated April 1, 2016, 4:23 a.m.)
Review request for Aurora and Bill Farner.
Repository: aurora
Description
-------
AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
Diffs (updated)
-----
RELEASE-NOTES.md 6fc3afeb5a9e2f2c2ba944fbc6d611d3494eb779
docs/operations/endpoints.md PRE-CREATION
src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/45511/diff/
Testing
-------
Added new unit test
Thanks,
Ashwin Murthy
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Ashwin Murthy <as...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/
-----------------------------------------------------------
(Updated April 1, 2016, 3:20 a.m.)
Review request for Aurora and Bill Farner.
Repository: aurora
Description
-------
AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
Diffs (updated)
-----
RELEASE-NOTES.md 6fc3afeb5a9e2f2c2ba944fbc6d611d3494eb779
docs/operations/endpoints.md PRE-CREATION
src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/45511/diff/
Testing
-------
Added new unit test
Thanks,
Ashwin Murthy
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Stephan Erb <se...@apache.org>.
> On March 31, 2016, 1:17 p.m., Stephan Erb wrote:
> > I have no clue about AWS & ELB, so just a couple of notes from the sideline:
> >
> > * The new feature should be listed in `RELEASE-NOTES.md`
> > * We have to figure out how to document that properly (beyond the release notes). Right now this is only done within the code. For a project as large as Aurora, this basically means the feature does not exist at all.
> >
> > * Option A: We create a new documentation page in `docs/operations` that document endpoints.
> > * Option B: We make sure the feature is self-documenting. This would mean we add it to the scheduler `index.html` and return a description of the feature in the response text. This would then be equivalent in how we handle the `mname` and `stuctdump` endpoints
>
> Ashwin Murthy wrote:
> I will update the release-notes.md. Is there a recommendation on which of the two options is the prefered way for documenting this feature/new endpoint? Both options seem to suggest that not all endpoints are currently documented. If we go with option A, we will need a separate JIRA ticket to move remaining endpoints to be documented here. I am fine either way, just want to see if there is a preferance eitherway
I believe `Option A` would probably be the better one, as it can be found via google :-)
But let's wait what the others think.
- Stephan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126297
-----------------------------------------------------------
On March 31, 2016, 7:06 a.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated March 31, 2016, 7:06 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Ashwin Murthy <as...@gmail.com>.
> On March 31, 2016, 11:17 a.m., Stephan Erb wrote:
> > I have no clue about AWS & ELB, so just a couple of notes from the sideline:
> >
> > * The new feature should be listed in `RELEASE-NOTES.md`
> > * We have to figure out how to document that properly (beyond the release notes). Right now this is only done within the code. For a project as large as Aurora, this basically means the feature does not exist at all.
> >
> > * Option A: We create a new documentation page in `docs/operations` that document endpoints.
> > * Option B: We make sure the feature is self-documenting. This would mean we add it to the scheduler `index.html` and return a description of the feature in the response text. This would then be equivalent in how we handle the `mname` and `stuctdump` endpoints
I will update the release-notes.md. Is there a recommendation on which of the two options is the prefered way for documenting this feature/new endpoint? Both options seem to suggest that not all endpoints are currently documented. If we go with option A, we will need a separate JIRA ticket to move remaining endpoints to be documented here. I am fine either way, just want to see if there is a preferance eitherway
- Ashwin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126297
-----------------------------------------------------------
On March 31, 2016, 5:06 a.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated March 31, 2016, 5:06 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Ashwin Murthy <as...@gmail.com>.
> On March 31, 2016, 11:17 a.m., Stephan Erb wrote:
> > I have no clue about AWS & ELB, so just a couple of notes from the sideline:
> >
> > * The new feature should be listed in `RELEASE-NOTES.md`
> > * We have to figure out how to document that properly (beyond the release notes). Right now this is only done within the code. For a project as large as Aurora, this basically means the feature does not exist at all.
> >
> > * Option A: We create a new documentation page in `docs/operations` that document endpoints.
> > * Option B: We make sure the feature is self-documenting. This would mean we add it to the scheduler `index.html` and return a description of the feature in the response text. This would then be equivalent in how we handle the `mname` and `stuctdump` endpoints
>
> Ashwin Murthy wrote:
> I will update the release-notes.md. Is there a recommendation on which of the two options is the prefered way for documenting this feature/new endpoint? Both options seem to suggest that not all endpoints are currently documented. If we go with option A, we will need a separate JIRA ticket to move remaining endpoints to be documented here. I am fine either way, just want to see if there is a preferance eitherway
>
> Stephan Erb wrote:
> I believe `Option A` would probably be the better one, as it can be found via google :-)
>
> But let's wait what the others think.
>
> Bill Farner wrote:
> Just noting that A & B are not mutually exclusive - we _could_ have both with great success!
>
> I think B is a nice suggestion. A first-time operator is quite likely to land at the HTTP root first and poke around.
I have taken option A and filed a JIRA task to add the other endpoints to this as well. The JIRA task has been linked in the newly added document called endpoints.md. I would prefer to do just option A since this seems very clear and is searchable. Option B seems like a nice to have and not a must :). Let me know if you think otherwise.
- Ashwin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126297
-----------------------------------------------------------
On April 1, 2016, 3:20 a.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated April 1, 2016, 3:20 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> RELEASE-NOTES.md 6fc3afeb5a9e2f2c2ba944fbc6d611d3494eb779
> docs/operations/endpoints.md PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Bill Farner <wf...@apache.org>.
> On March 31, 2016, 4:17 a.m., Stephan Erb wrote:
> > I have no clue about AWS & ELB, so just a couple of notes from the sideline:
> >
> > * The new feature should be listed in `RELEASE-NOTES.md`
> > * We have to figure out how to document that properly (beyond the release notes). Right now this is only done within the code. For a project as large as Aurora, this basically means the feature does not exist at all.
> >
> > * Option A: We create a new documentation page in `docs/operations` that document endpoints.
> > * Option B: We make sure the feature is self-documenting. This would mean we add it to the scheduler `index.html` and return a description of the feature in the response text. This would then be equivalent in how we handle the `mname` and `stuctdump` endpoints
>
> Ashwin Murthy wrote:
> I will update the release-notes.md. Is there a recommendation on which of the two options is the prefered way for documenting this feature/new endpoint? Both options seem to suggest that not all endpoints are currently documented. If we go with option A, we will need a separate JIRA ticket to move remaining endpoints to be documented here. I am fine either way, just want to see if there is a preferance eitherway
>
> Stephan Erb wrote:
> I believe `Option A` would probably be the better one, as it can be found via google :-)
>
> But let's wait what the others think.
Just noting that A & B are not mutually exclusive - we _could_ have both with great success!
I think B is a nice suggestion. A first-time operator is quite likely to land at the HTTP root first and poke around.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126297
-----------------------------------------------------------
On March 30, 2016, 10:06 p.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated March 30, 2016, 10:06 p.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Stephan Erb <se...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126297
-----------------------------------------------------------
I have no clue about AWS & ELB, so just a couple of notes from the sideline:
* The new feature should be listed in `RELEASE-NOTES.md`
* We have to figure out how to document that properly (beyond the release notes). Right now this is only done within the code. For a project as large as Aurora, this basically means the feature does not exist at all.
* Option A: We create a new documentation page in `docs/operations` that document endpoints.
* Option B: We make sure the feature is self-documenting. This would mean we add it to the scheduler `index.html` and return a description of the feature in the response text. This would then be equivalent in how we handle the `mname` and `stuctdump` endpoints
- Stephan Erb
On March 31, 2016, 7:06 a.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated March 31, 2016, 7:06 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Ashwin Murthy <as...@gmail.com>.
> On March 31, 2016, 6:26 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java, lines 54-58
> > <https://reviews.apache.org/r/45511/diff/4/?file=1320482#file1320482line54>
> >
> > `LeaderHealth` only cares about interfacing with `LeaderRedirect`, and therefore a unit test should aim to avoid pulling in other details of the `LeaderRedirect` implementation.
> >
> > You should be fine to only mock `LeaderRedirect` in this test.
I want to but seems like this is the easiest to test the three cases. For me to do that, I need to be able to set state on the LeaderRedirector or have it instantiated in one of the three states - Leader, NotLeader, NoLeader. I am probably missing some understanding of the mocking framework here since this is my first patch. But reusing the LeaderRedirector as is was the easiest to test these cases. Also, the LeaderHealth and LeaderRedirector are related closely. Let me know if there is another way.
- Ashwin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126383
-----------------------------------------------------------
On March 31, 2016, 5:06 a.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated March 31, 2016, 5:06 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Ashwin Murthy <as...@gmail.com>.
> On March 31, 2016, 6:26 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java, line 104
> > <https://reviews.apache.org/r/45511/diff/4/?file=1320482#file1320482line104>
> >
> > this should be +4 indent, not +8
Done
- Ashwin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126383
-----------------------------------------------------------
On March 31, 2016, 5:06 a.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated March 31, 2016, 5:06 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Bill Farner <wf...@apache.org>.
> On March 31, 2016, 11:26 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java, line 54
> > <https://reviews.apache.org/r/45511/diff/4/?file=1320481#file1320481line54>
> >
> > Is it useful to send a different status code for this state? Seems like it doesn't make a difference to a load balancer, and trying to differentiate forces use of an awkward code.
>
> Ashwin Murthy wrote:
> I thought about this and decided to go with different codes to disambiguate these two cases. My reasoning was If this endpoint ends up being used for other things like a canary monitoring the leader health and the different code could result in different action. Either way, not differentiating it seems like hiding this information which could be potentially useful in the future and I didnt see a good reason to hide it.
Perhaps, but IMHO 500 is potentially more insidious since it is the result of any unhandled exception in the scheduler. I'd prefer to reserve 500 for the truly unknown and triage-worthy.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126383
-----------------------------------------------------------
On March 30, 2016, 10:06 p.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated March 30, 2016, 10:06 p.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Ashwin Murthy <as...@gmail.com>.
> On March 31, 2016, 6:26 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java, line 47
> > <https://reviews.apache.org/r/45511/diff/4/?file=1320481#file1320481line47>
> >
> > Let's make this endpoint browser-friendly too and include some text. How about something like:
> >
> > ```
> > This is the leading scheduler.
> >
> > This endpoint can indicate to a load balancer which of a group of schedulers is leading. The response codes are:
> > 200: This is the leading scheduler.
> > xxx: Another scheduler is leading.
> > yyy: There is no leader currently, or the leader is unknown.
> > ```
Done
- Ashwin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126383
-----------------------------------------------------------
On March 31, 2016, 5:06 a.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated March 31, 2016, 5:06 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Ashwin Murthy <as...@gmail.com>.
> On March 31, 2016, 6:26 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java, line 54
> > <https://reviews.apache.org/r/45511/diff/4/?file=1320481#file1320481line54>
> >
> > Is it useful to send a different status code for this state? Seems like it doesn't make a difference to a load balancer, and trying to differentiate forces use of an awkward code.
I thought about this and decided to go with different codes to disambiguate these two cases. My reasoning was If this endpoint ends up being used for other things like a canary monitoring the leader health and the different code could result in different action. Either way, not differentiating it seems like hiding this information which could be potentially useful in the future and I didnt see a good reason to hide it.
> On March 31, 2016, 6:26 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java, line 52
> > <https://reviews.apache.org/r/45511/diff/4/?file=1320481#file1320481line52>
> >
> > I'm not fond of a 500, as it's typically reserved for cases of genuine unknown error. 503 seems more appropriate if it's all the same.
the RFC says 503 means "the service is having temporary issues and will be resolved soon, so please retry after some time" I feel this is more appropriate when there is no leader at all. If we agree it is ok to disambiguate no leader from the case where this instance is not the leader, 500 seemed the best option.
- Ashwin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126383
-----------------------------------------------------------
On March 31, 2016, 5:06 a.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated March 31, 2016, 5:06 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126383
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java (line 47)
<https://reviews.apache.org/r/45511/#comment189372>
Let's make this endpoint browser-friendly too and include some text. How about something like:
```
This is the leading scheduler.
This endpoint can indicate to a load balancer which of a group of schedulers is leading. The response codes are:
200: This is the leading scheduler.
xxx: Another scheduler is leading.
yyy: There is no leader currently, or the leader is unknown.
```
src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java (line 52)
<https://reviews.apache.org/r/45511/#comment189373>
I'm not fond of a 500, as it's typically reserved for cases of genuine unknown error. 503 seems more appropriate if it's all the same.
src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java (line 54)
<https://reviews.apache.org/r/45511/#comment189375>
Is it useful to send a different status code for this state? Seems like it doesn't make a difference to a load balancer, and trying to differentiate forces use of an awkward code.
src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java (lines 54 - 58)
<https://reviews.apache.org/r/45511/#comment189381>
`LeaderHealth` only cares about interfacing with `LeaderRedirect`, and therefore a unit test should aim to avoid pulling in other details of the `LeaderRedirect` implementation.
You should be fine to only mock `LeaderRedirect` in this test.
src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java (line 104)
<https://reviews.apache.org/r/45511/#comment189376>
this should be +4 indent, not +8
- Bill Farner
On March 30, 2016, 10:06 p.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated March 30, 2016, 10:06 p.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Ashwin Murthy <as...@gmail.com>.
> On March 31, 2016, 6:33 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java, line 49
> > <https://reviews.apache.org/r/45511/diff/4/?file=1320481#file1320481line49>
> >
> > Can we switch on this enum, rather than if/else branching? If we also add an explicit case for each enum value and reserve the default block for an unexpected status it will allow this code to continue to work as expected even if new values are added to the enum in the future (i.e. as things stand today, if we added a hypothetical state like `LeaderStatus.ALSO_LEADING`, it would fall into the `SERVICE_UNAVAILABLE` branch).
>
> Ashwin Murthy wrote:
> @Bill, @Joshua,
>
> Can I do the following?
>
> 1. Return 200 is leader
> 2. Return 501 (Not implemented) if this is not the leader but a leader exists
> 3. Return 503 (Service unavailable) if there is no current leader
> 4. Return 500 (Internal server error) for the default case that Joshua points out
>
> Would you be ok with this? I
>
> Joshua Cohen wrote:
> In the [LeaderRedirectFilter](https://github.com/apache/aurora/blob/master/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirectFilter.java#L70-L91) we do the following:
>
> Is Leading -> continue request (i.e. 200)
> No Leader -> 503
> Not Leading -> 307
> Unknown State -> 500
>
> So that meshes pretty well with what you describe, you're just replacing the redirect when the instance is not the leader with a non 2xx. I'm not sure if I love 501 for that case, but I'm having a hard time finding a better alternative.
>
> That being said, I feel like I've asked this before, but you can clarify why the existing `LeaderRedirectFilter` is not sufficient for ELB? If I'm reading the [docs](http://docs.aws.amazon.com/ElasticLoadBalancing/latest/DeveloperGuide/elb-healthchecks.html#health-check-configuration) right, it seems like any non-200 reply is sufficient to indicate the instance is not healthy (i.e. not the leader). So given that `LeaderRedirectFilter` already returns a 307 for this case, can we not just use that?
@Bill, @Joshua,
Actually, I changed my mind. The prev one is rather more complex for this simple scenario than needs to be.
How about?:
Return 200 if instance is leader
Return 503 (Service unavailablE) for the case when this instance is not the leader but a leader exists
Return 500 (internal server error) for all other (default) cases since this is truly an error.
Would you be ok with this?
- Ashwin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126394
-----------------------------------------------------------
On March 31, 2016, 5:06 a.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated March 31, 2016, 5:06 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Joshua Cohen <jc...@apache.org>.
> On March 31, 2016, 6:33 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java, line 49
> > <https://reviews.apache.org/r/45511/diff/4/?file=1320481#file1320481line49>
> >
> > Can we switch on this enum, rather than if/else branching? If we also add an explicit case for each enum value and reserve the default block for an unexpected status it will allow this code to continue to work as expected even if new values are added to the enum in the future (i.e. as things stand today, if we added a hypothetical state like `LeaderStatus.ALSO_LEADING`, it would fall into the `SERVICE_UNAVAILABLE` branch).
>
> Ashwin Murthy wrote:
> @Bill, @Joshua,
>
> Can I do the following?
>
> 1. Return 200 is leader
> 2. Return 501 (Not implemented) if this is not the leader but a leader exists
> 3. Return 503 (Service unavailable) if there is no current leader
> 4. Return 500 (Internal server error) for the default case that Joshua points out
>
> Would you be ok with this? I
In the [LeaderRedirectFilter](https://github.com/apache/aurora/blob/master/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirectFilter.java#L70-L91) we do the following:
Is Leading -> continue request (i.e. 200)
No Leader -> 503
Not Leading -> 307
Unknown State -> 500
So that meshes pretty well with what you describe, you're just replacing the redirect when the instance is not the leader with a non 2xx. I'm not sure if I love 501 for that case, but I'm having a hard time finding a better alternative.
That being said, I feel like I've asked this before, but you can clarify why the existing `LeaderRedirectFilter` is not sufficient for ELB? If I'm reading the [docs](http://docs.aws.amazon.com/ElasticLoadBalancing/latest/DeveloperGuide/elb-healthchecks.html#health-check-configuration) right, it seems like any non-200 reply is sufficient to indicate the instance is not healthy (i.e. not the leader). So given that `LeaderRedirectFilter` already returns a 307 for this case, can we not just use that?
- Joshua
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126394
-----------------------------------------------------------
On March 31, 2016, 5:06 a.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated March 31, 2016, 5:06 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Ashwin Murthy <as...@gmail.com>.
> On March 31, 2016, 6:33 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java, line 49
> > <https://reviews.apache.org/r/45511/diff/4/?file=1320481#file1320481line49>
> >
> > Can we switch on this enum, rather than if/else branching? If we also add an explicit case for each enum value and reserve the default block for an unexpected status it will allow this code to continue to work as expected even if new values are added to the enum in the future (i.e. as things stand today, if we added a hypothetical state like `LeaderStatus.ALSO_LEADING`, it would fall into the `SERVICE_UNAVAILABLE` branch).
@Bill, @Joshua,
Can I do the following?
1. Return 200 is leader
2. Return 501 (Not implemented) if this is not the leader but a leader exists
3. Return 503 (Service unavailable) if there is no current leader
4. Return 500 (Internal server error) for the default case that Joshua points out
Would you be ok with this? I
- Ashwin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126394
-----------------------------------------------------------
On March 31, 2016, 5:06 a.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated March 31, 2016, 5:06 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Ashwin Murthy <as...@gmail.com>.
> On March 31, 2016, 6:33 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java, line 49
> > <https://reviews.apache.org/r/45511/diff/4/?file=1320481#file1320481line49>
> >
> > Can we switch on this enum, rather than if/else branching? If we also add an explicit case for each enum value and reserve the default block for an unexpected status it will allow this code to continue to work as expected even if new values are added to the enum in the future (i.e. as things stand today, if we added a hypothetical state like `LeaderStatus.ALSO_LEADING`, it would fall into the `SERVICE_UNAVAILABLE` branch).
>
> Ashwin Murthy wrote:
> @Bill, @Joshua,
>
> Can I do the following?
>
> 1. Return 200 is leader
> 2. Return 501 (Not implemented) if this is not the leader but a leader exists
> 3. Return 503 (Service unavailable) if there is no current leader
> 4. Return 500 (Internal server error) for the default case that Joshua points out
>
> Would you be ok with this? I
>
> Joshua Cohen wrote:
> In the [LeaderRedirectFilter](https://github.com/apache/aurora/blob/master/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirectFilter.java#L70-L91) we do the following:
>
> Is Leading -> continue request (i.e. 200)
> No Leader -> 503
> Not Leading -> 307
> Unknown State -> 500
>
> So that meshes pretty well with what you describe, you're just replacing the redirect when the instance is not the leader with a non 2xx. I'm not sure if I love 501 for that case, but I'm having a hard time finding a better alternative.
>
> That being said, I feel like I've asked this before, but you can clarify why the existing `LeaderRedirectFilter` is not sufficient for ELB? If I'm reading the [docs](http://docs.aws.amazon.com/ElasticLoadBalancing/latest/DeveloperGuide/elb-healthchecks.html#health-check-configuration) right, it seems like any non-200 reply is sufficient to indicate the instance is not healthy (i.e. not the leader). So given that `LeaderRedirectFilter` already returns a 307 for this case, can we not just use that?
>
> Ashwin Murthy wrote:
> @Bill, @Joshua,
>
> Actually, I changed my mind. The prev one is rather more complex for this simple scenario than needs to be.
>
> How about?:
> Return 200 if instance is leader
> Return 503 (Service unavailablE) for the case when this instance is not the leader but a leader exists
> Return 500 (internal server error) for all other (default) cases since this is truly an error.
>
> Would you be ok with this?
The reason the 307 does not work in all cases is many LBs like HAProxy default to using response code 307 to mean healthy. Some LBs might even case the redirect. Since strictly speakin per HTTP RFC, this seems wrong to do, having a separate endpoint that returns a 500 level code is being pursued.
- Ashwin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126394
-----------------------------------------------------------
On March 31, 2016, 5:06 a.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated March 31, 2016, 5:06 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Joshua Cohen <jc...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126394
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java (line 49)
<https://reviews.apache.org/r/45511/#comment189387>
Can we switch on this enum, rather than if/else branching? If we also add an explicit case for each enum value and reserve the default block for an unexpected status it will allow this code to continue to work as expected even if new values are added to the enum in the future (i.e. as things stand today, if we added a hypothetical state like `LeaderStatus.ALSO_LEADING`, it would fall into the `SERVICE_UNAVAILABLE` branch).
- Joshua Cohen
On March 31, 2016, 5:06 a.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated March 31, 2016, 5:06 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126270
-----------------------------------------------------------
Ship it!
Master (193f17e) 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 31, 2016, 5:06 a.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated March 31, 2016, 5:06 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Ashwin Murthy <as...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/
-----------------------------------------------------------
(Updated March 31, 2016, 5:06 a.m.)
Review request for Aurora and Bill Farner.
Repository: aurora
Description
-------
AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/45511/diff/
Testing
-------
Added new unit test
Thanks,
Ashwin Murthy
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126257
-----------------------------------------------------------
Master (193f17e) is red with this patch.
./build-support/jenkins/build.sh
: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
:compileTestJava/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java:38: Note: Wrote forwarder org.apache.aurora.scheduler.thrift.aop.MockDecoratedThriftForwarder
@Forward(AnnotatedAuroraAdmin.class)
^
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
:processTestResources
:testClasses
:checkstyleTest[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java:22: error: Wrong order for 'java.util.Arrays' import. Order should be: java, javax, scala, com, net, org. Each group should be separated by a single blank line.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java:33: error: File contains a sequence of empty lines.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java:41:51: error: '{' is not preceded with whitespace.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java:44: error: 'lambda arguments' have incorrect indentation level 8, expected level should be 6.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java:65: error: 'method def rcurly' have incorrect indentation level 0, expected level should be 2.
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.html
* 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: 2 mins 39.143 secs
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On March 31, 2016, 3:36 a.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated March 31, 2016, 3:36 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Ashwin Murthy <as...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/
-----------------------------------------------------------
(Updated March 31, 2016, 3:36 a.m.)
Review request for Aurora and Bill Farner.
Repository: aurora
Description
-------
AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/45511/diff/
Testing
-------
Added new unit test
Thanks,
Ashwin Murthy
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/#review126247
-----------------------------------------------------------
Master (193f17e) is red with this patch.
./build-support/jenkins/build.sh
:generateBuildProperties
: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[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:29: error: 'class def modifier' have incorrect indentation level 2, expected level should be 0.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:30: error: 'class def modifier' have incorrect indentation level 2, expected level should be 0.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:32: error: 'member def modifier' have incorrect indentation level 4, expected level should be 2.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:34: error: 'ctor def modifier' have incorrect indentation level 4, expected level should be 2.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:36: error: 'ctor def' child have incorrect indentation level 6, expected level should be 4.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:37: error: 'ctor def rcurly' have incorrect indentation level 4, expected level should be 2.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:49:7: error: 'if' is not followed by whitespace.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:51:5: error: '}' at column 5 should be on the same line as the next part of a multi-block statement.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:52:12: error: 'if' is not followed by whitespace.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java:54:5: error: '}' at column 5 should be on the same line as the next part of a multi-block statement.
FAILED
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':checkstyleMain'.
> Checkstyle rule violations were found. See the report at: file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/checkstyle/main.html
* 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 36.345 secs
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On March 31, 2016, 2:35 a.m., Ashwin Murthy wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45511/
> -----------------------------------------------------------
>
> (Updated March 31, 2016, 2:35 a.m.)
>
>
> Review request for Aurora and Bill Farner.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45511/diff/
>
>
> Testing
> -------
>
> Added new unit test
>
>
> Thanks,
>
> Ashwin Murthy
>
>
Re: Review Request 45511: AURORA-1493: create ELB-friendly endpoint to
detect leading scheduler
Posted by Ashwin Murthy <as...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45511/
-----------------------------------------------------------
(Updated March 31, 2016, 2:35 a.m.)
Review request for Aurora and Bill Farner.
Repository: aurora
Description
-------
AURORA-1493: create ELB-friendly endpoint to detect leading scheduler. The fix is to add a new endpoint - "/leaderhealth" which returns http status code 200 (OK) if the instance is the leader. If the instance is not the leader but a leading exists, returns 500 (Internal server error). If there is no leader at all, returns 503 (Service unavailable)
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/http/LeaderHealth.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/45511/diff/
Testing
-------
Added new unit test
Thanks,
Ashwin Murthy