> On 2010-06-22 00:14:05, stack wrote: > > +1 (if it passes all tests). Nit-picks below.
Cool, I'll run it through my Hudson overnight, plus running a cluster test on my 5-node test cluster now. Will commit tomorrow midday with changes below addressed assuming testing goes OK. > On 2010-06-22 00:14:05, stack wrote: > > src/main/java/org/apache/hadoop/hbase/regionserver/Store.java, line 333 > > <http://review.hbase.org/r/216/diff/1/?file=1529#file1529line333> > > > > ? There was notion of a '_tmp' already? > > > > I'd say name it '.tmp'... since a '.' prefix seems to be our convention > > given logs dir at top-level has a '.' prefix. yea, the bulk load stuff uses a tmp dir if you ask the regionserver to load a file which is stored on a different filesystem than the region itself. I'll switch to .tmp though as you suggest (and move it into HConstants to replace the now-unused compactions.dir or what have you) > On 2010-06-22 00:14:05, stack wrote: > > src/main/java/org/apache/hadoop/hbase/regionserver/Store.java, line 464 > > <http://review.hbase.org/r/216/diff/1/?file=1529#file1529line464> > > > > Why not keep old name and just move dirs? Why create a new unique name? I was worried that what's unique in the old dir might not be unique in the new > On 2010-06-22 00:14:05, stack wrote: > > src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java, line 411 > > <http://review.hbase.org/r/216/diff/1/?file=1531#file1531line411> > > > > Nice test. thx! at some point I'd like to combine this with the one in TestFSErrorsExposed and make a little mini-framework out of it... something for next month perhaps. - Todd ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://review.hbase.org/r/216/#review261 ----------------------------------------------------------- On 2010-06-21 23:57:39, Todd Lipcon wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://review.hbase.org/r/216/ > ----------------------------------------------------------- > > (Updated 2010-06-21 23:57:39) > > > Review request for hbase, stack and Ryan Rawson. > > > Summary > ------- > > Fixes bugs where an exception in the middle of flushing a file leaves a > half-written StoreFile in the region dir, preventing that region from > recovering, or, in the case of transient errors, causing silent loss of half > a file's worth of data. > > I also got rid of the compaction dir here, and am just using one region-wide > tmp dir. Is there some reason this is a bad idea? > > > This addresses bug HBASE-2729. > http://issues.apache.org/jira/browse/HBASE-2729 > > > Diffs > ----- > > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1794df8 > src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 04b7522 > src/test/java/org/apache/hadoop/hbase/regionserver/TestCompaction.java > 9e5ca46 > src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java a65e947 > > Diff: http://review.hbase.org/r/216/diff > > > Testing > ------- > > Ran TestCompaction and TestStore. Will start a cluster test running before I > go to bed. > > > Thanks, > > Todd > >
