hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ramkrishna.s.vasudevan (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-16643) Reverse scanner heap creation may not allow MSLAB closure due to improper ref counting of segments
Date Mon, 26 Sep 2016 17:31:20 GMT

    [ 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

View raw message