Mailing-List: contact dev-help@hbase.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@hbase.apache.org
Received-SPF: pass (athena.apache.org: domain of chrajeshbabu32@gmail.com
 designates 209.85.192.175 as permitted sender)
MIME-Version: 1.0
In-Reply-To: 
 <CAADCt4KVD5kJM0cQnu_fYWSWoiv1OC6j-6B9PEX5AAZsuQ2npQ@mail.gmail.com>
References: 
 <CAPQQKxd1xBUDDPSdYU-oA9P02yYj0AW3XH_Z3XT0xWzoG+5vYg@mail.gmail.com>
	<CAADCt4+7GEGUMcauKjNjqep0k4VwfOJdWDoMUcv7ex0UzFohEw@mail.gmail.com>
	<CAADCt4J5vvopvaZjbd+yeJbeqCrSvRRLVLEqngeOg1k4HSrYSw@mail.gmail.com>
	<CAPQQKxcDxwacFs8uE5b-KdNsNiR1mRzvyXex04crCL=EeXJtuQ@mail.gmail.com>
	<CAADCt4KVD5kJM0cQnu_fYWSWoiv1OC6j-6B9PEX5AAZsuQ2npQ@mail.gmail.com>
Date: Tue, 17 Mar 2015 00:39:23 +0530
Message-ID: 
 <CAEhbsO6mFQw_8R67wtDkqK=Xvpmd87=VBi3Q_7cXVznZASQgDA@mail.gmail.com>
Subject: Re: Question on EnableTableHandler code
From: Rajeshbabu Chintaguntla <chrajeshbabu32@gmail.com>
To: "dev@hbase.apache.org" <dev@hbase.apache.org>
Content-Type: multipart/alternative; boundary=047d7bf0f4ec909ff205116c96d8

--047d7bf0f4ec909ff205116c96d8
Content-Type: text/plain; charset=UTF-8

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 <octo47@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 <syuanjiangdev@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 <octo47@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 <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.
> >>
> >
> >
>
>
> --
> Andrey.
>

--047d7bf0f4ec909ff205116c96d8--