steveloughran commented on code in PR #5144:
URL: https://github.com/apache/hadoop/pull/5144#discussion_r1043226728
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java:
##########
@@ -459,6 +458,13 @@ public void initialize(URI name, Configuration
originalConf)
AuditSpan span = null;
try {
LOG.debug("Initializing S3AFileSystem for {}", bucket);
+ if (LOG.isTraceEnabled()) {
+ // log a full trace for deep diagnostics of where an object is created,
+ // for tracking down memory leak issues.
+ LOG.trace("Filesystem for {} created; fs.s3a.impl.disable.cache = {}",
+ name, originalConf.getBoolean("fs.s3a.impl.disable.cache", false),
+ new RuntimeException(super.toString()));
Review Comment:
we don't throw it, just trace it. it can be anything. what is your
suggestion?
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java:
##########
@@ -3999,22 +4005,18 @@ public void close() throws IOException {
}
isClosed = true;
LOG.debug("Filesystem {} is closed", uri);
- if (getConf() != null) {
- String iostatisticsLoggingLevel =
- getConf().getTrimmed(IOSTATISTICS_LOGGING_LEVEL,
- IOSTATISTICS_LOGGING_LEVEL_DEFAULT);
- logIOStatisticsAtLevel(LOG, iostatisticsLoggingLevel, getIOStatistics());
- }
try {
super.close();
} finally {
stopAllServices();
- }
- // Log IOStatistics at debug.
- if (LOG.isDebugEnabled()) {
- // robust extract and convert to string
- LOG.debug("Statistics for {}: {}", uri,
- IOStatisticsLogging.ioStatisticsToPrettyString(getIOStatistics()));
+ // log IO statistics, including of any file deletion during
Review Comment:
it means "including iostatistics of any file deletion..." so IMO it's valid
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java:
##########
@@ -3999,22 +4005,18 @@ public void close() throws IOException {
}
isClosed = true;
LOG.debug("Filesystem {} is closed", uri);
- if (getConf() != null) {
- String iostatisticsLoggingLevel =
- getConf().getTrimmed(IOSTATISTICS_LOGGING_LEVEL,
- IOSTATISTICS_LOGGING_LEVEL_DEFAULT);
- logIOStatisticsAtLevel(LOG, iostatisticsLoggingLevel, getIOStatistics());
- }
try {
super.close();
} finally {
stopAllServices();
Review Comment:
not worried there. the system tests verify that you can still call
instrumentation methods safely, it is just unregistered
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AInstrumentation.java:
##########
@@ -257,7 +275,8 @@ private void registerAsMetricsSource(URI name) {
number = ++metricsSourceNameCounter;
}
String msName = METRICS_SOURCE_BASENAME + number;
- metricsSourceName = msName + "-" + name.getHost();
+ String metricsSourceName = msName + "-" + name.getHost();
+ metricsSourceReference = new WeakRefMetricsSource(metricsSourceName, this);
Review Comment:
not using this though, are we?
--
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]
