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