Unmap files to add eagerly in FileSmoother#50
Conversation
…urpose of this is to enable running Druid's IndexMergeBenchmark in Windows, but this is also universally healthy becauseit releases memory determenistically, otherwise files are unmapped when MappedByteBuffers are garbage-collected
|
Wait a sec, I need to double-check this. |
|
There are explicit tests in druid about properly unmapping files. It would be good to make sure this still functions in those tests. |
|
@drcrallen could you please point to them? This commit in java-util is only a part of the change. I'm also going to propose some fixes to druid itself, regarding specifically leaking file mappings. So those tests apparently don't catch the problems and should be updated too. |
| try { | ||
| add(name, mappedFileBuffer); | ||
| } finally { | ||
| ByteBufferUtils.unmap(mappedFileBuffer); |
There was a problem hiding this comment.
try-with-resources preferable here to avoid exception chomping.
There was a problem hiding this comment.
Yes... also there is #46, and also there is an idea to approach this more fundamentially with ResourceHandlers, discussed in apache/druid#3422. Another option is to move away from ByteBuffers, to e. g. Netty's, Aeron's or Chronicle's alternatives, which are better in many ways. So I close this PR for now anyway.
3 participants
The exact purpose of this is to enable running Druid's IndexMergeBenchmark in Windows, but this is also universally healthy becauseit releases memory determenistically, otherwise files are unmapped when MappedByteBuffers are garbage-collected