[
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
