You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "Vlad Dogaru (JIRA)" <ji...@apache.org> on 2011/08/11 08:55:27 UTC

[jira] [Created] (HBASE-4192) Optimize HLog for Throughput Using Delayed RPCs

Optimize HLog for Throughput Using Delayed RPCs
-----------------------------------------------

                 Key: HBASE-4192
                 URL: https://issues.apache.org/jira/browse/HBASE-4192
             Project: HBase
          Issue Type: New Feature
          Components: wal
    Affects Versions: 0.92.0
            Reporter: Vlad Dogaru
            Priority: Minor


Introduce a new HLog configuration parameter (batchEntries) for more aggressive batching of appends.  If this is enabled, HLog appends are not written to the HLog writer immediately, but batched and written either periodically or when a sync is requested.  Because sync times become larger, they use delayed RPCs to free up RPC handler threads.


--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (HBASE-4192) Optimize HLog for Throughput Using Delayed RPCs

Posted by "jiraposter@reviews.apache.org (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4192?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13083877#comment-13083877 ] 

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


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



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3272>

    Should this be wrapped in try/catch block ?
    We want to process all ready responses even if communication with some RPC client throws exception.


- Ted


On 2011-08-11 06:59:28, Vlad Dogaru wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/1463/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-08-11 06:59:28)
bq.  
bq.  
bq.  Review request for hbase.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  Changes:
bq.  
bq.  1.  Add hbase.region.wal.batchentries configuration parameter.  If this is
bq.  enabled, batch entries to the HLog in a queue.
bq.  2.  Use delayed RPCs for sync requests when aggresive batching is enabled.
bq.  This frees up RPC handler threads for the duration of the sync.
bq.  3.  Pass the RPC server instance all the way to down to HLog.  This is needed
bq.  to find out the current remote call, mark it as delayed, and finally complete
bq.  it when the sync is done.
bq.  4.  Use the region read-write consistency control to avoid exposing to
bq.  RegionScanners edits which have not yet been synced.
bq.  5.  Change a few tests which directly create HRegions or HLogs.  The
bq.  rpcServers passed in are null, HLog falls back to classic RPCs when it has no
bq.  knowledge of the RPC server.
bq.  6.  Add TestBatchEntriesLogRolling, which is identical to TestLogRolling,
bq.  except that it uses aggressive batching.  I'm not sure how to add tests that
bq.  verify the same functionality but don't duplicate code, suggestion are
bq.  welcome.
bq.  
bq.  The new parameter is disabled by default.
bq.  
bq.  
bq.  This addresses bug HBASE-4192.
bq.      https://issues.apache.org/jira/browse/HBASE-4192
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 6fb1da7 
bq.    src/main/java/org/apache/hadoop/hbase/master/HMaster.java a00b93d 
bq.    src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java 2f86f04 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 83ff7b2 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java f22fb6e 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/ReadWriteConsistencyControl.java 8ec53d3 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 887f736 
bq.    src/main/java/org/apache/hadoop/hbase/util/HMerge.java 9f0499e 
bq.    src/main/java/org/apache/hadoop/hbase/util/MetaUtils.java af8d734 
bq.    src/test/java/org/apache/hadoop/hbase/HBaseTestCase.java bf8004b 
bq.    src/test/java/org/apache/hadoop/hbase/TestScanMultipleVersions.java d7baba4 
bq.    src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorInterface.java 36816e8 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestBatchEntriesLogRolling.java PRE-CREATION 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogSplit.java de28418 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java dc43eb2 
bq.    src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 381ac90 
bq.  
bq.  Diff: https://reviews.apache.org/r/1463/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  All unit tests run with aggressive batching turned on and off.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Vlad
bq.  
bq.



> Optimize HLog for Throughput Using Delayed RPCs
> -----------------------------------------------
>
>                 Key: HBASE-4192
>                 URL: https://issues.apache.org/jira/browse/HBASE-4192
>             Project: HBase
>          Issue Type: New Feature
>          Components: wal
>    Affects Versions: 0.92.0
>            Reporter: Vlad Dogaru
>            Priority: Minor
>
> Introduce a new HLog configuration parameter (batchEntries) for more aggressive batching of appends.  If this is enabled, HLog appends are not written to the HLog writer immediately, but batched and written either periodically or when a sync is requested.  Because sync times become larger, they use delayed RPCs to free up RPC handler threads.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (HBASE-4192) Optimize HLog for Throughput Using Delayed RPCs

Posted by "jiraposter@reviews.apache.org (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4192?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13085453#comment-13085453 ] 

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


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



src/main/java/org/apache/hadoop/hbase/master/HMaster.java
<https://reviews.apache.org/r/1463/#comment3368>

    really what we want is not the RpcServer, but the current Call object. So I think threading that through (via ThreadLocals or parameters to append(), etc), makes more sense.


- Todd


On 2011-08-12 23:44:23, Vlad Dogaru wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/1463/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-08-12 23:44:23)
bq.  
bq.  
bq.  Review request for hbase.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  Changes:
bq.  
bq.  1.  Add hbase.region.wal.batchentries configuration parameter.  If this is
bq.  enabled, batch entries to the HLog in a queue.
bq.  2.  Use delayed RPCs for sync requests when aggresive batching is enabled.
bq.  This frees up RPC handler threads for the duration of the sync.
bq.  3.  Pass the RPC server instance all the way to down to HLog.  This is needed
bq.  to find out the current remote call, mark it as delayed, and finally complete
bq.  it when the sync is done.
bq.  4.  Use the region read-write consistency control to avoid exposing to
bq.  RegionScanners edits which have not yet been synced.
bq.  5.  Change a few tests which directly create HRegions or HLogs.  The
bq.  rpcServers passed in are null, HLog falls back to classic RPCs when it has no
bq.  knowledge of the RPC server.
bq.  6.  Add TestBatchEntriesLogRolling, which is identical to TestLogRolling,
bq.  except that it uses aggressive batching.  I'm not sure how to add tests that
bq.  verify the same functionality but don't duplicate code, suggestion are
bq.  welcome.
bq.  
bq.  The new parameter is disabled by default.
bq.  
bq.  
bq.  This addresses bug HBASE-4192.
bq.      https://issues.apache.org/jira/browse/HBASE-4192
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 7117bce 
bq.    src/main/java/org/apache/hadoop/hbase/master/HMaster.java a00b93d 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 83ff7b2 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 7a917da 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/ReadWriteConsistencyControl.java 8ec53d3 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 887f736 
bq.    src/main/resources/hbase-default.xml 66548ca 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java dc43eb2 
bq.    src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 381ac90 
bq.  
bq.  Diff: https://reviews.apache.org/r/1463/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  All unit tests run with aggressive batching turned on and off.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Vlad
bq.  
bq.



> Optimize HLog for Throughput Using Delayed RPCs
> -----------------------------------------------
>
>                 Key: HBASE-4192
>                 URL: https://issues.apache.org/jira/browse/HBASE-4192
>             Project: HBase
>          Issue Type: New Feature
>          Components: wal
>    Affects Versions: 0.92.0
>            Reporter: Vlad Dogaru
>            Priority: Minor
>
> Introduce a new HLog configuration parameter (batchEntries) for more aggressive batching of appends.  If this is enabled, HLog appends are not written to the HLog writer immediately, but batched and written either periodically or when a sync is requested.  Because sync times become larger, they use delayed RPCs to free up RPC handler threads.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (HBASE-4192) Optimize HLog for Throughput Using Delayed RPCs

Posted by "jiraposter@reviews.apache.org (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4192?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13082967#comment-13082967 ] 

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


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

Review request for hbase.


Summary
-------

Changes:

1.  Add hbase.region.wal.batchentries configuration parameter.  If this is
enabled, batch entries to the HLog in a queue.
2.  Use delayed RPCs for sync requests when aggresive batching is enabled.
This frees up RPC handler threads for the duration of the sync.
3.  Pass the RPC server instance all the way to down to HLog.  This is needed
to find out the current remote call, mark it as delayed, and finally complete
it when the sync is done.
4.  Use the region read-write consistency control to avoid exposing to
RegionScanners edits which have not yet been synced.
5.  Change a few tests which directly create HRegions or HLogs.  The
rpcServers passed in are null, HLog falls back to classic RPCs when it has no
knowledge of the RPC server.
6.  Add TestBatchEntriesLogRolling, which is identical to TestLogRolling,
except that it uses aggressive batching.  I'm not sure how to add tests that
verify the same functionality but don't duplicate code, suggestion are
welcome.

The new parameter is disabled by default.


This addresses bug HBASE-4192.
    https://issues.apache.org/jira/browse/HBASE-4192


Diffs
-----

  src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 6fb1da7 
  src/main/java/org/apache/hadoop/hbase/master/HMaster.java a00b93d 
  src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java 2f86f04 
  src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 83ff7b2 
  src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java f22fb6e 
  src/main/java/org/apache/hadoop/hbase/regionserver/ReadWriteConsistencyControl.java 8ec53d3 
  src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 887f736 
  src/main/java/org/apache/hadoop/hbase/util/HMerge.java 9f0499e 
  src/main/java/org/apache/hadoop/hbase/util/MetaUtils.java af8d734 
  src/test/java/org/apache/hadoop/hbase/HBaseTestCase.java bf8004b 
  src/test/java/org/apache/hadoop/hbase/TestScanMultipleVersions.java d7baba4 
  src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorInterface.java 36816e8 
  src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestBatchEntriesLogRolling.java PRE-CREATION 
  src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogSplit.java de28418 
  src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java dc43eb2 
  src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 381ac90 

Diff: https://reviews.apache.org/r/1463/diff


Testing
-------

All unit tests run with aggressive batching turned on and off.


Thanks,

Vlad



> Optimize HLog for Throughput Using Delayed RPCs
> -----------------------------------------------
>
>                 Key: HBASE-4192
>                 URL: https://issues.apache.org/jira/browse/HBASE-4192
>             Project: HBase
>          Issue Type: New Feature
>          Components: wal
>    Affects Versions: 0.92.0
>            Reporter: Vlad Dogaru
>            Priority: Minor
>
> Introduce a new HLog configuration parameter (batchEntries) for more aggressive batching of appends.  If this is enabled, HLog appends are not written to the HLog writer immediately, but batched and written either periodically or when a sync is requested.  Because sync times become larger, they use delayed RPCs to free up RPC handler threads.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (HBASE-4192) Optimize HLog for Throughput Using Delayed RPCs

Posted by "jiraposter@reviews.apache.org (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4192?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13084773#comment-13084773 ] 

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


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



src/main/java/org/apache/hadoop/hbase/regionserver/ReadWriteConsistencyControl.java
<https://reviews.apache.org/r/1463/#comment3352>

    The return value doesn't seem to be used.
    Can you clarify why ?


- Ted


On 2011-08-12 23:44:23, Vlad Dogaru wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/1463/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-08-12 23:44:23)
bq.  
bq.  
bq.  Review request for hbase.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  Changes:
bq.  
bq.  1.  Add hbase.region.wal.batchentries configuration parameter.  If this is
bq.  enabled, batch entries to the HLog in a queue.
bq.  2.  Use delayed RPCs for sync requests when aggresive batching is enabled.
bq.  This frees up RPC handler threads for the duration of the sync.
bq.  3.  Pass the RPC server instance all the way to down to HLog.  This is needed
bq.  to find out the current remote call, mark it as delayed, and finally complete
bq.  it when the sync is done.
bq.  4.  Use the region read-write consistency control to avoid exposing to
bq.  RegionScanners edits which have not yet been synced.
bq.  5.  Change a few tests which directly create HRegions or HLogs.  The
bq.  rpcServers passed in are null, HLog falls back to classic RPCs when it has no
bq.  knowledge of the RPC server.
bq.  6.  Add TestBatchEntriesLogRolling, which is identical to TestLogRolling,
bq.  except that it uses aggressive batching.  I'm not sure how to add tests that
bq.  verify the same functionality but don't duplicate code, suggestion are
bq.  welcome.
bq.  
bq.  The new parameter is disabled by default.
bq.  
bq.  
bq.  This addresses bug HBASE-4192.
bq.      https://issues.apache.org/jira/browse/HBASE-4192
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 7117bce 
bq.    src/main/java/org/apache/hadoop/hbase/master/HMaster.java a00b93d 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 83ff7b2 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 7a917da 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/ReadWriteConsistencyControl.java 8ec53d3 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 887f736 
bq.    src/main/resources/hbase-default.xml 66548ca 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java dc43eb2 
bq.    src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 381ac90 
bq.  
bq.  Diff: https://reviews.apache.org/r/1463/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  All unit tests run with aggressive batching turned on and off.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Vlad
bq.  
bq.



> Optimize HLog for Throughput Using Delayed RPCs
> -----------------------------------------------
>
>                 Key: HBASE-4192
>                 URL: https://issues.apache.org/jira/browse/HBASE-4192
>             Project: HBase
>          Issue Type: New Feature
>          Components: wal
>    Affects Versions: 0.92.0
>            Reporter: Vlad Dogaru
>            Priority: Minor
>
> Introduce a new HLog configuration parameter (batchEntries) for more aggressive batching of appends.  If this is enabled, HLog appends are not written to the HLog writer immediately, but batched and written either periodically or when a sync is requested.  Because sync times become larger, they use delayed RPCs to free up RPC handler threads.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (HBASE-4192) Optimize HLog for Throughput Using Delayed RPCs

Posted by "jiraposter@reviews.apache.org (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4192?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13084477#comment-13084477 ] 

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


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

(Updated 2011-08-12 23:44:23.648920)


Review request for hbase.


Changes
-------

Address most of Jon's feedback:

* change 'batchEntries' to 'useDelayedRpc'. Hopefully this describes the new feature better;
* revert the constructor usage in tests (where we were adding an additional null parameter);
* add a bunch of javadoc in HLog;
* document new parameters in hbase-default.xml;
* use wait/notify instead of yield when closing writer;
* unify the two differing append functions and change users of previous one;
* fix coprocessors; note that when using delayed RPCs the postWAL function is called in hlogFlush, when the entries are added to the writer;

Still TODO:
* parameterize TestLogRolling;
* benchmark results.


Summary
-------

Changes:

1.  Add hbase.region.wal.batchentries configuration parameter.  If this is
enabled, batch entries to the HLog in a queue.
2.  Use delayed RPCs for sync requests when aggresive batching is enabled.
This frees up RPC handler threads for the duration of the sync.
3.  Pass the RPC server instance all the way to down to HLog.  This is needed
to find out the current remote call, mark it as delayed, and finally complete
it when the sync is done.
4.  Use the region read-write consistency control to avoid exposing to
RegionScanners edits which have not yet been synced.
5.  Change a few tests which directly create HRegions or HLogs.  The
rpcServers passed in are null, HLog falls back to classic RPCs when it has no
knowledge of the RPC server.
6.  Add TestBatchEntriesLogRolling, which is identical to TestLogRolling,
except that it uses aggressive batching.  I'm not sure how to add tests that
verify the same functionality but don't duplicate code, suggestion are
welcome.

The new parameter is disabled by default.


This addresses bug HBASE-4192.
    https://issues.apache.org/jira/browse/HBASE-4192


Diffs (updated)
-----

  src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 7117bce 
  src/main/java/org/apache/hadoop/hbase/master/HMaster.java a00b93d 
  src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 83ff7b2 
  src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 7a917da 
  src/main/java/org/apache/hadoop/hbase/regionserver/ReadWriteConsistencyControl.java 8ec53d3 
  src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 887f736 
  src/main/resources/hbase-default.xml 66548ca 
  src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java dc43eb2 
  src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 381ac90 

Diff: https://reviews.apache.org/r/1463/diff


Testing
-------

All unit tests run with aggressive batching turned on and off.


Thanks,

Vlad



> Optimize HLog for Throughput Using Delayed RPCs
> -----------------------------------------------
>
>                 Key: HBASE-4192
>                 URL: https://issues.apache.org/jira/browse/HBASE-4192
>             Project: HBase
>          Issue Type: New Feature
>          Components: wal
>    Affects Versions: 0.92.0
>            Reporter: Vlad Dogaru
>            Priority: Minor
>
> Introduce a new HLog configuration parameter (batchEntries) for more aggressive batching of appends.  If this is enabled, HLog appends are not written to the HLog writer immediately, but batched and written either periodically or when a sync is requested.  Because sync times become larger, they use delayed RPCs to free up RPC handler threads.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (HBASE-4192) Optimize HLog for Throughput Using Delayed RPCs

Posted by "jiraposter@reviews.apache.org (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4192?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13085442#comment-13085442 ] 

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



bq.  On 2011-08-13 01:02:17, Todd Lipcon wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/master/HMaster.java, line 997
bq.  > <https://reviews.apache.org/r/1463/diff/2/?file=32490#file32490line997>
bq.  >
bq.  >     I really don't like that we're threading the RPCServer through to the HRegion and HLog objects. There's something really messy about it: imagine, for example, if we had two separate RPC servers on each daemon, listening on different ports (this is done in the Hadoop namenode in order to achieve some easy QoS). Then we'd have to thread two RPCServers through.
bq.  >     
bq.  >     Instead, I think the individual operations should use ThreadLocals to grab the current Call object, perhaps? If the ThreadLocal lookups are too expensive, then perhaps a new interface like "CallContext" which we thread through the processing of individual operations?

I don't understand how ThreadLocals would solve this problem.  We would still have to pass the ThreadLocal all the way from the region server to the HLog.  Is there something I am missing?


- Vlad


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


On 2011-08-12 23:44:23, Vlad Dogaru wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/1463/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-08-12 23:44:23)
bq.  
bq.  
bq.  Review request for hbase.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  Changes:
bq.  
bq.  1.  Add hbase.region.wal.batchentries configuration parameter.  If this is
bq.  enabled, batch entries to the HLog in a queue.
bq.  2.  Use delayed RPCs for sync requests when aggresive batching is enabled.
bq.  This frees up RPC handler threads for the duration of the sync.
bq.  3.  Pass the RPC server instance all the way to down to HLog.  This is needed
bq.  to find out the current remote call, mark it as delayed, and finally complete
bq.  it when the sync is done.
bq.  4.  Use the region read-write consistency control to avoid exposing to
bq.  RegionScanners edits which have not yet been synced.
bq.  5.  Change a few tests which directly create HRegions or HLogs.  The
bq.  rpcServers passed in are null, HLog falls back to classic RPCs when it has no
bq.  knowledge of the RPC server.
bq.  6.  Add TestBatchEntriesLogRolling, which is identical to TestLogRolling,
bq.  except that it uses aggressive batching.  I'm not sure how to add tests that
bq.  verify the same functionality but don't duplicate code, suggestion are
bq.  welcome.
bq.  
bq.  The new parameter is disabled by default.
bq.  
bq.  
bq.  This addresses bug HBASE-4192.
bq.      https://issues.apache.org/jira/browse/HBASE-4192
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 7117bce 
bq.    src/main/java/org/apache/hadoop/hbase/master/HMaster.java a00b93d 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 83ff7b2 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 7a917da 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/ReadWriteConsistencyControl.java 8ec53d3 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 887f736 
bq.    src/main/resources/hbase-default.xml 66548ca 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java dc43eb2 
bq.    src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 381ac90 
bq.  
bq.  Diff: https://reviews.apache.org/r/1463/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  All unit tests run with aggressive batching turned on and off.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Vlad
bq.  
bq.



> Optimize HLog for Throughput Using Delayed RPCs
> -----------------------------------------------
>
>                 Key: HBASE-4192
>                 URL: https://issues.apache.org/jira/browse/HBASE-4192
>             Project: HBase
>          Issue Type: New Feature
>          Components: wal
>    Affects Versions: 0.92.0
>            Reporter: Vlad Dogaru
>            Priority: Minor
>
> Introduce a new HLog configuration parameter (batchEntries) for more aggressive batching of appends.  If this is enabled, HLog appends are not written to the HLog writer immediately, but batched and written either periodically or when a sync is requested.  Because sync times become larger, they use delayed RPCs to free up RPC handler threads.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (HBASE-4192) Optimize HLog for Throughput Using Delayed RPCs

Posted by "dhruba borthakur (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4192?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13114823#comment-13114823 ] 

dhruba borthakur commented on HBASE-4192:
-----------------------------------------

I am proposing that we break up the patch in HB-4192 into two parts:

Part 1: Ability to release row locks while doing a sync to the HLog. This benefits the increment operation tremendously, especially when there are hot rows. The rpc handlers thread that is processing the increment operation will block till the HLog-sync is complete before returning status code to the client. No delayed RPCs here.

Part 2: Work with delayed RPCs as a succeeding patch. That will have the ability to not hold a RPC handler thread while the HLog sync is in progress.

> Optimize HLog for Throughput Using Delayed RPCs
> -----------------------------------------------
>
>                 Key: HBASE-4192
>                 URL: https://issues.apache.org/jira/browse/HBASE-4192
>             Project: HBase
>          Issue Type: New Feature
>          Components: wal
>    Affects Versions: 0.92.0
>            Reporter: Vlad Dogaru
>            Priority: Minor
>
> Introduce a new HLog configuration parameter (batchEntries) for more aggressive batching of appends.  If this is enabled, HLog appends are not written to the HLog writer immediately, but batched and written either periodically or when a sync is requested.  Because sync times become larger, they use delayed RPCs to free up RPC handler threads.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (HBASE-4192) Optimize HLog for Throughput Using Delayed RPCs

Posted by "jiraposter@reviews.apache.org (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4192?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13082997#comment-13082997 ] 

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


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


will look at this in detail tomorrow/fri, but, meanwhile, do you have some data on what the throughput improvement is for heavy write workloads?

- Todd


On 2011-08-11 06:59:28, Vlad Dogaru wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/1463/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-08-11 06:59:28)
bq.  
bq.  
bq.  Review request for hbase.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  Changes:
bq.  
bq.  1.  Add hbase.region.wal.batchentries configuration parameter.  If this is
bq.  enabled, batch entries to the HLog in a queue.
bq.  2.  Use delayed RPCs for sync requests when aggresive batching is enabled.
bq.  This frees up RPC handler threads for the duration of the sync.
bq.  3.  Pass the RPC server instance all the way to down to HLog.  This is needed
bq.  to find out the current remote call, mark it as delayed, and finally complete
bq.  it when the sync is done.
bq.  4.  Use the region read-write consistency control to avoid exposing to
bq.  RegionScanners edits which have not yet been synced.
bq.  5.  Change a few tests which directly create HRegions or HLogs.  The
bq.  rpcServers passed in are null, HLog falls back to classic RPCs when it has no
bq.  knowledge of the RPC server.
bq.  6.  Add TestBatchEntriesLogRolling, which is identical to TestLogRolling,
bq.  except that it uses aggressive batching.  I'm not sure how to add tests that
bq.  verify the same functionality but don't duplicate code, suggestion are
bq.  welcome.
bq.  
bq.  The new parameter is disabled by default.
bq.  
bq.  
bq.  This addresses bug HBASE-4192.
bq.      https://issues.apache.org/jira/browse/HBASE-4192
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 6fb1da7 
bq.    src/main/java/org/apache/hadoop/hbase/master/HMaster.java a00b93d 
bq.    src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java 2f86f04 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 83ff7b2 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java f22fb6e 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/ReadWriteConsistencyControl.java 8ec53d3 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 887f736 
bq.    src/main/java/org/apache/hadoop/hbase/util/HMerge.java 9f0499e 
bq.    src/main/java/org/apache/hadoop/hbase/util/MetaUtils.java af8d734 
bq.    src/test/java/org/apache/hadoop/hbase/HBaseTestCase.java bf8004b 
bq.    src/test/java/org/apache/hadoop/hbase/TestScanMultipleVersions.java d7baba4 
bq.    src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorInterface.java 36816e8 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestBatchEntriesLogRolling.java PRE-CREATION 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogSplit.java de28418 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java dc43eb2 
bq.    src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 381ac90 
bq.  
bq.  Diff: https://reviews.apache.org/r/1463/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  All unit tests run with aggressive batching turned on and off.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Vlad
bq.  
bq.



> Optimize HLog for Throughput Using Delayed RPCs
> -----------------------------------------------
>
>                 Key: HBASE-4192
>                 URL: https://issues.apache.org/jira/browse/HBASE-4192
>             Project: HBase
>          Issue Type: New Feature
>          Components: wal
>    Affects Versions: 0.92.0
>            Reporter: Vlad Dogaru
>            Priority: Minor
>
> Introduce a new HLog configuration parameter (batchEntries) for more aggressive batching of appends.  If this is enabled, HLog appends are not written to the HLog writer immediately, but batched and written either periodically or when a sync is requested.  Because sync times become larger, they use delayed RPCs to free up RPC handler threads.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (HBASE-4192) Optimize HLog for Throughput Using Delayed RPCs

Posted by "jiraposter@reviews.apache.org (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4192?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13085479#comment-13085479 ] 

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


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



src/main/resources/hbase-default.xml
<https://reviews.apache.org/r/1463/#comment3372>

    Check out WALEdit.heapSize()


- Ted


On 2011-08-12 23:44:23, Vlad Dogaru wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/1463/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-08-12 23:44:23)
bq.  
bq.  
bq.  Review request for hbase.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  Changes:
bq.  
bq.  1.  Add hbase.region.wal.batchentries configuration parameter.  If this is
bq.  enabled, batch entries to the HLog in a queue.
bq.  2.  Use delayed RPCs for sync requests when aggresive batching is enabled.
bq.  This frees up RPC handler threads for the duration of the sync.
bq.  3.  Pass the RPC server instance all the way to down to HLog.  This is needed
bq.  to find out the current remote call, mark it as delayed, and finally complete
bq.  it when the sync is done.
bq.  4.  Use the region read-write consistency control to avoid exposing to
bq.  RegionScanners edits which have not yet been synced.
bq.  5.  Change a few tests which directly create HRegions or HLogs.  The
bq.  rpcServers passed in are null, HLog falls back to classic RPCs when it has no
bq.  knowledge of the RPC server.
bq.  6.  Add TestBatchEntriesLogRolling, which is identical to TestLogRolling,
bq.  except that it uses aggressive batching.  I'm not sure how to add tests that
bq.  verify the same functionality but don't duplicate code, suggestion are
bq.  welcome.
bq.  
bq.  The new parameter is disabled by default.
bq.  
bq.  
bq.  This addresses bug HBASE-4192.
bq.      https://issues.apache.org/jira/browse/HBASE-4192
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 7117bce 
bq.    src/main/java/org/apache/hadoop/hbase/master/HMaster.java a00b93d 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 83ff7b2 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 7a917da 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/ReadWriteConsistencyControl.java 8ec53d3 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 887f736 
bq.    src/main/resources/hbase-default.xml 66548ca 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java dc43eb2 
bq.    src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 381ac90 
bq.  
bq.  Diff: https://reviews.apache.org/r/1463/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  All unit tests run with aggressive batching turned on and off.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Vlad
bq.  
bq.



> Optimize HLog for Throughput Using Delayed RPCs
> -----------------------------------------------
>
>                 Key: HBASE-4192
>                 URL: https://issues.apache.org/jira/browse/HBASE-4192
>             Project: HBase
>          Issue Type: New Feature
>          Components: wal
>    Affects Versions: 0.92.0
>            Reporter: Vlad Dogaru
>            Priority: Minor
>
> Introduce a new HLog configuration parameter (batchEntries) for more aggressive batching of appends.  If this is enabled, HLog appends are not written to the HLog writer immediately, but batched and written either periodically or when a sync is requested.  Because sync times become larger, they use delayed RPCs to free up RPC handler threads.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (HBASE-4192) Optimize HLog for Throughput Using Delayed RPCs

Posted by "jiraposter@reviews.apache.org (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4192?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13084618#comment-13084618 ] 

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


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



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3334>

    Some kind of counter for this case would be useful, I think.


- Ted


On 2011-08-12 23:44:23, Vlad Dogaru wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/1463/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-08-12 23:44:23)
bq.  
bq.  
bq.  Review request for hbase.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  Changes:
bq.  
bq.  1.  Add hbase.region.wal.batchentries configuration parameter.  If this is
bq.  enabled, batch entries to the HLog in a queue.
bq.  2.  Use delayed RPCs for sync requests when aggresive batching is enabled.
bq.  This frees up RPC handler threads for the duration of the sync.
bq.  3.  Pass the RPC server instance all the way to down to HLog.  This is needed
bq.  to find out the current remote call, mark it as delayed, and finally complete
bq.  it when the sync is done.
bq.  4.  Use the region read-write consistency control to avoid exposing to
bq.  RegionScanners edits which have not yet been synced.
bq.  5.  Change a few tests which directly create HRegions or HLogs.  The
bq.  rpcServers passed in are null, HLog falls back to classic RPCs when it has no
bq.  knowledge of the RPC server.
bq.  6.  Add TestBatchEntriesLogRolling, which is identical to TestLogRolling,
bq.  except that it uses aggressive batching.  I'm not sure how to add tests that
bq.  verify the same functionality but don't duplicate code, suggestion are
bq.  welcome.
bq.  
bq.  The new parameter is disabled by default.
bq.  
bq.  
bq.  This addresses bug HBASE-4192.
bq.      https://issues.apache.org/jira/browse/HBASE-4192
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 7117bce 
bq.    src/main/java/org/apache/hadoop/hbase/master/HMaster.java a00b93d 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 83ff7b2 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 7a917da 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/ReadWriteConsistencyControl.java 8ec53d3 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 887f736 
bq.    src/main/resources/hbase-default.xml 66548ca 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java dc43eb2 
bq.    src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 381ac90 
bq.  
bq.  Diff: https://reviews.apache.org/r/1463/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  All unit tests run with aggressive batching turned on and off.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Vlad
bq.  
bq.



> Optimize HLog for Throughput Using Delayed RPCs
> -----------------------------------------------
>
>                 Key: HBASE-4192
>                 URL: https://issues.apache.org/jira/browse/HBASE-4192
>             Project: HBase
>          Issue Type: New Feature
>          Components: wal
>    Affects Versions: 0.92.0
>            Reporter: Vlad Dogaru
>            Priority: Minor
>
> Introduce a new HLog configuration parameter (batchEntries) for more aggressive batching of appends.  If this is enabled, HLog appends are not written to the HLog writer immediately, but batched and written either periodically or when a sync is requested.  Because sync times become larger, they use delayed RPCs to free up RPC handler threads.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (HBASE-4192) Optimize HLog for Throughput Using Delayed RPCs

Posted by "jiraposter@reviews.apache.org (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4192?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13084468#comment-13084468 ] 

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



bq.  On 2011-08-11 20:11:34, Jonathan Gray wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java, lines 261-299
bq.  > <https://reviews.apache.org/r/1463/diff/1/?file=32100#file32100line261>
bq.  >
bq.  >     This is all static so there might be issues running multiple HRegionServers in a single JVM during unit tests?

I am not sure what you mean.  Should I change only the newly added functions or all of them?


bq.  On 2011-08-11 20:11:34, Jonathan Gray wrote:
bq.  > src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestBatchEntriesLogRolling.java, line 63
bq.  > <https://reviews.apache.org/r/1463/diff/1/?file=32106#file32106line63>
bq.  >
bq.  >     there are some ways to run all the unit tests with a varying set of initial parameters... look at TestCacheOnWrite on the new HFile v2 patch, it uses @RunWith(Parameterized.class)
bq.  >

Looking into that, test should also do some more careful cleanup between successive (parameterized) runs.


- Vlad


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


On 2011-08-11 06:59:28, Vlad Dogaru wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/1463/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-08-11 06:59:28)
bq.  
bq.  
bq.  Review request for hbase.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  Changes:
bq.  
bq.  1.  Add hbase.region.wal.batchentries configuration parameter.  If this is
bq.  enabled, batch entries to the HLog in a queue.
bq.  2.  Use delayed RPCs for sync requests when aggresive batching is enabled.
bq.  This frees up RPC handler threads for the duration of the sync.
bq.  3.  Pass the RPC server instance all the way to down to HLog.  This is needed
bq.  to find out the current remote call, mark it as delayed, and finally complete
bq.  it when the sync is done.
bq.  4.  Use the region read-write consistency control to avoid exposing to
bq.  RegionScanners edits which have not yet been synced.
bq.  5.  Change a few tests which directly create HRegions or HLogs.  The
bq.  rpcServers passed in are null, HLog falls back to classic RPCs when it has no
bq.  knowledge of the RPC server.
bq.  6.  Add TestBatchEntriesLogRolling, which is identical to TestLogRolling,
bq.  except that it uses aggressive batching.  I'm not sure how to add tests that
bq.  verify the same functionality but don't duplicate code, suggestion are
bq.  welcome.
bq.  
bq.  The new parameter is disabled by default.
bq.  
bq.  
bq.  This addresses bug HBASE-4192.
bq.      https://issues.apache.org/jira/browse/HBASE-4192
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 6fb1da7 
bq.    src/main/java/org/apache/hadoop/hbase/master/HMaster.java a00b93d 
bq.    src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java 2f86f04 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 83ff7b2 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java f22fb6e 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/ReadWriteConsistencyControl.java 8ec53d3 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 887f736 
bq.    src/main/java/org/apache/hadoop/hbase/util/HMerge.java 9f0499e 
bq.    src/main/java/org/apache/hadoop/hbase/util/MetaUtils.java af8d734 
bq.    src/test/java/org/apache/hadoop/hbase/HBaseTestCase.java bf8004b 
bq.    src/test/java/org/apache/hadoop/hbase/TestScanMultipleVersions.java d7baba4 
bq.    src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorInterface.java 36816e8 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestBatchEntriesLogRolling.java PRE-CREATION 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogSplit.java de28418 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java dc43eb2 
bq.    src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 381ac90 
bq.  
bq.  Diff: https://reviews.apache.org/r/1463/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  All unit tests run with aggressive batching turned on and off.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Vlad
bq.  
bq.



> Optimize HLog for Throughput Using Delayed RPCs
> -----------------------------------------------
>
>                 Key: HBASE-4192
>                 URL: https://issues.apache.org/jira/browse/HBASE-4192
>             Project: HBase
>          Issue Type: New Feature
>          Components: wal
>    Affects Versions: 0.92.0
>            Reporter: Vlad Dogaru
>            Priority: Minor
>
> Introduce a new HLog configuration parameter (batchEntries) for more aggressive batching of appends.  If this is enabled, HLog appends are not written to the HLog writer immediately, but batched and written either periodically or when a sync is requested.  Because sync times become larger, they use delayed RPCs to free up RPC handler threads.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (HBASE-4192) Optimize HLog for Throughput Using Delayed RPCs

Posted by "jiraposter@reviews.apache.org (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4192?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13083461#comment-13083461 ] 

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


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


Great stuff!  Mostly documentation stuff at this point but I think HLog will be much clearer by the end of this.


src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java
<https://reviews.apache.org/r/1463/#comment3221>

    isn't there an HRegion.createHRegion that doesn't take the rpc server and handles passing the null?



src/main/java/org/apache/hadoop/hbase/regionserver/ReadWriteConsistencyControl.java
<https://reviews.apache.org/r/1463/#comment3220>

    make this proper javadoc



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3222>

    i think now might be a good time to fill in javadoc for every single variable in this class (let's use this as an opportunity to clean up HLog a bit)



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3223>

    We shouldn't hard-code anything like this.  It should probably be marked final and set in the constructor.  You can always have a DEFAULT_DELAYED_RESPONSE_MAX_SIZE or something.



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3224>

    final?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3225>

    is this the best description for what this does?  my understanding is that there is actually always going to be batching, and it's not that this is really about making them any more aggressive (in all cases it loops around and immediately syncs anything pending).  this is really about... a non-blocking append, freeing of the handler thread, and the sync being called and response returned from a separate thread.  (a mouthful but there must be some concise way to describe this)



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3226>

    This is all static so there might be issues running multiple HRegionServers in a single JVM during unit tests?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3227>

    since you've kept the old constructors, why did we need to update anything elsewhere passing in null?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3228>

    what does this mean?  we should also add some documentation to the default xml and/or book about the new config params and this feature



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3229>

    yeah this is fine, so just don't set it when you declare it and make it final



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3230>

    should this throw an exception?  this seems like a bad configuration (user error) and we should halt instead of continue.



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3232>

    this this while() { yield() } the best pattern to use here?  Any reason not to use the typical wait/notify pattern?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3231>

    looks like there are double spaces on each line wrap



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3234>

    What's the contract of this method?  When should this be called?  Does it only append or does it also sync?  Is it blocking or non-blocking and what does it do with the RWCC if i pass it in?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3233>

    javadoc still needs to be updated



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3235>

    there's a bunch of javadoc on this method.  it's unclear when to use this method vs. the other above it.



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3236>

    should be info?  and need to add rwcc



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3237>

    You can get fancy with javadoc notation by referencing the classes/interfaces:  {@link Delayable} for example.



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3238>

    final



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3239>

    Any updates to the description of this thread?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3240>

    why no longer final?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3241>

    comment



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3244>

    these comments can all be changed to be the form:
    
    /** Comment about what this variable is */
    
    (then it will be loaded up as javadoc in eclipse and such)



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3242>

    and is used when?  when the server is shutting down?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3243>

    what happens if you overrun max?  (seems like this feature could use a doc describing what it does, the trade-offs, and configuration parameters)



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3245>

    I don't think we want to just do the printStackTrace() here which will go to stdout... You'll want to LOG.error("Something", e)



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3246>

    it doesn't always wait.  this method deserves a full javadoc



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3247>

    maybe maxDelayedResponses would be a better variable name



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3248>

    double space



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3249>

    convert this to javadoc and maybe add more description... it's not really the case that "All RPC threads that had delayed Responses are now allowed"... it's actually the case that it will send the responses for anything that was delayed and is now synced (some stuff could be delayed but not yet synced, thus not responded to)



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3250>

    blocking?  non-blocking?  this just queues and doesn't actually write to HLog?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3251>

    When is this called



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3252>

    same here.  can you add some comments about when this is used



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3253>

    javadoc



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3254>

    need to update javadoc



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3255>

    javadoc



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3257>

    don't you want to request a log roll if logRollRequested == true || sizeExceeded ?  (why do we need sizeExceeded at all?  couldn't we set logRollRequested=true?)



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3258>

    javadoc



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3259>

    javadoc... let's be sure to clearly state the contract of these public methods



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3261>

    this method could also use some doc (i know you didn't add this but would be very helpful)



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3260>

    this looks like a change in behavior?  previously the postWALWrite would happen even if this method returned true... now if it returns true we leave immediately



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3262>

    so looks like this option is not compatible with coprocessors?  or does this postWALWrite move elsewhere?
    
    this looks like the preWALWrite will get called if batchEntries is true but the postWALWrite never will



src/main/java/org/apache/hadoop/hbase/util/HMerge.java
<https://reviews.apache.org/r/1463/#comment3263>

    necessary change?  (here and remaining changes)



src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestBatchEntriesLogRolling.java
<https://reviews.apache.org/r/1463/#comment3264>

    2011



src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestBatchEntriesLogRolling.java
<https://reviews.apache.org/r/1463/#comment3265>

    there are some ways to run all the unit tests with a varying set of initial parameters... look at TestCacheOnWrite on the new HFile v2 patch, it uses @RunWith(Parameterized.class)
    


- Jonathan


On 2011-08-11 06:59:28, Vlad Dogaru wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/1463/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-08-11 06:59:28)
bq.  
bq.  
bq.  Review request for hbase.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  Changes:
bq.  
bq.  1.  Add hbase.region.wal.batchentries configuration parameter.  If this is
bq.  enabled, batch entries to the HLog in a queue.
bq.  2.  Use delayed RPCs for sync requests when aggresive batching is enabled.
bq.  This frees up RPC handler threads for the duration of the sync.
bq.  3.  Pass the RPC server instance all the way to down to HLog.  This is needed
bq.  to find out the current remote call, mark it as delayed, and finally complete
bq.  it when the sync is done.
bq.  4.  Use the region read-write consistency control to avoid exposing to
bq.  RegionScanners edits which have not yet been synced.
bq.  5.  Change a few tests which directly create HRegions or HLogs.  The
bq.  rpcServers passed in are null, HLog falls back to classic RPCs when it has no
bq.  knowledge of the RPC server.
bq.  6.  Add TestBatchEntriesLogRolling, which is identical to TestLogRolling,
bq.  except that it uses aggressive batching.  I'm not sure how to add tests that
bq.  verify the same functionality but don't duplicate code, suggestion are
bq.  welcome.
bq.  
bq.  The new parameter is disabled by default.
bq.  
bq.  
bq.  This addresses bug HBASE-4192.
bq.      https://issues.apache.org/jira/browse/HBASE-4192
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 6fb1da7 
bq.    src/main/java/org/apache/hadoop/hbase/master/HMaster.java a00b93d 
bq.    src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java 2f86f04 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 83ff7b2 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java f22fb6e 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/ReadWriteConsistencyControl.java 8ec53d3 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 887f736 
bq.    src/main/java/org/apache/hadoop/hbase/util/HMerge.java 9f0499e 
bq.    src/main/java/org/apache/hadoop/hbase/util/MetaUtils.java af8d734 
bq.    src/test/java/org/apache/hadoop/hbase/HBaseTestCase.java bf8004b 
bq.    src/test/java/org/apache/hadoop/hbase/TestScanMultipleVersions.java d7baba4 
bq.    src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorInterface.java 36816e8 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestBatchEntriesLogRolling.java PRE-CREATION 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogSplit.java de28418 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java dc43eb2 
bq.    src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 381ac90 
bq.  
bq.  Diff: https://reviews.apache.org/r/1463/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  All unit tests run with aggressive batching turned on and off.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Vlad
bq.  
bq.



> Optimize HLog for Throughput Using Delayed RPCs
> -----------------------------------------------
>
>                 Key: HBASE-4192
>                 URL: https://issues.apache.org/jira/browse/HBASE-4192
>             Project: HBase
>          Issue Type: New Feature
>          Components: wal
>    Affects Versions: 0.92.0
>            Reporter: Vlad Dogaru
>            Priority: Minor
>
> Introduce a new HLog configuration parameter (batchEntries) for more aggressive batching of appends.  If this is enabled, HLog appends are not written to the HLog writer immediately, but batched and written either periodically or when a sync is requested.  Because sync times become larger, they use delayed RPCs to free up RPC handler threads.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (HBASE-4192) Optimize HLog for Throughput Using Delayed RPCs

Posted by "jiraposter@reviews.apache.org (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4192?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13085474#comment-13085474 ] 

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



bq.  On 2011-08-13 01:02:17, Todd Lipcon wrote:
bq.  > src/main/resources/hbase-default.xml, line 222
bq.  > <https://reviews.apache.org/r/1463/diff/2/?file=32495#file32495line222>
bq.  >
bq.  >     I feel like this config should be based on size, not count... we've gotten in trouble in the past where we do things on count and people are doing very wide cells, etc, and we OOM

Would it be a good idea to force a sync once the total length of the unflushed edits exceeds a limit (maxUnflushedSize)?  If yes, is there any way to determine a WALEdit's approximate size other than toString() and iterating through its kvs? Thanks!


- Vlad


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


On 2011-08-12 23:44:23, Vlad Dogaru wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/1463/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-08-12 23:44:23)
bq.  
bq.  
bq.  Review request for hbase.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  Changes:
bq.  
bq.  1.  Add hbase.region.wal.batchentries configuration parameter.  If this is
bq.  enabled, batch entries to the HLog in a queue.
bq.  2.  Use delayed RPCs for sync requests when aggresive batching is enabled.
bq.  This frees up RPC handler threads for the duration of the sync.
bq.  3.  Pass the RPC server instance all the way to down to HLog.  This is needed
bq.  to find out the current remote call, mark it as delayed, and finally complete
bq.  it when the sync is done.
bq.  4.  Use the region read-write consistency control to avoid exposing to
bq.  RegionScanners edits which have not yet been synced.
bq.  5.  Change a few tests which directly create HRegions or HLogs.  The
bq.  rpcServers passed in are null, HLog falls back to classic RPCs when it has no
bq.  knowledge of the RPC server.
bq.  6.  Add TestBatchEntriesLogRolling, which is identical to TestLogRolling,
bq.  except that it uses aggressive batching.  I'm not sure how to add tests that
bq.  verify the same functionality but don't duplicate code, suggestion are
bq.  welcome.
bq.  
bq.  The new parameter is disabled by default.
bq.  
bq.  
bq.  This addresses bug HBASE-4192.
bq.      https://issues.apache.org/jira/browse/HBASE-4192
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 7117bce 
bq.    src/main/java/org/apache/hadoop/hbase/master/HMaster.java a00b93d 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 83ff7b2 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 7a917da 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/ReadWriteConsistencyControl.java 8ec53d3 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 887f736 
bq.    src/main/resources/hbase-default.xml 66548ca 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java dc43eb2 
bq.    src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 381ac90 
bq.  
bq.  Diff: https://reviews.apache.org/r/1463/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  All unit tests run with aggressive batching turned on and off.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Vlad
bq.  
bq.



> Optimize HLog for Throughput Using Delayed RPCs
> -----------------------------------------------
>
>                 Key: HBASE-4192
>                 URL: https://issues.apache.org/jira/browse/HBASE-4192
>             Project: HBase
>          Issue Type: New Feature
>          Components: wal
>    Affects Versions: 0.92.0
>            Reporter: Vlad Dogaru
>            Priority: Minor
>
> Introduce a new HLog configuration parameter (batchEntries) for more aggressive batching of appends.  If this is enabled, HLog appends are not written to the HLog writer immediately, but batched and written either periodically or when a sync is requested.  Because sync times become larger, they use delayed RPCs to free up RPC handler threads.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (HBASE-4192) Optimize HLog for Throughput Using Delayed RPCs

Posted by "jiraposter@reviews.apache.org (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4192?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13084503#comment-13084503 ] 

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


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


some new unit tests here would be nice - specifically if we can use mock objects and fire up a bunch of threads to pound on the various synchronization primitives. I haven't convinced myself that the synchronization is all correct yet, and some tests that "prove it" would help :)


src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java
<https://reviews.apache.org/r/1463/#comment3304>

    can you write a test case for this bug fix, separately from this HLog optimization? seems like you should be able to trigger it by delaying response to a different thread which runs immediately, then sleep for a second in the main RPC thread, right?



src/main/java/org/apache/hadoop/hbase/master/HMaster.java
<https://reviews.apache.org/r/1463/#comment3305>

    I really don't like that we're threading the RPCServer through to the HRegion and HLog objects. There's something really messy about it: imagine, for example, if we had two separate RPC servers on each daemon, listening on different ports (this is done in the Hadoop namenode in order to achieve some easy QoS). Then we'd have to thread two RPCServers through.
    
    Instead, I think the individual operations should use ThreadLocals to grab the current Call object, perhaps? If the ThreadLocal lookups are too expensive, then perhaps a new interface like "CallContext" which we thread through the processing of individual operations?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3315>

    this variable is never reset, so it really acts like a sequence number. I'm not sure if it needs to exist independently from the other sequence number that we have



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3311>

    it's the last transaction for which someone has requested a sync, right? i.e not the last transaction that has actually _been_ synced?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3306>

    making this a volatile long doesn't make the += atomic. You should use AtomicLong most likely?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3307>

    this too. I guess you were following the (incorrect) pattern above... may as well fix these.



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3308>

    rather than reading both of these as members of HLog, I think it would be cleaner to pass Configuration along to LogSyncer, since they're only used from within there, right? You could do the same with optionalFlushInterval while you're at it.



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3309>

    style: add braces



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3310>

    worth adding: assert Thread.currentThread().holdsLock(updateLock) here?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3312>

    if we got interrupted, that typically means that we should shutdown, right? swallowing the exception smells wrong



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3313>

    don't you mean "that do have"? ie if the edits have been cacheflushed, then we don't need to replay them.
    
    This javadoc might make more sense to put up at the top of the class?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3317>

    this can be a static inner class, yea?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3318>

    submit seems like a strange name for this... 'complete' maybe?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3322>

    the condition we're waiting on is actually queueNotEmpty, isn't it? or somethingToSync?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3321>

    make these final



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3323>

    I don't understand this sentence.. what is "0 or less"?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3324>

    should pass "e" as second arg, not stringify it - that way you get a stack trace



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3325>

    do you need to really do new Long? it should auto-box



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3326>

    e as 2nd arg. Since this error will result in a hung RPC client, it would be great to give as much detail as possible - can we somehow funnel the client IP, the transaction ID, etc into this message?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3327>

    i'm skeptical about the 'synchronized' keyword here - we have an internal lock, but we also use the object's monitor lock?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3328>

    hrm... does this do anything, given logSyncerThread is now waiting on Conditions instead of on the monitor obj?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3329>

    typo: and



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/1463/#comment3330>

    these three different paths scare me a bit... do we really need to support all three? ie are there any significant disadvantages to always doing the delayed responses, for example?



src/main/resources/hbase-default.xml
<https://reviews.apache.org/r/1463/#comment3331>

    I feel like this config should be based on size, not count... we've gotten in trouble in the past where we do things on count and people are doing very wide cells, etc, and we OOM


- Todd


On 2011-08-12 23:44:23, Vlad Dogaru wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/1463/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-08-12 23:44:23)
bq.  
bq.  
bq.  Review request for hbase.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  Changes:
bq.  
bq.  1.  Add hbase.region.wal.batchentries configuration parameter.  If this is
bq.  enabled, batch entries to the HLog in a queue.
bq.  2.  Use delayed RPCs for sync requests when aggresive batching is enabled.
bq.  This frees up RPC handler threads for the duration of the sync.
bq.  3.  Pass the RPC server instance all the way to down to HLog.  This is needed
bq.  to find out the current remote call, mark it as delayed, and finally complete
bq.  it when the sync is done.
bq.  4.  Use the region read-write consistency control to avoid exposing to
bq.  RegionScanners edits which have not yet been synced.
bq.  5.  Change a few tests which directly create HRegions or HLogs.  The
bq.  rpcServers passed in are null, HLog falls back to classic RPCs when it has no
bq.  knowledge of the RPC server.
bq.  6.  Add TestBatchEntriesLogRolling, which is identical to TestLogRolling,
bq.  except that it uses aggressive batching.  I'm not sure how to add tests that
bq.  verify the same functionality but don't duplicate code, suggestion are
bq.  welcome.
bq.  
bq.  The new parameter is disabled by default.
bq.  
bq.  
bq.  This addresses bug HBASE-4192.
bq.      https://issues.apache.org/jira/browse/HBASE-4192
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 7117bce 
bq.    src/main/java/org/apache/hadoop/hbase/master/HMaster.java a00b93d 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 83ff7b2 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 7a917da 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/ReadWriteConsistencyControl.java 8ec53d3 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 887f736 
bq.    src/main/resources/hbase-default.xml 66548ca 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java dc43eb2 
bq.    src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 381ac90 
bq.  
bq.  Diff: https://reviews.apache.org/r/1463/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  All unit tests run with aggressive batching turned on and off.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Vlad
bq.  
bq.



> Optimize HLog for Throughput Using Delayed RPCs
> -----------------------------------------------
>
>                 Key: HBASE-4192
>                 URL: https://issues.apache.org/jira/browse/HBASE-4192
>             Project: HBase
>          Issue Type: New Feature
>          Components: wal
>    Affects Versions: 0.92.0
>            Reporter: Vlad Dogaru
>            Priority: Minor
>
> Introduce a new HLog configuration parameter (batchEntries) for more aggressive batching of appends.  If this is enabled, HLog appends are not written to the HLog writer immediately, but batched and written either periodically or when a sync is requested.  Because sync times become larger, they use delayed RPCs to free up RPC handler threads.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (HBASE-4192) Optimize HLog for Throughput Using Delayed RPCs

Posted by "jiraposter@reviews.apache.org (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4192?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13085473#comment-13085473 ] 

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



bq.  On 2011-08-13 01:02:17, Todd Lipcon wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java, line 148
bq.  > <https://reviews.apache.org/r/1463/diff/2/?file=32494#file32494line148>
bq.  >
bq.  >     it's the last transaction for which someone has requested a sync, right? i.e not the last transaction that has actually _been_ synced?

It's actually the last transaction that has been synced, like the comment says.  The variable is updated in hflush().  I think a better name for it would be 'syncedTillHere' instead of 'syncTillHere'.


bq.  On 2011-08-13 01:02:17, Todd Lipcon wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java, line 1395
bq.  > <https://reviews.apache.org/r/1463/diff/2/?file=32494#file32494line1395>
bq.  >
bq.  >     hrm... does this do anything, given logSyncerThread is now waiting on Conditions instead of on the monitor obj?

We use logSyncerThread to wait for a flush to complete in cleanupCurrentWriter.


- Vlad


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


On 2011-08-12 23:44:23, Vlad Dogaru wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/1463/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-08-12 23:44:23)
bq.  
bq.  
bq.  Review request for hbase.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  Changes:
bq.  
bq.  1.  Add hbase.region.wal.batchentries configuration parameter.  If this is
bq.  enabled, batch entries to the HLog in a queue.
bq.  2.  Use delayed RPCs for sync requests when aggresive batching is enabled.
bq.  This frees up RPC handler threads for the duration of the sync.
bq.  3.  Pass the RPC server instance all the way to down to HLog.  This is needed
bq.  to find out the current remote call, mark it as delayed, and finally complete
bq.  it when the sync is done.
bq.  4.  Use the region read-write consistency control to avoid exposing to
bq.  RegionScanners edits which have not yet been synced.
bq.  5.  Change a few tests which directly create HRegions or HLogs.  The
bq.  rpcServers passed in are null, HLog falls back to classic RPCs when it has no
bq.  knowledge of the RPC server.
bq.  6.  Add TestBatchEntriesLogRolling, which is identical to TestLogRolling,
bq.  except that it uses aggressive batching.  I'm not sure how to add tests that
bq.  verify the same functionality but don't duplicate code, suggestion are
bq.  welcome.
bq.  
bq.  The new parameter is disabled by default.
bq.  
bq.  
bq.  This addresses bug HBASE-4192.
bq.      https://issues.apache.org/jira/browse/HBASE-4192
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 7117bce 
bq.    src/main/java/org/apache/hadoop/hbase/master/HMaster.java a00b93d 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 83ff7b2 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 7a917da 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/ReadWriteConsistencyControl.java 8ec53d3 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 887f736 
bq.    src/main/resources/hbase-default.xml 66548ca 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALActionsListener.java dc43eb2 
bq.    src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java 381ac90 
bq.  
bq.  Diff: https://reviews.apache.org/r/1463/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  All unit tests run with aggressive batching turned on and off.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Vlad
bq.  
bq.



> Optimize HLog for Throughput Using Delayed RPCs
> -----------------------------------------------
>
>                 Key: HBASE-4192
>                 URL: https://issues.apache.org/jira/browse/HBASE-4192
>             Project: HBase
>          Issue Type: New Feature
>          Components: wal
>    Affects Versions: 0.92.0
>            Reporter: Vlad Dogaru
>            Priority: Minor
>
> Introduce a new HLog configuration parameter (batchEntries) for more aggressive batching of appends.  If this is enabled, HLog appends are not written to the HLog writer immediately, but batched and written either periodically or when a sync is requested.  Because sync times become larger, they use delayed RPCs to free up RPC handler threads.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (HBASE-4192) Optimize HLog for Throughput Using Delayed RPCs

Posted by "Todd Lipcon (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HBASE-4192?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13115824#comment-13115824 ] 

Todd Lipcon commented on HBASE-4192:
------------------------------------

That sounds easier to review, thanks for the suggestion, dhruba.
                
> Optimize HLog for Throughput Using Delayed RPCs
> -----------------------------------------------
>
>                 Key: HBASE-4192
>                 URL: https://issues.apache.org/jira/browse/HBASE-4192
>             Project: HBase
>          Issue Type: New Feature
>          Components: wal
>    Affects Versions: 0.92.0
>            Reporter: Vlad Dogaru
>            Priority: Minor
>
> Introduce a new HLog configuration parameter (batchEntries) for more aggressive batching of appends.  If this is enabled, HLog appends are not written to the HLog writer immediately, but batched and written either periodically or when a sync is requested.  Because sync times become larger, they use delayed RPCs to free up RPC handler threads.

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