dlmarion commented on code in PR #3083:
URL: https://github.com/apache/accumulo/pull/3083#discussion_r1026358174
##########
core/src/main/java/org/apache/accumulo/core/conf/Property.java:
##########
@@ -964,6 +964,11 @@ public enum Property {
"1.3.5"),
TABLE_ARBITRARY_PROP_PREFIX("table.custom.", null, PropertyType.PREFIX,
"Prefix to be used for user defined arbitrary properties.", "1.7.0"),
+ TABLE_MAJC_OUTPUT_DROP_CACHE("table.compaction.major.output.drop.cache",
"false",
+ PropertyType.BOOLEAN,
+ "Setting this property to true will call"
+ + "FSDataOutputStream.setDropBehind(true) on the major compaction
output stream.",
+ "2.1.1"),
Review Comment:
> Can we justify adding it in 2.1.1?
I'm not sure we need to. There are plenty of examples of properties being
added in a patch release, including 4 of them added in version 1.9.3.
> Why not just always set it, and let it be cached when read?
> Making it configurable give a an escape hatch for that.
This is exactly my concern. I think it may be safe to always do this, but
I'm not 100% positive. The way that Hadoop implemented this, you can tell the
OS via the FADVISE_DONTNEED flag that the OS can purge the file from the page
cache when the read or write is done (in this case write). I read that this is
done by setting that advice on the file descriptor and that each process (the
DN in this case) has a file descriptor table. I don't know if re-opening the
same file shortly after would re-use the same file descriptor number and I
don't know if that advice may prevent caching of the file on a subsequent read
operation. My guess is that it's safe to call it on the majc output file, but I
don't know for sure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
