accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dennis Patrone (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (ACCUMULO-587) Add finalize to TabletServerBatchReader to catch when user forgets to close
Date Wed, 16 May 2012 18:43:08 GMT

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

Dennis Patrone commented on ACCUMULO-587:
-----------------------------------------

A minor change to the inner class BatchReaderThreadFactory (making it static and accepting
the 'batchReaderInstance' in the constructor rather than relying on the instance member for
thread naming) would allow a reader to be garbage collected without being closed-- even while
the thread pool was alive.

I understand the concern that people might get lazy and rely on finalize() rather than call
close() themselves.  But I'd argue you can't help those with that kind of attitude.  On the
other hand, a finalize with a logged warning would help those that *inadvertently* forgot
to call close and encounter a memory/thread leak to quickly recognize the source. 

And if you are really concerned about lazy folks, you could choose to not even call close()
in the finalize() method.  That is, leave the leak but add the log warning.  That way folks
can't rely on the finalize() to free up resources involved, but the system warn those that
forget to call close(); ultimately resulting in more stable Accumulo clients.
                
> Add finalize to TabletServerBatchReader to catch when user forgets to close
> ---------------------------------------------------------------------------
>
>                 Key: ACCUMULO-587
>                 URL: https://issues.apache.org/jira/browse/ACCUMULO-587
>             Project: Accumulo
>          Issue Type: Improvement
>          Components: client
>            Reporter: Dennis Patrone
>            Assignee: Billie Rinaldi
>            Priority: Trivial
>
> If a client forgets to close a BatchScanner or BatchDeleter, threads are leaked in the
TabletServerBatchReader implementation.  It would be nice if a finalize method were added
to check and warn the user of such a problem. The thread pool appeared to only be shared with
the TabletServerBatchReaderIterator, which maintains a reference to the TabletServerBatchReader
itself.  So AFAICT if the TabletServerBatchReader is eligible for garbage collection, there
can be no client references to that scanner or any iterators it created (i.e., it _should_
have been closed).
> For example:
> {code}
> protected void finalize() {
>    if (!queryThreadPool.isShutdown()) {
>       // add a logger reference in class initialization
>       log.warn("TabletServerBatchReader not shutdown; did you forget to call close()?");
>       close();
>    }
> }
> {code}
> The same might be true for the TabletServerBatchWriter (it has a close), but I didn't
look into that class.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Mime
View raw message