You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hbase.apache.org by Andrew Purtell <ap...@apache.org> on 2010/10/18 23:46:30 UTC
Review Request: [rest] publish endpoint and statistics into ZooKeeper
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1039/
-----------------------------------------------------------
Review request for hbase.
Summary
-------
This change allows the REST interface to publish its endpoint and metrics, currently only requests/sec, into ZooKeeper. By default a permanent znode tree is created as needed at /hbase/rest/status and Stargate instances create ephemeral children of this with names in the format <host>:<port>. The ephemeral znodes contain JSON serialized information about the instance, e.g.
{"connector":{"host":"restserver.example.com","port":"8080"},"statistics":{"requests":"13"}}
The function of Stargate itself is not affected, except for one significant change: now if the ZooKeeper service is lost, the Stargate instances will abort along with the rest of HBase.
This addresses bug HBASE-3119.
http://issues.apache.org/jira/browse/HBASE-3119
Diffs
-----
src/main/java/org/apache/hadoop/hbase/HConstants.java 71c3e7b
src/main/java/org/apache/hadoop/hbase/rest/Main.java 368b4b4
src/main/java/org/apache/hadoop/hbase/rest/RESTServlet.java ed92857
src/main/java/org/apache/hadoop/hbase/rest/metrics/RESTMetrics.java 284bbc5
Diff: http://review.cloudera.org/r/1039/diff
Testing
-------
Thanks,
Andrew
Re: Review Request: [rest] publish endpoint and statistics into ZooKeeper
Posted by Jonathan Gray <jg...@apache.org>.
> On 2010-10-18 15:34:45, Ryan Rawson wrote:
> > src/main/java/org/apache/hadoop/hbase/rest/RESTServlet.java, line 217
> > <http://review.cloudera.org/r/1039/diff/1/?file=14874#file14874line217>
> >
> > this looks clever, is it more generically useful to other parts of hbase?
>
> Andrew Purtell wrote:
> Other parts of HBase use ZKW methods to do this. I brought this in here to do the same without pulling in all of the behavior of ZKW I didn't want.
Which behaviors of ZKW are you referring to? Hopefully this component is generally reusable (the new ZooKeeperWatcher) and could be used even in limited contexts. Using it as the primary watcher and registering with it also helps when writing unit test. You'd then use ZKUtil methods for this kind of stuff and inherit work done there.
We are going to need one more level underneath ZKUtil or underlying ZKUtil that manages retry policies and such. I'm going to target that for 0.92. And if all our code uses these APIs then it will be easier to be consistent.
The patch looks fine to me though so we can work at unifying later and not blocking you on this.
- Jonathan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1039/#review1560
-----------------------------------------------------------
On 2010-10-18 14:46:30, Andrew Purtell wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/1039/
> -----------------------------------------------------------
>
> (Updated 2010-10-18 14:46:30)
>
>
> Review request for hbase.
>
>
> Summary
> -------
>
> This change allows the REST interface to publish its endpoint and metrics, currently only requests/sec, into ZooKeeper. By default a permanent znode tree is created as needed at /hbase/rest/status and Stargate instances create ephemeral children of this with names in the format <host>:<port>. The ephemeral znodes contain JSON serialized information about the instance, e.g.
>
> {"connector":{"host":"restserver.example.com","port":"8080"},"statistics":{"requests":"13"}}
>
> The function of Stargate itself is not affected, except for one significant change: now if the ZooKeeper service is lost, the Stargate instances will abort along with the rest of HBase.
>
>
> This addresses bug HBASE-3119.
> http://issues.apache.org/jira/browse/HBASE-3119
>
>
> Diffs
> -----
>
> src/main/java/org/apache/hadoop/hbase/HConstants.java 71c3e7b
> src/main/java/org/apache/hadoop/hbase/rest/Main.java 368b4b4
> src/main/java/org/apache/hadoop/hbase/rest/RESTServlet.java ed92857
> src/main/java/org/apache/hadoop/hbase/rest/metrics/RESTMetrics.java 284bbc5
>
> Diff: http://review.cloudera.org/r/1039/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrew
>
>
Re: Review Request: [rest] publish endpoint and statistics into ZooKeeper
Posted by Jonathan Gray <jg...@apache.org>.
> On 2010-10-18 15:34:45, Ryan Rawson wrote:
> > src/main/java/org/apache/hadoop/hbase/rest/RESTServlet.java, line 217
> > <http://review.cloudera.org/r/1039/diff/1/?file=14874#file14874line217>
> >
> > this looks clever, is it more generically useful to other parts of hbase?
>
> Andrew Purtell wrote:
> Other parts of HBase use ZKW methods to do this. I brought this in here to do the same without pulling in all of the behavior of ZKW I didn't want.
>
> Jonathan Gray wrote:
> Which behaviors of ZKW are you referring to? Hopefully this component is generally reusable (the new ZooKeeperWatcher) and could be used even in limited contexts. Using it as the primary watcher and registering with it also helps when writing unit test. You'd then use ZKUtil methods for this kind of stuff and inherit work done there.
>
> We are going to need one more level underneath ZKUtil or underlying ZKUtil that manages retry policies and such. I'm going to target that for 0.92. And if all our code uses these APIs then it will be easier to be consistent.
>
> The patch looks fine to me though so we can work at unifying later and not blocking you on this.
It also forces u into good behavior, for example, by needing to pass it an Abortable on construction.
- Jonathan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1039/#review1560
-----------------------------------------------------------
On 2010-10-18 14:46:30, Andrew Purtell wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/1039/
> -----------------------------------------------------------
>
> (Updated 2010-10-18 14:46:30)
>
>
> Review request for hbase.
>
>
> Summary
> -------
>
> This change allows the REST interface to publish its endpoint and metrics, currently only requests/sec, into ZooKeeper. By default a permanent znode tree is created as needed at /hbase/rest/status and Stargate instances create ephemeral children of this with names in the format <host>:<port>. The ephemeral znodes contain JSON serialized information about the instance, e.g.
>
> {"connector":{"host":"restserver.example.com","port":"8080"},"statistics":{"requests":"13"}}
>
> The function of Stargate itself is not affected, except for one significant change: now if the ZooKeeper service is lost, the Stargate instances will abort along with the rest of HBase.
>
>
> This addresses bug HBASE-3119.
> http://issues.apache.org/jira/browse/HBASE-3119
>
>
> Diffs
> -----
>
> src/main/java/org/apache/hadoop/hbase/HConstants.java 71c3e7b
> src/main/java/org/apache/hadoop/hbase/rest/Main.java 368b4b4
> src/main/java/org/apache/hadoop/hbase/rest/RESTServlet.java ed92857
> src/main/java/org/apache/hadoop/hbase/rest/metrics/RESTMetrics.java 284bbc5
>
> Diff: http://review.cloudera.org/r/1039/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrew
>
>
Re: Review Request: [rest] publish endpoint and statistics into ZooKeeper
Posted by Andrew Purtell <ap...@apache.org>.
> On 2010-10-18 15:34:45, Ryan Rawson wrote:
> > src/main/java/org/apache/hadoop/hbase/rest/RESTServlet.java, line 217
> > <http://review.cloudera.org/r/1039/diff/1/?file=14874#file14874line217>
> >
> > this looks clever, is it more generically useful to other parts of hbase?
>
> Andrew Purtell wrote:
> Other parts of HBase use ZKW methods to do this. I brought this in here to do the same without pulling in all of the behavior of ZKW I didn't want.
>
> Jonathan Gray wrote:
> Which behaviors of ZKW are you referring to? Hopefully this component is generally reusable (the new ZooKeeperWatcher) and could be used even in limited contexts. Using it as the primary watcher and registering with it also helps when writing unit test. You'd then use ZKUtil methods for this kind of stuff and inherit work done there.
>
> We are going to need one more level underneath ZKUtil or underlying ZKUtil that manages retry policies and such. I'm going to target that for 0.92. And if all our code uses these APIs then it will be easier to be consistent.
>
> The patch looks fine to me though so we can work at unifying later and not blocking you on this.
>
> Jonathan Gray wrote:
> It also forces u into good behavior, for example, by needing to pass it an Abortable on construction.
One issue is the constructor creates or checks znodes that the REST interface should not care about. (I'm thinking ahead to when ZK ACLs are in use a bit maybe.) Also I wanted automatic retry behavior for setData but that is something for which a wrapper around ZKUtil method calls would work.
Unifying later should not be a big deal.
- Andrew
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1039/#review1560
-----------------------------------------------------------
On 2010-10-18 14:46:30, Andrew Purtell wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/1039/
> -----------------------------------------------------------
>
> (Updated 2010-10-18 14:46:30)
>
>
> Review request for hbase.
>
>
> Summary
> -------
>
> This change allows the REST interface to publish its endpoint and metrics, currently only requests/sec, into ZooKeeper. By default a permanent znode tree is created as needed at /hbase/rest/status and Stargate instances create ephemeral children of this with names in the format <host>:<port>. The ephemeral znodes contain JSON serialized information about the instance, e.g.
>
> {"connector":{"host":"restserver.example.com","port":"8080"},"statistics":{"requests":"13"}}
>
> The function of Stargate itself is not affected, except for one significant change: now if the ZooKeeper service is lost, the Stargate instances will abort along with the rest of HBase.
>
>
> This addresses bug HBASE-3119.
> http://issues.apache.org/jira/browse/HBASE-3119
>
>
> Diffs
> -----
>
> src/main/java/org/apache/hadoop/hbase/HConstants.java 71c3e7b
> src/main/java/org/apache/hadoop/hbase/rest/Main.java 368b4b4
> src/main/java/org/apache/hadoop/hbase/rest/RESTServlet.java ed92857
> src/main/java/org/apache/hadoop/hbase/rest/metrics/RESTMetrics.java 284bbc5
>
> Diff: http://review.cloudera.org/r/1039/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrew
>
>
Re: Review Request: [rest] publish endpoint and statistics into ZooKeeper
Posted by Andrew Purtell <ap...@apache.org>.
> On 2010-10-18 15:34:45, Ryan Rawson wrote:
> > src/main/java/org/apache/hadoop/hbase/HConstants.java, line 139
> > <http://review.cloudera.org/r/1039/diff/1/?file=14872#file14872line139>
> >
> > do we want to migrate class-specific static config to those classes?
There are statics for other daemon ports nearby. I was following this convention.
> On 2010-10-18 15:34:45, Ryan Rawson wrote:
> > src/main/java/org/apache/hadoop/hbase/rest/RESTServlet.java, line 217
> > <http://review.cloudera.org/r/1039/diff/1/?file=14874#file14874line217>
> >
> > this looks clever, is it more generically useful to other parts of hbase?
Other parts of HBase use ZKW methods to do this. I brought this in here to do the same without pulling in all of the behavior of ZKW I didn't want.
> On 2010-10-18 15:34:45, Ryan Rawson wrote:
> > src/main/java/org/apache/hadoop/hbase/rest/RESTServlet.java, line 226
> > <http://review.cloudera.org/r/1039/diff/1/?file=14874#file14874line226>
> >
> > redundant assignment?
No.
> On 2010-10-18 15:34:45, Ryan Rawson wrote:
> > src/main/java/org/apache/hadoop/hbase/rest/RESTServlet.java, line 362
> > <http://review.cloudera.org/r/1039/diff/1/?file=14874#file14874line362>
> >
> > is this better called startZooKeeperClient?
Ok, can do on commit.
> On 2010-10-18 15:34:45, Ryan Rawson wrote:
> > src/main/java/org/apache/hadoop/hbase/rest/RESTServlet.java, line 381
> > <http://review.cloudera.org/r/1039/diff/1/?file=14874#file14874line381>
> >
> > does this mean that stats are only updated every minute by default?
Yes.
- Andrew
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1039/#review1560
-----------------------------------------------------------
On 2010-10-18 14:46:30, Andrew Purtell wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/1039/
> -----------------------------------------------------------
>
> (Updated 2010-10-18 14:46:30)
>
>
> Review request for hbase.
>
>
> Summary
> -------
>
> This change allows the REST interface to publish its endpoint and metrics, currently only requests/sec, into ZooKeeper. By default a permanent znode tree is created as needed at /hbase/rest/status and Stargate instances create ephemeral children of this with names in the format <host>:<port>. The ephemeral znodes contain JSON serialized information about the instance, e.g.
>
> {"connector":{"host":"restserver.example.com","port":"8080"},"statistics":{"requests":"13"}}
>
> The function of Stargate itself is not affected, except for one significant change: now if the ZooKeeper service is lost, the Stargate instances will abort along with the rest of HBase.
>
>
> This addresses bug HBASE-3119.
> http://issues.apache.org/jira/browse/HBASE-3119
>
>
> Diffs
> -----
>
> src/main/java/org/apache/hadoop/hbase/HConstants.java 71c3e7b
> src/main/java/org/apache/hadoop/hbase/rest/Main.java 368b4b4
> src/main/java/org/apache/hadoop/hbase/rest/RESTServlet.java ed92857
> src/main/java/org/apache/hadoop/hbase/rest/metrics/RESTMetrics.java 284bbc5
>
> Diff: http://review.cloudera.org/r/1039/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrew
>
>
Re: Review Request: [rest] publish endpoint and statistics into ZooKeeper
Posted by Jonathan Gray <jg...@apache.org>.
> On 2010-10-18 15:34:45, Ryan Rawson wrote:
> > src/main/java/org/apache/hadoop/hbase/rest/RESTServlet.java, line 217
> > <http://review.cloudera.org/r/1039/diff/1/?file=14874#file14874line217>
> >
> > this looks clever, is it more generically useful to other parts of hbase?
>
> Andrew Purtell wrote:
> Other parts of HBase use ZKW methods to do this. I brought this in here to do the same without pulling in all of the behavior of ZKW I didn't want.
>
> Jonathan Gray wrote:
> Which behaviors of ZKW are you referring to? Hopefully this component is generally reusable (the new ZooKeeperWatcher) and could be used even in limited contexts. Using it as the primary watcher and registering with it also helps when writing unit test. You'd then use ZKUtil methods for this kind of stuff and inherit work done there.
>
> We are going to need one more level underneath ZKUtil or underlying ZKUtil that manages retry policies and such. I'm going to target that for 0.92. And if all our code uses these APIs then it will be easier to be consistent.
>
> The patch looks fine to me though so we can work at unifying later and not blocking you on this.
>
> Jonathan Gray wrote:
> It also forces u into good behavior, for example, by needing to pass it an Abortable on construction.
>
> Andrew Purtell wrote:
> One issue is the constructor creates or checks znodes that the REST interface should not care about. (I'm thinking ahead to when ZK ACLs are in use a bit maybe.) Also I wanted automatic retry behavior for setData but that is something for which a wrapper around ZKUtil method calls would work.
>
> Unifying later should not be a big deal.
Good point, totally agree. That definitely does not belong in the ZKW constructor. The first master should do it and that's it. Filed HBASE-3123.
- Jonathan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1039/#review1560
-----------------------------------------------------------
On 2010-10-18 14:46:30, Andrew Purtell wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/1039/
> -----------------------------------------------------------
>
> (Updated 2010-10-18 14:46:30)
>
>
> Review request for hbase.
>
>
> Summary
> -------
>
> This change allows the REST interface to publish its endpoint and metrics, currently only requests/sec, into ZooKeeper. By default a permanent znode tree is created as needed at /hbase/rest/status and Stargate instances create ephemeral children of this with names in the format <host>:<port>. The ephemeral znodes contain JSON serialized information about the instance, e.g.
>
> {"connector":{"host":"restserver.example.com","port":"8080"},"statistics":{"requests":"13"}}
>
> The function of Stargate itself is not affected, except for one significant change: now if the ZooKeeper service is lost, the Stargate instances will abort along with the rest of HBase.
>
>
> This addresses bug HBASE-3119.
> http://issues.apache.org/jira/browse/HBASE-3119
>
>
> Diffs
> -----
>
> src/main/java/org/apache/hadoop/hbase/HConstants.java 71c3e7b
> src/main/java/org/apache/hadoop/hbase/rest/Main.java 368b4b4
> src/main/java/org/apache/hadoop/hbase/rest/RESTServlet.java ed92857
> src/main/java/org/apache/hadoop/hbase/rest/metrics/RESTMetrics.java 284bbc5
>
> Diff: http://review.cloudera.org/r/1039/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrew
>
>
Re: Review Request: [rest] publish endpoint and statistics into ZooKeeper
Posted by Ryan Rawson <ry...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1039/#review1560
-----------------------------------------------------------
src/main/java/org/apache/hadoop/hbase/HConstants.java
<http://review.cloudera.org/r/1039/#comment5259>
do we want to migrate class-specific static config to those classes?
src/main/java/org/apache/hadoop/hbase/rest/Main.java
<http://review.cloudera.org/r/1039/#comment5260>
where did DEFAULT_LISTEN_PORT go to? i am not seeing it removed?
src/main/java/org/apache/hadoop/hbase/rest/RESTServlet.java
<http://review.cloudera.org/r/1039/#comment5262>
this looks clever, is it more generically useful to other parts of hbase?
src/main/java/org/apache/hadoop/hbase/rest/RESTServlet.java
<http://review.cloudera.org/r/1039/#comment5261>
redundant assignment?
src/main/java/org/apache/hadoop/hbase/rest/RESTServlet.java
<http://review.cloudera.org/r/1039/#comment5263>
ditto on this one. i wonder if there might be an elegant way to retry or redo zk operations, but there might not be.
src/main/java/org/apache/hadoop/hbase/rest/RESTServlet.java
<http://review.cloudera.org/r/1039/#comment5264>
is this better called startZooKeeperClient?
src/main/java/org/apache/hadoop/hbase/rest/RESTServlet.java
<http://review.cloudera.org/r/1039/#comment5265>
does this mean that stats are only updated every minute by default?
- Ryan
On 2010-10-18 14:46:30, Andrew Purtell wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/1039/
> -----------------------------------------------------------
>
> (Updated 2010-10-18 14:46:30)
>
>
> Review request for hbase.
>
>
> Summary
> -------
>
> This change allows the REST interface to publish its endpoint and metrics, currently only requests/sec, into ZooKeeper. By default a permanent znode tree is created as needed at /hbase/rest/status and Stargate instances create ephemeral children of this with names in the format <host>:<port>. The ephemeral znodes contain JSON serialized information about the instance, e.g.
>
> {"connector":{"host":"restserver.example.com","port":"8080"},"statistics":{"requests":"13"}}
>
> The function of Stargate itself is not affected, except for one significant change: now if the ZooKeeper service is lost, the Stargate instances will abort along with the rest of HBase.
>
>
> This addresses bug HBASE-3119.
> http://issues.apache.org/jira/browse/HBASE-3119
>
>
> Diffs
> -----
>
> src/main/java/org/apache/hadoop/hbase/HConstants.java 71c3e7b
> src/main/java/org/apache/hadoop/hbase/rest/Main.java 368b4b4
> src/main/java/org/apache/hadoop/hbase/rest/RESTServlet.java ed92857
> src/main/java/org/apache/hadoop/hbase/rest/metrics/RESTMetrics.java 284bbc5
>
> Diff: http://review.cloudera.org/r/1039/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrew
>
>
Re: Review Request: [rest] publish endpoint and statistics into ZooKeeper
Posted by Ryan Rawson <ry...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1039/#review1565
-----------------------------------------------------------
Ship it!
- Ryan
On 2010-10-18 14:46:30, Andrew Purtell wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/1039/
> -----------------------------------------------------------
>
> (Updated 2010-10-18 14:46:30)
>
>
> Review request for hbase.
>
>
> Summary
> -------
>
> This change allows the REST interface to publish its endpoint and metrics, currently only requests/sec, into ZooKeeper. By default a permanent znode tree is created as needed at /hbase/rest/status and Stargate instances create ephemeral children of this with names in the format <host>:<port>. The ephemeral znodes contain JSON serialized information about the instance, e.g.
>
> {"connector":{"host":"restserver.example.com","port":"8080"},"statistics":{"requests":"13"}}
>
> The function of Stargate itself is not affected, except for one significant change: now if the ZooKeeper service is lost, the Stargate instances will abort along with the rest of HBase.
>
>
> This addresses bug HBASE-3119.
> http://issues.apache.org/jira/browse/HBASE-3119
>
>
> Diffs
> -----
>
> src/main/java/org/apache/hadoop/hbase/HConstants.java 71c3e7b
> src/main/java/org/apache/hadoop/hbase/rest/Main.java 368b4b4
> src/main/java/org/apache/hadoop/hbase/rest/RESTServlet.java ed92857
> src/main/java/org/apache/hadoop/hbase/rest/metrics/RESTMetrics.java 284bbc5
>
> Diff: http://review.cloudera.org/r/1039/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrew
>
>