hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From st...@duboce.net
Subject Re: Review Request: Reconstruction log playback has no bounds on memory used
Date Thu, 24 Jun 2010 06:09:06 GMT


> On 2010-06-22 16:07:23, Nicolas wrote:
> > couple questions after reviewing.  didn't look at previous reviews first, so sorry
if I duplicated commentary

Thanks for reviewing Nicolas.


> On 2010-06-22 16:07:23, Nicolas wrote:
> > src/main/java/org/apache/hadoop/hbase/HConstants.java, line 143
> > <http://review.hbase.org/r/179/diff/2/?file=1356#file1356line143>
> >
> >     don't you still want to keep around code to read oldlogfile.log & just remove
write path?  We're not changing Log file format between 0.20=>0.21, so a customer should
be able to cleanly upgrade.

I think the format has changed between 0.20 and 0.21, no? (we envelope all edits on a row
now, for example, whereas in 0.20 we just did edits as they came in).

So, to read in old WAL logs, we're talking migration -- reading w/ a class that understands
old format and converting to the new.   But, at least in the past, the first requirement migrating
has been a clean shutdown of old hbase cluster.  On clean shutdown, there should be no WAL
present.   In other words we've always gone out of our way for need migrating WALs across
*major* versions.


> On 2010-06-22 16:07:23, Nicolas wrote:
> > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 1897
> > <http://review.hbase.org/r/179/diff/2/?file=1358#file1358line1897>
> >
> >     technically, it's -1 if no outstanding log edits exist.  you store the max sequence
ID even if you skip all the edits.

Good point.


> On 2010-06-22 16:07:23, Nicolas wrote:
> > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 1921
> > <http://review.hbase.org/r/179/diff/2/?file=1358#file1358line1921>
> >
> >     is there a use case for putting HDFS in safe mode, then running HBase with hbase.skip.errors
do see the state of the cluster?  If so, fs.delete + fs.rename will both assert when this
is played on cluster restart.  Maybe you want to catch both and print errors?

Let me add the suggested print.

Regards what hbase does when FS under it flips out, there is https://issues.apache.org/jira/browse/HBASE-2183
that is for looking into this.


> On 2010-06-22 16:07:23, Nicolas wrote:
> > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 1981
> > <http://review.hbase.org/r/179/diff/2/?file=1358#file1358line1981>
> >
> >     do you want to update the currentEditSeqId even if it's from the wrong family?
 just making sure.

Yes.  I think thats right thing to do.  As we move through the log the seqid is increasing
regardless.


> On 2010-06-22 16:07:23, Nicolas wrote:
> > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 2002
> > <http://review.hbase.org/r/179/diff/2/?file=1358#file1358line2002>
> >
> >     do we want the option to store this HLog for post-mortem in this case?  we're
talking about CF-level, so this couldn't happen because of region splitting, right?

This condition should never happen.  Only reason it might would be if schema was edited between
log creation and new deploy.   It'd be cumbersome adding a keep log at this stage of the processing.
 Should I open an issue? 


> On 2010-06-22 16:07:23, Nicolas wrote:
> > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 2018
> > <http://review.hbase.org/r/179/diff/2/?file=1358#file1358line2018>
> >
> >     would it make more sense to have the interval be in seconds instead of count,
then have the update give the edit count?  Or is the difference in restoring large edits (~50k)
versus small ones inconsequential?

You are right.


- stack


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/179/#review269
-----------------------------------------------------------


On 2010-06-15 14:20:05, stack wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.hbase.org/r/179/
> -----------------------------------------------------------
> 
> (Updated 2010-06-15 14:20:05)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> + Moved replay of edits up from Store up into Region. This means we play the edits once
only rather than once per Store.
> + Lots of cleanup in the replay of edits code. Uses the new flag introduced by cosmin
– hbase.skip.errors. If set, and exception processing edits, we'll fail. If false, we'll
move the broke edits file aside and keep going. Renamed the method from doReconstructionLog
to replayRecoveredEdits (reconstruction?).
> + The main change in this patch is that recovering edits, we now go in via the HRegion
main API doing put and delete so that only one code path, so replayed edits are added to the
WAL, and so a flush will be triggered if we fill memory.
> + In HStore, lots of removed code and comments since no longer does log replay. Cleanup
of maximum seqid. Calculate it instead rather than save as a data member. Its only used once
on HRegion startup.
> + Change the HRegion#initialize. It used to take 'initial files' which is a notion never
used (it was a means of putting files in place after a split but split is done internal to
HRegion so can do things in HRegions guts w/o need of exposing notion of initial files). I
removed it and added overload that takes no args which is the usual way this method is invoked.
> + Rename the product of splits, 'recovered.edits' instead of 'oldlogfile.log'
> + Added small facility to HBaseTestingUtility for creating different user in a Configuration
so can have more than one Filesystem instance the easier.
> + Redid the test TestStoreReconstruction as TestWALReplay.
> 
> 
> This addresses bug hbase-1025.
>     http://issues.apache.org/jira/browse/hbase-1025
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/hadoop/hbase/HConstants.java f5d3e94 
>   src/main/java/org/apache/hadoop/hbase/HMerge.java 62f3561 
>   src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 06e022c 
>   src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java fe9aa8a 
>   src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 2a0dcee 
>   src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java 05826f5 
>   src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 05cf17f 
>   src/test/java/org/apache/hadoop/hbase/HBaseTestCase.java dbb21d4 
>   src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java 479c661 
>   src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java 43a8a28 
>   src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreReconstruction.java 4f0417d

>   src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogSplit.java 86cf4ea 
>   src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java PRE-CREATION

> 
> Diff: http://review.hbase.org/r/179/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> stack
> 
>


Mime
View raw message