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

Appy commented on HBASE-19675:
------------------------------

bq. Please consider accepting this patch as-is while I work on HBASE-19682
Followup sounds fine. 

+1 on v2 patch.

Just a quick note, though relevant to this patch, it's not a result of it. It's 
a pre-existing condition.
bq. Collection<HStoreFile> newFiles = Collections.emptyList(); // No new files.
The collection returned would be immutable where as it's not apparent that when 
it's passed to the next two functions, they will not try to change it. Looks 
like bug at first site and takes some looking around to verify that it's not. 
I'll change the method signature to make it apparent - one fn to take Stream<T> 
and other to take Iterable<T>.
Sg [~belugabehr]?


> Miscellaneous HStore Class Improvements
> ---------------------------------------
>
>                 Key: HBASE-19675
>                 URL: https://issues.apache.org/jira/browse/HBASE-19675
>             Project: HBase
>          Issue Type: Improvement
>          Components: hbase
>    Affects Versions: 3.0.0
>            Reporter: BELUGA BEHR
>            Assignee: BELUGA BEHR
>            Priority: Minor
>         Attachments: HBASE-19675.1.patch, HBASE-19675.2.patch
>
>
> * Remove logging code guards in favor of slf4j parameters
> * Use {{CollectionsUtils.isEmpty()}} consistently
> * Small check-style fixes
> * Remove flow control logic from {{trace}} statement {code}          if 
> (LOG.isTraceEnabled()) {
>             LOG.trace("No compacted files to archive");
>             return;
>           }{code}
> * Replace two calls to the same getter to ensure that the value doesn't 
> change between calls {code}      if (getCompactedFiles() != null) {
>         for (HStoreFile file : getCompactedFiles()) {
>           name2File.put(file.getFileInfo().getActiveFileName(), file);
>         }
>       }{code}
> * Make 'inputFiles' a Set for fast calls to {{contains}} method instead 
> {code}    //some of the input files might already be deleted
>     List<HStoreFile> inputStoreFiles = new 
> ArrayList<>(compactionInputs.size());
>     for (HStoreFile sf : this.getStorefiles()) {
>       if (inputFiles.contains(sf.getPath().getName())) {
>         inputStoreFiles.add(sf);
>       }
>     }{code}



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to