Re: Question on EnableTableHandler code

2015-03-16 Thread Stephen Jiang
Now (1) is under control (HBASE-13254). Let us talk about (2). Looks like we are doing best effort to online all regions of a table during 'enable table' operation. My argument is that we should be consistent with all conditions. Currently, we fail if bulk assignment failed with some reason;

Re: Question on EnableTableHandler code

2015-03-16 Thread Andrey Stepachev
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

Re: Question on EnableTableHandler code

2015-03-16 Thread Stephen Jiang
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

Re: Question on EnableTableHandler code

2015-03-16 Thread Andrey Stepachev
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 on EnableTableHandler code

2015-03-16 Thread Stephen Jiang
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

Re: Question on EnableTableHandler code

2015-03-16 Thread Andrey Stepachev
Stephen , you are right , that is my code and that thing was overlooked :) I think we need completely remove state cleanup code. Actually how it done tablestate manager could not return table which later will be rendered as nonexistent. Basically that means, that if we got nonexistent table in

Re: Question on EnableTableHandler code

2015-03-16 Thread Rajeshbabu Chintaguntla
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

Re: Question on EnableTableHandler code

2015-03-16 Thread Andrey Stepachev
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

Re: Question on EnableTableHandler code

2015-03-16 Thread Stephen Jiang
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