steveloughran commented on a change in pull request #1859: HADOOP-16885.
Encryption zone file copy failure leaks temp file ._COP…
URL: https://github.com/apache/hadoop/pull/1859#discussion_r384708404
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandWithDestination.java
##########
@@ -491,25 +490,18 @@ void writeStreamToFile(InputStream in, PathData target,
throws IOException {
FSDataOutputStream out = null;
try {
- out = create(target, lazyPersist, direct);
+ out = create(target, lazyPersist);
IOUtils.copyBytes(in, out, getConf(), true);
- } catch (IOException e) {
- // failure: clean up if we got as far as creating the file
- if (!direct && out != null) {
- try {
- fs.delete(target.path, false);
- } catch (IOException ignored) {
- }
- }
- throw e;
} finally {
+ if (!direct) {
+ deleteOnExit(target.path);
+ }
Review comment:
One thing I need to be confident that we're handling is the situation where
the file exists at the end, create fails with an overwrite error -we don't want
the deleteOnExit to suddenly delete that previous file, not if it is one we
care about.
For direct writes then: we don't do that delete (good), For indirect ones,
we are relying on the fact that the file being created is temporary. Because
we're going to end up stamping on it aren't we?
From an S3A perspective -I don't see the codepath creating a 404 as the
deleteOnExit registration takes place after the upload has succeeded: the HEAD
will find the file which has just been created.
----------------------------------------------------------------
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.
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]