[
https://issues.apache.org/jira/browse/HBASE-2696?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12888861#action_12888861
]
HBase Review Board commented on HBASE-2696:
-------------------------------------------
Message from: [email protected]
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/324/#review404
-----------------------------------------------------------
Ship it!
Looks good. Nothing to test in this new code (though most is just replacing
and adding new exception).
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/ServerController.java
<http://review.hbase.org/r/324/#comment1716>
Client "is a" ServerController or do you mean that you can do
ServerController actions via client? (I see later that whats going on here is
that the client is proxying Server calls)
Also, (don't kill me), but controller is bad name for this interface.
Going by the current set of methods in this interface, they are methods an
actual controller would make use of; e.g. a "Controller" would decide its time
to abort the server.
I still think it should be 'Server' or 'Process'
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/ServerController.java
<http://review.hbase.org/r/324/#comment1717>
Is this javadoc right? Mentions serverstatus
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
<http://review.hbase.org/r/324/#comment1718>
Should we remove this exception? Or deprecate it? The MasterNotRunning
one?
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
<http://review.hbase.org/r/324/#comment1719>
Having this bit of code inside each method was really bad... thats good you
fixed it
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
<http://review.hbase.org/r/324/#comment1720>
I'm for a super Interface. Its odd have server-side in here. How about
Abortable Interface?
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
<http://review.hbase.org/r/324/#comment1721>
There is a Configurable interface up in hadoop (but also has setConf which
you don't want I'd say)... just FYI.
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/regionserver/MasterAddressManager.java
<http://review.hbase.org/r/324/#comment1722>
Sorry, don't remember what I said in last review but did we agree that this
class actually 'manages' the master address? It has the watcher and updates
itself if change? That kinda thing?
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/regionserver/RSZookeeperUpdater.java
<http://review.hbase.org/r/324/#comment1723>
Call this RSZKUpdater to match your pattern naming classes that have to do
w/ zk (ZKUtil, ZKConf).
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
<http://review.hbase.org/r/324/#comment1726>
Do you mean ensemble?
branches/0.90_master_rewrite/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
<http://review.hbase.org/r/324/#comment1727>
Should probably be ensemble but probably a big change doing that?
- stack
> ZooKeeper cleanup and refactor
> ------------------------------
>
> Key: HBASE-2696
> URL: https://issues.apache.org/jira/browse/HBASE-2696
> Project: HBase
> Issue Type: Sub-task
> Components: master, regionserver, zookeeper
> Reporter: Jonathan Gray
> Assignee: Jonathan Gray
> Priority: Critical
> Fix For: 0.90.0
>
> Attachments: HBASE-2696-part1-NewClasses_NotIntegrated.patch,
> HBASE-2696-part1-v2-NewClasses_NotIntegrated.patch,
> HBASE-2696-part1-v3-NewClasses_RS.patch,
> HBASE-2696-part1-v4-NewClasses_RS_Tested.patch,
> HBASE-2696-part1-v5-NewClasses_RS_Tested.patch
>
>
> Currently almost everything we do with ZooKeeper is stuffed into a single
> class {{ZookeeperWrapper}}.
> This issue will deal with cleaning up our usage of ZK, adding some new
> abstractions to help with the master changes, splitting up watchers from
> utility methods, and nailing down the contracts of our ZK methods with
> respect to setting watchers, throwing exceptions, etc...
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
