[
https://issues.apache.org/jira/browse/HBASE-16643?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15522571#comment-15522571
]
ramkrishna.s.vasudevan commented on HBASE-16643:
------------------------------------------------
Thanks for the comments. I have replied to your comments in RB [~anastas].
bq.Ya this is what Ram checked and confirmed.. I dont think any logic abt first
calling seek / backwardSeek has changed recently. This is the core of the read
flow and very rarely change. Ram pls double confirm.
Yes. I verified that.
Still one thingis that for the close() call will it be better as I did in the
first patch like if heap is null we could iterate and close individual
scanners. Helps in test cases and any other future code that just tries to open
the scanner.
Also I tried to change the backward heap by first creating the heap and then
allowing to seek but it resulted in errors. I did not dive in deep in to that
so I prefer leaving things as in this patch.
> Reverse scanner heap creation may not allow MSLAB closure due to improper ref
> counting of segments
> --------------------------------------------------------------------------------------------------
>
> Key: HBASE-16643
> URL: https://issues.apache.org/jira/browse/HBASE-16643
> Project: HBase
> Issue Type: Bug
> Reporter: ramkrishna.s.vasudevan
> Assignee: ramkrishna.s.vasudevan
> Priority: Critical
> Fix For: 2.0.0
>
> Attachments: HBASE-16643.patch, HBASE-16643_1.patch,
> HBASE-16643_2.patch, HBASE-16643_3.patch, HBASE-16643_4.patch,
> HBASE-16643_5.patch, HBASE-16643_6.patch
>
>
> In the reverse scanner case,
> While doing 'initBackwardHeapIfNeeded' in MemstoreScanner for setting the
> backward heap, we do a
> {code}
> if ((backwardHeap == null) && (forwardHeap != null)) {
> forwardHeap.close();
> forwardHeap = null;
> // before building the heap seek for the relevant key on the scanners,
> // for the heap to be built from the scanners correctly
> for (KeyValueScanner scan : scanners) {
> if (toLast) {
> res |= scan.seekToLastRow();
> } else {
> res |= scan.backwardSeek(cell);
> }
> }
> {code}
> forwardHeap.close(). This would internally decrement the MSLAB ref counter
> for the current active segment and snapshot segment.
> When the scan is actually closed again we do close() and that will again
> decrement the count. Here chances are there that the count would go negative
> and hence the actual MSLAB closure that checks for refCount==0 will fail.
> Apart from this, when the refCount becomes 0 after the firstClose if any
> other thread requests to close the segment, then we will end up in corrupted
> segment because the segment could be put back to the MSLAB pool.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
