You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Jing Chen <mi...@gmail.com> on 2016/12/10 11:04:58 UTC
Review Request 54624: Expose stats on ZooKeeper connection state
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54624/
-----------------------------------------------------------
Review request for Aurora, Mehrdad Nurolahzade, Stephan Erb, and Zameer Manji.
Bugs: AURORA-1838
https://issues.apache.org/jira/browse/AURORA-1838
Repository: aurora
Description
-------
Expose stats on ZooKeeper connection state
Diffs
-----
src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java 999a542796858dcfe9e31601c47239189043fd52
Diff: https://reviews.apache.org/r/54624/diff/
Testing
-------
State Change is logged into aurora-scheduler.log:
```
I1210 10:44:37.973 [main-EventThread, ConnectionStateManager] State change: CONNECTED
```
Thanks,
Jing Chen
Re: Review Request 54624: Expose stats on ZooKeeper connection state
Posted by Jing Chen <mi...@gmail.com>.
> On Dec. 10, 2016, 8:14 p.m., Joshua Cohen wrote:
> > I think the idea behind the ticket was to track these as stats, not just into the log.
btw, I am not pretty sure if `AtomicLong`s here should be `static` or not
- Jing
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54624/#review158773
-----------------------------------------------------------
On Dec. 12, 2016, 1:32 a.m., Jing Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54624/
> -----------------------------------------------------------
>
> (Updated Dec. 12, 2016, 1:32 a.m.)
>
>
> Review request for Aurora, Joshua Cohen, Mehrdad Nurolahzade, Stephan Erb, and Zameer Manji.
>
>
> Bugs: AURORA-1838
> https://issues.apache.org/jira/browse/AURORA-1838
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Expose stats on ZooKeeper connection state
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java 999a542796858dcfe9e31601c47239189043fd52
>
> Diff: https://reviews.apache.org/r/54624/diff/
>
>
> Testing
> -------
>
> http://192.168.33.7:8081/vars
> ```
> zk_connection_state_connected 1
> zk_connection_state_lost 0
> zk_connection_state_readonly 0
> zk_connection_state_reconnected 0
> zk_connection_state_suspended 0
> zk_connection_state_unknown 0
> ```
>
>
> Thanks,
>
> Jing Chen
>
>
Re: Review Request 54624: Expose stats on ZooKeeper connection state
Posted by Joshua Cohen <jc...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54624/#review158773
-----------------------------------------------------------
I think the idea behind the ticket was to track these as stats, not just into the log.
- Joshua Cohen
On Dec. 10, 2016, 11:04 a.m., Jing Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54624/
> -----------------------------------------------------------
>
> (Updated Dec. 10, 2016, 11:04 a.m.)
>
>
> Review request for Aurora, Mehrdad Nurolahzade, Stephan Erb, and Zameer Manji.
>
>
> Bugs: AURORA-1838
> https://issues.apache.org/jira/browse/AURORA-1838
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Expose stats on ZooKeeper connection state
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java 999a542796858dcfe9e31601c47239189043fd52
>
> Diff: https://reviews.apache.org/r/54624/diff/
>
>
> Testing
> -------
>
> State Change is logged into aurora-scheduler.log:
> ```
> I1210 10:44:37.973 [main-EventThread, ConnectionStateManager] State change: CONNECTED
> ```
>
>
> Thanks,
>
> Jing Chen
>
>
Re: Review Request 54624: Expose stats on ZooKeeper connection state
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54624/#review158753
-----------------------------------------------------------
Master (c8e8953) is green with this patch.
./build-support/jenkins/build.sh
However, it appears that it might lack test coverage.
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On Dec. 10, 2016, 11:04 a.m., Jing Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54624/
> -----------------------------------------------------------
>
> (Updated Dec. 10, 2016, 11:04 a.m.)
>
>
> Review request for Aurora, Mehrdad Nurolahzade, Stephan Erb, and Zameer Manji.
>
>
> Bugs: AURORA-1838
> https://issues.apache.org/jira/browse/AURORA-1838
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Expose stats on ZooKeeper connection state
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java 999a542796858dcfe9e31601c47239189043fd52
>
> Diff: https://reviews.apache.org/r/54624/diff/
>
>
> Testing
> -------
>
> State Change is logged into aurora-scheduler.log:
> ```
> I1210 10:44:37.973 [main-EventThread, ConnectionStateManager] State change: CONNECTED
> ```
>
>
> Thanks,
>
> Jing Chen
>
>
Re: Review Request 54624: Expose stats on ZooKeeper connection state
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54624/#review158814
-----------------------------------------------------------
Master (c8e8953) is green with this patch.
./build-support/jenkins/build.sh
However, it appears that it might lack test coverage.
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On Dec. 12, 2016, 1:32 a.m., Jing Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54624/
> -----------------------------------------------------------
>
> (Updated Dec. 12, 2016, 1:32 a.m.)
>
>
> Review request for Aurora, Joshua Cohen, Mehrdad Nurolahzade, Stephan Erb, and Zameer Manji.
>
>
> Bugs: AURORA-1838
> https://issues.apache.org/jira/browse/AURORA-1838
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Expose stats on ZooKeeper connection state
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java 999a542796858dcfe9e31601c47239189043fd52
>
> Diff: https://reviews.apache.org/r/54624/diff/
>
>
> Testing
> -------
>
> http://192.168.33.7:8081/vars
> ```
> zk_connection_state_connected 1
> zk_connection_state_lost 0
> zk_connection_state_readonly 0
> zk_connection_state_reconnected 0
> zk_connection_state_suspended 0
> zk_connection_state_unknown 0
> ```
>
>
> Thanks,
>
> Jing Chen
>
>
Re: Review Request 54624: Expose stats on ZooKeeper connection state
Posted by David McLaughlin <da...@dmclaughlin.com>.
> On Dec. 12, 2016, 10:15 p.m., Mehrdad Nurolahzade wrote:
> > src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java, lines 135-151
> > <https://reviews.apache.org/r/54624/diff/2/?file=1581612#file1581612line135>
> >
> > I would rather prefer binary state (0/1 value) to counter-like stats here. See `org.apache.aurora.scheduler.SchedulerLifecycle` as an example.
>
> Stephan Erb wrote:
> Binary state gauges have the disadvantage that they only show the presence. This would imply we need to scrape `/vars` pretty often in order to detect ZK connection problems. Counters don't have that limitation.
>
> Mehrdad Nurolahzade wrote:
> I agree. Perhaps I am baised here because we already have a solution for polling and diagraming stats internally.
> So, in your world, how are going to use counters to contemplate current state?
We want both. A Gauge stat for current connection state and absolute counters for the events (as they are now).
- David
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54624/#review158871
-----------------------------------------------------------
On Dec. 12, 2016, 1:32 a.m., Jing Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54624/
> -----------------------------------------------------------
>
> (Updated Dec. 12, 2016, 1:32 a.m.)
>
>
> Review request for Aurora, Joshua Cohen, Mehrdad Nurolahzade, Stephan Erb, and Zameer Manji.
>
>
> Bugs: AURORA-1838
> https://issues.apache.org/jira/browse/AURORA-1838
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Expose stats on ZooKeeper connection state
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java 999a542796858dcfe9e31601c47239189043fd52
>
> Diff: https://reviews.apache.org/r/54624/diff/
>
>
> Testing
> -------
>
> http://192.168.33.7:8081/vars
> ```
> zk_connection_state_connected 1
> zk_connection_state_lost 0
> zk_connection_state_readonly 0
> zk_connection_state_reconnected 0
> zk_connection_state_suspended 0
> zk_connection_state_unknown 0
> ```
>
>
> Thanks,
>
> Jing Chen
>
>
Re: Review Request 54624: Expose stats on ZooKeeper connection state
Posted by Jing Chen <mi...@gmail.com>.
> On Dec. 12, 2016, 10:15 p.m., Mehrdad Nurolahzade wrote:
> > src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java, lines 135-151
> > <https://reviews.apache.org/r/54624/diff/2/?file=1581612#file1581612line135>
> >
> > I would rather prefer binary state (0/1 value) to counter-like stats here. See `org.apache.aurora.scheduler.SchedulerLifecycle` as an example.
>
> Stephan Erb wrote:
> Binary state gauges have the disadvantage that they only show the presence. This would imply we need to scrape `/vars` pretty often in order to detect ZK connection problems. Counters don't have that limitation.
>
> Mehrdad Nurolahzade wrote:
> I agree. Perhaps I am baised here because we already have a solution for polling and diagraming stats internally.
> So, in your world, how are going to use counters to contemplate current state?
>
> David McLaughlin wrote:
> We want both. A Gauge stat for current connection state and absolute counters for the events (as they are now).
AFAIC, it is easier to know how many state chagnes happened from a counter, can you tell me what we are able to get from the current state. Thanks
- Jing
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54624/#review158871
-----------------------------------------------------------
On Dec. 12, 2016, 1:32 a.m., Jing Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54624/
> -----------------------------------------------------------
>
> (Updated Dec. 12, 2016, 1:32 a.m.)
>
>
> Review request for Aurora, Joshua Cohen, Mehrdad Nurolahzade, Stephan Erb, and Zameer Manji.
>
>
> Bugs: AURORA-1838
> https://issues.apache.org/jira/browse/AURORA-1838
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Expose stats on ZooKeeper connection state
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java 999a542796858dcfe9e31601c47239189043fd52
>
> Diff: https://reviews.apache.org/r/54624/diff/
>
>
> Testing
> -------
>
> http://192.168.33.7:8081/vars
> ```
> zk_connection_state_connected 1
> zk_connection_state_lost 0
> zk_connection_state_readonly 0
> zk_connection_state_reconnected 0
> zk_connection_state_suspended 0
> zk_connection_state_unknown 0
> ```
>
>
> Thanks,
>
> Jing Chen
>
>
Re: Review Request 54624: Expose stats on ZooKeeper connection state
Posted by Stephan Erb <se...@apache.org>.
> On Dec. 12, 2016, 11:15 p.m., Mehrdad Nurolahzade wrote:
> > src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java, lines 135-151
> > <https://reviews.apache.org/r/54624/diff/2/?file=1581612#file1581612line135>
> >
> > I would rather prefer binary state (0/1 value) to counter-like stats here. See `org.apache.aurora.scheduler.SchedulerLifecycle` as an example.
Binary state gauges have the disadvantage that they only show the presence. This would imply we need to scrape `/vars` pretty often in order to detect ZK connection problems. Counters don't have that limitation.
- Stephan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54624/#review158871
-----------------------------------------------------------
On Dec. 12, 2016, 2:32 a.m., Jing Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54624/
> -----------------------------------------------------------
>
> (Updated Dec. 12, 2016, 2:32 a.m.)
>
>
> Review request for Aurora, Joshua Cohen, Mehrdad Nurolahzade, Stephan Erb, and Zameer Manji.
>
>
> Bugs: AURORA-1838
> https://issues.apache.org/jira/browse/AURORA-1838
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Expose stats on ZooKeeper connection state
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java 999a542796858dcfe9e31601c47239189043fd52
>
> Diff: https://reviews.apache.org/r/54624/diff/
>
>
> Testing
> -------
>
> http://192.168.33.7:8081/vars
> ```
> zk_connection_state_connected 1
> zk_connection_state_lost 0
> zk_connection_state_readonly 0
> zk_connection_state_reconnected 0
> zk_connection_state_suspended 0
> zk_connection_state_unknown 0
> ```
>
>
> Thanks,
>
> Jing Chen
>
>
Re: Review Request 54624: Expose stats on ZooKeeper connection state
Posted by Mehrdad Nurolahzade <me...@nurolahzade.com>.
> On Dec. 12, 2016, 2:15 p.m., Mehrdad Nurolahzade wrote:
> > src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java, lines 135-151
> > <https://reviews.apache.org/r/54624/diff/2/?file=1581612#file1581612line135>
> >
> > I would rather prefer binary state (0/1 value) to counter-like stats here. See `org.apache.aurora.scheduler.SchedulerLifecycle` as an example.
>
> Stephan Erb wrote:
> Binary state gauges have the disadvantage that they only show the presence. This would imply we need to scrape `/vars` pretty often in order to detect ZK connection problems. Counters don't have that limitation.
I agree. Perhaps I am baised here because we already have a solution for polling and diagraming stats internally.
So, in your world, how are going to use counters to contemplate current state?
- Mehrdad
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54624/#review158871
-----------------------------------------------------------
On Dec. 11, 2016, 5:32 p.m., Jing Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54624/
> -----------------------------------------------------------
>
> (Updated Dec. 11, 2016, 5:32 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Mehrdad Nurolahzade, Stephan Erb, and Zameer Manji.
>
>
> Bugs: AURORA-1838
> https://issues.apache.org/jira/browse/AURORA-1838
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Expose stats on ZooKeeper connection state
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java 999a542796858dcfe9e31601c47239189043fd52
>
> Diff: https://reviews.apache.org/r/54624/diff/
>
>
> Testing
> -------
>
> http://192.168.33.7:8081/vars
> ```
> zk_connection_state_connected 1
> zk_connection_state_lost 0
> zk_connection_state_readonly 0
> zk_connection_state_reconnected 0
> zk_connection_state_suspended 0
> zk_connection_state_unknown 0
> ```
>
>
> Thanks,
>
> Jing Chen
>
>
Re: Review Request 54624: Expose stats on ZooKeeper connection state
Posted by Mehrdad Nurolahzade <me...@nurolahzade.com>.
> On Dec. 12, 2016, 2:15 p.m., Mehrdad Nurolahzade wrote:
> > src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java, lines 135-151
> > <https://reviews.apache.org/r/54624/diff/2/?file=1581612#file1581612line135>
> >
> > I would rather prefer binary state (0/1 value) to counter-like stats here. See `org.apache.aurora.scheduler.SchedulerLifecycle` as an example.
>
> Stephan Erb wrote:
> Binary state gauges have the disadvantage that they only show the presence. This would imply we need to scrape `/vars` pretty often in order to detect ZK connection problems. Counters don't have that limitation.
>
> Mehrdad Nurolahzade wrote:
> I agree. Perhaps I am baised here because we already have a solution for polling and diagraming stats internally.
> So, in your world, how are going to use counters to contemplate current state?
>
> David McLaughlin wrote:
> We want both. A Gauge stat for current connection state and absolute counters for the events (as they are now).
>
> Jing Chen wrote:
> AFAIC, it is easier to know how many state chagnes happened from a counter, can you tell me what we are able to get from the current state. Thanks
Binary stats would give us connection state at sample times; like Stephan said above this implies that `/vars` is being sampled regularly, for example every second (that's what we do internally).
- Mehrdad
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54624/#review158871
-----------------------------------------------------------
On Dec. 11, 2016, 5:32 p.m., Jing Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54624/
> -----------------------------------------------------------
>
> (Updated Dec. 11, 2016, 5:32 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Mehrdad Nurolahzade, Stephan Erb, and Zameer Manji.
>
>
> Bugs: AURORA-1838
> https://issues.apache.org/jira/browse/AURORA-1838
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Expose stats on ZooKeeper connection state
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java 999a542796858dcfe9e31601c47239189043fd52
>
> Diff: https://reviews.apache.org/r/54624/diff/
>
>
> Testing
> -------
>
> http://192.168.33.7:8081/vars
> ```
> zk_connection_state_connected 1
> zk_connection_state_lost 0
> zk_connection_state_readonly 0
> zk_connection_state_reconnected 0
> zk_connection_state_suspended 0
> zk_connection_state_unknown 0
> ```
>
>
> Thanks,
>
> Jing Chen
>
>
Re: Review Request 54624: Expose stats on ZooKeeper connection state
Posted by Mehrdad Nurolahzade <me...@nurolahzade.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54624/#review158871
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java (lines 66 - 75)
<https://reviews.apache.org/r/54624/#comment229726>
Nit: You could have used a constant String prefix (`zk_connection_state_`) + `state.name()` for naming stats here.
src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java (line 67)
<https://reviews.apache.org/r/54624/#comment229687>
`zk_connection_state_connected`?
src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java (lines 135 - 151)
<https://reviews.apache.org/r/54624/#comment229728>
I would rather prefer binary state (0/1 value) to counter-like stats here. See `org.apache.aurora.scheduler.SchedulerLifecycle` as an example.
- Mehrdad Nurolahzade
On Dec. 11, 2016, 5:32 p.m., Jing Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54624/
> -----------------------------------------------------------
>
> (Updated Dec. 11, 2016, 5:32 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Mehrdad Nurolahzade, Stephan Erb, and Zameer Manji.
>
>
> Bugs: AURORA-1838
> https://issues.apache.org/jira/browse/AURORA-1838
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Expose stats on ZooKeeper connection state
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java 999a542796858dcfe9e31601c47239189043fd52
>
> Diff: https://reviews.apache.org/r/54624/diff/
>
>
> Testing
> -------
>
> http://192.168.33.7:8081/vars
> ```
> zk_connection_state_connected 1
> zk_connection_state_lost 0
> zk_connection_state_readonly 0
> zk_connection_state_reconnected 0
> zk_connection_state_suspended 0
> zk_connection_state_unknown 0
> ```
>
>
> Thanks,
>
> Jing Chen
>
>
Re: Review Request 54624: Expose stats on ZooKeeper connection state
Posted by Stephan Erb <se...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54624/#review158904
-----------------------------------------------------------
Ship it!
LGTM! Thanks
Please make sure you mention the new states in the RELEASE-NOTES.md.
- Stephan Erb
On Dec. 12, 2016, 2:32 a.m., Jing Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54624/
> -----------------------------------------------------------
>
> (Updated Dec. 12, 2016, 2:32 a.m.)
>
>
> Review request for Aurora, Joshua Cohen, Mehrdad Nurolahzade, Stephan Erb, and Zameer Manji.
>
>
> Bugs: AURORA-1838
> https://issues.apache.org/jira/browse/AURORA-1838
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Expose stats on ZooKeeper connection state
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java 999a542796858dcfe9e31601c47239189043fd52
>
> Diff: https://reviews.apache.org/r/54624/diff/
>
>
> Testing
> -------
>
> http://192.168.33.7:8081/vars
> ```
> zk_connection_state_connected 1
> zk_connection_state_lost 0
> zk_connection_state_readonly 0
> zk_connection_state_reconnected 0
> zk_connection_state_suspended 0
> zk_connection_state_unknown 0
> ```
>
>
> Thanks,
>
> Jing Chen
>
>
Re: Review Request 54624: Expose stats on ZooKeeper connection state
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54624/#review160473
-----------------------------------------------------------
Ship it!
Master (21ad18e) 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 Jan. 4, 2017, 9:33 a.m., Jing Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54624/
> -----------------------------------------------------------
>
> (Updated Jan. 4, 2017, 9:33 a.m.)
>
>
> Review request for Aurora, Joshua Cohen, Mehrdad Nurolahzade, Stephan Erb, and Zameer Manji.
>
>
> Bugs: AURORA-1838
> https://issues.apache.org/jira/browse/AURORA-1838
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Expose stats on ZooKeeper connection state
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java 999a542796858dcfe9e31601c47239189043fd52
> src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java d90192b10250b36c3b97055f87ff297fdf2a7dad
>
> Diff: https://reviews.apache.org/r/54624/diff/
>
>
> Testing
> -------
>
> http://192.168.33.7:8081/vars
> ```
> zk_connection_state_CONNECTED 1
> zk_connection_state_CONNECTED_counter 1
> zk_connection_state_LOST 0
> zk_connection_state_LOST_counter 0
> zk_connection_state_READ_ONLY 0
> zk_connection_state_READ_ONLY_counter 0
> zk_connection_state_RECONNECTED 0
> zk_connection_state_RECONNECTED_counter 0
> zk_connection_state_SUSPENDED 0
> zk_connection_state_SUSPENDED_counter 0
> ```
>
> * zk_connection_state_STATE shows 1 if STATE is current connection state, otherwise 0.
> * zk_connection_state_STATE_counter represents occurence times of the STATE since scheduler state
>
>
> Thanks,
>
> Jing Chen
>
>
Re: Review Request 54624: Expose stats on ZooKeeper connection state
Posted by Jing Chen <mi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54624/
-----------------------------------------------------------
(Updated Jan. 4, 2017, 9:33 a.m.)
Review request for Aurora, Joshua Cohen, Mehrdad Nurolahzade, Stephan Erb, and Zameer Manji.
Changes
-------
Injecting `StatsProvider` via provides methods
Bugs: AURORA-1838
https://issues.apache.org/jira/browse/AURORA-1838
Repository: aurora
Description
-------
Expose stats on ZooKeeper connection state
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java 999a542796858dcfe9e31601c47239189043fd52
src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java d90192b10250b36c3b97055f87ff297fdf2a7dad
Diff: https://reviews.apache.org/r/54624/diff/
Testing
-------
http://192.168.33.7:8081/vars
```
zk_connection_state_CONNECTED 1
zk_connection_state_CONNECTED_counter 1
zk_connection_state_LOST 0
zk_connection_state_LOST_counter 0
zk_connection_state_READ_ONLY 0
zk_connection_state_READ_ONLY_counter 0
zk_connection_state_RECONNECTED 0
zk_connection_state_RECONNECTED_counter 0
zk_connection_state_SUSPENDED 0
zk_connection_state_SUSPENDED_counter 0
```
* zk_connection_state_STATE shows 1 if STATE is current connection state, otherwise 0.
* zk_connection_state_STATE_counter represents occurence times of the STATE since scheduler state
Thanks,
Jing Chen
Re: Review Request 54624: Expose stats on ZooKeeper connection state
Posted by Mehrdad Nurolahzade <me...@nurolahzade.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54624/#review159726
-----------------------------------------------------------
Ship it!
Ship It!
- Mehrdad Nurolahzade
On Dec. 20, 2016, 1:33 a.m., Jing Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54624/
> -----------------------------------------------------------
>
> (Updated Dec. 20, 2016, 1:33 a.m.)
>
>
> Review request for Aurora, Joshua Cohen, Mehrdad Nurolahzade, Stephan Erb, and Zameer Manji.
>
>
> Bugs: AURORA-1838
> https://issues.apache.org/jira/browse/AURORA-1838
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Expose stats on ZooKeeper connection state
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java 999a542796858dcfe9e31601c47239189043fd52
>
> Diff: https://reviews.apache.org/r/54624/diff/
>
>
> Testing
> -------
>
> http://192.168.33.7:8081/vars
> ```
> zk_connection_state_CONNECTED 1
> zk_connection_state_CONNECTED_counter 1
> zk_connection_state_LOST 0
> zk_connection_state_LOST_counter 0
> zk_connection_state_READ_ONLY 0
> zk_connection_state_READ_ONLY_counter 0
> zk_connection_state_RECONNECTED 0
> zk_connection_state_RECONNECTED_counter 0
> zk_connection_state_SUSPENDED 0
> zk_connection_state_SUSPENDED_counter 0
> ```
>
> * zk_connection_state_STATE shows 1 if STATE is current connection state, otherwise 0.
> * zk_connection_state_STATE_counter represents occurence times of the STATE since scheduler state
>
>
> Thanks,
>
> Jing Chen
>
>
Re: Review Request 54624: Expose stats on ZooKeeper connection state
Posted by Jing Chen <mi...@gmail.com>.
> On Dec. 27, 2016, 5:33 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java, lines 68-77
> > <https://reviews.apache.org/r/54624/diff/3/?file=1589427#file1589427line68>
> >
> > It's prefereble to inject the `StatsProvider` instance and use that to create the counters in the constructor rather than to reference `Stats.STATS_PROVIDER` directly (if we ever bound another instance of `StatsProvider` in `AppModule` this class would not be aware of that, nor would we be able to use a mock stats provider for tests if we so desired.
is it a good idea to inject `StatsProvider` to the constructor in this case? Would it be better if `StatsProvide` were injected into provides methods and initializes counters?
- Jing
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54624/#review160175
-----------------------------------------------------------
On Dec. 20, 2016, 9:33 a.m., Jing Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54624/
> -----------------------------------------------------------
>
> (Updated Dec. 20, 2016, 9:33 a.m.)
>
>
> Review request for Aurora, Joshua Cohen, Mehrdad Nurolahzade, Stephan Erb, and Zameer Manji.
>
>
> Bugs: AURORA-1838
> https://issues.apache.org/jira/browse/AURORA-1838
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Expose stats on ZooKeeper connection state
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java 999a542796858dcfe9e31601c47239189043fd52
>
> Diff: https://reviews.apache.org/r/54624/diff/
>
>
> Testing
> -------
>
> http://192.168.33.7:8081/vars
> ```
> zk_connection_state_CONNECTED 1
> zk_connection_state_CONNECTED_counter 1
> zk_connection_state_LOST 0
> zk_connection_state_LOST_counter 0
> zk_connection_state_READ_ONLY 0
> zk_connection_state_READ_ONLY_counter 0
> zk_connection_state_RECONNECTED 0
> zk_connection_state_RECONNECTED_counter 0
> zk_connection_state_SUSPENDED 0
> zk_connection_state_SUSPENDED_counter 0
> ```
>
> * zk_connection_state_STATE shows 1 if STATE is current connection state, otherwise 0.
> * zk_connection_state_STATE_counter represents occurence times of the STATE since scheduler state
>
>
> Thanks,
>
> Jing Chen
>
>
Re: Review Request 54624: Expose stats on ZooKeeper connection state
Posted by Joshua Cohen <jc...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54624/#review160175
-----------------------------------------------------------
Fix it, then Ship it!
lgtm modulo the below.
src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java (lines 68 - 77)
<https://reviews.apache.org/r/54624/#comment231247>
It's prefereble to inject the `StatsProvider` instance and use that to create the counters in the constructor rather than to reference `Stats.STATS_PROVIDER` directly (if we ever bound another instance of `StatsProvider` in `AppModule` this class would not be aware of that, nor would we be able to use a mock stats provider for tests if we so desired.
- Joshua Cohen
On Dec. 20, 2016, 9:33 a.m., Jing Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54624/
> -----------------------------------------------------------
>
> (Updated Dec. 20, 2016, 9:33 a.m.)
>
>
> Review request for Aurora, Joshua Cohen, Mehrdad Nurolahzade, Stephan Erb, and Zameer Manji.
>
>
> Bugs: AURORA-1838
> https://issues.apache.org/jira/browse/AURORA-1838
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Expose stats on ZooKeeper connection state
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java 999a542796858dcfe9e31601c47239189043fd52
>
> Diff: https://reviews.apache.org/r/54624/diff/
>
>
> Testing
> -------
>
> http://192.168.33.7:8081/vars
> ```
> zk_connection_state_CONNECTED 1
> zk_connection_state_CONNECTED_counter 1
> zk_connection_state_LOST 0
> zk_connection_state_LOST_counter 0
> zk_connection_state_READ_ONLY 0
> zk_connection_state_READ_ONLY_counter 0
> zk_connection_state_RECONNECTED 0
> zk_connection_state_RECONNECTED_counter 0
> zk_connection_state_SUSPENDED 0
> zk_connection_state_SUSPENDED_counter 0
> ```
>
> * zk_connection_state_STATE shows 1 if STATE is current connection state, otherwise 0.
> * zk_connection_state_STATE_counter represents occurence times of the STATE since scheduler state
>
>
> Thanks,
>
> Jing Chen
>
>
Re: Review Request 54624: Expose stats on ZooKeeper connection state
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54624/#review159690
-----------------------------------------------------------
Master (38b9311) is green with this patch.
./build-support/jenkins/build.sh
However, it appears that it might lack test coverage.
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On Dec. 20, 2016, 9:33 a.m., Jing Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54624/
> -----------------------------------------------------------
>
> (Updated Dec. 20, 2016, 9:33 a.m.)
>
>
> Review request for Aurora, Joshua Cohen, Mehrdad Nurolahzade, Stephan Erb, and Zameer Manji.
>
>
> Bugs: AURORA-1838
> https://issues.apache.org/jira/browse/AURORA-1838
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Expose stats on ZooKeeper connection state
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java 999a542796858dcfe9e31601c47239189043fd52
>
> Diff: https://reviews.apache.org/r/54624/diff/
>
>
> Testing
> -------
>
> http://192.168.33.7:8081/vars
> ```
> zk_connection_state_CONNECTED 1
> zk_connection_state_CONNECTED_counter 1
> zk_connection_state_LOST 0
> zk_connection_state_LOST_counter 0
> zk_connection_state_READ_ONLY 0
> zk_connection_state_READ_ONLY_counter 0
> zk_connection_state_RECONNECTED 0
> zk_connection_state_RECONNECTED_counter 0
> zk_connection_state_SUSPENDED 0
> zk_connection_state_SUSPENDED_counter 0
> ```
>
> * zk_connection_state_STATE shows 1 if STATE is current connection state, otherwise 0.
> * zk_connection_state_STATE_counter represents occurence times of the STATE since scheduler state
>
>
> Thanks,
>
> Jing Chen
>
>
Re: Review Request 54624: Expose stats on ZooKeeper connection state
Posted by Jing Chen <mi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54624/
-----------------------------------------------------------
(Updated Dec. 20, 2016, 9:33 a.m.)
Review request for Aurora, Joshua Cohen, Mehrdad Nurolahzade, Stephan Erb, and Zameer Manji.
Changes
-------
per review feedback, add binary gauge to export current connection state, keep counters to keep track of state appearance times
Bugs: AURORA-1838
https://issues.apache.org/jira/browse/AURORA-1838
Repository: aurora
Description
-------
Expose stats on ZooKeeper connection state
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java 999a542796858dcfe9e31601c47239189043fd52
Diff: https://reviews.apache.org/r/54624/diff/
Testing (updated)
-------
http://192.168.33.7:8081/vars
```
zk_connection_state_CONNECTED 1
zk_connection_state_CONNECTED_counter 1
zk_connection_state_LOST 0
zk_connection_state_LOST_counter 0
zk_connection_state_READ_ONLY 0
zk_connection_state_READ_ONLY_counter 0
zk_connection_state_RECONNECTED 0
zk_connection_state_RECONNECTED_counter 0
zk_connection_state_SUSPENDED 0
zk_connection_state_SUSPENDED_counter 0
```
* zk_connection_state_STATE shows 1 if STATE is current connection state, otherwise 0.
* zk_connection_state_STATE_counter represents occurence times of the STATE since scheduler state
Thanks,
Jing Chen
Re: Review Request 54624: Expose stats on ZooKeeper connection state
Posted by Jing Chen <mi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54624/
-----------------------------------------------------------
(Updated Dec. 12, 2016, 1:32 a.m.)
Review request for Aurora, Joshua Cohen, Mehrdad Nurolahzade, Stephan Erb, and Zameer Manji.
Bugs: AURORA-1838
https://issues.apache.org/jira/browse/AURORA-1838
Repository: aurora
Description
-------
Expose stats on ZooKeeper connection state
Diffs
-----
src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java 999a542796858dcfe9e31601c47239189043fd52
Diff: https://reviews.apache.org/r/54624/diff/
Testing (updated)
-------
http://192.168.33.7:8081/vars
```
zk_connection_state_connected 1
zk_connection_state_lost 0
zk_connection_state_readonly 0
zk_connection_state_reconnected 0
zk_connection_state_suspended 0
zk_connection_state_unknown 0
```
Thanks,
Jing Chen
Re: Review Request 54624: Expose stats on ZooKeeper connection state
Posted by Jing Chen <mi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54624/
-----------------------------------------------------------
(Updated Dec. 12, 2016, 1:28 a.m.)
Review request for Aurora, Mehrdad Nurolahzade, Stephan Erb, and Zameer Manji.
Changes
-------
expose stats on connection state changes
Bugs: AURORA-1838
https://issues.apache.org/jira/browse/AURORA-1838
Repository: aurora
Description
-------
Expose stats on ZooKeeper connection state
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java 999a542796858dcfe9e31601c47239189043fd52
Diff: https://reviews.apache.org/r/54624/diff/
Testing
-------
State Change is logged into aurora-scheduler.log:
```
I1210 10:44:37.973 [main-EventThread, ConnectionStateManager] State change: CONNECTED
```
Thanks,
Jing Chen