Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

HADOOP-18373. IOStatisticsContext tuning #4705

Merged
merged 2 commits into from
Aug 8, 2022
Merged

HADOOP-18373. IOStatisticsContext tuning #4705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
127d230
HADOOP-18373. IOStatisticsContext tuning
virajjasani Aug 4, 2022
e6292d5
addressing review comments
virajjasani Aug 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions ...mon-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeys.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -480,13 +480,13 @@ public class CommonConfigurationKeys extends CommonConfigurationKeysPublic {
* Thread-level IOStats Support.
* {@value}
*/
public static final String THREAD_LEVEL_IOSTATISTICS_ENABLED =
"fs.thread.level.iostatistics.enabled";
public static final String IOSTATISTICS_THREAD_LEVEL_ENABLED =
"fs.iostatistics.thread.level.enabled";

/**
* Default value for Thread-level IOStats Support is true.
*/
public static final boolean THREAD_LEVEL_IOSTATISTICS_ENABLED_DEFAULT =
public static final boolean IOSTATISTICS_THREAD_LEVEL_ENABLED_DEFAULT =
true;

}
10 changes: 10 additions & 0 deletions ...ject/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/IOStatisticsContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,14 @@ static void setThreadIOStatisticsContext(
IOStatisticsContextIntegration.setThreadIOStatisticsContext(
statisticsContext);
}

/**
* Static probe to check if the thread-level IO statistics enabled.
*
* @return if the thread-level IO statistics enabled.
*/
static boolean enabled() {
return IOStatisticsContextIntegration.isIOStatisticsThreadLevelEnabled();
}

}
17 changes: 13 additions & 4 deletions ...on/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContextIntegration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@
import org.apache.hadoop.fs.impl.WeakReferenceThreadMap;
import org.apache.hadoop.fs.statistics.IOStatisticsContext;

import static org.apache.hadoop.fs.CommonConfigurationKeys.THREAD_LEVEL_IOSTATISTICS_ENABLED;
import static org.apache.hadoop.fs.CommonConfigurationKeys.THREAD_LEVEL_IOSTATISTICS_ENABLED_DEFAULT;
import static org.apache.hadoop.fs.CommonConfigurationKeys.IOSTATISTICS_THREAD_LEVEL_ENABLED;
import static org.apache.hadoop.fs.CommonConfigurationKeys.IOSTATISTICS_THREAD_LEVEL_ENABLED_DEFAULT;

/**
* A Utility class for IOStatisticsContext, which helps in creating and
Expand Down Expand Up @@ -76,8 +76,17 @@ public final class IOStatisticsContextIntegration {
// Work out if the current context has thread level IOStatistics enabled.
final Configuration configuration = new Configuration();
isThreadIOStatsEnabled =
configuration.getBoolean(THREAD_LEVEL_IOSTATISTICS_ENABLED,
THREAD_LEVEL_IOSTATISTICS_ENABLED_DEFAULT);
configuration.getBoolean(IOSTATISTICS_THREAD_LEVEL_ENABLED,
IOSTATISTICS_THREAD_LEVEL_ENABLED_DEFAULT);
}

/**
* Static probe to check if the thread-level IO statistics enabled.
*
* @return if the thread-level IO statistics enabled.
*/
public static boolean isIOStatisticsThreadLevelEnabled() {
return isThreadIOStatsEnabled;
}

/**
Expand Down
2 changes: 2 additions & 0 deletions ...-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AIOStatisticsContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,8 @@ public void testS3AInputStreamIOStatisticsContext()
* @return thread context
*/
private static IOStatisticsContext getAndResetThreadStatisticsContext() {
assertTrue("thread-level IOStatistics should be enabled by default",
IOStatisticsContext.enabled());
IOStatisticsContext context =
IOStatisticsContext.getCurrentIOStatisticsContext();
context.reset();
Expand Down
6 changes: 6 additions & 0 deletions hadoop-tools/hadoop-aws/src/test/resources/core-site.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,12 @@
<value>true</value>
</property>

<!-- Enable IOStatisticsContext support for Thread level. -->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mixed feelings here. hive team and others who serialize configs don't like us overfilling the xml file with defaults

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They might not have issue with test resource xml changes right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no issues there at all

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets go with it

<property>
<name>fs.iostatistics.thread.level.enabled</name>
<value>true</value>
</property>

<!--
To run these tests.

Expand Down