[
https://issues.apache.org/jira/browse/FLINK-2369?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14631211#comment-14631211
]
Gabor Gevay commented on FLINK-2369:
------------------------------------
This is worse than I thought. The code is totally inconsistent about the
relationship between OutputFormat.close() and
CleanupWhenUnsuccessful.tryCleanupOnError(). If you do a find usages on
tryCleanupOnError it gets called in four places in three different relations to
close:
1. DataSinkTask.cancel(): After close
2. Exception handler in DataSinkTask.invoke: Before close
3. FileSinkFunction.close: When catching an exception thrown by close
4. FileSinkFunction.flush: This is before close in FileSinkFunction.close.
I see two approaches to fixing this:
A. Fix all of the above to call tryCleanupOnError only after close.
- 2. is easy: just introduce a flag shouldCallTryCleanup, and set it to true
instead where now tryCleanupOnError is called, and do the actual
tryCleanupOnError call after the close in the finally block if the flag is set.
- You can't really do anything in case of 3., because things must be really
messed up if we get to that catch block. I mean, the close already failed here,
so something very bad must be going on.
- I am not sure about 4: Does the same approach of introducing a flag and
attending to it after close would work? FileSinkFunction.close would be a
natural place to attend to the flag, but I am not even sure whether it gets
called in case of an error.
B. Modify the contract of tryCleanupOnError to not give any guarantees about
the relation to close.
This means that it should do a close on its own if neccessary. This would
basically mean, that the implementor of the interface got all the burden of
making sure that it can handle all of the above 4 cases. What makes this
unpleasant is that the javadoc of close says that "When this method is called,
the output format it guaranteed to be opened.", which means that some subclass
might get "surprised" if we just call close from
FileOutputFormat.tryCleanupOnError and it ends up called twice.
> On Windows, in testFailingSortingDataSinkTask the temp file is not removed
> --------------------------------------------------------------------------
>
> Key: FLINK-2369
> URL: https://issues.apache.org/jira/browse/FLINK-2369
> Project: Flink
> Issue Type: Bug
> Components: Distributed Runtime
> Environment: Windows 7 64-bit, JDK 8u51
> Reporter: Gabor Gevay
> Assignee: Gabor Gevay
> Priority: Minor
>
> The test fails with the assert at the very end ("Temp output file has not
> been removed"). This happens because FileOutputFormat.tryCleanupOnError can't
> delete the file, because it is still open (note, that Linux happily deletes
> open files).
> A fix would be to have the this.format.close(); not just in the finally
> block (in DataSinkTask.invoke), but also before the tryCleanupOnError call
> (around line 217).
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
