leventov commented on a change in pull request #6677: FileUtils: Sync directory
entry too on writeAtomically.
URL: https://github.com/apache/incubator-druid/pull/6677#discussion_r237182258
##########
File path:
core/src/main/java/org/apache/druid/java/util/common/CompressionUtils.java
##########
@@ -78,16 +78,12 @@ public static long zip(File directory, File outputZipFile,
boolean fsync) throws
log.warn("No .zip suffix[%s], putting files from [%s] into it anyway.",
outputZipFile, directory);
}
- try (final FileOutputStream out = new FileOutputStream(outputZipFile)) {
- long bytes = zip(directory, out);
-
- // For explanation of why fsyncing here is a good practice:
- //
https://github.com/apache/incubator-druid/pull/5187#pullrequestreview-85188984
- if (fsync) {
- out.getChannel().force(true);
+ if (fsync) {
+ return FileUtils.writeAtomically(outputZipFile, out -> zip(directory,
out));
+ } else {
+ try (final FileOutputStream out = new FileOutputStream(outputZipFile)) {
Review comment:
Personally I've only *heard* that all types of references from
`java.lang.ref` are somehow bad for GC and could lead to effective memory
leaks, both *especially* true for finalizable objects. But I cannot prove that.
Anecdote:
We are constantly suffering GC and apparent memory leak problems with pretty
much all Druid node types, that only get worse with Druid updates, but we
cannot understand the reasons. We only try to make incremental improvements on
this front, such as #6370 and other PRs.
On the suffering nodes, we cannot made full heap dumps to anylize them
because heaps are too big, but we can see heap stats (a-la `jmap -histo`) and
they show that there are, for example, on some Historical node: *more than 10k*
of finalizable objects on the heap (determined by the number of
`java.lang.ref.Finalizer` objects on the heap), *tens of thousands* of
`java.lang.ref.PhantomReference`s, and several thousands of both WeakReferences
and SoftReferences. I don't know where the hell all this comes from. (Except
for `PhantomReference`s that are apparently connected to direct `ByteBuffers`,
#4773 could help here.)
----------------------------------------------------------------
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]
