memory leak on TmpFileIOPeon, io.druid.segment.realtime.firehose.ReplayableFirehoseFactory

[valenpo]

@Override
 public OutputStream makeOutputStream(String filename) throws IOException
 {
   File retFile = createdFiles.get(filename);
   if (retFile == null) {
     retFile = File.createTempFile("filePeon", filename);
     retFile.deleteOnExit();
     createdFiles.put(filename, retFile);
     return new BufferedOutputStream(new FileOutputStream(retFile));
   } else if (allowOverwrite) {
     return new BufferedOutputStream(new FileOutputStream(retFile));
   } else {
     throw new IOException("tmp file conflicts, file[" + filename + "] already exist!");
   }
 }

retFile.deleteOnExit(); - create a memory leak due to JVM are exited.

[valenpo]

Same story for io.druid.segment.realtime.firehose.ReplayableFirehoseFactory.

public void deleteOnExit()
Requests that the file or directory denoted by this abstract pathname be deleted when the virtual machine terminates. Files (or directories) are deleted in the reverse order that they are registered. Invoking this method to delete a file or directory that is already registered for deletion has no effect. Deletion will be attempted only for normal termination of the virtual machine, as defined by the Java Language Specification.
Once deletion has been requested, it is not possible to cancel the request. This method should therefore be used with care.

Note: this method should not be used for file-locking, as the resulting protocol cannot be made to work reliably. The FileLock facility should be used instead.

Throws:
SecurityException - If a security manager exists and its SecurityManager.checkDelete(java.lang.String) method denies delete access to the file
Since:
1.2
See Also:
delete()

[gianm]

Thanks for the report @valenpo.

I think this is probably fine for ReplayableFirehoseFactory, since it's only meant to be used from short-lived indexing tasks.

TmpFileIOPeon, not so sure, I don't recall where it's used exactly. But it does have its own "cleanup" method that, if always called, would make the deleteOnExit redundant.

[valenpo]

@gianm it is used on IndexMerger. deleteOnExit not should be used on long running application. If it is necessary I think better use DELETE_ON_CLOSE on NIO2

[akashdw]

@gianm GenericIndexedWriter gets TmpFileIOPeon to create header and value files. Probably we should remove deleteOnExit() to prevent any side effects. We could make IOPean Closeable and do the cleanup in close method.

[KurtYoung]

We have met this issue in our production environment, and just delete the deleteOnExit works fine.

[valenpo]

@KurtYoung we do same. It is wrong using deleteOnExit on production.

[stale]

This issue has been marked as stale due to 280 days of inactivity. It will be closed in 2 weeks if no further activity occurs. If this issue is still relevant, please simply write any comment. Even if closed, you can still revive the issue at any time or discuss it on the dev@druid.apache.org list. Thank you for your contributions.

[stale]

This issue has been closed due to lack of activity. If you think that is incorrect, or the issue requires additional review, you can revive the issue at any time.