hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "jiraposter@reviews.apache.org (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-4528) The put operation can release the rowlock before sync-ing the Hlog
Date Tue, 18 Oct 2011 18:53:11 GMT

    [ https://issues.apache.org/jira/browse/HBASE-4528?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13129940#comment-13129940
] 

jiraposter@reviews.apache.org commented on HBASE-4528:
------------------------------------------------------


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2141/#review2652
-----------------------------------------------------------

Ship it!


+1 for commit to trunk.

Great work, Dhruba.  Really nice comments explaining the changes too.

A few minor nits, mostly docs and stuff.  My major issue is around a better test of some of
the potential race conditions.  Let's file a follow-up JIRA to deal with that.  I'm good with
committing now as this is only going to trunk and there's plenty of time to continue making
improvements and better tests.  We'll be brining this to our 92 branch so it'll get cluster
testing as well.


/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
<https://reviews.apache.org/r/2141/#comment5963>

    it's not before we commit, it's before we even begin iterating memstore / writing the
storefiles, right?  (a distinction we discussed yesterday)



/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
<https://reviews.apache.org/r/2141/#comment5964>

    nice fix



/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
<https://reviews.apache.org/r/2141/#comment5965>

    this is an awesome comment



/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
<https://reviews.apache.org/r/2141/#comment5966>

    This is not really Write to WAL?  This is just building the WALEdit?



/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
<https://reviews.apache.org/r/2141/#comment5967>

    good



/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
<https://reviews.apache.org/r/2141/#comment5968>

    nit, whitespace



/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
<https://reviews.apache.org/r/2141/#comment5969>

    add something like ", or null to create and complete an rwcc transaction within the call?



/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
<https://reviews.apache.org/r/2141/#comment5970>

    the description seems a little vague.  this is specifically for exceptions during log
syncing when using early-lock-release?  or does this / will this potentially cover other failures?
also, some whitespace in this method.



/src/main/java/org/apache/hadoop/hbase/regionserver/MemStore.java
<https://reviews.apache.org/r/2141/#comment5974>

    TODO: optimize  (not a big deal because this is rarely used but you can use tailMaps/iterators
and such to make this more efficient, especially if the set of input KVs are sorted)



/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
<https://reviews.apache.org/r/2141/#comment5975>

    rollbacks only happen on the MemStore?  never the snapshot?  are there race conditions
here?



/src/test/java/org/apache/hadoop/hbase/regionserver/TestParallelPut.java
<https://reviews.apache.org/r/2141/#comment5976>

    should we be flushing concurrently?  and doing other nasty things like mocking an exception
out of the sync and verifying the rollback?  i think a majority of the change is easy enough
to verify through code analysis, and i think it's fine to commit this change as-is, but i'm
worried about some kind of race between snapshotting, waiting for the read point to advance,
an hlog exception, and rollback of the memstore.  need to ensure rwcc still moves forward,
need to ensure edits don't make it to hlog / get replayed, if post-snapshot there are new
edits going to memstore and a new hlog, that the right thing happens w.r.t. that hlog sync
either succeeding or failing again.
    
    i'm cool with filing a follow-up JIRA about making this test more thorough.


- Jonathan


On 2011-10-18 07:04:47, Dhruba Borthakur wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/2141/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-10-18 07:04:47)
bq.  
bq.  
bq.  Review request for hbase.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  The changes the multiPut operation so that the sync to the wal occurs outside the rowlock.
bq.  
bq.  This enhancement is done only to HRegion.mut(Put[]) because this is the only method that
gets invoked from an application. The HRegion.put(Put) is used only by unit tests and should
possibly be deprecated.
bq.  
bq.  
bq.  This addresses bug HBASE-4528.
bq.      https://issues.apache.org/jira/browse/HBASE-4528
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 1185500 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueSkipListSet.java 1185500

bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/MemStore.java 1185500 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/ReadWriteConsistencyControl.java
1185500 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 1185500 
bq.    /src/main/java/org/apache/hadoop/hbase/regionserver/StoreFlusher.java 1185500 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestParallelPut.java PRE-CREATION

bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/TestStore.java 1185500 
bq.    /src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestLogRolling.java 1185500

bq.  
bq.  Diff: https://reviews.apache.org/r/2141/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  I ran TestLogRolling over and over again, about 50 times, not failed a single time.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Dhruba
bq.  
bq.


                
> The put operation can release the rowlock before sync-ing the Hlog
> ------------------------------------------------------------------
>
>                 Key: HBASE-4528
>                 URL: https://issues.apache.org/jira/browse/HBASE-4528
>             Project: HBase
>          Issue Type: Improvement
>          Components: regionserver
>            Reporter: dhruba borthakur
>            Assignee: dhruba borthakur
>             Fix For: 0.94.0
>
>         Attachments: appendNoSync5.txt, appendNoSyncPut1.txt, appendNoSyncPut2.txt, appendNoSyncPut3.txt,
appendNoSyncPut4.txt, appendNoSyncPut5.txt, appendNoSyncPut6.txt
>
>
> This allows for better throughput when there are hot rows. A single row update improves
from 100 puts/sec/server to 5000 puts/sec/server.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Mime
View raw message