> On 2010-06-11 09:59:29, stack wrote: > > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 1439 > > <http://review.hbase.org/r/167/diff/1/?file=1237#file1237line1439> > > > > Should this be public? Isn't it just used internally?
good call, will make it package private > On 2010-06-11 09:59:29, stack wrote: > > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 1442 > > <http://review.hbase.org/r/167/diff/1/?file=1237#file1237line1442> > > > > Is this a copy? Yep. I did it the simple way following the pseudocode in the JIRA, but we can definitely do it more efficiently by indexing into a list or arraylist. My bet is that the efficiency gains are marginal compared to the cost of actually writing to the WAL, etc, and this patch should already be a big gain over what we used to have. After this is committed let's file a followup JIRA to remove extra shallow copies. I'll add a TODO in the code also. > On 2010-06-11 09:59:29, stack wrote: > > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 1467 > > <http://review.hbase.org/r/167/diff/1/?file=1237#file1237line1467> > > > > You were going to replace these w/ something from guava (or is this it?) The guava Preconditions thing is supposed to be more for parameter checking, etc, than for internal assertions. Currently these don't get run, but I think we should enable -ea at least for unit tests. > On 2010-06-11 09:59:29, stack wrote: > > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 1482 > > <http://review.hbase.org/r/167/diff/1/?file=1237#file1237line1482> > > > > This creates new Map, pass in Map.Entry instead? I thought both .keySet() and .entrySet() were just views into the existing map? (and p.getFamilyMap() just returns a member) > On 2010-06-11 09:59:29, stack wrote: > > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 1777 > > <http://review.hbase.org/r/167/diff/1/?file=1237#file1237line1777> > > > > w can never be null here? (There was null check previous) yea, since it's assigned first in the try block, and that function doesn't throw exceptions. - Todd ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://review.hbase.org/r/167/#review181 ----------------------------------------------------------- On 2010-06-11 00:50:05, Todd Lipcon wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://review.hbase.org/r/167/ > ----------------------------------------------------------- > > (Updated 2010-06-11 00:50:05) > > > Review request for hbase, Kannan Muthukkaruppan and Ryan Rawson. > > > Summary > ------- > > I implemented the "mini batching" idea we talked about on the JIRA. > > This currently breaks some of the error handling, so I dont intend to commit > as is, but everyone is busy so wanted to put a review up now while I tidy up > the rest. > > > This addresses bug HBASE-2353. > http://issues.apache.org/jira/browse/HBASE-2353 > > > Diffs > ----- > > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 6b6d098 > src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java > a1baff4 > src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 034690e > > Diff: http://review.hbase.org/r/167/diff > > > Testing > ------- > > Some PEs on a real sync-enabled cluster, seems faster but haven't done > scientific benchmarking. > > > Thanks, > > Todd > >
