Mailing-List: contact issues-help@hbase.apache.org; run by ezmlm
Precedence: bulk
Date: Thu, 29 Oct 2015 04:03:28 +0000 (UTC)
From: "ramkrishna.s.vasudevan (JIRA)" <jira@apache.org>
To: issues@hbase.apache.org
Message-ID: <JIRA.12776620.1424500574000.91988.1446091408096@Atlassian.JIRA>
In-Reply-To: <JIRA.12776620.1424500574000@Atlassian.JIRA>
References: <JIRA.12776620.1424500574000@Atlassian.JIRA>
 <JIRA.12776620.1424500574747@arcas>
Subject: [jira] [Commented] (HBASE-13082) Coarsen StoreScanner locks to
 RegionScanner
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


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

ramkrishna.s.vasudevan commented on HBASE-13082:
------------------------------------------------

Regarding the memstore files getting updated in the current scanner, I think we can solve it this way - let me try to explain here
In the current code 
{code}
    this.lock.writeLock().lock();
    try {
      this.storeEngine.getStoreFileManager().insertNewFiles(sfs);
      if (snapshotId > 0) {
        this.memstore.clearSnapshot(snapshotId);
      }
    } finally {
      // We need the lock, as long as we are updating the storeFiles
      // or changing the memstore. Let us release it before calling
      // notifyChangeReadersObservers. See HBASE-4485 for a possible
      // deadlock scenario that could have happened if continue to hold
      // the lock.
      this.lock.writeLock().unlock();
    }
    notifyChangedReadersObservers();
{code}
As you can see that before we notify the observer we clear the snapshot. So this notification wil try to hold the storescanner's lock and do the heap nullify and the scanner will continue to reset the heap seeing it to be null every time it enters any scanner API like next(), seek() etc.  So here if there is an ongoing scanner going on assume it is in StoreScanner#next() it would have already held the lock and so the notify has to wait for nullifying the heap. So though the snapshot is cleared the storescanner operates on the older reference of the snapshot which cannot be GCed till the current scanner finishes the current API operation. 
So we can change this logic slightly in such a way that let the flush code set a volatile boolean in all the StoreScanner ( the observers). Note that the flush also has cleared the snapshot but still the reference cannot be GCed. Now the current on going scanner every time it enters the scan APIs like next(), seek(), reseek() etc. check for the volatile boolean if it is true. If so, it means that the flush has happened so go ahead and nullfiy the heap and create a new heap. All this will be done by the Storescanner itself. Note that within a storescanner it is always single threaded. 
Now even while entering a scan API if the scanner does not see the volatile boolean as true - it will continue to use the existing heap only and the scan will
still work because we have the older memstore snapshot ref that is not yet GCed - (similar to how things work now).  So this is more of a lazy  model and there may be lag in nullifying the heap and resetting it but should be okie considering the existing way things work.  Thoughts?!!

> Coarsen StoreScanner locks to RegionScanner
> -------------------------------------------
>
>                 Key: HBASE-13082
>                 URL: https://issues.apache.org/jira/browse/HBASE-13082
>             Project: HBase
>          Issue Type: Bug
>            Reporter: Lars Hofhansl
>            Assignee: ramkrishna.s.vasudevan
>         Attachments: 13082-test.txt, 13082-v2.txt, 13082-v3.txt, 13082-v4.txt, 13082.txt, 13082.txt, HBASE-13082.pdf, HBASE-13082_1_WIP.patch, HBASE-13082_2_WIP.patch, HBASE-13082_3.patch, HBASE-13082_4.patch, gc.png, gc.png, gc.png, hits.png, next.png, next.png
>
>
> Continuing where HBASE-10015 left of.
> We can avoid locking (and memory fencing) inside StoreScanner by deferring to the lock already held by the RegionScanner.
> In tests this shows quite a scan improvement and reduced CPU (the fences make the cores wait for memory fetches).
> There are some drawbacks too:
> * All calls to RegionScanner need to be remain synchronized
> * Implementors of coprocessors need to be diligent in following the locking contract. For example Phoenix does not lock RegionScanner.nextRaw() and required in the documentation (not picking on Phoenix, this one is my fault as I told them it's OK)
> * possible starving of flushes and compaction with heavy read load. RegionScanner operations would keep getting the locks and the flushes/compactions would not be able finalize the set of files.
> I'll have a patch soon.


--
This message was sent by Atlassian JIRA
(v6.3.4#6332)