You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hbase.apache.org by Jean-Daniel Cryans <jd...@apache.org> on 2010/08/20 02:57:26 UTC

Review Request: HLog cleanup is done under the updateLock, major slowdown

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/694/
-----------------------------------------------------------

Review request for hbase.


Summary
-------

Simply moves the LogActionListeners and outputfiles handling out of the updateLock synchronization.


This addresses bug HBASE-2922.
    http://issues.apache.org/jira/browse/HBASE-2922


Diffs
-----

  /trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 987355 

Diff: http://review.cloudera.org/r/694/diff


Testing
-------

Unit tests and some PEs.


Thanks,

Jean-Daniel


Re: Review Request: HLog cleanup is done under the updateLock, major slowdown

Posted by st...@duboce.net.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/694/#review980
-----------------------------------------------------------

Ship it!


OK

+1

- stack


On 2010-08-20 15:58:27, Jean-Daniel Cryans wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/694/
> -----------------------------------------------------------
> 
> (Updated 2010-08-20 15:58:27)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> Simply moves the LogActionListeners and outputfiles handling out of the updateLock synchronization, as well as the creation of the next log file.
> 
> 
> This addresses bug HBASE-2922.
>     http://issues.apache.org/jira/browse/HBASE-2922
> 
> 
> Diffs
> -----
> 
>   /trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 987355 
> 
> Diff: http://review.cloudera.org/r/694/diff
> 
> 
> Testing
> -------
> 
> Unit tests and some PEs.
> 
> 
> Thanks,
> 
> Jean-Daniel
> 
>


Re: Review Request: HLog cleanup is done under the updateLock, major slowdown

Posted by st...@duboce.net.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/694/#review976
-----------------------------------------------------------



/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<http://review.cloudera.org/r/694/#comment3169>

    This is an invariant?  Will multiple threads arriving here at same time pickup same filenum?



/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<http://review.cloudera.org/r/694/#comment3168>

    Should this be inside the update lock?  If many threads, each will create a writer?  Or is thought that this is mostly single-threaded and that if multiple threads arrive here at same time, its unusual and just let each roll?



/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<http://review.cloudera.org/r/694/#comment3170>

    Yeah, should this be inside the update lock so many concurrent threads don't all create instances of these streams?



/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<http://review.cloudera.org/r/694/#comment3172>

    Yes, this should be outside of the sync block.



/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<http://review.cloudera.org/r/694/#comment3171>

    Yeah, is this safe?  If two threads come in here at same time, they proceed in series but when the second gets to here he's going to claim he rolled the wrong old file?


- stack


On 2010-08-20 15:58:27, Jean-Daniel Cryans wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/694/
> -----------------------------------------------------------
> 
> (Updated 2010-08-20 15:58:27)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> Simply moves the LogActionListeners and outputfiles handling out of the updateLock synchronization, as well as the creation of the next log file.
> 
> 
> This addresses bug HBASE-2922.
>     http://issues.apache.org/jira/browse/HBASE-2922
> 
> 
> Diffs
> -----
> 
>   /trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 987355 
> 
> Diff: http://review.cloudera.org/r/694/diff
> 
> 
> Testing
> -------
> 
> Unit tests and some PEs.
> 
> 
> Thanks,
> 
> Jean-Daniel
> 
>


Re: Review Request: HLog cleanup is done under the updateLock, major slowdown

Posted by Jean-Daniel Cryans <jd...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/694/
-----------------------------------------------------------

(Updated 2010-08-20 15:58:27.771023)


Review request for hbase.


Summary (updated)
-------

Simply moves the LogActionListeners and outputfiles handling out of the updateLock synchronization, as well as the creation of the next log file.


This addresses bug HBASE-2922.
    http://issues.apache.org/jira/browse/HBASE-2922


Diffs
-----

  /trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 987355 

Diff: http://review.cloudera.org/r/694/diff


Testing
-------

Unit tests and some PEs.


Thanks,

Jean-Daniel


Re: Review Request: HLog cleanup is done under the updateLock, major slowdown

Posted by Jean-Daniel Cryans <jd...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/694/
-----------------------------------------------------------

(Updated 2010-08-20 15:51:17.366257)


Review request for hbase.


Changes
-------

New patch, I decided to dig even more into this problem as I'm thinking that what happens in the updateLock should be as quick as possible. The difference with the previous patch is that the new HLog is created before we take the updateLock, as well as all the preparation surrounding that. Then I tested on a randomWrite 4 + 6 incrementing threads on a single RS that was instrumented to sysout the time it took to go through the sync block in rollWriter.

Without the patch:
Test duration: 406835ms
Total sync block duration: 56048ms
Percentage of the time updates were blocked: 13.7%
Average time in sync block: 610ms
Median time in sync block: 37ms

With the patch:
Test duration: 383503ms
Total sync block duration: 14072ms
Percentage of the time updates were blocked: 3.6%
Average time in sync block: 142ms
Median time in sync block: 3ms

Comments:
I was surprised by how closing a HLog could take a wildly varying time. In both cases I saw multi-seconds to close a writer (as you can tell by the averages compared to medians). In any case, my patch makes it much faster and less blocking. Your mileage may vary.


Summary
-------

Simply moves the LogActionListeners and outputfiles handling out of the updateLock synchronization.


This addresses bug HBASE-2922.
    http://issues.apache.org/jira/browse/HBASE-2922


Diffs (updated)
-----

  /trunk/src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 987355 

Diff: http://review.cloudera.org/r/694/diff


Testing
-------

Unit tests and some PEs.


Thanks,

Jean-Daniel