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