steveloughran commented on a change in pull request #2624:
URL: https://github.com/apache/hadoop/pull/2624#discussion_r565187998



##########
File path: 
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/lib/output/FileOutputCommitter.java
##########
@@ -455,53 +455,50 @@ protected void commitJobInternal(JobContext context) 
throws IOException {
    */
   private void mergePaths(FileSystem fs, final FileStatus from,
       final Path to, JobContext context) throws IOException {
-    long timeStartNs = -1L;
-    if (LOG.isDebugEnabled()) {
-      timeStartNs = System.nanoTime();
-      LOG.debug("Merging data from " + from + " to " + to);
-    }
-    reportProgress(context);
-    FileStatus toStat;
-    try {
-      toStat = fs.getFileStatus(to);
-    } catch (FileNotFoundException fnfe) {
-      toStat = null;
-    }
-
-    if (from.isFile()) {
-      if (toStat != null) {
-        if (!fs.delete(to, true)) {
-          throw new IOException("Failed to delete " + to);
-        }
+    try (DurationInfo d = new DurationInfo(LOG,
+        false,
+        "Merged data from %s to %s", from.getPath(), to)) {

Review comment:
       we actually print this *at start* as well as end; end has timings. So 
you don't need lines 461 & 462

##########
File path: 
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/lib/output/FileOutputCommitter.java
##########
@@ -455,53 +455,50 @@ protected void commitJobInternal(JobContext context) 
throws IOException {
    */
   private void mergePaths(FileSystem fs, final FileStatus from,
       final Path to, JobContext context) throws IOException {
-    long timeStartNs = -1L;
-    if (LOG.isDebugEnabled()) {
-      timeStartNs = System.nanoTime();
-      LOG.debug("Merging data from " + from + " to " + to);
-    }
-    reportProgress(context);
-    FileStatus toStat;
-    try {
-      toStat = fs.getFileStatus(to);
-    } catch (FileNotFoundException fnfe) {
-      toStat = null;
-    }
-
-    if (from.isFile()) {
-      if (toStat != null) {
-        if (!fs.delete(to, true)) {
-          throw new IOException("Failed to delete " + to);
-        }
+    try (DurationInfo d = new DurationInfo(LOG,
+        false,
+        "Merged data from %s to %s", from.getPath(), to)) {
+      if (LOG.isDebugEnabled()) {

Review comment:
       cut these; duplicate now




----------------------------------------------------------------
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]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to