hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jonathan Gray" <jg...@apache.org>
Subject Re: Review Request: Fix periodic major compaction (HBASE-2990) and expiration check (HBASE-3083)
Date Thu, 11 Nov 2010 17:26:34 GMT


> On 2010-11-11 06:23:59, Kannan Muthukkaruppan wrote:
> > Last major should be tracked on per region+CF basis, rather than just a per region
basis, right?
> > 
> > Nicolas's & Amit's recent changes to compaction algorithm already makes it such
that different column families within the same region could end up having their major compactions
done at different times. [A minor which includes all files will implicitly get promoted to
a major].
> > 
> > Is there some boolean state/flag in the HFile's meta data already that tells us
 if the file was the result of a major compaction or not?
> > 
> > If so, isn't it sufficient to use the timestamp of the file which has the "major
compaction" tag to determine when a particular region+CF was last major compacted?
> >
> 
> stack wrote:
>     Kannan, yes, there is a flag on end of HFile already which says if file is result
of a major compaction and yes, it would make more sense to do this on a per-Store level (though
up to this its been at the Region level).  And thanks, your note reminds me that all is different
since the new compaction code went in.   I'm thinking the latter probably even 'fixes' the
original issue.  Lets see.  Meantime, I think we should move this issue out of 0.90.  We can
then take the time with the implementation.

I think I'm -1 on moving out of 0.90.  This is an actual bug and others have already -1'd
the punting from 0.90 when I tried to do that earlier :)

@Kannan, I don't see how Amit's change allows majors to be done on one family but not the
other.  This is a change from Nicolas' patch (though it's actually undocumented from what
I read), but looking at the code it does upgrade to major if all files selected.

In any case, I don't actually think that's necessarily a deal breaker.  This check is only
for periodic major compaction checking.  I would say the expected behavior there is that if
one family did get a minor turned into major but the other didn't, you would respect the lastMajor
of the family that didn't get the previous major compaction upgrade.  (Periodic major must
happen on all stores for now while we live in the world of this stuff being done at region
level not store level).  So it wouldn't break that case.

The case that it's one family, or all families get upgraded, then you would be working off
of an 'old' lastMajor stamp, yes.  However, at the least, you would avoid doing anything crazy
like re-major compacting a single file.  There are checks that even if the period is expired,
if there's only one file and it's the result of a major compaction, then don't do the major.
 Once a new file did show up in at least one store, then a periodic major compaction would
happen, yes.

If we think this is a big deal, I could think of one additional way to mitigate.  We make
the decision to do a major compaction at the Store level (the periodic major compact check
calls to the region, the region iterates the stores and if any returns yes then it does a
major).  We could bake in to the Store check, in addition to looking at the region-level lastMajor,
looking at the current files of the store for any that are major compacted.  If there is a
major compacted file, and it is more recent than the region-level lastMajor, then we use that
stamp to check against the period expiration check.  I think that would work.  lastMajorStamp
= Math.max(regionLevelLastMajor, lastMajorFromFilesOfThisStore)... if the filesFromThisStore
don't have any major compacted files, then will always use regionLevelLastMajor which is the
fix for HBASE-2990.


- Jonathan


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


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


Mime
View raw message