[ 
https://issues.apache.org/jira/browse/HBASE-3112?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12929860#action_12929860
 ] 

HBase Review Board commented on HBASE-3112:
-------------------------------------------

Message from: "Jonathan Gray" <jg...@apache.org>

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1187/#review1853
-----------------------------------------------------------

Ship it!


I can live without the state-checking methods in ZKTable but it's fine, no big 
deal as is.

Only issue is that we're hitting ZK on each assign().

Otherwise, if tests are passing and it's working for you up on cluster, I'm +1 
on this.

If you add async/sync support, let's be sure to nail the javadoc.


trunk/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
<http://review.cloudera.org/r/1187/#comment6017>

    why describe 'enabling' state in our client API?  do we let out 'enabling' 
at all anywhere?
    
    and do we need to make mention that if it seems to never finish, this 
method should be retried?



trunk/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
<http://review.cloudera.org/r/1187/#comment6019>

    same.  i think the javadoc is descriptive enough with this explanation of 
states.  just not sure if we need any javadoc about retrying?



trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
<http://review.cloudera.org/r/1187/#comment6022>

    isTableDisabling is actually a ZK read rather than checking in-memory 
state.  Should be move enabling/disabling to in-memory state as well?  I'm not 
sure it's a good idea to have to read from ZK on every assign() call, 
especially for something as rare as disabling.


- Jonathan





> Enable and disable of table needs a bit of loving in new master
> ---------------------------------------------------------------
>
>                 Key: HBASE-3112
>                 URL: https://issues.apache.org/jira/browse/HBASE-3112
>             Project: HBase
>          Issue Type: Bug
>            Reporter: stack
>            Assignee: stack
>            Priority: Critical
>             Fix For: 0.90.0
>
>         Attachments: 3112-v2.txt, 3112-v3.txt, 3112.txt
>
>
> The tools are in place to do a more reliable enable/disable of tables.  Some 
> work has been done to hack in a basic enable/disable but its not enough -- 
> see the test avro/thrift tests where a disable/enable/disable switchback can 
> confuse the table state (and has been disabled until this issue addressed).
> This issue is about finishing off enable/disable in the new master.   I think 
> we need to add to the table znode an enabling/disabling state rather than 
> have them binary with a watcher that will stop an enable (or disable) 
> starting until the previous completes (Currently we atomically switch the 
> state though the region close/open lags -- some work in enable/disable 
> handlers helps in that they won't complete till all regions have 
> transitioned.. but its not enough).
> Need to add tests too.
> Marking issue critical bug because loads of the questions we get on lists are 
> about enable/disable probs.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to