ramkrishna.s.vasudevan updated HBASE-16643:
    Attachment: HBASE-16643_3.patch

Now we will initialize the heap in memstore scanner lazily depending on what 
type of scanner API is called on it for the first time.
Removes synchronized from all the methods as it was mentioned in the earlier 
review comment.
The patch becomes smaller because no test cases were changed and no interface 
is updated though I would like to update the Store#getScanners() API and unify 
them. Not sure how to do that if we cannot break without deprecating. So I have 
deferred it to another JIRA.
Still I prefer changing the Store interface as it is much cleaner and passing 
the reverse scan info to the getScanners() API in some form is useful and 
cleaner. Will submit for QA.

> 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
> 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

Reply via email to