You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hbase.apache.org by Jonathan Gray <jg...@apache.org> on 2010/10/25 17:33:33 UTC

Review Request: Fix periodic major compaction (HBASE-2990) and expiration check (HBASE-3083)

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1089/
-----------------------------------------------------------

Review request for hbase, stack and khemani.


Summary
-------

This is a somewhat misguided attempt.  It's not done but it shows the fairly simple change to the actual major compaction code.

The hard part is:

+    long lastMajor = region.getRegionInfo().getRegionData().getLastMajor();

And the question is where to persist that.

This patch adds a new class called HRegionData into HRegionInfo that contains any number of key-value pairs of data that get persisted with the HRI.  Not really sure how I ended up there but this data seemed like an odd-man-out so adding another field seemed weird.  We also need some kind of versioning in HRI so we can add stuff w/o migrating.  There's some versioned stuff in HRData.

Just looking for some feedback / ideas.


This addresses bugs HBASE-2990 and HBASE-3083.
    http://issues.apache.org/jira/browse/HBASE-2990
    http://issues.apache.org/jira/browse/HBASE-3083


Diffs
-----

  trunk/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java 1026935 
  trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionData.java PRE-CREATION 
  trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1026935 

Diff: http://review.cloudera.org/r/1089/diff


Testing
-------


Thanks,

Jonathan


Re: Review Request: Fix periodic major compaction (HBASE-2990) and expiration check (HBASE-3083)

Posted by st...@duboce.net.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1089/#review1647
-----------------------------------------------------------



trunk/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java
<http://review.cloudera.org/r/1089/#comment5527>

    I thought you could add random key/values to HRI already?  Hmm... no, you can't.  Its only to HCD.  Ok.  Its versioned but we don't actually write out the versioning.   How you going to persist the change?   Write it to FS?  Everytime we major compact?  Want to write it into .META. instead?  Can it be ephemeral info on the HRI or does it have to persist?



trunk/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java
<http://review.cloudera.org/r/1089/#comment5528>

    Yeah, migrating to new HRI will be a prob. w/o versioning.



trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionData.java
<http://review.cloudera.org/r/1089/#comment5529>

    Check HCD?


- stack


On 2010-10-25 08:33:33, Jonathan Gray wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/1089/
> -----------------------------------------------------------
> 
> (Updated 2010-10-25 08:33:33)
> 
> 
> Review request for hbase, stack and khemani.
> 
> 
> Summary
> -------
> 
> This is a somewhat misguided attempt.  It's not done but it shows the fairly simple change to the actual major compaction code.
> 
> The hard part is:
> 
> +    long lastMajor = region.getRegionInfo().getRegionData().getLastMajor();
> 
> And the question is where to persist that.
> 
> This patch adds a new class called HRegionData into HRegionInfo that contains any number of key-value pairs of data that get persisted with the HRI.  Not really sure how I ended up there but this data seemed like an odd-man-out so adding another field seemed weird.  We also need some kind of versioning in HRI so we can add stuff w/o migrating.  There's some versioned stuff in HRData.
> 
> Just looking for some feedback / ideas.
> 
> 
> This addresses bugs HBASE-2990 and HBASE-3083.
>     http://issues.apache.org/jira/browse/HBASE-2990
>     http://issues.apache.org/jira/browse/HBASE-3083
> 
> 
> Diffs
> -----
> 
>   trunk/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java 1026935 
>   trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionData.java PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1026935 
> 
> Diff: http://review.cloudera.org/r/1089/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jonathan
> 
>


Re: Review Request: Fix periodic major compaction (HBASE-2990) and expiration check (HBASE-3083)

Posted by Jonathan Gray <jg...@apache.org>.

> On 2010-11-10 21:35:09, stack wrote:
> > Looks good to me.  Have you tested it works?  Or, you want to write a unit test at least for the reading/writing of the new file?

I did a little bit of testing but not much.  Let's finish other discussion about this jira but I will write a unit test today that verifies we are properly reading/writing the file.


- Jonathan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1089/#review1904
-----------------------------------------------------------


On 2010-11-10 17:04:39, Jonathan Gray wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/1089/
> -----------------------------------------------------------
> 
> (Updated 2010-11-10 17:04:39)
> 
> 
> Review request for hbase, stack and khemani.
> 
> 
> Summary
> -------
> 
> This is a somewhat misguided attempt.  It's not done but it shows the fairly simple change to the actual major compaction code.
> 
> The hard part is:
> 
> +    long lastMajor = region.getRegionInfo().getRegionData().getLastMajor();
> 
> And the question is where to persist that.
> 
> This patch adds a new class called HRegionData into HRegionInfo that contains any number of key-value pairs of data that get persisted with the HRI.  Not really sure how I ended up there but this data seemed like an odd-man-out so adding another field seemed weird.  We also need some kind of versioning in HRI so we can add stuff w/o migrating.  There's some versioned stuff in HRData.
> 
> Just looking for some feedback / ideas.
> 
> 
> This addresses bugs HBASE-2990 and HBASE-3083.
>     http://issues.apache.org/jira/browse/HBASE-2990
>     http://issues.apache.org/jira/browse/HBASE-3083
> 
> 
> Diffs
> -----
> 
>   trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1033780 
>   trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1033780 
> 
> Diff: http://review.cloudera.org/r/1089/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jonathan
> 
>


Re: Review Request: Fix periodic major compaction (HBASE-2990) and expiration check (HBASE-3083)

Posted by st...@duboce.net.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1089/#review1904
-----------------------------------------------------------

Ship it!


Looks good to me.  Have you tested it works?  Or, you want to write a unit test at least for the reading/writing of the new file?

- stack


On 2010-11-10 17:04:39, Jonathan Gray wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/1089/
> -----------------------------------------------------------
> 
> (Updated 2010-11-10 17:04:39)
> 
> 
> Review request for hbase, stack and khemani.
> 
> 
> Summary
> -------
> 
> This is a somewhat misguided attempt.  It's not done but it shows the fairly simple change to the actual major compaction code.
> 
> The hard part is:
> 
> +    long lastMajor = region.getRegionInfo().getRegionData().getLastMajor();
> 
> And the question is where to persist that.
> 
> This patch adds a new class called HRegionData into HRegionInfo that contains any number of key-value pairs of data that get persisted with the HRI.  Not really sure how I ended up there but this data seemed like an odd-man-out so adding another field seemed weird.  We also need some kind of versioning in HRI so we can add stuff w/o migrating.  There's some versioned stuff in HRData.
> 
> Just looking for some feedback / ideas.
> 
> 
> This addresses bugs HBASE-2990 and HBASE-3083.
>     http://issues.apache.org/jira/browse/HBASE-2990
>     http://issues.apache.org/jira/browse/HBASE-3083
> 
> 
> Diffs
> -----
> 
>   trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1033780 
>   trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1033780 
> 
> Diff: http://review.cloudera.org/r/1089/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jonathan
> 
>


Re: Review Request: Fix periodic major compaction (HBASE-2990) and expiration check (HBASE-3083)

Posted by Jonathan Gray <jg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1089/
-----------------------------------------------------------

(Updated 2010-11-11 09:53:02.338546)


Review request for hbase, stack and khemani.


Changes
-------

Adds change described in comment to Kannan.  If the oldest file is the result of a major compaction, we use the latest stamp between the region-level lastMajorTime and this store-level lastMajorTime (the modification time of the major compacted file).  This should address the issue of dealing w/ minors upgraded to majors not being accounted for in the region-level lastMajorTime.


Summary
-------

This is a somewhat misguided attempt.  It's not done but it shows the fairly simple change to the actual major compaction code.

The hard part is:

+    long lastMajor = region.getRegionInfo().getRegionData().getLastMajor();

And the question is where to persist that.

This patch adds a new class called HRegionData into HRegionInfo that contains any number of key-value pairs of data that get persisted with the HRI.  Not really sure how I ended up there but this data seemed like an odd-man-out so adding another field seemed weird.  We also need some kind of versioning in HRI so we can add stuff w/o migrating.  There's some versioned stuff in HRData.

Just looking for some feedback / ideas.


This addresses bugs HBASE-2990 and HBASE-3083.
    http://issues.apache.org/jira/browse/HBASE-2990
    http://issues.apache.org/jira/browse/HBASE-3083


Diffs (updated)
-----

  trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1034008 
  trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1034008 

Diff: http://review.cloudera.org/r/1089/diff


Testing
-------


Thanks,

Jonathan


Re: Review Request: Fix periodic major compaction (HBASE-2990) and expiration check (HBASE-3083)

Posted by Jonathan Gray <jg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1089/
-----------------------------------------------------------

(Updated 2010-11-10 17:04:39.281251)


Review request for hbase, stack and khemani.


Changes
-------

Fixes heap size calculation on HRegion (TestHeapSize failed)


Summary
-------

This is a somewhat misguided attempt.  It's not done but it shows the fairly simple change to the actual major compaction code.

The hard part is:

+    long lastMajor = region.getRegionInfo().getRegionData().getLastMajor();

And the question is where to persist that.

This patch adds a new class called HRegionData into HRegionInfo that contains any number of key-value pairs of data that get persisted with the HRI.  Not really sure how I ended up there but this data seemed like an odd-man-out so adding another field seemed weird.  We also need some kind of versioning in HRI so we can add stuff w/o migrating.  There's some versioned stuff in HRData.

Just looking for some feedback / ideas.


This addresses bugs HBASE-2990 and HBASE-3083.
    http://issues.apache.org/jira/browse/HBASE-2990
    http://issues.apache.org/jira/browse/HBASE-3083


Diffs (updated)
-----

  trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1033780 
  trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1033780 

Diff: http://review.cloudera.org/r/1089/diff


Testing
-------


Thanks,

Jonathan


Re: Review Request: Fix periodic major compaction (HBASE-2990) and expiration check (HBASE-3083)

Posted by Jonathan Gray <jg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1089/
-----------------------------------------------------------

(Updated 2010-11-10 16:55:42.907573)


Review request for hbase, stack and khemani.


Changes
-------

whitespace fix


Summary
-------

This is a somewhat misguided attempt.  It's not done but it shows the fairly simple change to the actual major compaction code.

The hard part is:

+    long lastMajor = region.getRegionInfo().getRegionData().getLastMajor();

And the question is where to persist that.

This patch adds a new class called HRegionData into HRegionInfo that contains any number of key-value pairs of data that get persisted with the HRI.  Not really sure how I ended up there but this data seemed like an odd-man-out so adding another field seemed weird.  We also need some kind of versioning in HRI so we can add stuff w/o migrating.  There's some versioned stuff in HRData.

Just looking for some feedback / ideas.


This addresses bugs HBASE-2990 and HBASE-3083.
    http://issues.apache.org/jira/browse/HBASE-2990
    http://issues.apache.org/jira/browse/HBASE-3083


Diffs (updated)
-----

  trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1033780 
  trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1033780 

Diff: http://review.cloudera.org/r/1089/diff


Testing
-------


Thanks,

Jonathan


Re: Review Request: Fix periodic major compaction (HBASE-2990) and expiration check (HBASE-3083)

Posted by Jonathan Gray <jg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1089/
-----------------------------------------------------------

(Updated 2010-11-10 16:53:29.452432)


Review request for hbase, stack and khemani.


Changes
-------

Okay, completely scrapped my two previous implementations.

Instead, this is more of a hack but there's minimal code invasion.  We persist a file similar to .regioninfo in the regiondir called .lastmajor.  It contains a long of the last major compaction.  We use it on the periodic major compaction check and persist it when it is updated at the end of a major.

If the file does not exist (as it never will the first time a region is opened w/ this code), it sets/persists the last major stamp as now().


Summary
-------

This is a somewhat misguided attempt.  It's not done but it shows the fairly simple change to the actual major compaction code.

The hard part is:

+    long lastMajor = region.getRegionInfo().getRegionData().getLastMajor();

And the question is where to persist that.

This patch adds a new class called HRegionData into HRegionInfo that contains any number of key-value pairs of data that get persisted with the HRI.  Not really sure how I ended up there but this data seemed like an odd-man-out so adding another field seemed weird.  We also need some kind of versioning in HRI so we can add stuff w/o migrating.  There's some versioned stuff in HRData.

Just looking for some feedback / ideas.


This addresses bugs HBASE-2990 and HBASE-3083.
    http://issues.apache.org/jira/browse/HBASE-2990
    http://issues.apache.org/jira/browse/HBASE-3083


Diffs (updated)
-----

  trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1033780 
  trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1033780 

Diff: http://review.cloudera.org/r/1089/diff


Testing
-------


Thanks,

Jonathan


Re: Review Request: Fix periodic major compaction (HBASE-2990) and expiration check (HBASE-3083)

Posted by Jonathan Gray <jg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1089/
-----------------------------------------------------------

(Updated 2010-11-01 16:31:36.124095)


Review request for hbase, stack and khemani.


Changes
-------

Latest version.  I guess I should write some tests...


Summary
-------

This is a somewhat misguided attempt.  It's not done but it shows the fairly simple change to the actual major compaction code.

The hard part is:

+    long lastMajor = region.getRegionInfo().getRegionData().getLastMajor();

And the question is where to persist that.

This patch adds a new class called HRegionData into HRegionInfo that contains any number of key-value pairs of data that get persisted with the HRI.  Not really sure how I ended up there but this data seemed like an odd-man-out so adding another field seemed weird.  We also need some kind of versioning in HRI so we can add stuff w/o migrating.  There's some versioned stuff in HRData.

Just looking for some feedback / ideas.


This addresses bugs HBASE-2990 and HBASE-3083.
    http://issues.apache.org/jira/browse/HBASE-2990
    http://issues.apache.org/jira/browse/HBASE-3083


Diffs (updated)
-----

  trunk/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java 1029789 
  trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java 1029789 
  trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionData.java PRE-CREATION 
  trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 1029789 
  trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1029789 

Diff: http://review.cloudera.org/r/1089/diff


Testing
-------


Thanks,

Jonathan