azagrebin commented on a change in pull request #8646: [FLINK-12735][network]
Make shuffle environment implementation independent with IOManager
URL: https://github.com/apache/flink/pull/8646#discussion_r296229508
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/io/disk/iomanager/IOManager.java
##########
@@ -59,46 +53,15 @@
* @param tempDirs The basic directories for files underlying anonymous
channels.
*/
protected IOManager(String[] tempDirs) {
- if (tempDirs == null || tempDirs.length == 0) {
- throw new IllegalArgumentException("The temporary
directories must not be null or empty.");
- }
-
- this.random = new Random();
- this.nextPath = 0;
-
- this.paths = new File[tempDirs.length];
- for (int i = 0; i < tempDirs.length; i++) {
- File baseDir = new File(tempDirs[i]);
- String subfolder = String.format("flink-io-%s",
UUID.randomUUID().toString());
- File storageDir = new File(baseDir, subfolder);
-
- if (!storageDir.exists() && !storageDir.mkdirs()) {
- throw new RuntimeException(
- "Could not create storage
directory for IOManager: " + storageDir.getAbsolutePath());
- }
- paths[i] = storageDir;
- LOG.info("I/O manager uses directory {} for spill
files.", storageDir.getAbsolutePath());
- }
+ this.fileChannelManager = new
FileChannelManagerImpl(Preconditions.checkNotNull(tempDirs));
}
- /**
- * Close method, marks the I/O manager as closed
- * and removed all temporary files.
- */
@Override
public void close() {
- // remove all of our temp directories
- for (File path : paths) {
- try {
- if (path != null) {
- if (path.exists()) {
- FileUtils.deleteDirectory(path);
- LOG.info("I/O manager removed
spill file directory {}", path.getAbsolutePath());
- }
- }
- } catch (Throwable t) {
- LOG.error("IOManager failed to properly clean
up temp file directory: " + path, t);
- }
+ try {
+ fileChannelManager.close();
+ } catch (Throwable t) {
+ LOG.warn("Cannot close file channel manager properly.",
t);
Review comment:
Not sure about value of this extra warning, because only IOException is
expected and it will be already logged in `fileChannelManager.close`.
`Throwable` is again too broad and unexpected. The close should fail fast for
unexpected failures which should be caught only at some high level, e.g.
TaskExecutor.onStop or RpcService.
----------------------------------------------------------------
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
