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

Ted Yu commented on HBASE-21088:
--------------------------------

bq. Where did this issue come from? A bug found in a deploy? A debug session? 
Or is it just random code reading?

I noticed the increase in open files when running test suite. After checking 
recently modified code, I came to this particular method.

bq. Without it, there is no sense of import, or damage done, or rate of 
incidence?

The implication of current code is that many open StoreFile's would be dangling 
after hasReferences() returns.

bq. why not a finally block?

The finally block would be applied to {{StoreUtils.hasReferences}}.
However, HStoreFile::isReference is used to check for reference which isn't 
declared to throw IOE.
If you still think finally block is needed, I can add it in the next patch.

bq. The IOE could come out and the files will be still open, no?
Looking at the following method:
{code}
  private List<HStoreFile> openStoreFiles(Collection<StoreFileInfo> files) 
throws IOException {
{code}
we can see that open StoreFile readers would be closed in case of IOE:
{code}
    if (ioe != null) {
      // close StoreFile readers
      boolean evictOnClose =
          cacheConf != null? cacheConf.shouldEvictOnClose(): true;
      for (HStoreFile file : results) {
{code}
So there is no more open reader to close from the point of view of 
hasReferences().

> HStoreFile should be closed in HStore#hasReferences
> ---------------------------------------------------
>
>                 Key: HBASE-21088
>                 URL: https://issues.apache.org/jira/browse/HBASE-21088
>             Project: HBase
>          Issue Type: Bug
>            Reporter: Ted Yu
>            Assignee: Ted Yu
>            Priority: Major
>         Attachments: 21088.v1.txt
>
>
> {code}
>       reloadedStoreFiles = loadStoreFiles();
>       return StoreUtils.hasReferences(reloadedStoreFiles);
> {code}
> The intention of obtaining the HStoreFile's is to check for references.
> The loaded HStoreFile's should be closed prior to return to prevent leak.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to