Hi,
On behalf of Chunhui, I am requesting review for HBASE-7403 Online Merge.

This JIRA was created 3 months ago.
Chunhui has responded to review comments very promptly, including a major
rewrite around the time split transaction was rewritten.

This feature has widely been requested. I feel the patch is mostly ready to
go in.
Here is brief recap of the steps.

Process of merging two regions:

a.client sends RPC (dispatch merging regions) to master
b.master moves the regions together (on the regionserver where the more
heavily loaded region resided)
c.master sends RPC (merge regions) to this regionserver
d.Regionserver executes the region merge transaction in the thread pool

I think step b is a nice simplification for the problem. In previous
versions of the patch, the two merging regions stay on respective servers
which required more complex coordination through zookeeper.

High level comment as well as detailed review are both welcome.

Thanks

Hi Ted,

I jut gave it a look.

I have updated it on the RB.

Overall, this is very good and I'm eager to see that integrated! I'm
waiting for this feature since the beginning ;)

Regarding non adjacent regions merge? Will the system still be
consistent after that? Or will hbck report some regions overlaps?

JM
2013/3/16 Ted Yu <[EMAIL PROTECTED]>:
> Hi,
> On behalf of Chunhui, I am requesting review for HBASE-7403 Online Merge.
>
> This JIRA was created 3 months ago.
> Chunhui has responded to review comments very promptly, including a major
> rewrite around the time split transaction was rewritten.
>
> This feature has widely been requested. I feel the patch is mostly ready to
> go in.
> Here is brief recap of the steps.
>
> Process of merging two regions:
>
> a.client sends RPC (dispatch merging regions) to master
> b.master moves the regions together (on the regionserver where the more
> heavily loaded region resided)
> c.master sends RPC (merge regions) to this regionserver
> d.Regionserver executes the region merge transaction in the thread pool
>
> I think step b is a nice simplification for the problem. In previous
> versions of the patch, the two merging regions stay on respective servers
> which required more complex coordination through zookeeper.
>
> High level comment as well as detailed review are both welcome.
>
> Thanks

Hey,JM,
 
When regions exist hole or overlap, administrator could merge non adjacent regions to keep table consistency,
otherwise we shouldn't merge non adjacent regions. I would point this out in the annotation

Thanks for the review

 

Chunhui

At 2013-03-16 20:52:48,"Jean-Marc Spaggiari" <[EMAIL PROTECTED]> wrote:
>Hi Ted,
>
>I jut gave it a look.
>
>I have updated it on the RB.
>
>Overall, this is very good and I'm eager to see that integrated! I'm
>waiting for this feature since the beginning ;)
>
>Regarding non adjacent regions merge? Will the system still be
>consistent after that? Or will hbck report some regions overlaps?
>
>JM
>
>
>2013/3/16 Ted Yu <[EMAIL PROTECTED]>:
>> Hi,
>> On behalf of Chunhui, I am requesting review for HBASE-7403 Online Merge.
>>
>> This JIRA was created 3 months ago.
>> Chunhui has responded to review comments very promptly, including a major
>> rewrite around the time split transaction was rewritten.
>>
>> This feature has widely been requested. I feel the patch is mostly ready to
>> go in.
>> Here is brief recap of the steps.
>>
>> Process of merging two regions:
>>
>> a.client sends RPC (dispatch merging regions) to master
>> b.master moves the regions together (on the regionserver where the more
>> heavily loaded region resided)
>> c.master sends RPC (merge regions) to this regionserver
>> d.Regionserver executes the region merge transaction in the thread pool
>>
>> I think step b is a nice simplification for the problem. In previous
>> versions of the patch, the two merging regions stay on respective servers
>> which required more complex coordination through zookeeper.
>>
>> High level comment as well as detailed review are both welcome.
>>
>> Thanks

Chunhui replied to this question on review board.

Basically the force option is to repair overlapping regions or table with hole in its regions.

Personally I think online merge should detect merging regions with hole in between them and not require force flag in that case because logically they're adjacent.

Cheers

On Mar 16, 2013, at 5:52 AM, Jean-Marc Spaggiari <[EMAIL PROTECTED]> wrote:

> Hi Ted,
>
> I jut gave it a look.
>
> I have updated it on the RB.
>
> Overall, this is very good and I'm eager to see that integrated! I'm
> waiting for this feature since the beginning ;)
>
> Regarding non adjacent regions merge? Will the system still be
> consistent after that? Or will hbck report some regions overlaps?
>
> JM
>
>
> 2013/3/16 Ted Yu <[EMAIL PROTECTED]>:
>> Hi,
>> On behalf of Chunhui, I am requesting review for HBASE-7403 Online Merge.
>>
>> This JIRA was created 3 months ago.
>> Chunhui has responded to review comments very promptly, including a major
>> rewrite around the time split transaction was rewritten.
>>
>> This feature has widely been requested. I feel the patch is mostly ready to
>> go in.
>> Here is brief recap of the steps.
>>
>> Process of merging two regions:
>>
>> a.client sends RPC (dispatch merging regions) to master
>> b.master moves the regions together (on the regionserver where the more
>> heavily loaded region resided)
>> c.master sends RPC (merge regions) to this regionserver
>> d.Regionserver executes the region merge transaction in the thread pool
>>
>> I think step b is a nice simplification for the problem. In previous
>> versions of the patch, the two merging regions stay on respective servers
>> which required more complex coordination through zookeeper.
>>
>> High level comment as well as detailed review are both welcome.
>>
>> Thanks

Ted,

  Why would we use that merge tool when hbck will repair that?  Should we
throw a warning and tell the user to run repair first?
On Mar 16, 2013 9:17 AM, "Ted" <[EMAIL PROTECTED]> wrote:

> Chunhui replied to this question on review board.
>
> Basically the force option is to repair overlapping regions or table with
> hole in its regions.
>
> Personally I think online merge should detect merging regions with hole in
> between them and not require force flag in that case because logically
> they're adjacent.
>
> Cheers
>
> On Mar 16, 2013, at 5:52 AM, Jean-Marc Spaggiari <[EMAIL PROTECTED]>
> wrote:
>
> > Hi Ted,
> >
> > I jut gave it a look.
> >
> > I have updated it on the RB.
> >
> > Overall, this is very good and I'm eager to see that integrated! I'm
> > waiting for this feature since the beginning ;)
> >
> > Regarding non adjacent regions merge? Will the system still be
> > consistent after that? Or will hbck report some regions overlaps?
> >
> > JM
> >
> >
> > 2013/3/16 Ted Yu <[EMAIL PROTECTED]>:
> >> Hi,
> >> On behalf of Chunhui, I am requesting review for HBASE-7403 Online
> Merge.
> >>
> >> This JIRA was created 3 months ago.
> >> Chunhui has responded to review comments very promptly, including a
> major
> >> rewrite around the time split transaction was rewritten.
> >>
> >> This feature has widely been requested. I feel the patch is mostly
> ready to
> >> go in.
> >> Here is brief recap of the steps.
> >>
> >> Process of merging two regions:
> >>
> >> a.client sends RPC (dispatch merging regions) to master
> >> b.master moves the regions together (on the regionserver where the more
> >> heavily loaded region resided)
> >> c.master sends RPC (merge regions) to this regionserver
> >> d.Regionserver executes the region merge transaction in the thread pool
> >>
> >> I think step b is a nice simplification for the problem. In previous
> >> versions of the patch, the two merging regions stay on respective
> servers
> >> which required more complex coordination through zookeeper.
> >>
> >> High level comment as well as detailed review are both welcome.
> >>
> >> Thanks
>

Kevin:
I have a few thoughts on your questions.

But I think we should give Chunhui a chance to answer these questions first.

Btw the improvement in handling region split by Enis is fairly new. Meaning hbck hasn't utilized this new model yet.

Cheers

On Mar 16, 2013, at 6:21 AM, "Kevin O'dell" <[EMAIL PROTECTED]> wrote:

> Ted,
>
>  Why would we use that merge tool when hbck will repair that?  Should we
> throw a warning and tell the user to run repair first?
> On Mar 16, 2013 9:17 AM, "Ted" <[EMAIL PROTECTED]> wrote:
>
>> Chunhui replied to this question on review board.
>>
>> Basically the force option is to repair overlapping regions or table with
>> hole in its regions.
>>
>> Personally I think online merge should detect merging regions with hole in
>> between them and not require force flag in that case because logically
>> they're adjacent.
>>
>> Cheers
>>
>> On Mar 16, 2013, at 5:52 AM, Jean-Marc Spaggiari <[EMAIL PROTECTED]>
>> wrote:
>>
>>> Hi Ted,
>>>
>>> I jut gave it a look.
>>>
>>> I have updated it on the RB.
>>>
>>> Overall, this is very good and I'm eager to see that integrated! I'm
>>> waiting for this feature since the beginning ;)
>>>
>>> Regarding non adjacent regions merge? Will the system still be
>>> consistent after that? Or will hbck report some regions overlaps?
>>>
>>> JM
>>>
>>>
>>> 2013/3/16 Ted Yu <[EMAIL PROTECTED]>:
>>>> Hi,
>>>> On behalf of Chunhui, I am requesting review for HBASE-7403 Online
>> Merge.
>>>>
>>>> This JIRA was created 3 months ago.
>>>> Chunhui has responded to review comments very promptly, including a
>> major
>>>> rewrite around the time split transaction was rewritten.
>>>>
>>>> This feature has widely been requested. I feel the patch is mostly
>> ready to
>>>> go in.
>>>> Here is brief recap of the steps.
>>>>
>>>> Process of merging two regions:
>>>>
>>>> a.client sends RPC (dispatch merging regions) to master
>>>> b.master moves the regions together (on the regionserver where the more
>>>> heavily loaded region resided)
>>>> c.master sends RPC (merge regions) to this regionserver
>>>> d.Regionserver executes the region merge transaction in the thread pool
>>>>
>>>> I think step b is a nice simplification for the problem. In previous
>>>> versions of the patch, the two merging regions stay on respective
>> servers
>>>> which required more complex coordination through zookeeper.
>>>>
>>>> High level comment as well as detailed review are both welcome.
>>>>
>>>> Thanks
>>

I see.

I agree that we should throw a warning at least, when someone is
trying to merge such regions. Might still be good to have this
possibility.

JM

2013/3/16 Kevin O'dell <[EMAIL PROTECTED]>:
> Ted,
>
>   Why would we use that merge tool when hbck will repair that?  Should we
> throw a warning and tell the user to run repair first?
> On Mar 16, 2013 9:17 AM, "Ted" <[EMAIL PROTECTED]> wrote:
>
>> Chunhui replied to this question on review board.
>>
>> Basically the force option is to repair overlapping regions or table with
>> hole in its regions.
>>
>> Personally I think online merge should detect merging regions with hole in
>> between them and not require force flag in that case because logically
>> they're adjacent.
>>
>> Cheers
>>
>> On Mar 16, 2013, at 5:52 AM, Jean-Marc Spaggiari <[EMAIL PROTECTED]>
>> wrote:
>>
>> > Hi Ted,
>> >
>> > I jut gave it a look.
>> >
>> > I have updated it on the RB.
>> >
>> > Overall, this is very good and I'm eager to see that integrated! I'm
>> > waiting for this feature since the beginning ;)
>> >
>> > Regarding non adjacent regions merge? Will the system still be
>> > consistent after that? Or will hbck report some regions overlaps?
>> >
>> > JM
>> >
>> >
>> > 2013/3/16 Ted Yu <[EMAIL PROTECTED]>:
>> >> Hi,
>> >> On behalf of Chunhui, I am requesting review for HBASE-7403 Online
>> Merge.
>> >>
>> >> This JIRA was created 3 months ago.
>> >> Chunhui has responded to review comments very promptly, including a
>> major
>> >> rewrite around the time split transaction was rewritten.
>> >>
>> >> This feature has widely been requested. I feel the patch is mostly
>> ready to
>> >> go in.
>> >> Here is brief recap of the steps.
>> >>
>> >> Process of merging two regions:
>> >>
>> >> a.client sends RPC (dispatch merging regions) to master
>> >> b.master moves the regions together (on the regionserver where the more
>> >> heavily loaded region resided)
>> >> c.master sends RPC (merge regions) to this regionserver
>> >> d.Regionserver executes the region merge transaction in the thread pool
>> >>
>> >> I think step b is a nice simplification for the problem. In previous
>> >> versions of the patch, the two merging regions stay on respective
>> servers
>> >> which required more complex coordination through zookeeper.
>> >>
>> >> High level comment as well as detailed review are both welcome.
>> >>
>> >> Thanks
>>

Kevin,
Could we repair overlapping regions by hbck now, except offline merge?  I think not, if so, please tell me the issue, thanks
 
Regarding non adjacent regions merge,  we will throw the exception as the following:
 
+    if (!forcible && !HRegionInfo.areAdjacent(regionStateA.getRegion(),
+            regionStateB.getRegion())) {
+      throw new ServiceException("Unable to merge not adjacent regions "
+          + regionStateA.getRegion().getRegionNameAsString() + ", "
+          + regionStateB.getRegion().getRegionNameAsString()
+          + " where forcible = " + forcible);
+    }
 
Using force flag to merge non adjacent regions, I think it's easy to understand and could manually merge any two regions if user want. Not only for repairing case, also testing case
 
Therefore, I think Ted' thought would limit the freedom of regions merge.
 
Maybe we are worried about the inconsistency if user incorrectly call a force regions merge,  I think it is similar with HBaseAdmin#assignRegion possibly triggering multi-assign
 
Cheers
Chunhui

At 2013-03-16 21:21:44,"Kevin O'dell" <[EMAIL PROTECTED]> wrote:
>Ted,
>
>  Why would we use that merge tool when hbck will repair that?  Should we
>throw a warning and tell the user to run repair first?
>On Mar 16, 2013 9:17 AM, "Ted" <[EMAIL PROTECTED]> wrote:
>
>> Chunhui replied to this question on review board.
>>
>> Basically the force option is to repair overlapping regions or table with
>> hole in its regions.
>>
>> Personally I think online merge should detect merging regions with hole in
>> between them and not require force flag in that case because logically
>> they're adjacent.
>>
>> Cheers
>>
>> On Mar 16, 2013, at 5:52 AM, Jean-Marc Spaggiari <[EMAIL PROTECTED]>
>> wrote:
>>
>> > Hi Ted,
>> >
>> > I jut gave it a look.
>> >
>> > I have updated it on the RB.
>> >
>> > Overall, this is very good and I'm eager to see that integrated! I'm
>> > waiting for this feature since the beginning ;)
>> >
>> > Regarding non adjacent regions merge? Will the system still be
>> > consistent after that? Or will hbck report some regions overlaps?
>> >
>> > JM
>> >
>> >
>> > 2013/3/16 Ted Yu <[EMAIL PROTECTED]>:
>> >> Hi,
>> >> On behalf of Chunhui, I am requesting review for HBASE-7403 Online
>> Merge.
>> >>
>> >> This JIRA was created 3 months ago.
>> >> Chunhui has responded to review comments very promptly, including a
>> major
>> >> rewrite around the time split transaction was rewritten.
>> >>
>> >> This feature has widely been requested. I feel the patch is mostly
>> ready to
>> >> go in.
>> >> Here is brief recap of the steps.
>> >>
>> >> Process of merging two regions:
>> >>
>> >> a.client sends RPC (dispatch merging regions) to master
>> >> b.master moves the regions together (on the regionserver where the more
>> >> heavily loaded region resided)
>> >> c.master sends RPC (merge regions) to this regionserver
>> >> d.Regionserver executes the region merge transaction in the thread pool
>> >>
>> >> I think step b is a nice simplification for the problem. In previous
>> >> versions of the patch, the two merging regions stay on respective
>> servers
>> >> which required more complex coordination through zookeeper.
>> >>
>> >> High level comment as well as detailed review are both welcome.
>> >>
>> >> Thanks
>>

On Sat, Mar 16, 2013 at 7:25 AM, Chunhui Shen <[EMAIL PROTECTED]> wrote:

> Kevin,
> Could we repair overlapping regions by hbck now, except offline merge?  I
> think not, if so, please tell me the issue, thanks
>
> hbck has two options to repair overlapping regions -- either merge them or
"sideline" them so they can data can be restored via a bulk load.

http://hbase.apache.org/book/apbs04.html
--
// Jonathan Hsieh (shay)
// Software Engineer, Cloudera
// [EMAIL PROTECTED]