PR comments 2

apache · Dec 12, 2024 · 082e2b5 · 082e2b5
1 parent d642287
commit 082e2b5
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 20 deletions.
diff --git a/tez-api/src/main/java/org/apache/tez/common/counters/FileSystemCounter.java b/tez-api/src/main/java/org/apache/tez/common/counters/FileSystemCounter.java
@@ -32,7 +32,6 @@ public enum FileSystemCounter {
   LARGE_READ_OPS("largeReadOps"),
   WRITE_OPS("writeOps"),
 
-  // Additional counters from HADOOP-13305
   // Additional counters from HADOOP-13305
   OP_APPEND(CommonStatisticNames.OP_APPEND),
   OP_COPY_FROM_LOCAL_FILE(CommonStatisticNames.OP_COPY_FROM_LOCAL_FILE),

diff --git a/tez-runtime-internals/src/main/java/org/apache/tez/runtime/metrics/TaskCounterUpdater.java b/tez-runtime-internals/src/main/java/org/apache/tez/runtime/metrics/TaskCounterUpdater.java
@@ -21,7 +21,6 @@
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
-import java.util.TreeMap;
 
 import org.apache.hadoop.fs.GlobalStorageStatistics;
 import org.apache.hadoop.fs.StorageStatistics;
@@ -49,16 +48,10 @@ public class TaskCounterUpdater {
   private final TezCounters tezCounters;
   private final Configuration conf;
 
-//  /**
-//   * A Map where Key-> URIScheme and value->FileSystemStatisticUpdater
-//   */
-//  private Map<String, FileSystemStatisticUpdater> statisticUpdaters =
-//     new HashMap<>();
   /**
    * A Map where Key-> URIScheme and value->Map<Name, FileSystemStatisticUpdater>
    */
-  private Map<String, Map<String, FileSystemStatisticUpdater>> statisticUpdaters =
-      new HashMap<>();
+  private final Map<String, Map<String, FileSystemStatisticUpdater>> statisticUpdaters = new HashMap<>();
   protected final GcTimeUpdater gcUpdater;
   private ResourceCalculatorProcessTree pTree;
   private long initCpuCumulativeTime = 0;
@@ -73,15 +66,15 @@ public TaskCounterUpdater(TezCounters counters, Configuration conf, String pid)
     recordInitialCpuStats();
   }
 
-  
+
   public void updateCounters() {
     GlobalStorageStatistics globalStorageStatistics = FileSystem.getGlobalStorageStatistics();
     Iterator<StorageStatistics> iter = globalStorageStatistics.iterator();
     while (iter.hasNext()) {
       StorageStatistics stats = iter.next();
       // Fetch or initialize the updater set for the scheme
       Map<String, FileSystemStatisticUpdater> updaterSet = statisticUpdaters
-          .computeIfAbsent(stats.getScheme(), k -> new TreeMap<>());
+          .computeIfAbsent(stats.getScheme(), k -> new HashMap<>());
       // Fetch or create the updater for the specific statistic
       FileSystemStatisticUpdater updater = updaterSet
           .computeIfAbsent(stats.getName(), k -> new FileSystemStatisticUpdater(tezCounters, stats));

diff --git a/...nternals/src/test/java/org/apache/tez/runtime/metrics/TestFileSystemStatisticUpdater.java b/...nternals/src/test/java/org/apache/tez/runtime/metrics/TestFileSystemStatisticUpdater.java
@@ -30,6 +30,7 @@
 import org.apache.tez.common.counters.TezCounters;
 import org.junit.AfterClass;
 import org.junit.Assert;
+import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.slf4j.Logger;
@@ -49,14 +50,8 @@ public class TestFileSystemStatisticUpdater {
       TestFileSystemStatisticUpdater.class.getName() + "-tmpDir";
 
   @BeforeClass
-  public static void setup() throws IOException {
-    try {
-      CONF.set(MiniDFSCluster.HDFS_MINIDFS_BASEDIR, TEST_ROOT_DIR);
-      dfsCluster = new MiniDFSCluster.Builder(CONF).numDataNodes(2).build();
-      remoteFs = dfsCluster.getFileSystem();
-    } catch (IOException io) {
-      throw new RuntimeException("problem starting mini dfs cluster", io);
-    }
+  public static void beforeClass() throws Exception {
+    CONF.set(MiniDFSCluster.HDFS_MINIDFS_BASEDIR, TEST_ROOT_DIR);
   }
 
   @AfterClass
@@ -67,6 +62,21 @@ public static void tearDown() {
     }
   }
 
+  @Before
+  public void setup() throws IOException {
+    FileSystem.clearStatistics();
+    try {
+      // tear down the whole cluster before each test to completely get rid of file system statistics
+      if (dfsCluster != null) {
+        dfsCluster.shutdown();
+      }
+      dfsCluster = new MiniDFSCluster.Builder(CONF).numDataNodes(2).build();
+      remoteFs = dfsCluster.getFileSystem();
+    } catch (IOException io) {
+      throw new RuntimeException("problem starting mini dfs cluster", io);
+    }
+  }
+
   @Test
   public void basicTest() throws IOException {
     TezCounters counters = new TezCounters();
@@ -94,7 +104,6 @@ public void basicTest() throws IOException {
     assertCounter(counters, FileSystemCounter.OP_CREATE, 2);
 
     // Ensure all numbers are reset
-    FileSystem.clearStatistics();
     updater.updateCounters();
     LOG.info("Counters (after third update): {}", counters);
     // counter holds its value after clearStatistics + updateCounters