clintropolis commented on a change in pull request #6699: release mmap
immediately after merge indexes
URL: https://github.com/apache/incubator-druid/pull/6699#discussion_r242046581
##########
File path:
server/src/main/java/org/apache/druid/segment/realtime/appenderator/AppenderatorImpl.java
##########
@@ -716,8 +717,20 @@ private DataSegment mergeAndPush(final SegmentIdentifier
identifier, final Sink
closer.register(segmentAndCloseable.rhs);
}
- mergedFile = indexMerger.mergeQueryableIndex(
- indexes,
Review comment:
Cool, glad we could sort out what is going on, and apologies it took so long
for me to understand what the point was :+1:
Since this doesn't really seem to particularly have any negative
consequences, and since it is doing something useful in some cases at least,
I'll have another look at this PR.
Also in jdk 10+, I believe it will be [possible to open files with
`O_DIRECT`](https://bugs.openjdk.java.net/browse/JDK-8164900), which is
probably what we really want here if we want merge to be done out of the query
path and not have a significant impact on page cache usage, though it would
potentially lower merge performance. I think it would be worth putting a note
and adding this link so our future selves remember to consider if we ever make
it out of java 8.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
