[
https://issues.apache.org/jira/browse/HBASE-15716?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15362226#comment-15362226
]
Hiroshi Ikeda commented on HBASE-15716:
---------------------------------------
bq. I should make getReadPoint private. It is only for use in this class. I
should remove 'abstract long getMvccReadPoint();'?
Sorry I forgot to delete getReadPoint. I experimentally saw how the code
becomes messy if I collect the duplicate logic about getting a read point with
a isolation level from HRegion.getReadpoint (as a comment I wrote a few days
ago).
The abstract method getMvccReadPoint is needed to avoid taking the whole
MultiVersionConsistencyControl instance, that is regrettably large concrete
object. Using such a concrete class would make our class unnecessarily be noisy
and off the point, and also it would be difficult to create a test.
bq. 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?
Yes. In fact it is not needed to do that operation here, because that operation
will be eventually done when it is actually required. We just allow the thread
to wipe up its operation's mess and/or help GC just a little.
bq. 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?
Do you mean logic in getEntry? In this case, you are right and always
tail.readPoint <= mvccReadpoint because getting the tail happens before getting
the mvccReadPoint. Even if the relation would be not established,
tail.readPoint > mvccReadPoint means, the mvccReadPoint is already stale and
another thread updates the tail with the newer mvccReadPoint, and then it would
be enough to get a lift in it. It is enough to get the read point which is
equal to or newer than the mvccReadPoint at the moment you enter the method.
Or do you mean about the mvccReadPoint's advancing? I wrote getMvccReadPoint is
always advancing in the javadoc but I just meant that doesn't go back and
possibility that returns the same value.
{quote}
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;
{quote}
The same thing can be said and always head.readPoint <= mvccReadPoint.
bq. Any suggestions on how to test for correctness?
That's difficult... :(
> 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)
