[
https://issues.apache.org/jira/browse/HBASE-15061?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15076044#comment-15076044
]
Jonathan Hsieh commented on HBASE-15061:
----------------------------------------
[~anoopsamjohn] Having to look up what the (..., false, true,false, false...)
means everytime by reading the chain of 3 flavors of getScannerForStoreFiles
makes it harder to read/review the code. I prefer going to one java doc to
find out which settings are on and off by default. If we say what the defaults
are there, the invocations highlight when we purposely are choosing different
settings instead of hiding them. When the next extension comes along the
series of booleans gets more difficult to grok.
I've chosen to require the list of files and readpoint in the constructor and
making them require arguments. Is your suggestion to potentially moving the
caching setting into the constructor to make it a required argument? At least
from the patch, that values is essentially always set false except for when an
override comes in from the scan.
Bigger picture -- related to HBASE-15051, there is more coming. I'm going
through and trying to regularize and cleanup all the argument handling and
instantiation around hfiles and their reader/writers. I'm working my way
through this and plan on getting to somewhere where all the flavors of hfile
reader/writer options (Conf, HTD, HCF, HFileContexts, StoreFileInfos,
CacheConfig, HalfStoreFile, References, HFileLinks) are a bit more unified.
[~larsh] Good point. I'll do a profiling run before considering a commit to
trunk.
> Refactor StoreFileScanner creation to builder pattern
> -----------------------------------------------------
>
> Key: HBASE-15061
> URL: https://issues.apache.org/jira/browse/HBASE-15061
> Project: HBase
> Issue Type: Improvement
> Affects Versions: 2.0.0
> Reporter: Jonathan Hsieh
> Assignee: Jonathan Hsieh
> Fix For: 2.0.0
>
> Attachments: hbase-15061.patch, hbase-15061.v2.patch
>
>
> There are several falvors of calls that creates a list of StoreFileScanners,
> and new feature have been added to this recently. This patch converts the
> somewhat difficult to read (need to go to javadoc) call:
> {code}
> // which args are the most relevant to this?
> - List<StoreFileScanner> sfScanners =
> StoreFileScanner.getScannersForStoreFiles(sfs,
> - cacheMobBlocks, true, false, false, readPt);
> {code}
> into one that is more literate:
> {code}
> // ah, very clearly we are using defaults except for the caching settings
> + List<StoreFileScanner> sfScanners = new
> StoreFileScanner.ListBuilder(sfs, readPt)
> + .withCacheBlocks(cacheMobBlocks).build();
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
