hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "HBase Review Board (JIRA)" <j...@apache.org>
Subject [jira] Commented: (HBASE-2696) ZooKeeper cleanup and refactor
Date Thu, 15 Jul 2010 17:24:50 GMT

    [ 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: stack@duboce.net

-----------------------------------------------------------
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.


Mime
View raw message