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-13082) Coarsen StoreScanner locks to RegionScanner
Date Fri, 20 Nov 2015 10:46:11 GMT

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

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

Thanks for the reviews Stack.
bq. Need to update comment and rename variable else we'll stay confused.
Will update the comment and say that 'these files are not included in reads'? The name 'compactedfiles'
is still better?

bq.Only one thread involved here?
bq.public ImmutableCollection<StoreFile> clearCompactedFiles() {
Yes it will be single threaded. Used only while closing the store files.
bq.Suggest you change this method to return the Collection rather than set the data member
internall: i.e remove the 'set' part from sortAndSetCompactedFiles Do the set on return. Methods
like this with 'side effects' can be tough to follow.
Okie.
bq.You know the size of the array to allocate here: 124	newCompactedFiles = Lists.newArrayList();
...
Yes done some refactoring there.
bq. DISCARDED and ACTIVE?
We will make it ACTIVE and COMPACTEDAWAY (as you suggested in another comment).?
bq.I don't follow how we were checking for references when we went to merge but in this patch
it changes to a check for compactions:
I think you were referring to some old patch. The patch that was latest was _14.
bq.Fix formatting here abouts if you are doing a new patch: if (!SystemUtils.IS_OS_WINDOWS)
{
This is not there in the latest patch.
bq.Where we explain what it does?
There is a javadoc explaining what it does.
bq.as to be public because its in the Interface? Does it have to be:
We access Store not directly from HStore but from Store.java. So it is better to add there
and anyway this is going to be common for that store.
bq.Might want to note that expectation is that access on methods like this one are single-threaded:
clearCompactedFiles
Okie.
bq.Do you have to stop the chore in the region or store close? Before you do your close?
Yes good catch. Done now.
bq.void closeAndArchiveCompactedFiles(List<StoreFile> compactedStorefiles) throws IOException;
In my next version will remove this but keep the other one void closeAndArchiveCompactedFiles()
throws IOException;
bq.Do they have to be so specific? Can they be made more generic?
Generic in the sense?
bq.t seems like the compacted or not belongs in StoreFileInfo rather than in StoreFile. Is
this fact persisted across open/close?
We cannot have this in StorefileInfo because we only cache the Storefile (in the StorefileManager)
and not the StorefileInfo. StoreFileInfos are created every time from the hfile path.
bq.Maybe 'compactedAway'?
Ya we can have ACTIVE and COMPACTED_AWAY.?

> 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.pdf, HBASE-13082_12.patch, HBASE-13082_13.patch,
HBASE-13082_14.patch, HBASE-13082_1_WIP.patch, HBASE-13082_2.pdf, HBASE-13082_2_WIP.patch,
HBASE-13082_3.patch, HBASE-13082_4.patch, HBASE-13082_9.patch, HBASE-13082_9.patch, HBASE-13082_withoutpatch.jpg,
HBASE-13082_withpatch.jpg, LockVsSynchronized.java, 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)

Mime
View raw message