Stephen, would you like to create jira for case (1)?
Thank you.
On Mon, Mar 16, 2015 at 5:58 PM, Andrey Stepachev <oct...@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 <syuanjiang...@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.
