You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by John Sirois <js...@apache.org> on 2016/04/08 00:32:07 UTC
Review Request 45902: Introduce a Curator-based `ServiceGroupMonitor`.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45902/
-----------------------------------------------------------
Review request for Aurora, Bill Farner and Zameer Manji.
Bugs: AURORA-1468
https://issues.apache.org/jira/browse/AURORA-1468
Repository: aurora
Description
-------
build.gradle | 5 ++
src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java | 94 +++++++++++++++++++++++
src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 304 insertions(+)
Diffs
-----
build.gradle ad5ec5c4afe4dd5c2189d0680692a7409f0417a9
src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/45902/diff/
Testing
-------
Locally green: `./gradlew -Pq build`.
Thanks,
John Sirois
Re: Review Request 45902: Introduce a Curator-based
`ServiceGroupMonitor`.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45902/#review127687
-----------------------------------------------------------
Ship it!
Master (c0cb631) 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 7, 2016, 10:32 p.m., John Sirois wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45902/
> -----------------------------------------------------------
>
> (Updated April 7, 2016, 10:32 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Bugs: AURORA-1468
> https://issues.apache.org/jira/browse/AURORA-1468
>
>
> Repository: aurora
>
>
> Description
> -------
>
> build.gradle | 5 ++
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java | 94 +++++++++++++++++++++++
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 304 insertions(+)
>
>
> Diffs
> -----
>
> build.gradle ad5ec5c4afe4dd5c2189d0680692a7409f0417a9
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45902/diff/
>
>
> Testing
> -------
>
> Locally green: `./gradlew -Pq build`.
>
>
> Thanks,
>
> John Sirois
>
>
Re: Review Request 45902: Introduce a Curator-based
`ServiceGroupMonitor`.
Posted by John Sirois <js...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45902/#review127683
-----------------------------------------------------------
Helpful reference links:
The path cache recipe used: http://curator.apache.org/curator-recipes/path-cache.html
The APIs (for 3.1.0, but all the relevant bits used are the same): http://curator.apache.org/apidocs/index.html
- John Sirois
On April 7, 2016, 4:32 p.m., John Sirois wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45902/
> -----------------------------------------------------------
>
> (Updated April 7, 2016, 4:32 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Bugs: AURORA-1468
> https://issues.apache.org/jira/browse/AURORA-1468
>
>
> Repository: aurora
>
>
> Description
> -------
>
> build.gradle | 5 ++
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java | 94 +++++++++++++++++++++++
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 304 insertions(+)
>
>
> Diffs
> -----
>
> build.gradle ad5ec5c4afe4dd5c2189d0680692a7409f0417a9
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45902/diff/
>
>
> Testing
> -------
>
> Locally green: `./gradlew -Pq build`.
>
>
> Thanks,
>
> John Sirois
>
>
Re: Review Request 45902: Introduce a Curator-based
`ServiceGroupMonitor`.
Posted by John Sirois <js...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45902/#review128046
-----------------------------------------------------------
@ReviewBot retry
- John Sirois
On April 9, 2016, 8:37 p.m., John Sirois wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45902/
> -----------------------------------------------------------
>
> (Updated April 9, 2016, 8:37 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Bugs: AURORA-1468
> https://issues.apache.org/jira/browse/AURORA-1468
>
>
> Repository: aurora
>
>
> Description
> -------
>
> build.gradle | 5 ++
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java | 94 +++++++++++++++++++++++
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 304 insertions(+)
>
>
> Diffs
> -----
>
> build.gradle fc61adfcbaf12c8ea45f71c7efe69d4fe39597c6
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45902/diff/
>
>
> Testing
> -------
>
> Locally green: `./gradlew -Pq build`.
>
>
> Thanks,
>
> John Sirois
>
>
Re: Review Request 45902: Introduce a Curator-based
`ServiceGroupMonitor`.
Posted by John Sirois <js...@apache.org>.
> On April 9, 2016, 8:41 p.m., Aurora ReviewBot wrote:
> > Master (0dd096d) is red with this patch.
> > ./build-support/jenkins/build.sh
> >
> > :buildSrc:compileTestJava UP-TO-DATE
> > :buildSrc:compileTestGroovy UP-TO-DATE
> > :buildSrc:processTestResources UP-TO-DATE
> > :buildSrc:testClasses UP-TO-DATE
> > :buildSrc:test UP-TO-DATE
> > :buildSrc:check UP-TO-DATE
> > :buildSrc:build
> > :clean UP-TO-DATE
> > :api:clean UP-TO-DATE
> > :buildSrc:clean UP-TO-DATE
> > :commons:clean UP-TO-DATE
> > :commons-args:clean UP-TO-DATE
> > :enforceVersion UP-TO-DATE
> > :api:generateThriftJava
> > make: Entering directory `/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift'
> > sha256=$(curl -s --show-error https://archive.apache.org/dist/thrift/0.9.1/thrift-0.9.1.tar.gz | tee thrift-0.9.1.tar.gz | openssl dgst -sha256 | cut -d' ' -f2) && \
> > [ "${sha256}" = "ac175080c8cac567b0331e394f23ac306472c071628396db2850cb00c41b0017" ] && \
> > tar zxvf thrift-0.9.1.tar.gz && \
> > cd thrift-0.9.1 && \
> > sha256=$(curl -s --show-error https://issues.apache.org/jira/secure/attachment/12632477/yylex.patch | tee thrift.patch | openssl dgst -sha256 | cut -d' ' -f2) && \
> > [ "${sha256}" = "70f20b4e5b2e004b8a0d075b80a52750bce5be02ed83efdc60adbc45ec386a6c" ] && \
> > patch -p1 < thrift.patch && \
> > ./configure --disable-dependency-tracking --disable-shared --without-c_glib --without-cpp --without-csharp --without-d --without-erlang --without-go --without-java --without-haskell --without-perl --without-php --without-php_extension --without-pic --without-python --without-qt4 --without-ruby --without-tests && \
> > make -j4
> > make: Leaving directory `/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift'
> > make: *** [thrift-0.9.1/compiler/cpp/thrift] Error 1
> > :api:generateThriftJava FAILED
> >
> > FAILURE: Build failed with an exception.
> >
> > * What went wrong:
> > Execution failed for task ':api:generateThriftJava'.
> > > Process 'command '/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift/thriftw'' finished with non-zero exit value 2
> >
> > * 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: 16.445 secs
> >
> >
> > I will refresh this build result if you post a review containing "@ReviewBot retry"
This is waiting on apache infra maintenance of http://archive.apache.org/ to complete.
- John
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45902/#review128016
-----------------------------------------------------------
On April 9, 2016, 8:37 p.m., John Sirois wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45902/
> -----------------------------------------------------------
>
> (Updated April 9, 2016, 8:37 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Bugs: AURORA-1468
> https://issues.apache.org/jira/browse/AURORA-1468
>
>
> Repository: aurora
>
>
> Description
> -------
>
> build.gradle | 5 ++
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java | 94 +++++++++++++++++++++++
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 304 insertions(+)
>
>
> Diffs
> -----
>
> build.gradle fc61adfcbaf12c8ea45f71c7efe69d4fe39597c6
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45902/diff/
>
>
> Testing
> -------
>
> Locally green: `./gradlew -Pq build`.
>
>
> Thanks,
>
> John Sirois
>
>
Re: Review Request 45902: Introduce a Curator-based
`ServiceGroupMonitor`.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45902/#review128016
-----------------------------------------------------------
Master (0dd096d) is red with this patch.
./build-support/jenkins/build.sh
:buildSrc:compileTestJava UP-TO-DATE
:buildSrc:compileTestGroovy UP-TO-DATE
:buildSrc:processTestResources UP-TO-DATE
:buildSrc:testClasses UP-TO-DATE
:buildSrc:test UP-TO-DATE
:buildSrc:check UP-TO-DATE
:buildSrc:build
:clean UP-TO-DATE
:api:clean UP-TO-DATE
:buildSrc:clean UP-TO-DATE
:commons:clean UP-TO-DATE
:commons-args:clean UP-TO-DATE
:enforceVersion UP-TO-DATE
:api:generateThriftJava
make: Entering directory `/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift'
sha256=$(curl -s --show-error https://archive.apache.org/dist/thrift/0.9.1/thrift-0.9.1.tar.gz | tee thrift-0.9.1.tar.gz | openssl dgst -sha256 | cut -d' ' -f2) && \
[ "${sha256}" = "ac175080c8cac567b0331e394f23ac306472c071628396db2850cb00c41b0017" ] && \
tar zxvf thrift-0.9.1.tar.gz && \
cd thrift-0.9.1 && \
sha256=$(curl -s --show-error https://issues.apache.org/jira/secure/attachment/12632477/yylex.patch | tee thrift.patch | openssl dgst -sha256 | cut -d' ' -f2) && \
[ "${sha256}" = "70f20b4e5b2e004b8a0d075b80a52750bce5be02ed83efdc60adbc45ec386a6c" ] && \
patch -p1 < thrift.patch && \
./configure --disable-dependency-tracking --disable-shared --without-c_glib --without-cpp --without-csharp --without-d --without-erlang --without-go --without-java --without-haskell --without-perl --without-php --without-php_extension --without-pic --without-python --without-qt4 --without-ruby --without-tests && \
make -j4
make: Leaving directory `/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift'
make: *** [thrift-0.9.1/compiler/cpp/thrift] Error 1
:api:generateThriftJava FAILED
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':api:generateThriftJava'.
> Process 'command '/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift/thriftw'' finished with non-zero exit value 2
* 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: 16.445 secs
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On April 10, 2016, 2:37 a.m., John Sirois wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45902/
> -----------------------------------------------------------
>
> (Updated April 10, 2016, 2:37 a.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Bugs: AURORA-1468
> https://issues.apache.org/jira/browse/AURORA-1468
>
>
> Repository: aurora
>
>
> Description
> -------
>
> build.gradle | 5 ++
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java | 94 +++++++++++++++++++++++
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 304 insertions(+)
>
>
> Diffs
> -----
>
> build.gradle fc61adfcbaf12c8ea45f71c7efe69d4fe39597c6
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45902/diff/
>
>
> Testing
> -------
>
> Locally green: `./gradlew -Pq build`.
>
>
> Thanks,
>
> John Sirois
>
>
Re: Review Request 45902: Introduce a Curator-based
`ServiceGroupMonitor`.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45902/#review128162
-----------------------------------------------------------
Ship it!
Ship It!
- Bill Farner
On April 9, 2016, 7:37 p.m., John Sirois wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45902/
> -----------------------------------------------------------
>
> (Updated April 9, 2016, 7:37 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Bugs: AURORA-1468
> https://issues.apache.org/jira/browse/AURORA-1468
>
>
> Repository: aurora
>
>
> Description
> -------
>
> build.gradle | 5 ++
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java | 94 +++++++++++++++++++++++
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 304 insertions(+)
>
>
> Diffs
> -----
>
> build.gradle fc61adfcbaf12c8ea45f71c7efe69d4fe39597c6
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45902/diff/
>
>
> Testing
> -------
>
> Locally green: `./gradlew -Pq build`.
>
>
> Thanks,
>
> John Sirois
>
>
Re: Review Request 45902: Introduce a Curator-based
`ServiceGroupMonitor`.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45902/#review128048
-----------------------------------------------------------
Ship it!
Master (0dd096d) 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 10, 2016, 2:37 a.m., John Sirois wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45902/
> -----------------------------------------------------------
>
> (Updated April 10, 2016, 2:37 a.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Bugs: AURORA-1468
> https://issues.apache.org/jira/browse/AURORA-1468
>
>
> Repository: aurora
>
>
> Description
> -------
>
> build.gradle | 5 ++
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java | 94 +++++++++++++++++++++++
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 304 insertions(+)
>
>
> Diffs
> -----
>
> build.gradle fc61adfcbaf12c8ea45f71c7efe69d4fe39597c6
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45902/diff/
>
>
> Testing
> -------
>
> Locally green: `./gradlew -Pq build`.
>
>
> Thanks,
>
> John Sirois
>
>
Re: Review Request 45902: Introduce a Curator-based
`ServiceGroupMonitor`.
Posted by John Sirois <js...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45902/
-----------------------------------------------------------
(Updated April 9, 2016, 8:37 p.m.)
Review request for Aurora, Bill Farner and Zameer Manji.
Changes
-------
Cleanup CuratorServiceGroupMonitor and test.
src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java | 8 +-------
src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java | 24 +++++++++++-------------
2 files changed, 12 insertions(+), 20 deletions(-)
Bugs: AURORA-1468
https://issues.apache.org/jira/browse/AURORA-1468
Repository: aurora
Description
-------
build.gradle | 5 ++
src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java | 94 +++++++++++++++++++++++
src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 304 insertions(+)
Diffs (updated)
-----
build.gradle fc61adfcbaf12c8ea45f71c7efe69d4fe39597c6
src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/45902/diff/
Testing
-------
Locally green: `./gradlew -Pq build`.
Thanks,
John Sirois
Re: Review Request 45902: Introduce a Curator-based
`ServiceGroupMonitor`.
Posted by John Sirois <js...@apache.org>.
> On April 8, 2016, 11:20 a.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java, line 76
> > <https://reviews.apache.org/r/45902/diff/2/?file=1331376#file1331376line76>
> >
> > This causes the ordering of `eventTypes` to matter, and appears to block forever if the ordering is incorrect.. Is that what you want?
Killed.
> On April 8, 2016, 11:20 a.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java, line 75
> > <https://reviews.apache.org/r/45902/diff/2/?file=1331376#file1331376line75>
> >
> > Varargs is a bit funny here - always used with exactly 1 arg. If you keep varargs, it would be nice to design the method signature to require at least one
Yeah - my use was factored away. Killed.
> On April 8, 2016, 11:20 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java, line 92
> > <https://reviews.apache.org/r/45902/diff/2/?file=1331375#file1331375line92>
> >
> > ditto
Killed.
> On April 8, 2016, 11:20 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java, line 85
> > <https://reviews.apache.org/r/45902/diff/2/?file=1331375#file1331375line85>
> >
> > `groupCache` is always non-null
Yeah - missed clean-up after refactor. Killed.
- John
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45902/#review127817
-----------------------------------------------------------
On April 7, 2016, 5:07 p.m., John Sirois wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45902/
> -----------------------------------------------------------
>
> (Updated April 7, 2016, 5:07 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Bugs: AURORA-1468
> https://issues.apache.org/jira/browse/AURORA-1468
>
>
> Repository: aurora
>
>
> Description
> -------
>
> build.gradle | 5 ++
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java | 94 +++++++++++++++++++++++
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 304 insertions(+)
>
>
> Diffs
> -----
>
> build.gradle ad5ec5c4afe4dd5c2189d0680692a7409f0417a9
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45902/diff/
>
>
> Testing
> -------
>
> Locally green: `./gradlew -Pq build`.
>
>
> Thanks,
>
> John Sirois
>
>
Re: Review Request 45902: Introduce a Curator-based
`ServiceGroupMonitor`.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45902/#review127817
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java (line 85)
<https://reviews.apache.org/r/45902/#comment191196>
`groupCache` is always non-null
src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java (line 92)
<https://reviews.apache.org/r/45902/#comment191197>
ditto
src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java (line 75)
<https://reviews.apache.org/r/45902/#comment191198>
Varargs is a bit funny here - always used with exactly 1 arg. If you keep varargs, it would be nice to design the method signature to require at least one
src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java (line 76)
<https://reviews.apache.org/r/45902/#comment191199>
This causes the ordering of `eventTypes` to matter, and appears to block forever if the ordering is incorrect.. Is that what you want?
- Bill Farner
On April 7, 2016, 4:07 p.m., John Sirois wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45902/
> -----------------------------------------------------------
>
> (Updated April 7, 2016, 4:07 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Bugs: AURORA-1468
> https://issues.apache.org/jira/browse/AURORA-1468
>
>
> Repository: aurora
>
>
> Description
> -------
>
> build.gradle | 5 ++
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java | 94 +++++++++++++++++++++++
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 304 insertions(+)
>
>
> Diffs
> -----
>
> build.gradle ad5ec5c4afe4dd5c2189d0680692a7409f0417a9
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45902/diff/
>
>
> Testing
> -------
>
> Locally green: `./gradlew -Pq build`.
>
>
> Thanks,
>
> John Sirois
>
>
Re: Review Request 45902: Introduce a Curator-based
`ServiceGroupMonitor`.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45902/#review127691
-----------------------------------------------------------
Ship it!
Master (c0cb631) 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 7, 2016, 11:07 p.m., John Sirois wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45902/
> -----------------------------------------------------------
>
> (Updated April 7, 2016, 11:07 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Bugs: AURORA-1468
> https://issues.apache.org/jira/browse/AURORA-1468
>
>
> Repository: aurora
>
>
> Description
> -------
>
> build.gradle | 5 ++
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java | 94 +++++++++++++++++++++++
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 304 insertions(+)
>
>
> Diffs
> -----
>
> build.gradle ad5ec5c4afe4dd5c2189d0680692a7409f0417a9
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45902/diff/
>
>
> Testing
> -------
>
> Locally green: `./gradlew -Pq build`.
>
>
> Thanks,
>
> John Sirois
>
>
Re: Review Request 45902: Introduce a Curator-based
`ServiceGroupMonitor`.
Posted by Zameer Manji <zm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45902/#review127713
-----------------------------------------------------------
Ship it!
Ship It!
- Zameer Manji
On April 7, 2016, 4:07 p.m., John Sirois wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45902/
> -----------------------------------------------------------
>
> (Updated April 7, 2016, 4:07 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Bugs: AURORA-1468
> https://issues.apache.org/jira/browse/AURORA-1468
>
>
> Repository: aurora
>
>
> Description
> -------
>
> build.gradle | 5 ++
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java | 94 +++++++++++++++++++++++
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 304 insertions(+)
>
>
> Diffs
> -----
>
> build.gradle ad5ec5c4afe4dd5c2189d0680692a7409f0417a9
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45902/diff/
>
>
> Testing
> -------
>
> Locally green: `./gradlew -Pq build`.
>
>
> Thanks,
>
> John Sirois
>
>
Re: Review Request 45902: Introduce a Curator-based
`ServiceGroupMonitor`.
Posted by John Sirois <js...@apache.org>.
> On April 7, 2016, 6:42 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java, line 108
> > <https://reviews.apache.org/r/45902/diff/2/?file=1331375#file1331375line108>
> >
> > Shouldn't this be fatal? To me to looks like this exception can only throw if there is a ZK problem `data.getData()` is null, or the data is malformed (bug in a follower).
That's certainly how the commons impl treats it - it throws a RuntimeException. I clearly re-thought that, though did not add a stat. It seemed to me on re-examination that malformed data is not conceptually different from a non-member node name collision. In that case I'd think the right answer would be to proceed and log (and possibly alert); ie: a ~malicious actor made a "member_000000X" node - this presumably shouldn't stop the scheduler.
LMK what you think.
- John
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45902/#review127708
-----------------------------------------------------------
On April 7, 2016, 5:07 p.m., John Sirois wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45902/
> -----------------------------------------------------------
>
> (Updated April 7, 2016, 5:07 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Bugs: AURORA-1468
> https://issues.apache.org/jira/browse/AURORA-1468
>
>
> Repository: aurora
>
>
> Description
> -------
>
> build.gradle | 5 ++
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java | 94 +++++++++++++++++++++++
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 304 insertions(+)
>
>
> Diffs
> -----
>
> build.gradle ad5ec5c4afe4dd5c2189d0680692a7409f0417a9
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45902/diff/
>
>
> Testing
> -------
>
> Locally green: `./gradlew -Pq build`.
>
>
> Thanks,
>
> John Sirois
>
>
Re: Review Request 45902: Introduce a Curator-based
`ServiceGroupMonitor`.
Posted by John Sirois <js...@apache.org>.
> On April 7, 2016, 6:42 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java, line 108
> > <https://reviews.apache.org/r/45902/diff/2/?file=1331375#file1331375line108>
> >
> > Shouldn't this be fatal? To me to looks like this exception can only throw if there is a ZK problem `data.getData()` is null, or the data is malformed (bug in a follower).
>
> John Sirois wrote:
> That's certainly how the commons impl treats it - it throws a RuntimeException. I clearly re-thought that, though did not add a stat. It seemed to me on re-examination that malformed data is not conceptually different from a non-member node name collision. In that case I'd think the right answer would be to proceed and log (and possibly alert); ie: a ~malicious actor made a "member_000000X" node - this presumably shouldn't stop the scheduler.
>
> LMK what you think.
>
> Zameer Manji wrote:
> Your reasoning is solid, I'll drop this issue.
It does deserve a test though, so I'll circle back to add that. It turns out the JSON Codec has been pretty broken for a while wrt its exception contract, so I'd like to fix that in a smaller change.
- John
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45902/#review127708
-----------------------------------------------------------
On April 7, 2016, 5:07 p.m., John Sirois wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45902/
> -----------------------------------------------------------
>
> (Updated April 7, 2016, 5:07 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Bugs: AURORA-1468
> https://issues.apache.org/jira/browse/AURORA-1468
>
>
> Repository: aurora
>
>
> Description
> -------
>
> build.gradle | 5 ++
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java | 94 +++++++++++++++++++++++
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 304 insertions(+)
>
>
> Diffs
> -----
>
> build.gradle ad5ec5c4afe4dd5c2189d0680692a7409f0417a9
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45902/diff/
>
>
> Testing
> -------
>
> Locally green: `./gradlew -Pq build`.
>
>
> Thanks,
>
> John Sirois
>
>
Re: Review Request 45902: Introduce a Curator-based
`ServiceGroupMonitor`.
Posted by Zameer Manji <zm...@apache.org>.
> On April 7, 2016, 5:42 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java, line 108
> > <https://reviews.apache.org/r/45902/diff/2/?file=1331375#file1331375line108>
> >
> > Shouldn't this be fatal? To me to looks like this exception can only throw if there is a ZK problem `data.getData()` is null, or the data is malformed (bug in a follower).
>
> John Sirois wrote:
> That's certainly how the commons impl treats it - it throws a RuntimeException. I clearly re-thought that, though did not add a stat. It seemed to me on re-examination that malformed data is not conceptually different from a non-member node name collision. In that case I'd think the right answer would be to proceed and log (and possibly alert); ie: a ~malicious actor made a "member_000000X" node - this presumably shouldn't stop the scheduler.
>
> LMK what you think.
Your reasoning is solid, I'll drop this issue.
- Zameer
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45902/#review127708
-----------------------------------------------------------
On April 7, 2016, 4:07 p.m., John Sirois wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45902/
> -----------------------------------------------------------
>
> (Updated April 7, 2016, 4:07 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Bugs: AURORA-1468
> https://issues.apache.org/jira/browse/AURORA-1468
>
>
> Repository: aurora
>
>
> Description
> -------
>
> build.gradle | 5 ++
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java | 94 +++++++++++++++++++++++
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 304 insertions(+)
>
>
> Diffs
> -----
>
> build.gradle ad5ec5c4afe4dd5c2189d0680692a7409f0417a9
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45902/diff/
>
>
> Testing
> -------
>
> Locally green: `./gradlew -Pq build`.
>
>
> Thanks,
>
> John Sirois
>
>
Re: Review Request 45902: Introduce a Curator-based
`ServiceGroupMonitor`.
Posted by Zameer Manji <zm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45902/#review127708
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java (line 108)
<https://reviews.apache.org/r/45902/#comment191097>
Shouldn't this be fatal? To me to looks like this exception can only throw if there is a ZK problem `data.getData()` is null, or the data is malformed (bug in a follower).
- Zameer Manji
On April 7, 2016, 4:07 p.m., John Sirois wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45902/
> -----------------------------------------------------------
>
> (Updated April 7, 2016, 4:07 p.m.)
>
>
> Review request for Aurora, Bill Farner and Zameer Manji.
>
>
> Bugs: AURORA-1468
> https://issues.apache.org/jira/browse/AURORA-1468
>
>
> Repository: aurora
>
>
> Description
> -------
>
> build.gradle | 5 ++
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java | 94 +++++++++++++++++++++++
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 304 insertions(+)
>
>
> Diffs
> -----
>
> build.gradle ad5ec5c4afe4dd5c2189d0680692a7409f0417a9
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java PRE-CREATION
> src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/45902/diff/
>
>
> Testing
> -------
>
> Locally green: `./gradlew -Pq build`.
>
>
> Thanks,
>
> John Sirois
>
>
Re: Review Request 45902: Introduce a Curator-based
`ServiceGroupMonitor`.
Posted by John Sirois <js...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45902/
-----------------------------------------------------------
(Updated April 7, 2016, 5:07 p.m.)
Review request for Aurora, Bill Farner and Zameer Manji.
Changes
-------
Document `CuratorServiceGroupMonitor`.
Also tighten the contract of the `memberSelector` `Predicate`, passing
it just the node name and not the full path.
src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java | 20 +++++++++++++++++++-
src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java | 2 +-
2 files changed, 20 insertions(+), 2 deletions(-)
Bugs: AURORA-1468
https://issues.apache.org/jira/browse/AURORA-1468
Repository: aurora
Description
-------
build.gradle | 5 ++
src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java | 94 +++++++++++++++++++++++
src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 304 insertions(+)
Diffs (updated)
-----
build.gradle ad5ec5c4afe4dd5c2189d0680692a7409f0417a9
src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitor.java PRE-CREATION
src/test/java/org/apache/aurora/scheduler/discovery/CuratorServiceGroupMonitorTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/45902/diff/
Testing
-------
Locally green: `./gradlew -Pq build`.
Thanks,
John Sirois