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/11/02 00:31:36 UTC

Re: 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/
-----------------------------------------------------------

(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

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>.

(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.

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>.

(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.

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