jordepic commented on code in PR #14501:
URL: https://github.com/apache/iceberg/pull/14501#discussion_r2499436517
##########
core/src/main/java/org/apache/iceberg/hadoop/HadoopFileIO.java:
##########
@@ -211,6 +212,19 @@ private ExecutorService executorService() {
return executorService;
}
+ private void deletePath(FileSystem fs, Path toDelete, boolean recursive)
throws IOException {
+ Trash trash = new Trash(fs, getConf());
Review Comment:
It's a good call. I've added a new toggle that can be put in the hadoop
configuration to determine if we want to use the trash for iceberg, following
Russel Spitzer's example in other HadoopFileIO changes.
I've taken a look regarding object reuse. The trash can change due to lots
of changes in configuration (meaning I'd have to create a cache based on 5+
configuration values which are susceptible to change in the future), unlike the
file system (Key doesn't actually rely on conf, just relies on the URI and user
group information). With that being said, the change that I made to check for
hadoop configuration first makes it so that we don't create the Trash object
unless specifically opted into. I hope that this is good enough for now - an
iceberg user will now have to opt into this change to experience any possible
object churn.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
