steveloughran commented on code in PR #6551:
URL: https://github.com/apache/hadoop/pull/6551#discussion_r1773612613
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileUtil.java:
##########
@@ -2108,4 +2108,21 @@ public static void
maybeIgnoreMissingDirectory(FileSystem fs,
LOG.info("Ignoring missing directory {}", path);
LOG.debug("Directory missing", e);
}
+
+ /**
+ * Return true if the FS implements {@link WithErasureCoding} and
+ * supports EC_POLICY option in {@link Options.OpenFileOptions}
Review Comment:
1. add a . at the end of this sentence
2. add a second sentence saying that a message is logged when not available
##########
hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java:
##########
@@ -380,6 +380,8 @@ public FSDataInputStream open(PathHandle fd, int bufferSize)
@Override
public String getErasureCodingPolicyName(FileStatus fileStatus) {
+ if (!(fileStatus instanceof HdfsFileStatus))
Review Comment:
has to have curly braces round. Not for us, but for our successors in
maintaining this code
##########
hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/mapred/RetriableFileCopyCommand.java:
##########
@@ -208,8 +208,8 @@ private long copyToFile(Path targetPath, FileSystem
targetFS,
String ecPolicyName = null;
if (preserveEC && sourceStatus.isErasureCoded()
- && checkFSSupportsEC(sourceStatus.getPath(), sourceFS)
- && checkFSSupportsEC(targetPath, targetFS)) {
+ && checkFSSupportsEC(sourceFS,sourceStatus.getPath())
Review Comment:
nit: space after ,
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileUtil.java:
##########
@@ -2108,4 +2108,21 @@ public static void
maybeIgnoreMissingDirectory(FileSystem fs,
LOG.info("Ignoring missing directory {}", path);
LOG.debug("Directory missing", e);
}
+
+ /**
+ * Return true if the FS implements {@link WithErasureCoding} and
+ * supports EC_POLICY option in {@link Options.OpenFileOptions}
+ * @param fs filesystem
+ * @param path path
+ * @return true if the Filesystem supports EC
+ * @throws IOException if there is a failure in hasPathCapability call
+ */
+ public static boolean checkFSSupportsEC(FileSystem fs, Path path) throws
IOException {
+ if (fs instanceof WithErasureCoding &&
+ fs.hasPathCapability(path,
Options.OpenFileOptions.FS_OPTION_OPENFILE_EC_POLICY)) {
+ return true;
+ }
+ LOG.warn("FS with scheme {} does not support EC", fs.getScheme());
Review Comment:
* prefer "Erasure Coding" to "EC"
* do you think we should print the full path?
--
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]
