Re:Re: review request: HBASE-7403 Online Merge

2013-03-16 Thread Chunhui Shen
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 jean-m...@spaggiari.org 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 yuzhih...@gmail.com:
 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


Re:Re: review request: HBASE-7403 Online Merge

2013-03-16 Thread Chunhui Shen
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 kevin.od...@cloudera.com 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 yuzhih...@gmail.com 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 jean-m...@spaggiari.org
 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 yuzhih...@gmail.com:
  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