hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From st...@duboce.net
Subject Re: Review Request: Batch writes should sync to HLog in batches
Date Fri, 11 Jun 2010 17:58:18 GMT


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


Mime
View raw message