thanks, Rajeshbabu, HBASE-10215 is not the last change, The HBASE-7767
(hello, Andrey [?]) removed the exception throw code after setting up the
table state, what we really want is as follows (if Andrey agrees with the
change, I will create a JIRA and send out the patch today):
// 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) {
this.assignmentManager.getTableStateManager().setDeletedTable(
tableName);
}
throw new TableNotFoundException(tableName);
}
On Mon, Mar 16, 2015 at 12:09 PM, Rajeshbabu Chintaguntla <
chrajeshbab...@gmail.com> wrote:
> Hi Stephen and Andrey,
>
> The first step added to remove stale znodes if table creation fails after
> znode creation.
> See HBASE-10215 <https://issues.apache.org/jira/browse/HBASE-10215>. Not
> sure still we need it or not.
>
> Thanks,
> Rajeshbabu.
>
>
>
>
> On Tue, Mar 17, 2015 at 12:18 AM, Andrey Stepachev <oct...@gmail.com>
> wrote:
>
> > Thanks Stephen.
> >
> > on (2): I think that much better to guarantee that table was enabled
> (i.e.
> > all internal structures reflect that fact and balancer knows about new
> > table). But result of that could be checked asyncronically from Admin.
> > Does it make sense?
> >
> > On Mon, Mar 16, 2015 at 6:10 PM, Stephen Jiang <syuanjiang...@gmail.com>
> > wrote:
> >
> > > Andrey, I will take care of (1).
> > >
> > > And (2) :-) if your guys agree. Because it is not consistent, if the
> > bulk
> > > assigned failed, we would fail the enabling table; however, if the bulk
> > > assign not starts, we would enable table with offline regions - really
> > > inconsistent - we either all fail in those scenarios or all succeed
> with
> > > offline region (best effort approach).
> > >
> > > Thanks
> > > Stephen
> > >
> > > On Mon, Mar 16, 2015 at 11:01 AM, Andrey Stepachev <oct...@gmail.com>
> > > wrote:
> > >
> > >> 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.
> > >>
> > >
> > >
> >
> >
> > --
> > Andrey.
> >
>
