[
https://issues.apache.org/jira/browse/HBASE-4528?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13129940#comment-13129940
]
jirapos...@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
