hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Anastasia Braginsky (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 Sun, 25 Sep 2016 09:12:20 GMT

    [ https://issues.apache.org/jira/browse/HBASE-16643?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15520507#comment-15520507

Anastasia Braginsky commented on HBASE-16643:

Hi [~ram_krish],

I have just reviewed the recent patch version you published on the review board, and left
some comments there. 
Although all this refactoring wasn't needed to resolve the bug explained in the title, I am
OK with this refactoring.
I like removing the "synchronized" I wanted to do it myself, but I wasn't brave enough  :)

It appears to me that you assume first action for forward scan is seek/reseek() and first
action for reverse scan is backwardSeek/seekToPreviousRow(). This helps you to avoid using
two heaps variants.
I recall I tried to do this myself, but it didn't work for me. Maybe something changed since
them or you just arrange it better.
Bottom line, I am OK with this change.
Please consider two small comments I Ieft on the review board.


> 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

View raw message