steveloughran commented on code in PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#discussion_r928703332
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AInputStream.java:
##########
@@ -1351,6 +1351,7 @@ public boolean hasCapability(String capability) {
case StreamCapabilities.READAHEAD:
case StreamCapabilities.UNBUFFER:
case StreamCapabilities.VECTOREDIO:
+ case StreamCapabilities.IOSTATISTICS_CONTEXT:
Review Comment:
put it in alphabetical order
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContextIntegration.java:
##########
@@ -116,13 +116,17 @@ public static IOStatisticsContext
getCurrentIOStatisticsContext() {
/**
* Set the IOStatisticsContext for the current thread.
* @param statisticsContext IOStatistics context instance for the
- * current thread.
+ * current thread. If null, the context is reset.
*/
public static void setThreadIOStatisticsContext(
IOStatisticsContext statisticsContext) {
- if (IS_THREAD_IOSTATS_ENABLED &&
- ACTIVE_IOSTATS_CONTEXT.getForCurrentThread() != statisticsContext) {
- ACTIVE_IOSTATS_CONTEXT.setForCurrentThread(statisticsContext);
+ if (IS_THREAD_IOSTATS_ENABLED) {
Review Comment:
we should have a test to verify this doesn't trigger an NPE and removes the
context
test
* get thread context
* set the current context to null
* get the thread context again
* verify the ID values don't match, because a new context was generated
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContextIntegration.java:
##########
@@ -152,4 +152,13 @@ public static IOStatisticsContext
getThreadSpecificIOStatisticsContext(long test
}
return null;
}
+
+ /**
+ * A method to enable IOStatisticsContext to override if set otherwise in
+ * the configurations for tests.
+ */
+ @VisibleForTesting
+ public static void enableIOStatisticsContext() {
+ IS_THREAD_IOSTATS_ENABLED = true;
Review Comment:
how about, if it wasn't already true, log at info that it is being set
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContextIntegration.java:
##########
@@ -53,7 +53,7 @@ public final class IOStatisticsContextIntegration {
/**
* Is thread-level IO Statistics enabled?
*/
- private static final boolean IS_THREAD_IOSTATS_ENABLED;
+ private static boolean IS_THREAD_IOSTATS_ENABLED;
Review Comment:
needs to become lower case
--
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]
