> 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? > > Todd Lipcon wrote: > 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.
Fine by me > 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?) > > Todd Lipcon wrote: > 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. NM. Just saw this "By default, Surefire enables JVM assertions for the execution of your test cases." So, keep your asserts as is (we should all take them on) > 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? > > Todd Lipcon wrote: > I thought both .keySet() and .entrySet() were just views into the > existing map? (and p.getFamilyMap() just returns a member) Looks like I'm wrong, at least regards JDK1.6. Internally it seems to use entrySet. Below is from java.util.AbstractMap: public Set<K> keySet() { if (keySet == null) { keySet = new AbstractSet<K>() { public Iterator<K> iterator() { return new Iterator<K>() { private Iterator<Entry<K,V>> i = entrySet().iterator(); .... - stack ----------------------------------------------------------- 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 > >
