[
https://issues.apache.org/jira/browse/HBASE-16643?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15523687#comment-15523687
]
ramkrishna.s.vasudevan commented on HBASE-16643:
------------------------------------------------
bq.t just init reverse KVHeap.. Within the init we do seekToLast/seekPrevious
stuff.. This is what I asked previously. Can we do like
initReverseKVHeapIfNeeded do just heap create (as in other method) and the
actual methods do seekToLast/ seekPrevious work?
I think I replied to this in the RB.
Yes this will init the heap after doing the seek. In the above case it is
seekToLastRow(). I tried to do the following change by just creating the heap
and then allow the API call to do the actual seek. It was creating test
failures. So it requires some more investigation as why that fails. My guess is
that the heap creation depends on what cell the scanners could peek. So after
doing seek if we peek (this is specific to reverse) we are able to do the right
cells to peek from the ReverseKVHeap.
So if that change has to be done then we need to do some more investigation and
may lead to more changes as part of this patch. Let me know what you think. I
believe that we could have this committed and see those in another JIRA.
> 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, HBASE-16643_7.patch,
> HBASE-16643_8.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)
