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