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