hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "stack (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-6317) Master clean start up and Partially enabled tables make region assignment inconsistent.
Date Mon, 09 Jul 2012 20:37:34 GMT

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

stack commented on HBASE-6317:
------------------------------



Does this need to be public?  Can it be package private?

{code}
+  public List<HRegionInfo> getEnablingTableRegions(String tableName) {
{code}

What does this protect against?  Concurrent assignment by another thread?

{code}
+  public boolean addPlanIfNotPresent(HRegionInfo hri, RegionPlan plan) {
{code}

Can we not ask if regionInTransition rather than add this new method?

How can we get this better noticed?  The table will be offline right?

{code}
+    } catch (InterruptedException e) {
+      LOG.error("Error trying to enable the table " + this.tableNameStr, e);
     }
{code}

Why is this:

{code}
+    if(this.failover == false && this.enablingTables.size() > 0){
+      this.failover = true;
+    }
{code}

We are testing if failover is necessary and if there are tables in an enabling state when
new master comes up, then that is good enough reason to run the failover code?  A comment
would be good here I'd say (especially if I have it wrong).

Should we at least look at the RegionState before we remove the item from the list below (or
do you think it would never be other than OPENING or something?)

{code}
+    List<HRegionInfo> hris = this.enablingTables.get(regionInfo.getTableNameAsString());
+    if(hris != null && !hris.isEmpty()){
+      hris.remove(regionInfo);
+    }
{code}

Why do we do tests like if (false == checkIfRegionBelongsToDisabled(regionInfo)... instead
of if (!checkIfRegionBelongsToDisabled(regionInfo)...?  (Its not you but it looks weird your
replicating this oddity).

I'm not I follow all that is going on in here but it looks right... I can try a review again
later.  Too hard to write a test I suppose lads?  Its difficult state to reproduce, this master
startup stuff?
                
> Master clean start up and Partially enabled tables make region assignment inconsistent.
> ---------------------------------------------------------------------------------------
>
>                 Key: HBASE-6317
>                 URL: https://issues.apache.org/jira/browse/HBASE-6317
>             Project: HBase
>          Issue Type: Bug
>            Reporter: ramkrishna.s.vasudevan
>            Assignee: rajeshbabu
>             Fix For: 0.92.2, 0.96.0, 0.94.1
>
>         Attachments: HBASE-6317_94.patch
>
>
> If we have a  table in partially enabled state (ENABLING) then on HMaster restart we
treat it as a clean cluster start up and do a bulk assign.  Currently in 0.94 bulk assign
will not handle ALREADY_OPENED scenarios and it leads to region assignment problems.  Analysing
more on this we found that we have better way to handle these scenarios.
> {code}
> if (false == checkIfRegionBelongsToDisabled(regionInfo)
>             && false == checkIfRegionsBelongsToEnabling(regionInfo)) {
>           synchronized (this.regions) {
>             regions.put(regionInfo, regionLocation);
>             addToServers(regionLocation, regionInfo);
>           }
> {code}
> We dont add to regions map so that enable table handler can handle it.  But as nothing
is added to regions map we think it as a clean cluster start up.
> Will come up with a patch tomorrow.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Mime
View raw message