[GitHub] [hadoop] mehakmeet commented on a diff in pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A

[GitBox <gi...@apache.org>]

mehakmeet commented on code in PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#discussion_r917829041


##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContext.java:
##########
@@ -51,21 +51,21 @@ public class IOStatisticsContext {
   private static final boolean IS_THREAD_IOSTATS_ENABLED;
 
   private static final WeakReferenceThreadMap<IOStatisticsContext>
-      ACTIVE_IOSTATS_CONTEXT = new WeakReferenceThreadMap<>(
-      IOStatisticsContext::createNewInstance,
-      IOStatisticsContext::referenceLostContext
+      ACTIVE_IOSTATS_CONTEXT =
+      new WeakReferenceThreadMap<>(IOStatisticsContext::createNewInstance,
+          IOStatisticsContext::referenceLostContext
   );
 
   /**
    * Collecting IOStatistics per thread.
    */
   private final WeakReferenceThreadMap<IOStatisticsAggregator>

Review Comment:
   Okay, I'll do that. My thinking behind this was to also have a way to get EmptyIOstatisticsSource as one of the implementations we could return for a thread ID, which would have a no-op for aggregation, but thinking this through, it seems like having this thread map solely for "enabled" thread IOStats makes more sense and we can return the EmptyIOstatisticsSource, while getting the IOstats without setting it in. Makes resetting easier too. 



-- 
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: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org