hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrey Stepachev <oct...@gmail.com>
Subject Re: Question on EnableTableHandler code
Date Mon, 16 Mar 2015 18:01:24 GMT
Stephen, would you like to create jira for case (1)?

Thank you.

On Mon, Mar 16, 2015 at 5:58 PM, Andrey Stepachev <octo47@gmail.com> wrote:

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



-- 
Andrey.

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message