hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "jiraposter@reviews.apache.org (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-3759) Eliminate use of ThreadLocals for CoprocessorEnvironment bypass() and complete()
Date Wed, 13 Apr 2011 01:11:05 GMT

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

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


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

Review request for hbase.


Summary
-------

Profiling the HRegionServer process with a RegionObserver coprocessor loaded shows a fair
amount of runnable thread CPU time spent getting the bypass and complete flag ThreadLocal
values by RegionCoprocessorHost.  See the HBASE-3759 JIRA for some attached graphs.

With the caveat that this is runnable CPU time and not threads in all states, this still seems
like a significant processing bottleneck on a hot call path.  The workload profiled was a
put-based bulk load, so for each multi-put request, RegionCoprocessorHost.prePut() could be
called many times.

Instead of using ThreadLocal variable for bypass/complete, which will incur contention on
the underlying map of values, I think we can eliminate the bottleneck by using locally scoped
variables for each preXXX/putXXX method called in the RegionCoprocessorHost, MasterCoprocessorHost
and WALCoprocessorHost classes.

The attached patch refactors the current RegionObserver, MasterObserver and WALObserver APIs
to provide a locally scoped ObserverContext object for storing and checking the bypass and
complete values.

Summary of changes:

* adds a new ObserverContext<T extends CoprocessorEnvironment> class, containing references
for bypass, complete and the environment instance
* in each pre/post method in RegionObserver, the RegionCoprocessorEnvironment parameter is
replaced by ObserverContext<RegionCoprocessorEnvironment>
* in each pre/post method in MasterObserver, the MasterCoprocessorEnvironment parameter is
replaced by ObserverContext<MasterCoprocessorEnvironment>
* in each pre/post method in WALObserver, the WALCoprocessorEnvironment parameter is replace
by ObserverContext<WALCoprocesorEnvironment>


This is obviously a large bulk change to the existing API.  I could avoid the API change with
hacky modification underneath the *CoprocessorEnvironment interfaces.  But since we do not
yet have a public release with coprocessors, I would prefer to take the time to make the initial
API the best we can before we push it out.

Please let me know your thoughts on this approach.


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


Diffs
-----

  src/main/java/org/apache/hadoop/hbase/coprocessor/BaseMasterObserver.java 9576c48 
  src/main/java/org/apache/hadoop/hbase/coprocessor/BaseRegionObserverCoprocessor.java 5a0f095

  src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorEnvironment.java d45b950 
  src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java a82f62b 
  src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java db0870b 
  src/main/java/org/apache/hadoop/hbase/coprocessor/ObserverContext.java PRE-CREATION 
  src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java 3501958 
  src/main/java/org/apache/hadoop/hbase/coprocessor/WALObserver.java 7a34d18 
  src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java 019bbde 
  src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java 60efa12 
  src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALCoprocessorHost.java a3f3b31 
  src/test/java/org/apache/hadoop/hbase/coprocessor/SampleRegionWALObserver.java 834283f 
  src/test/java/org/apache/hadoop/hbase/coprocessor/SimpleRegionObserver.java 0ce2147 
  src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorInterface.java 0db5001

  src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java a15d53a 
  src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionObserverStacking.java 2c1e4a0


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


Testing
-------


Thanks,

Gary



> Eliminate use of ThreadLocals for CoprocessorEnvironment bypass() and complete()
> --------------------------------------------------------------------------------
>
>                 Key: HBASE-3759
>                 URL: https://issues.apache.org/jira/browse/HBASE-3759
>             Project: HBase
>          Issue Type: Improvement
>          Components: coprocessors
>            Reporter: Gary Helmling
>            Assignee: Gary Helmling
>         Attachments: cp_bypass.tar.gz
>
>
> In the current coprocessor framework, ThreadLocal objects are used for the bypass and
complete booleans in CoprocessorEnvironment.  This allows the *CoprocessorHost implementations
to identify when to short-circuit processing the the preXXX and postXXX hook methods.
> Profiling the region server, however, shows that these ThreadLocals can become a contention
point when on a hot code path (such as prePut()).  We should refactor the CoprocessorHost
pre/post implementations to remove usage of the ThreadLocal variables and replace them with
locally scoped variables to eliminate contention between handler threads.

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

Mime
View raw message