[
https://issues.apache.org/jira/browse/HBASE-15716?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15361997#comment-15361997
]
stack commented on HBASE-15716:
-------------------------------
Let me upload a new patch w/ your changes included. A few comments on the patch
below:
I should make getReadPoint private. It is only for use in this class. I should
remove 'abstract long getMvccReadPoint();'?
On this comment, " // Ignore the result; Another thread already did
for you.", you mean another thread has move the tail on further than what we
wanted to set it too?
Should we be upping the tail reference count? We only checks if the
tail.readPoint is less than mvccReadPoint. We didn't check it is equal. Could
tail be > mvccReadPoint? It shouldn't ever be I suppose.. Is that what you are
depending on here?
This should never happen (mvcc read point should always be > head.readPoint...)?
110 + long mvccReadPoint = getMvccReadPoint();
111 + if (head.readPoint >= mvccReadPoint) {
112 + return head.readPoint;
113 + }
Otherwise, patch looks good. Any suggestions on how to test for correctness? (I
can check perf easy). Thanks [~ikeda]
> HRegion#RegionScannerImpl scannerReadPoints synchronization constrains random
> read
> ----------------------------------------------------------------------------------
>
> Key: HBASE-15716
> URL: https://issues.apache.org/jira/browse/HBASE-15716
> Project: HBase
> Issue Type: Bug
> Components: Performance
> Reporter: stack
> Assignee: stack
> Attachments:
> 15716.implementation.using.ScannerReadPoints.branch-1.patch,
> 15716.prune.synchronizations.patch, 15716.prune.synchronizations.v3.patch,
> 15716.prune.synchronizations.v4.patch, 15716.prune.synchronizations.v4.patch,
> 15716.wip.more_to_be_done.patch, HBASE-15716.branch-1.001.patch,
> HBASE-15716.branch-1.002.patch, HBASE-15716.branch-1.003.patch,
> HBASE-15716.branch-1.004.patch, HBASE-15716.branch-1.005.patch,
> ScannerReadPoints.java, ScannerReadPoints.v2.java, Screen Shot 2016-04-26 at
> 2.05.45 PM.png, Screen Shot 2016-04-26 at 2.06.14 PM.png, Screen Shot
> 2016-04-26 at 2.07.06 PM.png, Screen Shot 2016-04-26 at 2.25.26 PM.png,
> Screen Shot 2016-04-26 at 6.02.29 PM.png, Screen Shot 2016-04-27 at 9.49.35
> AM.png, Screen Shot 2016-06-30 at 9.52.52 PM.png, Screen Shot 2016-06-30 at
> 9.54.08 PM.png, TestScannerReadPoints.java, before_after.png,
> current-branch-1.vs.NoSynchronization.vs.Patch.png, hits.png,
> remove.locks.patch, remove_cslm.patch
>
>
> Here is a [~lhofhansl] special.
> When we construct the region scanner, we get our read point and then store it
> with the scanner instance in a Region scoped CSLM. This is done under a
> synchronize on the CSLM.
> This synchronize on a region-scoped Map creating region scanners is the
> outstanding point of lock contention according to flight recorder (My work
> load is workload c, random reads).
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
