[
https://issues.apache.org/jira/browse/HBASE-10015?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13832315#comment-13832315
]
Lars Hofhansl commented on HBASE-10015:
---------------------------------------
The trick will be to do all this without the need to synchronize anything in
StoreScanner.
Maybe there is a way to bring my idea of lock coarsening further: Above I
suggest to just have updateReaders lock on the RegionScannerImpl, because that
is locked anyway. The problem was that - at least in trunk - we also want to be
notified during flushes and compactions and in that case we do not have a
RegionScannerImpl. So idea is: Lock the StoreScanner itself from the
compaction/flush code and pass itself as the lock object. For flushes we do it
in a single loop, for compaction we could do that in chunks of 10000 or so.
Now in this issue I already attached a bunch of different patches. Lemme commit
the -lock patch here and close this. We can discuss further on a new jira.
Thanks [~apurtell], [~stack], and [[email protected]] for running the perf
unittests.
> Replace intrinsic locking with explicit locks in StoreScanner
> -------------------------------------------------------------
>
> Key: HBASE-10015
> URL: https://issues.apache.org/jira/browse/HBASE-10015
> Project: HBase
> Issue Type: Bug
> Reporter: Lars Hofhansl
> Assignee: Lars Hofhansl
> Labels: performance
> Attachments: 10015-0.94-lock.txt, 10015-0.94-new-sample.txt,
> 10015-0.94-v2.txt, 10015-0.94-v3.txt, 10015-0.94-v4.txt,
> 10015-0.94-withtest.txt, 10015-0.94.txt, 10015-trunk-v2.txt,
> 10015-trunk-v3.txt, 10015-trunk-v4.txt, 10015-trunk-v4.txt,
> 10015-trunk-v4.txt, 10015-trunk.txt, TestLoad.java
>
>
> Did some more profiling (this time with a sampling profiler) and
> StoreScanner.peek() showed up a lot in the samples. At first that was
> surprising, but peek is synchronized, so it seems a lot of the sync'ing cost
> is eaten there.
> It seems the only reason we have to synchronize all these methods is because
> a concurrent flush or compaction can change the scanner stack, other than
> that only a single thread should access a StoreScanner at any given time.
> So replaced updateReaders() with some code that just indicates to the scanner
> that the readers should be updated and then make it the using thread's
> responsibility to do the work.
> The perf improvement from this is staggering. I am seeing somewhere around 3x
> scan performance improvement across all scenarios.
> Now, the hard part is to reason about whether this is 100% correct. I ran
> TestAtomicOperation and TestAcidGuarantees a few times in a loop, all still
> pass.
> Will attach a sample patch.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
