Mailing-List: contact dev-help@hbase.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@hbase.apache.org
Received-SPF: pass (nike.apache.org: domain of octo47@gmail.com designates
 209.85.214.169 as permitted sender)
MIME-Version: 1.0
In-Reply-To: 
 <CAPQQKxd1xBUDDPSdYU-oA9P02yYj0AW3XH_Z3XT0xWzoG+5vYg@mail.gmail.com>
References: 
 <CAPQQKxd1xBUDDPSdYU-oA9P02yYj0AW3XH_Z3XT0xWzoG+5vYg@mail.gmail.com>
From: Andrey Stepachev <octo47@gmail.com>
Date: Mon, 16 Mar 2015 17:58:30 +0000
Message-ID: 
 <CAADCt4+7GEGUMcauKjNjqep0k4VwfOJdWDoMUcv7ex0UzFohEw@mail.gmail.com>
Subject: Re: Question on EnableTableHandler code
To: dev@hbase.apache.org
Content-Type: multipart/alternative; boundary=001a113d631e3b056305116b9ab6

--001a113d631e3b056305116b9ab6
Content-Type: text/plain; charset=UTF-8

Thanks Stephen.

Looks like you are right. For (1) case we really don't need there
state cleanup. That is a bug. Should throw TableNotFoundException.

As for (2) in case of no online region servers available we could
leave table enabled, but no regions would be assigned.

Actually that rises good question what enable table means,
i.e. do we really need to guarantee that on table enable absolutely
all regions are online, or that could be done in Admin on client side.

So for now it seems that Enable handler do what is best, and leave
table enabled but unassigned to be later assigned by Balancer.

On Mon, Mar 16, 2015 at 5:34 PM, Stephen Jiang <syuanjiangdev@gmail.com>
wrote:

> I want to make sure that the following logic in EnableTableHandler is
> correct:
>
> (1). In EnableTableHandler#prepare - if the table is not existed, it marked
> the table as deleted and not throw exception.  The result is the table lock
> is released and the caller has no knowledge that the table not exist or
> already deleted, it would continue the next step.
>
> Currently, this would happen during recovery (the caller is
> AssignmentManager#recoverTableInEnablingState()) - however, looking at
> recovery code, it expects TableNotFoundException  Should we always throw
> exception - if the table not exist?  I want to make sure that I don't break
> recovery logic by modifying.
>
> public EnableTableHandler prepare() {
>
> ...
>
> // Check if table exists
>
>       if (!MetaTableAccessor.tableExists(this.server.getConnection(),
> tableName)) {
>
>         // retainAssignment is true only during recovery.  In normal case
> it is false
>
>         if (!this.skipTableStateCheck) {
>
>           throw new TableNotFoundException(tableName);
>
>         }
>
>         this.assignmentManager.getTableStateManager().setDeletedTable(
> tableName);
>
>       }
>
> ...
>
> }
> (2). In EnableTableHandler#handleEnableTable() - if the bulk assign plan
> could not be find, it would leave regions to be offline and declare enable
> table succeed - i think this is a bug and we should retry or fail - but I
> want to make sure that there are some merit behind this logic
>
>   private void handleEnableTable() {
>
>     Map<ServerName, List<HRegionInfo>> bulkPlan =
>
>         this.assignmentManager.getBalancer().retainAssignment(
> regionsToAssign, onlineServers);
>
>     if (bulkPlan != null) {
>
>           ...
>
>       } else {
>
>       LOG.info("Balancer was unable to find suitable servers for table " +
> tableName
>
>           + ", leaving unassigned");
>
>       done = true;
>
>     }
>
>     if (done) {
>
>       // Flip the table to enabled.
>
>       this.assignmentManager.getTableStateManager().setTableState(
>
>         this.tableName, TableState.State.ENABLED);
>
>       LOG.info("Table '" + this.tableName
>
>       + "' was successfully enabled. Status: done=" + done);
>
>    }
>
>      ...
>
> }
>
>
> thanks
> Stephen
>


-- 
Andrey.

--001a113d631e3b056305116b9ab6--