You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2022/05/24 13:38:59 UTC
[GitHub] [hadoop] mehakmeet opened a new pull request, #4352: HADOOP-17461. Thread-level IOStatistics in S3A
mehakmeet opened a new pull request, #4352:
URL: https://github.com/apache/hadoop/pull/4352
### Description of PR
Adding Thread-level IOStatsitics in hadoop-common and implementing it in S3A Streams.
### How was this patch tested?
Region: ap-south-1
`mvn clean verify -Dparallel-tests -DtestsThreadCount=4 -Dscale`
All tests ran fine.
### For code changes:
- [X] Does the title or this PR starts with the corresponding JIRA issue id (e.g. 'HADOOP-17799. Your PR title ...')?
- [X] Object storage: have the integration tests been executed and the endpoint declared according to the connector-specific documentation?
- [X] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [X] If applicable, have you updated the `LICENSE`, `LICENSE-binary`, `NOTICE-binary` files?
--
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
[GitHub] [hadoop] hadoop-yetus commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1164284637
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 2m 18s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 4 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 15m 10s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 26m 24s | | trunk passed |
| +1 :green_heart: | compile | 23m 45s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | compile | 21m 44s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 4m 5s | | trunk passed |
| +1 :green_heart: | mvnsite | 3m 25s | | trunk passed |
| +1 :green_heart: | javadoc | 2m 31s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 2m 16s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 5m 2s | | trunk passed |
| +1 :green_heart: | shadedclient | 22m 54s | | branch has no errors when building and testing our client artifacts. |
| -0 :warning: | patch | 23m 22s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 32s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 54s | | the patch passed |
| +1 :green_heart: | compile | 24m 42s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javac | 24m 42s | | the patch passed |
| +1 :green_heart: | compile | 21m 7s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 21m 7s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 4m 24s | | the patch passed |
| +1 :green_heart: | mvnsite | 3m 27s | | the patch passed |
| +1 :green_heart: | javadoc | 2m 22s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| -1 :x: | javadoc | 1m 0s | [/results-javadoc-javadoc-hadoop-tools_hadoop-aws-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/5/artifact/out/results-javadoc-javadoc-hadoop-tools_hadoop-aws-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.txt) | hadoop-tools_hadoop-aws-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 generated 2 new + 37 unchanged - 0 fixed = 39 total (was 37) |
| +1 :green_heart: | spotbugs | 4m 56s | | the patch passed |
| +1 :green_heart: | shadedclient | 23m 6s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 19m 5s | | hadoop-common in the patch passed. |
| +1 :green_heart: | unit | 2m 56s | | hadoop-aws in the patch passed. |
| +1 :green_heart: | asflicense | 1m 27s | | The patch does not generate ASF License warnings. |
| | | 246m 42s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/5/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/4352 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 4b91d0b6f8a6 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / bcb4c253b695a1f84f829044a006c491ee3d359d |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/5/testReport/ |
| Max. process+thread count | 2841 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/5/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hadoop] steveloughran commented on a diff in pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
steveloughran commented on code in PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#discussion_r920337912
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContextImpl.java:
##########
@@ -0,0 +1,224 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.statistics.impl;
+
+import java.lang.ref.WeakReference;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.impl.WeakReferenceThreadMap;
+import org.apache.hadoop.fs.statistics.IOStatistics;
+import org.apache.hadoop.fs.statistics.IOStatisticsAggregator;
+import org.apache.hadoop.fs.statistics.IOStatisticsSnapshot;
+
+import static org.apache.hadoop.fs.CommonConfigurationKeys.THREAD_LEVEL_IOSTATISTICS_ENABLED;
+import static org.apache.hadoop.fs.CommonConfigurationKeys.THREAD_LEVEL_IOSTATISTICS_ENABLED_DEFAULT;
+
+/**
+ * Implementing the IOStatisticsContext.
+ *
+ * A Context defined for IOStatistics collection per thread which captures
+ * each worker thread's work in FS streams and stores it in the form of
+ * IOStatisticsSnapshot if thread level aggregation is enabled else returning
+ * an instance of EmptyIOStatisticsStore for the FS. An active instance of
+ * the IOStatisticsContext can be used to collect the statistics.
+ *
+ * For the current thread the IOStatisticsSnapshot can be used as a way to
+ * move the IOStatistics data between applications using the Serializable
+ * nature of the class.
+ */
+public class IOStatisticsContextImpl implements IOStatisticsContext {
+ private static final Logger LOG =
+ LoggerFactory.getLogger(IOStatisticsContextImpl.class);
+ private static final boolean IS_THREAD_IOSTATS_ENABLED;
+
+ /**
+ * Active IOStatistics Context containing different worker thread's
+ * statistics. Weak Reference so that it gets cleaned up during GC and we
+ * avoid any memory leak issues due to long lived references.
+ */
+ private static final WeakReferenceThreadMap<IOStatisticsContext>
+ ACTIVE_IOSTATS_CONTEXT =
+ new WeakReferenceThreadMap<>(IOStatisticsContextImpl::createNewInstance,
+ IOStatisticsContextImpl::referenceLostContext
+ );
+
+ /**
+ * Collecting IOStatistics per thread.
+ */
+ private final WeakReferenceThreadMap<IOStatisticsSnapshot>
+ threadLevelIOStatisticsMap =
+ new WeakReferenceThreadMap<>(this::getIOStatisticsSnapshotFactory,
+ this::referenceLost);
+
+ static {
+ // Work out if the current context has thread level IOStatistics enabled.
+ final Configuration configuration = new Configuration();
+ IS_THREAD_IOSTATS_ENABLED =
+ configuration.getBoolean(THREAD_LEVEL_IOSTATISTICS_ENABLED,
+ THREAD_LEVEL_IOSTATISTICS_ENABLED_DEFAULT);
+ }
+
+ /**
+ * Creating a new IOStatisticsContext instance for a FS to be used. If
Review Comment:
cropped sentence at end
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContextImpl.java:
##########
@@ -0,0 +1,224 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.statistics.impl;
+
+import java.lang.ref.WeakReference;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.impl.WeakReferenceThreadMap;
+import org.apache.hadoop.fs.statistics.IOStatistics;
+import org.apache.hadoop.fs.statistics.IOStatisticsAggregator;
+import org.apache.hadoop.fs.statistics.IOStatisticsSnapshot;
+
+import static org.apache.hadoop.fs.CommonConfigurationKeys.THREAD_LEVEL_IOSTATISTICS_ENABLED;
+import static org.apache.hadoop.fs.CommonConfigurationKeys.THREAD_LEVEL_IOSTATISTICS_ENABLED_DEFAULT;
+
+/**
+ * Implementing the IOStatisticsContext.
+ *
+ * A Context defined for IOStatistics collection per thread which captures
+ * each worker thread's work in FS streams and stores it in the form of
+ * IOStatisticsSnapshot if thread level aggregation is enabled else returning
+ * an instance of EmptyIOStatisticsStore for the FS. An active instance of
+ * the IOStatisticsContext can be used to collect the statistics.
+ *
+ * For the current thread the IOStatisticsSnapshot can be used as a way to
+ * move the IOStatistics data between applications using the Serializable
+ * nature of the class.
+ */
+public class IOStatisticsContextImpl implements IOStatisticsContext {
+ private static final Logger LOG =
+ LoggerFactory.getLogger(IOStatisticsContextImpl.class);
+ private static final boolean IS_THREAD_IOSTATS_ENABLED;
+
+ /**
+ * Active IOStatistics Context containing different worker thread's
+ * statistics. Weak Reference so that it gets cleaned up during GC and we
+ * avoid any memory leak issues due to long lived references.
+ */
+ private static final WeakReferenceThreadMap<IOStatisticsContext>
+ ACTIVE_IOSTATS_CONTEXT =
+ new WeakReferenceThreadMap<>(IOStatisticsContextImpl::createNewInstance,
+ IOStatisticsContextImpl::referenceLostContext
+ );
+
+ /**
+ * Collecting IOStatistics per thread.
+ */
+ private final WeakReferenceThreadMap<IOStatisticsSnapshot>
+ threadLevelIOStatisticsMap =
+ new WeakReferenceThreadMap<>(this::getIOStatisticsSnapshotFactory,
+ this::referenceLost);
+
+ static {
+ // Work out if the current context has thread level IOStatistics enabled.
+ final Configuration configuration = new Configuration();
+ IS_THREAD_IOSTATS_ENABLED =
+ configuration.getBoolean(THREAD_LEVEL_IOSTATISTICS_ENABLED,
+ THREAD_LEVEL_IOSTATISTICS_ENABLED_DEFAULT);
+ }
+
+ /**
+ * Creating a new IOStatisticsContext instance for a FS to be used. If
+ *
+ * @param key Thread ID that represents which thread the context belongs to.
+ * @return an instance of IOStatisticsContext.
+ */
+ private static IOStatisticsContext createNewInstance(Long key) {
+ if (!IS_THREAD_IOSTATS_ENABLED) {
+ return new EmptyIOStatisticsContextImpl();
+ }
+ return new IOStatisticsContextImpl();
+ }
+
+ /**
+ * A Method to act as an IOStatisticsSnapshot factory, in a
+ * WeakReferenceThreadMap.
+ *
+ * @param key ThreadID.
+ * @return an Instance of IOStatisticsSnapshot.
+ */
+ private IOStatisticsSnapshot getIOStatisticsSnapshotFactory(Long key) {
+ return new IOStatisticsSnapshot();
+ }
+
+ /**
+ * In case of reference loss.
+ *
+ * @param key ThreadID.
+ */
+ private void referenceLost(Long key) {
+ LOG.debug("Reference lost for threadID: {}", key);
+ }
+
+ /**
+ * In case of reference loss for IOStatisticsContext.
+ *
+ * @param key ThreadID.
+ */
+ private static void referenceLostContext(Long key) {
+ LOG.debug("Reference lost for threadID for the context: {}", key);
+ }
+
+ /**
+ * Get the current thread's IOStatisticsContext instance. If no instance is
+ * present for this thread ID, create one using the factory.
+ *
+ * @return instance of IOStatisticsContext.
+ */
+ @Override
+ public IOStatisticsContext getCurrentIOStatisticsContext() {
+ return ACTIVE_IOSTATS_CONTEXT.getForCurrentThread();
+ }
+
+ /**
+ * A Method to get the IOStatisticsAggregator of the currentThread. This
+ * denotes the aggregated IOStatistics per thread.
+ *
+ * @return the instance of IOStatisticsAggregator for the current thread.
+ */
+ @Override
+ public IOStatisticsAggregator getThreadIOStatisticsAggregator() {
+ // If the current Thread ID already have an aggregator assigned, return
+ // that.
+ boolean isThreadIOStatsPresent =
Review Comment:
use the context map and ask it for its aggregator
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/impl/WeakReferenceThreadMap.java:
##########
@@ -48,7 +49,15 @@ public long currentThreadId() {
}
public V setForCurrentThread(V newVal) {
- return put(currentThreadId(), newVal);
+ long id = currentThreadId();
Review Comment:
do we need a precondition on a null value here?
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContext.java:
##########
@@ -20,22 +20,28 @@
import org.apache.hadoop.fs.statistics.IOStatisticsAggregator;
import org.apache.hadoop.fs.statistics.IOStatisticsSnapshot;
+import org.apache.hadoop.fs.statistics.IOStatisticsSource;
/**
* An interface defined to capture thread-level IOStatistics by using per
- * thread context consisting of IOStatisticsSnapshot thread map for each
- * worker thread.
- * EmptyIOStatisticsSource is returned as an aggregator if this feature is
- * disabled, resulting in a no-op in aggregation.
+ * thread context.
+ * <p>
+ * The aggregator should be collected in their constructor by statistics-generating
+ * classes to obtain the aggregator to update <i>across all threads</i>.
+ * <p>
+ * The {@link #snapshot()} call creates a snapshot of the statistics;
+ * <p>
+ * The {@link #reset()} call resets the statistics in the current thread so
+ * that later snapshots will get the incremental data.
*/
-public interface IOStatisticsContext {
+public interface IOStatisticsContext extends IOStatisticsSource {
/**
- * Get the current thread's IOStatisticsContext.
+ * Get the IOStatisticsAggregator for the current thread.
Review Comment:
"for the context"
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContext.java:
##########
@@ -20,22 +20,28 @@
import org.apache.hadoop.fs.statistics.IOStatisticsAggregator;
import org.apache.hadoop.fs.statistics.IOStatisticsSnapshot;
+import org.apache.hadoop.fs.statistics.IOStatisticsSource;
/**
* An interface defined to capture thread-level IOStatistics by using per
- * thread context consisting of IOStatisticsSnapshot thread map for each
- * worker thread.
- * EmptyIOStatisticsSource is returned as an aggregator if this feature is
- * disabled, resulting in a no-op in aggregation.
+ * thread context.
+ * <p>
+ * The aggregator should be collected in their constructor by statistics-generating
+ * classes to obtain the aggregator to update <i>across all threads</i>.
+ * <p>
+ * The {@link #snapshot()} call creates a snapshot of the statistics;
+ * <p>
+ * The {@link #reset()} call resets the statistics in the current thread so
+ * that later snapshots will get the incremental data.
*/
-public interface IOStatisticsContext {
+public interface IOStatisticsContext extends IOStatisticsSource {
/**
- * Get the current thread's IOStatisticsContext.
+ * Get the IOStatisticsAggregator for the current thread.
*
- * @return instance of IOStatisticsContext for the current thread.
+ * @return return the aggregator for current thread.
*/
- IOStatisticsContext getCurrentIOStatisticsContext();
+ IOStatisticsAggregator getAggregator();
/**
* Capture the snapshot of current thread's IOStatistics.
Review Comment:
"of the context's"
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContextImpl.java:
##########
@@ -0,0 +1,224 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.statistics.impl;
+
+import java.lang.ref.WeakReference;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.impl.WeakReferenceThreadMap;
+import org.apache.hadoop.fs.statistics.IOStatistics;
+import org.apache.hadoop.fs.statistics.IOStatisticsAggregator;
+import org.apache.hadoop.fs.statistics.IOStatisticsSnapshot;
+
+import static org.apache.hadoop.fs.CommonConfigurationKeys.THREAD_LEVEL_IOSTATISTICS_ENABLED;
+import static org.apache.hadoop.fs.CommonConfigurationKeys.THREAD_LEVEL_IOSTATISTICS_ENABLED_DEFAULT;
+
+/**
+ * Implementing the IOStatisticsContext.
+ *
+ * A Context defined for IOStatistics collection per thread which captures
+ * each worker thread's work in FS streams and stores it in the form of
+ * IOStatisticsSnapshot if thread level aggregation is enabled else returning
+ * an instance of EmptyIOStatisticsStore for the FS. An active instance of
+ * the IOStatisticsContext can be used to collect the statistics.
+ *
+ * For the current thread the IOStatisticsSnapshot can be used as a way to
+ * move the IOStatistics data between applications using the Serializable
+ * nature of the class.
+ */
+public class IOStatisticsContextImpl implements IOStatisticsContext {
+ private static final Logger LOG =
+ LoggerFactory.getLogger(IOStatisticsContextImpl.class);
+ private static final boolean IS_THREAD_IOSTATS_ENABLED;
+
+ /**
+ * Active IOStatistics Context containing different worker thread's
+ * statistics. Weak Reference so that it gets cleaned up during GC and we
+ * avoid any memory leak issues due to long lived references.
+ */
+ private static final WeakReferenceThreadMap<IOStatisticsContext>
+ ACTIVE_IOSTATS_CONTEXT =
+ new WeakReferenceThreadMap<>(IOStatisticsContextImpl::createNewInstance,
+ IOStatisticsContextImpl::referenceLostContext
+ );
+
+ /**
+ * Collecting IOStatistics per thread.
+ */
+ private final WeakReferenceThreadMap<IOStatisticsSnapshot>
Review Comment:
the context map can support this; just look it up and call getThreadIOStatisticsAggregator() on it
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AIOStatisticsContext.java:
##########
@@ -66,20 +66,21 @@ public void setUp() throws Exception {
*/
@BeforeClass
public static void beforeClass() throws Exception {
- currentIOStatisticsContext().resetThreadIOStatisticsForCurrentThread();
// Do some work in constructor thread.
S3AFileSystem fs = new S3AFileSystem();
Configuration conf = new Configuration();
fs.initialize(new URI(conf.get(TEST_FS_S3A_NAME)), conf);
Path path = new Path("testConstructor");
+ IOStatisticsContextImpl ioStatisticsContext = fs.getIoStatisticsContext();
Review Comment:
should be IOStatisticsContext
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java:
##########
@@ -5231,4 +5234,13 @@ public RequestFactory getRequestFactory() {
public boolean isCSEEnabled() {
return isCSEEnabled;
}
+
+ /**
+ * Get the FileSystem's IOStatisticsContext.
+ * @return the instance of IOStatisticsContextImpl.
+ */
+ @VisibleForTesting
+ public IOStatisticsContextImpl getIoStatisticsContext() {
Review Comment:
don't cast. if tests want to cast it, and they know it is safe, they can do so
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/commit/AbstractCommitITest.java:
##########
@@ -66,6 +70,12 @@ public abstract class AbstractCommitITest extends AbstractS3ATestBase {
private static final Logger LOG =
LoggerFactory.getLogger(AbstractCommitITest.class);
+ /**
+ * Job statistics accrued across all test cases
Review Comment:
needs a .
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContext.java:
##########
@@ -20,22 +20,28 @@
import org.apache.hadoop.fs.statistics.IOStatisticsAggregator;
import org.apache.hadoop.fs.statistics.IOStatisticsSnapshot;
+import org.apache.hadoop.fs.statistics.IOStatisticsSource;
/**
* An interface defined to capture thread-level IOStatistics by using per
- * thread context consisting of IOStatisticsSnapshot thread map for each
- * worker thread.
- * EmptyIOStatisticsSource is returned as an aggregator if this feature is
- * disabled, resulting in a no-op in aggregation.
+ * thread context.
+ * <p>
+ * The aggregator should be collected in their constructor by statistics-generating
+ * classes to obtain the aggregator to update <i>across all threads</i>.
+ * <p>
+ * The {@link #snapshot()} call creates a snapshot of the statistics;
+ * <p>
+ * The {@link #reset()} call resets the statistics in the current thread so
+ * that later snapshots will get the incremental data.
*/
-public interface IOStatisticsContext {
+public interface IOStatisticsContext extends IOStatisticsSource {
/**
- * Get the current thread's IOStatisticsContext.
+ * Get the IOStatisticsAggregator for the current thread.
*
- * @return instance of IOStatisticsContext for the current thread.
+ * @return return the aggregator for current thread.
Review Comment:
"for the context"
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContextImpl.java:
##########
@@ -0,0 +1,224 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.statistics.impl;
+
+import java.lang.ref.WeakReference;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.impl.WeakReferenceThreadMap;
+import org.apache.hadoop.fs.statistics.IOStatistics;
+import org.apache.hadoop.fs.statistics.IOStatisticsAggregator;
+import org.apache.hadoop.fs.statistics.IOStatisticsSnapshot;
+
+import static org.apache.hadoop.fs.CommonConfigurationKeys.THREAD_LEVEL_IOSTATISTICS_ENABLED;
+import static org.apache.hadoop.fs.CommonConfigurationKeys.THREAD_LEVEL_IOSTATISTICS_ENABLED_DEFAULT;
+
+/**
+ * Implementing the IOStatisticsContext.
+ *
+ * A Context defined for IOStatistics collection per thread which captures
+ * each worker thread's work in FS streams and stores it in the form of
+ * IOStatisticsSnapshot if thread level aggregation is enabled else returning
+ * an instance of EmptyIOStatisticsStore for the FS. An active instance of
+ * the IOStatisticsContext can be used to collect the statistics.
+ *
+ * For the current thread the IOStatisticsSnapshot can be used as a way to
+ * move the IOStatistics data between applications using the Serializable
+ * nature of the class.
+ */
+public class IOStatisticsContextImpl implements IOStatisticsContext {
+ private static final Logger LOG =
+ LoggerFactory.getLogger(IOStatisticsContextImpl.class);
+ private static final boolean IS_THREAD_IOSTATS_ENABLED;
+
+ /**
+ * Active IOStatistics Context containing different worker thread's
+ * statistics. Weak Reference so that it gets cleaned up during GC and we
+ * avoid any memory leak issues due to long lived references.
+ */
+ private static final WeakReferenceThreadMap<IOStatisticsContext>
+ ACTIVE_IOSTATS_CONTEXT =
+ new WeakReferenceThreadMap<>(IOStatisticsContextImpl::createNewInstance,
+ IOStatisticsContextImpl::referenceLostContext
+ );
+
+ /**
+ * Collecting IOStatistics per thread.
+ */
+ private final WeakReferenceThreadMap<IOStatisticsSnapshot>
+ threadLevelIOStatisticsMap =
+ new WeakReferenceThreadMap<>(this::getIOStatisticsSnapshotFactory,
+ this::referenceLost);
+
+ static {
+ // Work out if the current context has thread level IOStatistics enabled.
+ final Configuration configuration = new Configuration();
+ IS_THREAD_IOSTATS_ENABLED =
+ configuration.getBoolean(THREAD_LEVEL_IOSTATISTICS_ENABLED,
+ THREAD_LEVEL_IOSTATISTICS_ENABLED_DEFAULT);
+ }
+
+ /**
+ * Creating a new IOStatisticsContext instance for a FS to be used. If
+ *
+ * @param key Thread ID that represents which thread the context belongs to.
+ * @return an instance of IOStatisticsContext.
+ */
+ private static IOStatisticsContext createNewInstance(Long key) {
+ if (!IS_THREAD_IOSTATS_ENABLED) {
+ return new EmptyIOStatisticsContextImpl();
+ }
+ return new IOStatisticsContextImpl();
+ }
+
+ /**
+ * A Method to act as an IOStatisticsSnapshot factory, in a
+ * WeakReferenceThreadMap.
+ *
+ * @param key ThreadID.
+ * @return an Instance of IOStatisticsSnapshot.
+ */
+ private IOStatisticsSnapshot getIOStatisticsSnapshotFactory(Long key) {
+ return new IOStatisticsSnapshot();
+ }
+
+ /**
+ * In case of reference loss.
+ *
+ * @param key ThreadID.
+ */
+ private void referenceLost(Long key) {
+ LOG.debug("Reference lost for threadID: {}", key);
+ }
+
+ /**
+ * In case of reference loss for IOStatisticsContext.
+ *
+ * @param key ThreadID.
+ */
+ private static void referenceLostContext(Long key) {
+ LOG.debug("Reference lost for threadID for the context: {}", key);
+ }
+
+ /**
+ * Get the current thread's IOStatisticsContext instance. If no instance is
+ * present for this thread ID, create one using the factory.
+ *
+ * @return instance of IOStatisticsContext.
+ */
+ @Override
+ public IOStatisticsContext getCurrentIOStatisticsContext() {
+ return ACTIVE_IOSTATS_CONTEXT.getForCurrentThread();
+ }
+
+ /**
+ * A Method to get the IOStatisticsAggregator of the currentThread. This
+ * denotes the aggregated IOStatistics per thread.
+ *
+ * @return the instance of IOStatisticsAggregator for the current thread.
+ */
+ @Override
+ public IOStatisticsAggregator getThreadIOStatisticsAggregator() {
+ // If the current Thread ID already have an aggregator assigned, return
+ // that.
+ boolean isThreadIOStatsPresent =
+ threadLevelIOStatisticsMap.containsKey(Thread.currentThread().getId());
+ if (isThreadIOStatsPresent) {
+ return threadLevelIOStatisticsMap.getForCurrentThread();
+ }
+ LOG.debug("No thread iostats present creating it for :{}",
+ Thread.currentThread().getId());
+ // If no aggregator is defined to the thread ID, create one and assign it.
+ IOStatisticsSnapshot ioStatisticsSnapshot = new IOStatisticsSnapshot();
+ setThreadIOStatistics(ioStatisticsSnapshot);
+ return ioStatisticsSnapshot;
+ }
+
+ /**
+ * Set the IOStatisticsSnapshot for the current context for a specific
+ * thread.
+ *
+ * @param ioStatisticsSnapshot IOStatisticsAggregator instance for the
+ * current thread.
+ */
+ public void setThreadIOStatistics(
+ IOStatisticsSnapshot ioStatisticsSnapshot) {
+ threadLevelIOStatisticsMap.setForCurrentThread(ioStatisticsSnapshot);
+ }
+
+ /**
+ * Returns a snapshot of the current thread's IOStatistics.
+ *
+ * @return IOStatisticsSnapshot of the current thread.
+ */
+ @Override
+ public IOStatisticsSnapshot snapshot() {
+ if (threadLevelIOStatisticsMap.containsKey(getCurrentThreadID())) {
+ return threadLevelIOStatisticsMap.get(getCurrentThreadID());
+ }
+ return new IOStatisticsSnapshot();
+ }
+
+ /**
+ * Reset the thread IOStatistics for current thread.
+ */
+ @Override
+ public void reset() {
+ WeakReference<IOStatisticsSnapshot> ioStatisticsSnapshotRef =
+ threadLevelIOStatisticsMap.lookup(getCurrentThreadID());
+ if (ioStatisticsSnapshotRef != null) {
+ IOStatisticsSnapshot ioStatisticsSnapshot = ioStatisticsSnapshotRef.get();
+ // Get the snapshot for the current thread ID and clear it.
+ if(ioStatisticsSnapshot != null) {
+ ioStatisticsSnapshot.clear();
+ }
+ }
+ }
+
+ /**
+ * Get the current thread's ID.
+ * @return long value of the thread ID.
+ */
+ private Long getCurrentThreadID() {
+ return Thread.currentThread().getId();
+ }
+
+ /**
+ * Get thread ID specific IOStatistics values.
+ *
+ * @param testThreadId thread ID.
+ * @return IOStatistics instance.
+ */
+ @VisibleForTesting
+ public IOStatistics getThreadSpecificIOStatistics(long testThreadId) {
Review Comment:
tests should also look at the context map; lets them test more
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContext.java:
##########
@@ -20,22 +20,28 @@
import org.apache.hadoop.fs.statistics.IOStatisticsAggregator;
import org.apache.hadoop.fs.statistics.IOStatisticsSnapshot;
+import org.apache.hadoop.fs.statistics.IOStatisticsSource;
/**
* An interface defined to capture thread-level IOStatistics by using per
- * thread context consisting of IOStatisticsSnapshot thread map for each
- * worker thread.
- * EmptyIOStatisticsSource is returned as an aggregator if this feature is
- * disabled, resulting in a no-op in aggregation.
+ * thread context.
+ * <p>
+ * The aggregator should be collected in their constructor by statistics-generating
+ * classes to obtain the aggregator to update <i>across all threads</i>.
+ * <p>
+ * The {@link #snapshot()} call creates a snapshot of the statistics;
+ * <p>
+ * The {@link #reset()} call resets the statistics in the current thread so
+ * that later snapshots will get the incremental data.
*/
-public interface IOStatisticsContext {
+public interface IOStatisticsContext extends IOStatisticsSource {
Review Comment:
all the javadocs saying "the current thread" except for the static method to do the lookup MUST be changed to just say ""the context"
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/impl/WeakReferenceThreadMap.java:
##########
@@ -48,7 +49,15 @@ public long currentThreadId() {
}
public V setForCurrentThread(V newVal) {
- return put(currentThreadId(), newVal);
+ long id = currentThreadId();
+
+ // if the same object is already in the map, just return it.
+ WeakReference<V> ref = lookup(id);
+ if (ref != null && ref.get() == newVal) {
Review Comment:
if there's been a GC ref.get could be null. now, as we are doing == and not Object.equals that is safe.
we should mention that as a comment
--
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
[GitHub] [hadoop] steveloughran commented on a diff in pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
steveloughran commented on code in PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#discussion_r926485594
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContextIntegration.java:
##########
@@ -0,0 +1,151 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.statistics.impl;
+
+import java.lang.ref.WeakReference;
+import java.util.concurrent.atomic.AtomicLong;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.conf.Configuration;
+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;
+
+/**
+ * A Utility class for IOStatisticsContext, which helps in creating and
+ * getting the current active context. Static methods in this class allows to
+ * get the current context to start aggregating the IOStatistics.
+ *
+ * Static initializer is used to work out if the feature to collect
+ * thread-level IOStatistics is enabled or not and the corresponding
+ * implementation class is called for it.
+ *
+ * Weak Reference thread map to be used to keep track of different context's
+ * to avoid long-lived memory leakages as these references would be cleaned
+ * up at GC.
+ */
+public final class IOStatisticsContextIntegration {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(IOStatisticsContextIntegration.class);
+
+ /**
+ * Is thread-level IO Statistics enabled?
+ */
+ private static final boolean IS_THREAD_IOSTATS_ENABLED;
+
+ /**
+ * ID for next instance to create.
+ */
+ public static final AtomicLong INSTANCE_ID = new AtomicLong(1);
+
+ /**
+ * Active IOStatistics Context containing different worker thread's
+ * statistics. Weak Reference so that it gets cleaned up during GC and we
+ * avoid any memory leak issues due to long lived references.
+ */
+ private static final WeakReferenceThreadMap<IOStatisticsContext>
+ ACTIVE_IOSTATS_CONTEXT =
+ new WeakReferenceThreadMap<>(
+ IOStatisticsContextIntegration::createNewInstance,
+ IOStatisticsContextIntegration::referenceLostContext
+ );
+
+ static {
+ // Work out if the current context has thread level IOStatistics enabled.
+ final Configuration configuration = new Configuration();
+ IS_THREAD_IOSTATS_ENABLED =
+ configuration.getBoolean(THREAD_LEVEL_IOSTATISTICS_ENABLED,
+ THREAD_LEVEL_IOSTATISTICS_ENABLED_DEFAULT);
+ }
+
+ /**
+ * Private constructor for a utility class to be used in IOStatisticsContext.
+ */
+ private IOStatisticsContextIntegration() {}
+
+ /**
+ * Creating a new IOStatisticsContext instance for a FS to be used.
+ * @param key Thread ID that represents which thread the context belongs to.
+ * @return an instance of IOStatisticsContext.
+ */
+ private static IOStatisticsContext createNewInstance(Long key) {
+ return new IOStatisticsContextImpl(key, INSTANCE_ID.getAndIncrement());
+ }
+
+ /**
+ * In case of reference loss for IOStatisticsContext.
+ * @param key ThreadID.
+ */
+ private static void referenceLostContext(Long key) {
+ LOG.debug("Reference lost for threadID for the context: {}", key);
+ }
+
+ /**
+ * Get the current thread's IOStatisticsContext instance. If no instance is
+ * present for this thread ID, create one using the factory.
+ * @return instance of IOStatisticsContext.
+ */
+ public static IOStatisticsContext getCurrentIOStatisticsContext() {
+ return IS_THREAD_IOSTATS_ENABLED
+ ? ACTIVE_IOSTATS_CONTEXT.getForCurrentThread()
+ : EmptyIOStatisticsContextImpl.getInstance();
+ }
+
+ /**
+ * Set the IOStatisticsContext for the current thread.
+ * @param statisticsContext IOStatistics context instance for the
+ * current thread.
+ */
+ public static void setThreadIOStatisticsContext(
+ IOStatisticsContext statisticsContext) {
+ if (IS_THREAD_IOSTATS_ENABLED &&
+ ACTIVE_IOSTATS_CONTEXT.getForCurrentThread() != statisticsContext) {
+ ACTIVE_IOSTATS_CONTEXT.setForCurrentThread(statisticsContext);
Review Comment:
if statisticsContext is null, call remove() instead; lets us use it as the resetter
--
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
[GitHub] [hadoop] mehakmeet commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
mehakmeet commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1190642189
added your latest commit + Testing for full suite done for both enabled and disabled `fs.s3a.committer.experimental.collect.iostatistics` 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
[GitHub] [hadoop] mehakmeet commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
mehakmeet commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1186772913
Cherry-picked S3A committers integration and lining up with spark build. Some javadocs would need modification, adding that in a commit. Need to see how to test this on a spark build locally, since spark might not be using the correct hadoop version iirc, then we could get this in
--
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
[GitHub] [hadoop] mehakmeet commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
mehakmeet commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1162654911
Had to force push to resolve conflicts.
Some changes in the latest commit:
- Moved thread context initialization and config for enabling in hadoop-common.
- Weak ref thread map for iostats context, have a doubt here regarding this whether having context and the iostats as weak ref would cause any issues, since both are now mapped to thread IDs. Thinking in terms of one context per s3afs with iostats per thread.
- Added new test
- Disabling iostats returns EmptyIOstatisticsStore instance with no-op aggregation.
--
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
[GitHub] [hadoop] mehakmeet commented on a diff in pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
mehakmeet commented on code in PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#discussion_r898719209
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/test/LambdaTestUtils.java:
##########
@@ -65,6 +66,15 @@ private LambdaTestUtils() {
*/
public static final String NULL_RESULT = "(null)";
+ /**
+ * Atomic references to be used to re-throw an Exception or an ASE
+ * caught inside a lambda function.
+ */
+ public static final AtomicReference<Exception> futureExcp =
Review Comment:
ah I see, since we wanted the atomic ref to these exceptions for lambda functions, I thought this might be the best place for these, you reckon we move this out of here?
--
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
[GitHub] [hadoop] hadoop-yetus commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1180254837
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 41s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 4 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 14m 46s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 27m 30s | | trunk passed |
| +1 :green_heart: | compile | 23m 57s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | compile | 20m 40s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 4m 23s | | trunk passed |
| +1 :green_heart: | mvnsite | 3m 44s | | trunk passed |
| +1 :green_heart: | javadoc | 2m 59s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 2m 41s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 5m 0s | | trunk passed |
| +1 :green_heart: | shadedclient | 22m 0s | | branch has no errors when building and testing our client artifacts. |
| -0 :warning: | patch | 22m 32s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 31s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 47s | | the patch passed |
| +1 :green_heart: | compile | 22m 13s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javac | 22m 13s | | the patch passed |
| +1 :green_heart: | compile | 20m 26s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 20m 26s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 4m 13s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/6/artifact/out/results-checkstyle-root.txt) | root: The patch generated 1 new + 75 unchanged - 0 fixed = 76 total (was 75) |
| +1 :green_heart: | mvnsite | 3m 44s | | the patch passed |
| +1 :green_heart: | javadoc | 2m 50s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| -1 :x: | javadoc | 1m 21s | [/results-javadoc-javadoc-hadoop-tools_hadoop-aws-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/6/artifact/out/results-javadoc-javadoc-hadoop-tools_hadoop-aws-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.txt) | hadoop-tools_hadoop-aws-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 generated 2 new + 37 unchanged - 0 fixed = 39 total (was 37) |
| +1 :green_heart: | spotbugs | 5m 4s | | the patch passed |
| +1 :green_heart: | shadedclient | 23m 47s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 19m 24s | | hadoop-common in the patch passed. |
| +1 :green_heart: | unit | 3m 10s | | hadoop-aws in the patch passed. |
| +1 :green_heart: | asflicense | 1m 38s | | The patch does not generate ASF License warnings. |
| | | 245m 43s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/6/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/4352 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 6a018cb07b95 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 232d6b9017a816b2c41ae9a2207ec1c72258fa13 |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/6/testReport/ |
| Max. process+thread count | 1263 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/6/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hadoop] steveloughran commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
steveloughran commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1192543668
good point.
how about adding a static method to enable it in the integration class?
disabling it would be harder...what if contexts had already been generated? easier to only allow the caller to go from off to on.
also, had an idea:
add a stream capability `fs.capability.iocontext.supported` for streams which update iocontext.
then add asserts to the test to verify this
1. lines up for making the tests contract tests
2. if the prefetching stream isn't (yet) context aware, we will know why the tests are failing
--
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
[GitHub] [hadoop] steveloughran commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
steveloughran commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1159135030
we need a single IOStatisticsContext for all FS instances, so that a task reading from one fs and writing to another would have the stats updated from both actions.
And it needs to be in hadoop-common, just like the common audit context, so that code can compile and link against it even without having hadoop-aws or hadoop-azure on the classpath.
Making the context a weak ref map ensures that GCs will trigger cleanup. This would lose stats, but not while any stream was active, *or if some code picked up a reference to the thread stats before executing work. That is what I plan to do in spark; we will grab that ref before starting the work, and after it is finished, take a snapshot of it. Oh, and reset the values before work starts -we need that context there 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
[GitHub] [hadoop] mehakmeet commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
mehakmeet commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1181759094
Changed the way streams are accessing the thread IOStatistics, now we would directly get them from the current active context in the stream's constructor rather than pass them around through the builders, as it didn't seem to add anything if we can directly get it due to static nature of the context. Also, made the weakRef as IOstatisticsSnapshot rather than an aggregator and then cast as discussed above.
--
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
[GitHub] [hadoop] hadoop-yetus commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1194187132
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 53s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 6 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 14m 48s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 28m 47s | | trunk passed |
| +1 :green_heart: | compile | 25m 18s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | compile | 21m 53s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 4m 32s | | trunk passed |
| +1 :green_heart: | mvnsite | 3m 13s | | trunk passed |
| +1 :green_heart: | javadoc | 2m 23s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 2m 5s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 4m 39s | | trunk passed |
| +1 :green_heart: | shadedclient | 24m 44s | | branch has no errors when building and testing our client artifacts. |
| -0 :warning: | patch | 25m 11s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 26s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 42s | | the patch passed |
| +1 :green_heart: | compile | 24m 25s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javac | 24m 25s | | the patch passed |
| +1 :green_heart: | compile | 21m 51s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 21m 51s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 4m 24s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/15/artifact/out/results-checkstyle-root.txt) | root: The patch generated 1 new + 77 unchanged - 0 fixed = 78 total (was 77) |
| +1 :green_heart: | mvnsite | 3m 11s | | the patch passed |
| +1 :green_heart: | javadoc | 2m 15s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 2m 8s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 5m 25s | | the patch passed |
| +1 :green_heart: | shadedclient | 27m 1s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 18m 56s | | hadoop-common in the patch passed. |
| +1 :green_heart: | unit | 3m 8s | | hadoop-aws in the patch passed. |
| +1 :green_heart: | asflicense | 1m 18s | | The patch does not generate ASF License warnings. |
| | | 253m 20s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/15/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/4352 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 76bd37db2c7b 4.15.0-175-generic #184-Ubuntu SMP Thu Mar 24 17:48:36 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / cdc9915d9c52644d35057844c101653858ed10eb |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/15/testReport/ |
| Max. process+thread count | 1263 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/15/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hadoop] mehakmeet commented on a diff in pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
mehakmeet commented on code in PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#discussion_r898719852
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AReadOpContext.java:
##########
@@ -77,11 +80,13 @@ public S3AReadOpContext(
Invoker invoker,
@Nullable FileSystem.Statistics stats,
S3AStatisticsContext instrumentation,
- FileStatus dstFileStatus) {
+ FileStatus dstFileStatus,
+ final IOStatisticsContext ioStatisticsContext) {
super(invoker, stats, instrumentation,
dstFileStatus);
this.path = requireNonNull(path);
+ this.ioStatisticsContext = ioStatisticsContext;
Review Comment:
Adding EmptyIOStatisticsContext for the disabled scenario with EmptyIOStatisticsStore as the aggregator which is a no-op.
--
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
[GitHub] [hadoop] hadoop-yetus commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1162865131
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 48s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 4 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 14m 33s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 26m 22s | | trunk passed |
| +1 :green_heart: | compile | 25m 35s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | compile | 22m 14s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 4m 8s | | trunk passed |
| +1 :green_heart: | mvnsite | 3m 1s | | trunk passed |
| +1 :green_heart: | javadoc | 2m 24s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 2m 3s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 4m 53s | | trunk passed |
| +1 :green_heart: | shadedclient | 24m 36s | | branch has no errors when building and testing our client artifacts. |
| -0 :warning: | patch | 25m 5s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 32s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 53s | | the patch passed |
| +1 :green_heart: | compile | 25m 25s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javac | 25m 25s | | the patch passed |
| +1 :green_heart: | compile | 22m 30s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 22m 30s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 4m 6s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/4/artifact/out/results-checkstyle-root.txt) | root: The patch generated 3 new + 74 unchanged - 0 fixed = 77 total (was 74) |
| +1 :green_heart: | mvnsite | 3m 8s | | the patch passed |
| +1 :green_heart: | javadoc | 2m 18s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| -1 :x: | javadoc | 1m 2s | [/results-javadoc-javadoc-hadoop-tools_hadoop-aws-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/4/artifact/out/results-javadoc-javadoc-hadoop-tools_hadoop-aws-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.txt) | hadoop-tools_hadoop-aws-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 generated 2 new + 38 unchanged - 0 fixed = 40 total (was 38) |
| +1 :green_heart: | spotbugs | 4m 48s | | the patch passed |
| +1 :green_heart: | shadedclient | 22m 24s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 18m 43s | | hadoop-common in the patch passed. |
| +1 :green_heart: | unit | 2m 54s | | hadoop-aws in the patch passed. |
| +1 :green_heart: | asflicense | 1m 20s | | The patch does not generate ASF License warnings. |
| | | 247m 29s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/4/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/4352 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux a31c5c724321 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 5badccde12beee84d9fdfec0327346fa1a09f50e |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/4/testReport/ |
| Max. process+thread count | 1263 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/4/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hadoop] mehakmeet commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
mehakmeet commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1192316937
Pushed the changes, makes sense in case of null IOStatisticsContext. There is one more issue, in case we have `fs.thread.level.iostatistics.enabled=false` which means empty counters, hence the test getting failed in the test in this PR only, since it requires thread-level IOstatsitics to be enabled, now we do the `removeBaseAndBucketOverrides(configuration,
THREAD_LEVEL_IOSTATISTICS_ENABLED);
configuration.setBoolean(THREAD_LEVEL_IOSTATISTICS_ENABLED, true);`
But, since the way this is toggled is in a static initializer for the `IOStatisticsContextIntegration` utility class, it does it's calculation before this, and setting it afterwards doesn't have an affect, how do we ensure that this property is enabled for this test always? Reload the class using classLoaders(not sure if this works since I tried using the Thread.currentThread.getClassLoaderContext)?
Also
> Contributed by Mehakmeet Singh
*and Steve Loughran
--
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
[GitHub] [hadoop] hadoop-yetus commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1195733167
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 44s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 1s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 6 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 15m 39s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 25m 30s | | trunk passed |
| +1 :green_heart: | compile | 23m 9s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | compile | 20m 44s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 4m 30s | | trunk passed |
| +1 :green_heart: | mvnsite | 3m 44s | | trunk passed |
| +1 :green_heart: | javadoc | 3m 1s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 2m 42s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 5m 4s | | trunk passed |
| +1 :green_heart: | shadedclient | 22m 11s | | branch has no errors when building and testing our client artifacts. |
| -0 :warning: | patch | 22m 42s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 30s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 46s | | the patch passed |
| +1 :green_heart: | compile | 22m 31s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javac | 22m 31s | | the patch passed |
| +1 :green_heart: | compile | 21m 30s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 21m 30s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 4m 17s | | root: The patch generated 0 new + 97 unchanged - 1 fixed = 97 total (was 98) |
| +1 :green_heart: | mvnsite | 3m 24s | | the patch passed |
| +1 :green_heart: | javadoc | 2m 21s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 2m 7s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 4m 45s | | the patch passed |
| +1 :green_heart: | shadedclient | 23m 30s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 18m 36s | | hadoop-common in the patch passed. |
| +1 :green_heart: | unit | 3m 7s | | hadoop-aws in the patch passed. |
| +1 :green_heart: | asflicense | 1m 26s | | The patch does not generate ASF License warnings. |
| | | 241m 14s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/17/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/4352 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux fca9fdd4589a 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 0db1a76e21be9996a6d3f257630ba1ce0ca8b745 |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/17/testReport/ |
| Max. process+thread count | 1263 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/17/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hadoop] steveloughran commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
steveloughran commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1195331924
(the streams need to cache the aggregator and update in close)
--
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
[GitHub] [hadoop] steveloughran commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
steveloughran commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1195331465
-1 to the last patch. pick up 9dd221dda28 from my PR
--
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
[GitHub] [hadoop] hadoop-yetus commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1138786813
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 53s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 3 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 14m 26s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 26m 38s | | trunk passed |
| -1 :x: | compile | 4m 21s | [/branch-compile-root-jdkPrivateBuild-11.0.15+10-Ubuntu-0ubuntu0.20.04.1.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/2/artifact/out/branch-compile-root-jdkPrivateBuild-11.0.15+10-Ubuntu-0ubuntu0.20.04.1.txt) | root in trunk failed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1. |
| -1 :x: | compile | 3m 32s | [/branch-compile-root-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/2/artifact/out/branch-compile-root-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.txt) | root in trunk failed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07. |
| +1 :green_heart: | checkstyle | 3m 50s | | trunk passed |
| +1 :green_heart: | mvnsite | 2m 26s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 42s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 1m 30s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 4m 4s | | trunk passed |
| +1 :green_heart: | shadedclient | 22m 1s | | branch has no errors when building and testing our client artifacts. |
| -0 :warning: | patch | 22m 29s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 28s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 40s | | the patch passed |
| -1 :x: | compile | 3m 50s | [/patch-compile-root-jdkPrivateBuild-11.0.15+10-Ubuntu-0ubuntu0.20.04.1.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/2/artifact/out/patch-compile-root-jdkPrivateBuild-11.0.15+10-Ubuntu-0ubuntu0.20.04.1.txt) | root in the patch failed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1. |
| -1 :x: | javac | 3m 50s | [/patch-compile-root-jdkPrivateBuild-11.0.15+10-Ubuntu-0ubuntu0.20.04.1.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/2/artifact/out/patch-compile-root-jdkPrivateBuild-11.0.15+10-Ubuntu-0ubuntu0.20.04.1.txt) | root in the patch failed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1. |
| -1 :x: | compile | 3m 25s | [/patch-compile-root-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/2/artifact/out/patch-compile-root-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.txt) | root in the patch failed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07. |
| -1 :x: | javac | 3m 25s | [/patch-compile-root-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/2/artifact/out/patch-compile-root-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.txt) | root in the patch failed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07. |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 3m 22s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/2/artifact/out/results-checkstyle-root.txt) | root: The patch generated 3 new + 6 unchanged - 0 fixed = 9 total (was 6) |
| +1 :green_heart: | mvnsite | 2m 19s | | the patch passed |
| +1 :green_heart: | javadoc | 1m 26s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| -1 :x: | javadoc | 0m 34s | [/results-javadoc-javadoc-hadoop-tools_hadoop-aws-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/2/artifact/out/results-javadoc-javadoc-hadoop-tools_hadoop-aws-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.txt) | hadoop-tools_hadoop-aws-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 generated 2 new + 38 unchanged - 0 fixed = 40 total (was 38) |
| +1 :green_heart: | spotbugs | 4m 4s | | the patch passed |
| +1 :green_heart: | shadedclient | 23m 59s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 19m 3s | | hadoop-common in the patch passed. |
| +1 :green_heart: | unit | 2m 32s | | hadoop-aws in the patch passed. |
| +1 :green_heart: | asflicense | 0m 50s | | The patch does not generate ASF License warnings. |
| | | 156m 24s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/4352 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 246b2439a9b5 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / cf1200bb8256da8dfc3eee6f93d01dbcc1b2b99f |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/2/testReport/ |
| Max. process+thread count | 3149 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/2/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hadoop] steveloughran commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
steveloughran commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1195904127
ok, merged. Can you do a PR/cherrypick into branch-3.3 now?
--
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
[GitHub] [hadoop] steveloughran commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
steveloughran commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1189482342
I now have a branch of spark set up to use this, albeit not production ready
https://github.com/steveloughran/hadoop/tree/s3/HADOOP-17461-iostatisticsContext
* TaskMetrics includes an IOStatisticsSnapshot in its serialized data
* IOStatisticsContext is retrieved and reset at start of read/write work; updated at end.
Based on where the read bytes/write bytes counters were read.
Having done that, I've realised that the rdds and writers are not quite the correct place as the IOStats collect all IO on the thread, and are both the readers and writers will be working on that thread.
The place to do it is actually in the ResultTask, with IOStatisticsSnapshot being one of the accumulators which is sent back. the spark driver would keep the core IOStatisticsSnapshot up to date with results from success and failure tasks.
--
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
[GitHub] [hadoop] steveloughran commented on a diff in pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
steveloughran commented on code in PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#discussion_r926503951
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContextIntegration.java:
##########
@@ -0,0 +1,151 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.statistics.impl;
+
+import java.lang.ref.WeakReference;
+import java.util.concurrent.atomic.AtomicLong;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.conf.Configuration;
+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;
+
+/**
+ * A Utility class for IOStatisticsContext, which helps in creating and
+ * getting the current active context. Static methods in this class allows to
+ * get the current context to start aggregating the IOStatistics.
+ *
+ * Static initializer is used to work out if the feature to collect
+ * thread-level IOStatistics is enabled or not and the corresponding
+ * implementation class is called for it.
+ *
+ * Weak Reference thread map to be used to keep track of different context's
+ * to avoid long-lived memory leakages as these references would be cleaned
+ * up at GC.
+ */
+public final class IOStatisticsContextIntegration {
+
+ private static final Logger LOG =
+ LoggerFactory.getLogger(IOStatisticsContextIntegration.class);
+
+ /**
+ * Is thread-level IO Statistics enabled?
+ */
+ private static final boolean IS_THREAD_IOSTATS_ENABLED;
+
+ /**
+ * ID for next instance to create.
+ */
+ public static final AtomicLong INSTANCE_ID = new AtomicLong(1);
+
+ /**
+ * Active IOStatistics Context containing different worker thread's
+ * statistics. Weak Reference so that it gets cleaned up during GC and we
+ * avoid any memory leak issues due to long lived references.
+ */
+ private static final WeakReferenceThreadMap<IOStatisticsContext>
+ ACTIVE_IOSTATS_CONTEXT =
+ new WeakReferenceThreadMap<>(
+ IOStatisticsContextIntegration::createNewInstance,
+ IOStatisticsContextIntegration::referenceLostContext
+ );
+
+ static {
+ // Work out if the current context has thread level IOStatistics enabled.
+ final Configuration configuration = new Configuration();
+ IS_THREAD_IOSTATS_ENABLED =
+ configuration.getBoolean(THREAD_LEVEL_IOSTATISTICS_ENABLED,
+ THREAD_LEVEL_IOSTATISTICS_ENABLED_DEFAULT);
+ }
+
+ /**
+ * Private constructor for a utility class to be used in IOStatisticsContext.
+ */
+ private IOStatisticsContextIntegration() {}
+
+ /**
+ * Creating a new IOStatisticsContext instance for a FS to be used.
+ * @param key Thread ID that represents which thread the context belongs to.
+ * @return an instance of IOStatisticsContext.
+ */
+ private static IOStatisticsContext createNewInstance(Long key) {
+ return new IOStatisticsContextImpl(key, INSTANCE_ID.getAndIncrement());
+ }
+
+ /**
+ * In case of reference loss for IOStatisticsContext.
+ * @param key ThreadID.
+ */
+ private static void referenceLostContext(Long key) {
+ LOG.debug("Reference lost for threadID for the context: {}", key);
+ }
+
+ /**
+ * Get the current thread's IOStatisticsContext instance. If no instance is
+ * present for this thread ID, create one using the factory.
+ * @return instance of IOStatisticsContext.
+ */
+ public static IOStatisticsContext getCurrentIOStatisticsContext() {
+ return IS_THREAD_IOSTATS_ENABLED
+ ? ACTIVE_IOSTATS_CONTEXT.getForCurrentThread()
+ : EmptyIOStatisticsContextImpl.getInstance();
+ }
+
+ /**
+ * Set the IOStatisticsContext for the current thread.
+ * @param statisticsContext IOStatistics context instance for the
+ * current thread.
+ */
+ public static void setThreadIOStatisticsContext(
+ IOStatisticsContext statisticsContext) {
+ if (IS_THREAD_IOSTATS_ENABLED &&
+ ACTIVE_IOSTATS_CONTEXT.getForCurrentThread() != statisticsContext) {
+ ACTIVE_IOSTATS_CONTEXT.setForCurrentThread(statisticsContext);
Review Comment:
will show in a pr
--
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
[GitHub] [hadoop] hadoop-yetus commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1194055457
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 43s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 6 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 15m 20s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 25m 48s | | trunk passed |
| +1 :green_heart: | compile | 24m 51s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | compile | 21m 56s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 4m 38s | | trunk passed |
| +1 :green_heart: | mvnsite | 3m 30s | | trunk passed |
| +1 :green_heart: | javadoc | 2m 23s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 2m 5s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 4m 31s | | trunk passed |
| +1 :green_heart: | shadedclient | 22m 3s | | branch has no errors when building and testing our client artifacts. |
| -0 :warning: | patch | 22m 33s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 35s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 49s | | the patch passed |
| +1 :green_heart: | compile | 23m 53s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javac | 23m 53s | | the patch passed |
| +1 :green_heart: | compile | 22m 1s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 22m 1s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 4m 23s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/14/artifact/out/results-checkstyle-root.txt) | root: The patch generated 2 new + 77 unchanged - 0 fixed = 79 total (was 77) |
| +1 :green_heart: | mvnsite | 3m 46s | | the patch passed |
| +1 :green_heart: | javadoc | 2m 20s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 2m 29s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 5m 6s | | the patch passed |
| +1 :green_heart: | shadedclient | 23m 12s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 18m 51s | | hadoop-common in the patch passed. |
| +1 :green_heart: | unit | 2m 59s | | hadoop-aws in the patch passed. |
| +1 :green_heart: | asflicense | 1m 34s | | The patch does not generate ASF License warnings. |
| | | 245m 27s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/14/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/4352 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux e41385851cf2 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 81c0480a7d0eabd171c9e7cdaccbb3939ec85559 |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/14/testReport/ |
| Max. process+thread count | 3045 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/14/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hadoop] steveloughran commented on a diff in pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
steveloughran commented on code in PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#discussion_r924896943
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContext.java:
##########
@@ -31,34 +31,34 @@
* <p>
* The {@link #snapshot()} call creates a snapshot of the statistics;
* <p>
- * The {@link #reset()} call resets the statistics in the current thread so
+ * The {@link #reset()} call resets the statistics in the context so
* that later snapshots will get the incremental data.
*/
public interface IOStatisticsContext extends IOStatisticsSource {
Review Comment:
this needs to get moved to org.apache.hadoop.fs.statistics.IOStatisticsContext as it is the public api the apps need; the impl stuff is kept private for the filesystems.
--
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
[GitHub] [hadoop] steveloughran commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
steveloughran commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1191326991
This is my draft commit message btw
----
Adds a new IOStatistics class IOStatisticsContext.
This is the active collector of thread-level statistics for
The current thread.
The S3A Filesystem's input and output streams, and listing
operations, all update this context when close() is called on
them (and not before!), so there is effectively automatic
aggregation of all IO statistics performed by a single thread.
The IOStatisticsContext of a thread can be retrieved and
cached for invocation in other threads. Holding such a
reference also ensures that the context will not be garbage
collected.
To collect statistics on a thread:
1. Retrieve the active context with a call to
IOStatisticsContext.getCurrentIOStatisticsContext()
2. Call IOStatisticsContext.reset() to reset all statistics
3. Call getIOStatistics() on it for the latest values, or
snapshot() for a snapshot of them.
To instrument filesystem objects for thread-level
IOStatistics
1. Cache the current IOStatisticsContext context or just its
aggregator in the object constructor.
2. In the close() operation, aggregate() the object's own statistics.
3. Pass the context into worker threads performing work
on behalf of this thread, through
IOStatisticsContext.setThreadIOStatisticsContext();
set it to null afterwards.
TaskPool does the context propagation and reset
automatically.
Contributed by Mehakmeet Singh
---
--
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
[GitHub] [hadoop] steveloughran commented on a diff in pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
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: 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
[GitHub] [hadoop] steveloughran commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
steveloughran commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1194488631
ok, one more change, as I write up something on integration. The RawLocal streams should support IOContext too.
why so? makes testing integration easier, e.g. for distcp, spark etc. no need to wait until object store tests.
This also lines up for moving ITestS3AIOStatisticsContext into hadoop common unit tests as a contract test.
I'm not going to make that a requirement of this PR, but for adding abfs in we should do that
--
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
[GitHub] [hadoop] hadoop-yetus commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1182052415
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 41s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 1s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 2 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 14m 35s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 25m 8s | | trunk passed |
| +1 :green_heart: | compile | 23m 11s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | compile | 20m 24s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 4m 22s | | trunk passed |
| +1 :green_heart: | mvnsite | 3m 45s | | trunk passed |
| +1 :green_heart: | javadoc | 3m 0s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 2m 41s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 4m 57s | | trunk passed |
| +1 :green_heart: | shadedclient | 22m 28s | | branch has no errors when building and testing our client artifacts. |
| -0 :warning: | patch | 23m 0s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 32s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 48s | | the patch passed |
| +1 :green_heart: | compile | 22m 14s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javac | 22m 14s | | the patch passed |
| +1 :green_heart: | compile | 20m 49s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 20m 49s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 4m 15s | | the patch passed |
| +1 :green_heart: | mvnsite | 3m 43s | | the patch passed |
| +1 :green_heart: | javadoc | 2m 51s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 2m 42s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 5m 8s | | the patch passed |
| +1 :green_heart: | shadedclient | 23m 20s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 18m 40s | | hadoop-common in the patch passed. |
| +1 :green_heart: | unit | 3m 20s | | hadoop-aws in the patch passed. |
| +1 :green_heart: | asflicense | 1m 37s | | The patch does not generate ASF License warnings. |
| | | 241m 55s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/7/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/4352 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux d849f9acb1b6 4.15.0-169-generic #177-Ubuntu SMP Thu Feb 3 10:50:38 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 82bc205d749d727d03750a1698adf34f7c2a88fd |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/7/testReport/ |
| Max. process+thread count | 1616 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/7/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hadoop] hadoop-yetus commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1162856649
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 38s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 1s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 4 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 14m 42s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 24m 49s | | trunk passed |
| +1 :green_heart: | compile | 23m 1s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | compile | 20m 33s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 4m 22s | | trunk passed |
| +1 :green_heart: | mvnsite | 3m 43s | | trunk passed |
| +1 :green_heart: | javadoc | 3m 0s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 2m 41s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 4m 59s | | trunk passed |
| +1 :green_heart: | shadedclient | 22m 9s | | branch has no errors when building and testing our client artifacts. |
| -0 :warning: | patch | 22m 42s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 33s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 44s | | the patch passed |
| +1 :green_heart: | compile | 23m 1s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javac | 23m 1s | | the patch passed |
| +1 :green_heart: | compile | 21m 54s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 21m 54s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 4m 15s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/3/artifact/out/results-checkstyle-root.txt) | root: The patch generated 3 new + 74 unchanged - 0 fixed = 77 total (was 74) |
| +1 :green_heart: | mvnsite | 3m 41s | | the patch passed |
| +1 :green_heart: | javadoc | 2m 42s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| -1 :x: | javadoc | 1m 16s | [/results-javadoc-javadoc-hadoop-tools_hadoop-aws-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/3/artifact/out/results-javadoc-javadoc-hadoop-tools_hadoop-aws-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.txt) | hadoop-tools_hadoop-aws-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 generated 2 new + 38 unchanged - 0 fixed = 40 total (was 38) |
| +1 :green_heart: | spotbugs | 5m 16s | | the patch passed |
| +1 :green_heart: | shadedclient | 22m 46s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 18m 14s | | hadoop-common in the patch passed. |
| +1 :green_heart: | unit | 3m 12s | | hadoop-aws in the patch passed. |
| +1 :green_heart: | asflicense | 1m 38s | | The patch does not generate ASF License warnings. |
| | | 241m 22s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/3/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/4352 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 6be2be9d4b79 4.15.0-169-generic #177-Ubuntu SMP Thu Feb 3 10:50:38 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 5badccde12beee84d9fdfec0327346fa1a09f50e |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/3/testReport/ |
| Max. process+thread count | 3158 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/3/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hadoop] hadoop-yetus commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1192486376
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 41s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 6 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 15m 10s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 25m 44s | | trunk passed |
| +1 :green_heart: | compile | 23m 40s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | compile | 22m 22s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 4m 10s | | trunk passed |
| +1 :green_heart: | mvnsite | 3m 8s | | trunk passed |
| +1 :green_heart: | javadoc | 2m 28s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 2m 17s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 5m 28s | | trunk passed |
| +1 :green_heart: | shadedclient | 23m 8s | | branch has no errors when building and testing our client artifacts. |
| -0 :warning: | patch | 23m 33s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 31s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 2m 12s | | the patch passed |
| +1 :green_heart: | compile | 24m 33s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javac | 24m 33s | | the patch passed |
| +1 :green_heart: | compile | 22m 6s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 22m 6s | | the patch passed |
| -1 :x: | blanks | 0m 0s | [/blanks-eol.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/13/artifact/out/blanks-eol.txt) | The patch has 1 line(s) that end in blanks. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply |
| +1 :green_heart: | checkstyle | 4m 1s | | the patch passed |
| +1 :green_heart: | mvnsite | 3m 15s | | the patch passed |
| +1 :green_heart: | javadoc | 2m 33s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 2m 6s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 5m 30s | | the patch passed |
| +1 :green_heart: | shadedclient | 22m 56s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 19m 3s | | hadoop-common in the patch passed. |
| +1 :green_heart: | unit | 3m 8s | | hadoop-aws in the patch passed. |
| +1 :green_heart: | asflicense | 1m 29s | | The patch does not generate ASF License warnings. |
| | | 245m 55s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/13/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/4352 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 0377eb935cf3 4.15.0-169-generic #177-Ubuntu SMP Thu Feb 3 10:50:38 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 359206d7a5726bde59ad77997f698989f18e18cc |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/13/testReport/ |
| Max. process+thread count | 2496 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/13/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hadoop] steveloughran commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
steveloughran commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1191322172
i was writing a commit message with instructions on use, realised that the task pools sharing meant we needed a way to reset the context, then that the pool should do this...adding that and I concluded that the TaskPool should automatically pick up and propagate the context. Which it now does.
no new tests, not tested at all...
--
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
[GitHub] [hadoop] hadoop-yetus commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1183670262
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 40s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 3 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 44m 22s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 25m 13s | | trunk passed |
| +1 :green_heart: | compile | 23m 12s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | compile | 20m 36s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 4m 26s | | trunk passed |
| +1 :green_heart: | mvnsite | 3m 47s | | trunk passed |
| +1 :green_heart: | javadoc | 2m 58s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 2m 42s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 5m 0s | | trunk passed |
| +1 :green_heart: | shadedclient | 22m 5s | | branch has no errors when building and testing our client artifacts. |
| -0 :warning: | patch | 22m 38s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 29s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 46s | | the patch passed |
| +1 :green_heart: | compile | 22m 7s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javac | 22m 7s | | the patch passed |
| +1 :green_heart: | compile | 20m 40s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 20m 40s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 4m 13s | | the patch passed |
| +1 :green_heart: | mvnsite | 3m 43s | | the patch passed |
| +1 :green_heart: | javadoc | 2m 52s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 2m 41s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 5m 11s | | the patch passed |
| +1 :green_heart: | shadedclient | 22m 9s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 18m 53s | | hadoop-common in the patch passed. |
| +1 :green_heart: | unit | 3m 9s | | hadoop-aws in the patch passed. |
| +1 :green_heart: | asflicense | 1m 37s | | The patch does not generate ASF License warnings. |
| | | 270m 13s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/8/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/4352 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux dca9ac835711 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / c2dcad142b639640681fa17960202b00befef2f4 |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/8/testReport/ |
| Max. process+thread count | 1263 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/8/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hadoop] steveloughran commented on a diff in pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
steveloughran commented on code in PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#discussion_r917816203
##########
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>
Review Comment:
let's add a brief comment here as it is a key part of the design
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContext.java:
##########
@@ -164,13 +165,25 @@ public void setThreadIOStatistics(
*
* @return IOStatisticsSnapshot of the current thread.
*/
- public IOStatisticsSnapshot getThreadIOStatisticsSnapshot() {
+ public IOStatisticsSnapshot snapshotCurrentThreadIOStatistics() {
if (IS_THREAD_IOSTATS_ENABLED) {
return (IOStatisticsSnapshot) getThreadIOStatistics();
}
return new IOStatisticsSnapshot();
}
+ /**
+ * Reset the thread IOStatistics for current thread.
+ */
+ public void resetThreadIOStatisticsForCurrentThread() {
+ if (IS_THREAD_IOSTATS_ENABLED) {
+ IOStatisticsSnapshot currentThreadIOStatsSnapshot =
+ (IOStatisticsSnapshot) getThreadIOStatistics();
+ currentThreadIOStatsSnapshot.clear();
+ setThreadIOStatistics(currentThreadIOStatsSnapshot);
Review Comment:
don't think this is needed
##########
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:
now we are casting to a snapshot, why not make this of type `IOStatisticsSnapshot`
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContext.java:
##########
@@ -164,13 +165,25 @@ public void setThreadIOStatistics(
*
* @return IOStatisticsSnapshot of the current thread.
*/
- public IOStatisticsSnapshot getThreadIOStatisticsSnapshot() {
+ public IOStatisticsSnapshot snapshotCurrentThreadIOStatistics() {
if (IS_THREAD_IOSTATS_ENABLED) {
return (IOStatisticsSnapshot) getThreadIOStatistics();
}
return new IOStatisticsSnapshot();
}
+ /**
+ * Reset the thread IOStatistics for current thread.
+ */
+ public void resetThreadIOStatisticsForCurrentThread() {
+ if (IS_THREAD_IOSTATS_ENABLED) {
+ IOStatisticsSnapshot currentThreadIOStatsSnapshot =
+ (IOStatisticsSnapshot) getThreadIOStatistics();
Review Comment:
This is going to demand create a snapshot context even if there wasn't one needing resetting. How about you just look in the reference map (via lookup(currentThreadId())); and only if a ref is returned do the resetting?
--
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
[GitHub] [hadoop] steveloughran commented on a diff in pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
steveloughran commented on code in PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#discussion_r919262464
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContext.java:
##########
@@ -0,0 +1,238 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.statistics.impl;
+
+import java.lang.ref.WeakReference;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.impl.WeakReferenceThreadMap;
+import org.apache.hadoop.fs.statistics.IOStatistics;
+import org.apache.hadoop.fs.statistics.IOStatisticsAggregator;
+import org.apache.hadoop.fs.statistics.IOStatisticsSnapshot;
+
+import static org.apache.hadoop.fs.CommonConfigurationKeys.THREAD_LEVEL_IOSTATISTICS_ENABLED;
+import static org.apache.hadoop.fs.CommonConfigurationKeys.THREAD_LEVEL_IOSTATISTICS_ENABLED_DEFAULT;
+
+/**
+ * Implementing the IOStatisticsContext.
+ *
+ * A Context defined for IOStatistics collection per thread which captures
+ * each worker thread's work in FS streams and stores it in the form of
+ * IOStatisticsSnapshot if thread level aggregation is enabled else returning
+ * an instance of EmptyIOStatisticsStore for the FS. An active instance of
+ * the IOStatisticsContext can be used to collect the statistics.
+ *
+ * For the current thread the IOStatisticsSnapshot can be used as a way to
+ * move the IOStatistics data between applications using the Serializable
+ * nature of the class.
+ */
+public class IOStatisticsContext {
+ private static final Logger LOG =
+ LoggerFactory.getLogger(IOStatisticsContext.class);
+ private static final boolean IS_THREAD_IOSTATS_ENABLED;
+
+ /**
+ * Active IOStatistics Context containing different worker thread's
+ * statistics. Weak Reference so that it gets cleaned up during GC and we
+ * avoid any memory leak issues due to long lived references.
+ */
+ private static final WeakReferenceThreadMap<IOStatisticsContext>
+ ACTIVE_IOSTATS_CONTEXT =
+ new WeakReferenceThreadMap<>(IOStatisticsContext::createNewInstance,
+ IOStatisticsContext::referenceLostContext
+ );
+
+ /**
+ * Collecting IOStatistics per thread.
+ */
+ private final WeakReferenceThreadMap<IOStatisticsSnapshot>
+ threadIOStatsContext =
+ new WeakReferenceThreadMap<>(this::getIOStatisticsSnapshotFactory,
+ this::referenceLost);
+
+ static {
+ // Work out if the current context has thread level IOStatistics enabled.
+ final Configuration configuration = new Configuration();
+ IS_THREAD_IOSTATS_ENABLED =
+ configuration.getBoolean(THREAD_LEVEL_IOSTATISTICS_ENABLED,
+ THREAD_LEVEL_IOSTATISTICS_ENABLED_DEFAULT);
+ }
+
+ /**
+ * Creating a new IOStatisticsContext instance for a FS to be used.
+ *
+ * @param key Thread ID that represents which thread the context belongs to.
+ * @return an instance of IOStatisticsContext.
+ */
+ private static IOStatisticsContext createNewInstance(Long key) {
+ return new IOStatisticsContext();
+ }
+
+ /**
+ * Get the current IOStatisticsContext.
+ *
+ * @return current IOStatisticsContext instance.
+ */
+ public static IOStatisticsContext currentIOStatisticsContext() {
+ return ACTIVE_IOSTATS_CONTEXT.get(Thread.currentThread().getId());
+ }
+
+ /**
+ * A Method to act as an IOStatisticsSnapshot factory, in a
+ * WeakReferenceThreadMap.
+ *
+ * @param key ThreadID.
+ * @return an Instance of IOStatisticsSnapshot.
+ */
+ private IOStatisticsSnapshot getIOStatisticsSnapshotFactory(Long key) {
+ return new IOStatisticsSnapshot();
+ }
+
+ /**
+ * In case of reference loss.
+ *
+ * @param key ThreadID.
+ */
+ private void referenceLost(Long key) {
+ LOG.debug("Reference lost for threadID: {}", key);
+ }
+
+ /**
+ * In case of reference loss for IOStatisticsContext.
+ *
+ * @param key ThreadID.
+ */
+ private static void referenceLostContext(Long key) {
+ LOG.debug("Reference lost for threadID for the context: {}", key);
+ }
+
+ /**
+ * A Method to get the IOStatisticsAggregator of the currentThread. This
+ * denotes the aggregated IOStatistics per thread.
+ *
+ * @return the instance of IOStatisticsAggregator for the current thread.
+ */
+ public IOStatisticsAggregator getThreadIOStatistics() {
Review Comment:
let's call this ThreadIOStatisticsAggregator
--
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
[GitHub] [hadoop] mehakmeet commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
mehakmeet commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1164037866
More merge conflicts due to vectored IO merging.
Interesting bit of info on the new test, seems to fail in maven verify but works fine in intellij, I remember Steve saying something about how tests run differently in mvn than intellij wrt Threads, need to check that.
--
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
[GitHub] [hadoop] hadoop-yetus commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1195291441
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 37s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 6 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 15m 13s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 25m 27s | | trunk passed |
| +1 :green_heart: | compile | 23m 25s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | compile | 21m 40s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 4m 48s | | trunk passed |
| +1 :green_heart: | mvnsite | 3m 35s | | trunk passed |
| +1 :green_heart: | javadoc | 2m 54s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 2m 28s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 5m 3s | | trunk passed |
| +1 :green_heart: | shadedclient | 22m 8s | | branch has no errors when building and testing our client artifacts. |
| -0 :warning: | patch | 22m 40s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 31s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 46s | | the patch passed |
| +1 :green_heart: | compile | 22m 29s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javac | 22m 29s | | the patch passed |
| +1 :green_heart: | compile | 20m 45s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 20m 45s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 4m 17s | | the patch passed |
| +1 :green_heart: | mvnsite | 3m 44s | | the patch passed |
| +1 :green_heart: | javadoc | 2m 50s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 2m 43s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 5m 11s | | the patch passed |
| +1 :green_heart: | shadedclient | 22m 20s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 18m 40s | | hadoop-common in the patch passed. |
| +1 :green_heart: | unit | 3m 19s | | hadoop-aws in the patch passed. |
| +1 :green_heart: | asflicense | 1m 37s | | The patch does not generate ASF License warnings. |
| | | 243m 1s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/16/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/4352 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 18348d0e00b5 4.15.0-169-generic #177-Ubuntu SMP Thu Feb 3 10:50:38 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / de12dbe2a061edf49f147e634c8cee29a31b09ed |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/16/testReport/ |
| Max. process+thread count | 2608 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/16/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hadoop] mehakmeet commented on a diff in pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by 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
[GitHub] [hadoop] steveloughran commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
steveloughran commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1184422633
ok, i have some changes but I will add them as a commit for you to review/cherrypick...i want to make sure they line up with a local spark build.
key point: interfaces now support static methods, so looking up the current context can/should be a static method in IOStatisticsContext
--
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
[GitHub] [hadoop] steveloughran commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
steveloughran commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1187742820
yeah, reviewed javadocs and code. criticised my own work. checkstyles
```
./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContextIntegration.java:46:public class IOStatisticsContextIntegration {:1: Utility classes should not have a public or default constructor. [HideUtilityClassConstructor]
./hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/commit/impl/CommitContext.java:370: /**: First sentence should end with a period. [JavadocStyle]
./hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java:135:import org.apache.hadoop.fs.statistics.impl.IOStatisticsContextImpl;:8: Unused import - org.apache.hadoop.fs.statistics.impl.IOStatisticsContextImpl. [UnusedImports]
./hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/commit/AbstractCommitITest.java:73: /**: First sentence should end with a period. [JavadocStyle]
./hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AIOStatisticsContext.java:168: final ExecutorService executor =:27: 'executor' hides a field. [HiddenField]
```
--
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
[GitHub] [hadoop] hadoop-yetus commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1190110412
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 42s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 1s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 5 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 15m 5s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 25m 21s | | trunk passed |
| +1 :green_heart: | compile | 24m 5s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | compile | 22m 6s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 4m 21s | | trunk passed |
| +1 :green_heart: | mvnsite | 3m 35s | | trunk passed |
| +1 :green_heart: | javadoc | 2m 59s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 2m 43s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 5m 3s | | trunk passed |
| +1 :green_heart: | shadedclient | 22m 21s | | branch has no errors when building and testing our client artifacts. |
| -0 :warning: | patch | 22m 54s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 35s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 45s | | the patch passed |
| +1 :green_heart: | compile | 22m 25s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javac | 22m 25s | | the patch passed |
| +1 :green_heart: | compile | 20m 50s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 20m 50s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 4m 14s | | the patch passed |
| +1 :green_heart: | mvnsite | 3m 33s | | the patch passed |
| +1 :green_heart: | javadoc | 2m 45s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 2m 40s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 5m 17s | | the patch passed |
| +1 :green_heart: | shadedclient | 23m 11s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 19m 8s | | hadoop-common in the patch passed. |
| +1 :green_heart: | unit | 3m 10s | | hadoop-aws in the patch passed. |
| +1 :green_heart: | asflicense | 1m 20s | | The patch does not generate ASF License warnings. |
| | | 244m 37s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/11/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/4352 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux bd453dad2670 4.15.0-169-generic #177-Ubuntu SMP Thu Feb 3 10:50:38 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 6f7e72ee80015a6f9396a3ab1d8896a142fef464 |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/11/testReport/ |
| Max. process+thread count | 3158 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/11/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hadoop] steveloughran commented on a diff in pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
steveloughran commented on code in PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#discussion_r893822412
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContextImpl.java:
##########
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.statistics.impl;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hadoop.fs.impl.WeakReferenceThreadMap;
+import org.apache.hadoop.fs.statistics.IOStatisticsContext;
+import org.apache.hadoop.fs.statistics.IOStatisticsSnapshot;
+
+/**
+ * Implementing the IOStatisticsContext interface.
+ */
+public class IOStatisticsContextImpl implements IOStatisticsContext {
+ private static final Logger LOG =
+ LoggerFactory.getLogger(IOStatisticsContextImpl.class);
+
+ /**
+ * Collecting IOStatistics per thread.
+ */
+ private final WeakReferenceThreadMap<IOStatisticsSnapshot>
+ threadIOStatsContext = new WeakReferenceThreadMap<>(
+ this::getIOStatisticsSnapshotFactory,
Review Comment:
needs indentation
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContextImpl.java:
##########
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.statistics.impl;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hadoop.fs.impl.WeakReferenceThreadMap;
+import org.apache.hadoop.fs.statistics.IOStatisticsContext;
+import org.apache.hadoop.fs.statistics.IOStatisticsSnapshot;
+
+/**
+ * Implementing the IOStatisticsContext interface.
+ */
+public class IOStatisticsContextImpl implements IOStatisticsContext {
+ private static final Logger LOG =
+ LoggerFactory.getLogger(IOStatisticsContextImpl.class);
+
+ /**
+ * Collecting IOStatistics per thread.
+ */
+ private final WeakReferenceThreadMap<IOStatisticsSnapshot>
+ threadIOStatsContext = new WeakReferenceThreadMap<>(
+ this::getIOStatisticsSnapshotFactory,
+ this::referenceLost);
+
+ /**
+ * A Method to act as an IOStatisticsSnapshot factory, in a
+ * WeakReferenceThreadMap.
+ *
+ * @param key ThreadID.
+ * @return an Instance of IOStatisticsSnapshot.
+ */
+ private IOStatisticsSnapshot getIOStatisticsSnapshotFactory(Long key) {
+ return new IOStatisticsSnapshot();
+ }
+
+ /**
+ * In case of reference loss.
+ *
+ * @param key ThreadID.
+ */
+ private void referenceLost(Long key) {
+ LOG.info("Reference lost for threadID: {}", key);
Review Comment:
maybe debug
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContextImpl.java:
##########
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.statistics.impl;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hadoop.fs.impl.WeakReferenceThreadMap;
+import org.apache.hadoop.fs.statistics.IOStatisticsContext;
+import org.apache.hadoop.fs.statistics.IOStatisticsSnapshot;
+
+/**
+ * Implementing the IOStatisticsContext interface.
+ */
+public class IOStatisticsContextImpl implements IOStatisticsContext {
+ private static final Logger LOG =
+ LoggerFactory.getLogger(IOStatisticsContextImpl.class);
+
+ /**
+ * Collecting IOStatistics per thread.
+ */
+ private final WeakReferenceThreadMap<IOStatisticsSnapshot>
+ threadIOStatsContext = new WeakReferenceThreadMap<>(
+ this::getIOStatisticsSnapshotFactory,
+ this::referenceLost);
+
+ /**
+ * A Method to act as an IOStatisticsSnapshot factory, in a
+ * WeakReferenceThreadMap.
+ *
+ * @param key ThreadID.
+ * @return an Instance of IOStatisticsSnapshot.
+ */
+ private IOStatisticsSnapshot getIOStatisticsSnapshotFactory(Long key) {
+ return new IOStatisticsSnapshot();
+ }
+
+ /**
+ * In case of reference loss.
+ *
+ * @param key ThreadID.
+ */
+ private void referenceLost(Long key) {
+ LOG.info("Reference lost for threadID: {}", key);
+ }
+
+ /**
+ * A Method to get the IOStatisticsSnapshot of the currentThread. This
+ * denotes the aggregated IOStatistics per thread.
+ *
Review Comment:
comment that it is not thread safe as it doesn't need to be
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContextImpl.java:
##########
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.statistics.impl;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hadoop.fs.impl.WeakReferenceThreadMap;
+import org.apache.hadoop.fs.statistics.IOStatisticsContext;
+import org.apache.hadoop.fs.statistics.IOStatisticsSnapshot;
+
+/**
+ * Implementing the IOStatisticsContext interface.
+ */
+public class IOStatisticsContextImpl implements IOStatisticsContext {
+ private static final Logger LOG =
+ LoggerFactory.getLogger(IOStatisticsContextImpl.class);
+
+ /**
+ * Collecting IOStatistics per thread.
+ */
+ private final WeakReferenceThreadMap<IOStatisticsSnapshot>
+ threadIOStatsContext = new WeakReferenceThreadMap<>(
+ this::getIOStatisticsSnapshotFactory,
+ this::referenceLost);
+
+ /**
+ * A Method to act as an IOStatisticsSnapshot factory, in a
+ * WeakReferenceThreadMap.
+ *
+ * @param key ThreadID.
+ * @return an Instance of IOStatisticsSnapshot.
+ */
+ private IOStatisticsSnapshot getIOStatisticsSnapshotFactory(Long key) {
+ return new IOStatisticsSnapshot();
+ }
+
+ /**
+ * In case of reference loss.
+ *
+ * @param key ThreadID.
+ */
+ private void referenceLost(Long key) {
+ LOG.info("Reference lost for threadID: {}", key);
+ }
+
+ /**
+ * A Method to get the IOStatisticsSnapshot of the currentThread. This
+ * denotes the aggregated IOStatistics per thread.
+ *
+ * @return the instance of IOStatisticsSnapshot.
+ */
+ @Override
+ public IOStatisticsSnapshot getThreadIOStatistics() {
+ IOStatisticsSnapshot ioStatisticsSnapshot =
+ threadIOStatsContext.getForCurrentThread();
+ if (ioStatisticsSnapshot == null) {
+ // Create and set IOStatsSnapshot in the ThreadPool.
+ ioStatisticsSnapshot = new IOStatisticsSnapshot();
+ threadIOStatsContext.setForCurrentThread(ioStatisticsSnapshot);
+ }
+ // If an instance is present return it.
+ return ioStatisticsSnapshot;
+ }
+
+ @Override
+ public String toString() {
+ return getThreadIOStatistics().toString();
Review Comment:
this is potentially expensive and give different values when called in different threads. best cut
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java:
##########
@@ -670,6 +685,19 @@ public Statistics getInstanceStatistics() {
return S3AFileSystem.this.statistics;
}
});
+ // If Thread-level IOStats support is enabled, initialize the context.
+ // This would initialize a ThreadLocal Instance.
+ if(isIOStatsContextEnabled()) {
+ this.ioStatisticsContext = new IOStatisticsContextImpl();
+ }
+ }
+
+ /**
+ *
Review Comment:
nit: javadocs
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContextImpl.java:
##########
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.statistics.impl;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hadoop.fs.impl.WeakReferenceThreadMap;
+import org.apache.hadoop.fs.statistics.IOStatisticsContext;
+import org.apache.hadoop.fs.statistics.IOStatisticsSnapshot;
+
+/**
+ * Implementing the IOStatisticsContext interface.
+ */
+public class IOStatisticsContextImpl implements IOStatisticsContext {
+ private static final Logger LOG =
+ LoggerFactory.getLogger(IOStatisticsContextImpl.class);
+
+ /**
+ * Collecting IOStatistics per thread.
+ */
+ private final WeakReferenceThreadMap<IOStatisticsSnapshot>
+ threadIOStatsContext = new WeakReferenceThreadMap<>(
+ this::getIOStatisticsSnapshotFactory,
+ this::referenceLost);
+
+ /**
+ * A Method to act as an IOStatisticsSnapshot factory, in a
+ * WeakReferenceThreadMap.
+ *
+ * @param key ThreadID.
+ * @return an Instance of IOStatisticsSnapshot.
+ */
+ private IOStatisticsSnapshot getIOStatisticsSnapshotFactory(Long key) {
+ return new IOStatisticsSnapshot();
+ }
+
+ /**
+ * In case of reference loss.
+ *
+ * @param key ThreadID.
+ */
+ private void referenceLost(Long key) {
+ LOG.info("Reference lost for threadID: {}", key);
+ }
+
+ /**
+ * A Method to get the IOStatisticsSnapshot of the currentThread. This
Review Comment:
nit: space between current and thread
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/test/LambdaTestUtils.java:
##########
@@ -65,6 +66,15 @@ private LambdaTestUtils() {
*/
public static final String NULL_RESULT = "(null)";
+ /**
+ * Atomic references to be used to re-throw an Exception or an ASE
+ * caught inside a lambda function.
+ */
+ public static final AtomicReference<Exception> futureExcp =
Review Comment:
1. not sure that this is the right place for what's been a fairly stable class for a while
1. give full title in their names, upper case
2. make private
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AInputStream.java:
##########
@@ -561,6 +566,12 @@ public synchronized void close() throws IOException {
try {
// close or abort the stream; blocking
awaitFuture(closeStream("close() operation", false, true));
+ // Collect ThreadLevel IOStats
+ if (ioStatisticsContext != null) {
+ ioStatisticsContext.getThreadIOStatistics().aggregate(streamStatistics.getIOStatistics());
Review Comment:
ioStatisticsContext.getThreadIOStatistics() needs to be cached in the constructor, so that even when closed *in a different thread* the original stat context is updated. this also guarantees that the reference doesn't get cleaned up while a stream is open
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AIOStatisticsContext.java:
##########
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.s3a;
+
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+
+import org.junit.Test;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FSDataInputStream;
+import org.apache.hadoop.fs.FSDataOutputStream;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.statistics.IOStatisticAssertions;
+import org.apache.hadoop.fs.statistics.IOStatistics;
+import org.apache.hadoop.fs.statistics.IOStatisticsContext;
+import org.apache.hadoop.fs.statistics.StreamStatisticNames;
+import org.apache.hadoop.util.concurrent.HadoopExecutors;
+
+import static org.apache.hadoop.fs.contract.ContractTestUtils.dataset;
+import static org.apache.hadoop.fs.contract.ContractTestUtils.writeDataset;
+import static org.apache.hadoop.fs.s3a.Constants.THREAD_LEVEL_IOSTATS_ENABLED;
+import static org.apache.hadoop.fs.s3a.S3ATestUtils.removeBaseAndBucketOverrides;
+import static org.apache.hadoop.test.LambdaTestUtils.maybeReThrowFutureASE;
+import static org.apache.hadoop.test.LambdaTestUtils.maybeReThrowFutureException;
+import static org.apache.hadoop.test.LambdaTestUtils.setFutureASE;
+import static org.apache.hadoop.test.LambdaTestUtils.setFutureException;
+
+public class ITestS3AIOStatisticsContext extends AbstractS3ATestBase {
Review Comment:
1. add javadoc of test purpose
2. add tests to verify the thread used in the contructor has its stats updated for both input and output io. open in the junit thread, do work and close in others. assert on the junit thread values
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AReadOpContext.java:
##########
@@ -77,11 +80,13 @@ public S3AReadOpContext(
Invoker invoker,
@Nullable FileSystem.Statistics stats,
S3AStatisticsContext instrumentation,
- FileStatus dstFileStatus) {
+ FileStatus dstFileStatus,
+ final IOStatisticsContext ioStatisticsContext) {
super(invoker, stats, instrumentation,
dstFileStatus);
this.path = requireNonNull(path);
+ this.ioStatisticsContext = ioStatisticsContext;
Review Comment:
can this be null? prefer not
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3ABlockOutputStream.java:
##########
@@ -193,6 +198,7 @@ class S3ABlockOutputStream extends OutputStream implements
initMultipartUpload();
}
this.isCSEEnabled = builder.isCSEEnabled;
+ this.ioStatisticsContext = builder.ioStatisticsContext;
Review Comment:
cache the thread iostats here and use in close(), something like `threadIOStatistics`
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AIOStatisticsContext.java:
##########
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.s3a;
+
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+
+import org.junit.Test;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FSDataInputStream;
+import org.apache.hadoop.fs.FSDataOutputStream;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.statistics.IOStatisticAssertions;
+import org.apache.hadoop.fs.statistics.IOStatistics;
+import org.apache.hadoop.fs.statistics.IOStatisticsContext;
+import org.apache.hadoop.fs.statistics.StreamStatisticNames;
+import org.apache.hadoop.util.concurrent.HadoopExecutors;
+
+import static org.apache.hadoop.fs.contract.ContractTestUtils.dataset;
+import static org.apache.hadoop.fs.contract.ContractTestUtils.writeDataset;
+import static org.apache.hadoop.fs.s3a.Constants.THREAD_LEVEL_IOSTATS_ENABLED;
+import static org.apache.hadoop.fs.s3a.S3ATestUtils.removeBaseAndBucketOverrides;
+import static org.apache.hadoop.test.LambdaTestUtils.maybeReThrowFutureASE;
+import static org.apache.hadoop.test.LambdaTestUtils.maybeReThrowFutureException;
+import static org.apache.hadoop.test.LambdaTestUtils.setFutureASE;
+import static org.apache.hadoop.test.LambdaTestUtils.setFutureException;
+
+public class ITestS3AIOStatisticsContext extends AbstractS3ATestBase {
+
+ private static final int SMALL_THREADS = 2;
+ private static final int READ_BYTES_FIRST = 100;
+ private static final int READ_BYTES_SECOND = 50;
+
+ @Override
+ protected Configuration createConfiguration() {
+ Configuration configuration = super.createConfiguration();
+ removeBaseAndBucketOverrides(configuration, THREAD_LEVEL_IOSTATS_ENABLED);
+ return configuration;
+ }
+
+ /**
+ * Verify that S3AInputStream aggregates per thread IOStats collection
+ * correctly.
+ */
+ @Test
+ public void testS3AInputStreamIOStatisticsContext()
+ throws Exception {
+ S3AFileSystem fs = getFileSystem();
+ Path path = path(getMethodName());
+ byte[] data = dataset(256, 'a', 'z');
+ byte[] readDataFirst = new byte[READ_BYTES_FIRST];
+ byte[] readDataSecond = new byte[READ_BYTES_SECOND];
+ writeDataset(fs, path, data, data.length, 1024, true);
+
+ final ExecutorService executor =
+ HadoopExecutors.newFixedThreadPool(SMALL_THREADS);
+ CountDownLatch latch = new CountDownLatch(SMALL_THREADS);
+
+ try {
+ for (int i = 0; i < SMALL_THREADS; i++) {
+ executor.submit(() -> {
+ try {
+ IOStatistics ioStatisticsFirst;
+ try (FSDataInputStream in = fs.open(path)) {
+ in.seek(50);
+ in.read(readDataFirst);
+ in.close();
+ ioStatisticsFirst = assertThreadStatisticsBytesRead(in,
+ READ_BYTES_FIRST);
+ }
+ // Stream is closed for a thread. Re-open and do more operations.
+ IOStatistics ioStatisticsSecond;
+ try (FSDataInputStream in = fs.open(path)) {
+ in.seek(100);
+ in.read(readDataSecond);
+ in.close();
+ ioStatisticsSecond = assertThreadStatisticsBytesRead(in,
+ READ_BYTES_FIRST + READ_BYTES_SECOND);
+ }
+ latch.countDown();
+ } catch (Exception e) {
+ latch.countDown();
+ setFutureException(e);
+ LOG.error("An error occurred while doing a task in the thread", e);
+ } catch (AssertionError ase) {
+ latch.countDown();
+ setFutureASE(ase);
+ throw ase;
+ }
+ });
+ }
+ // wait for tasks to finish.
+ latch.await();
+ } finally {
+ executor.shutdown();
+ }
+
+ // Check if an Excp or ASE was caught while the test threads were running.
+ maybeReThrowFutureException();
+ maybeReThrowFutureASE();
+
+ }
+
+ @Test
+ public void testS3ABlockOutputStreamIOStatisticsContext()
+ throws Exception {
+ S3AFileSystem fs = getFileSystem();
+ Path path = path(getMethodName());
+ byte[] data = dataset(256, 'a', 'z');
+ byte[] writeDataFirst = new byte[READ_BYTES_FIRST];
+ byte[] writeDataSecond = new byte[READ_BYTES_SECOND];
+ writeDataset(fs, path, data, data.length, 1024, true);
+
+ final ExecutorService executor =
+ HadoopExecutors.newFixedThreadPool(SMALL_THREADS);
+ CountDownLatch latch = new CountDownLatch(SMALL_THREADS);
+
+ try {
+ for (int i = 0; i < SMALL_THREADS; i++) {
+ executor.submit(() -> {
+ try {
+ IOStatistics ioStatisticsFirst;
+ try (FSDataOutputStream out = fs.create(path)) {
+ out.write(writeDataFirst);
+ out.close();
+ ioStatisticsFirst = assertThreadStatisticsBytesWrite(out,
+ READ_BYTES_FIRST);
+ }
+ // Stream is closed for a thread. Re-open and do more operations.
+ IOStatistics ioStatisticsSecond;
+ try (FSDataOutputStream out = fs.create(path)) {
+ out.write(writeDataSecond);
+ out.close();
+ ioStatisticsSecond = assertThreadStatisticsBytesWrite(out,
+ READ_BYTES_FIRST + READ_BYTES_SECOND);
+ }
+ latch.countDown();
+ } catch (Exception e) {
+ latch.countDown();
+ setFutureException(e);
+ LOG.error("An error occurred while doing a task in the thread", e);
+ } catch (AssertionError ase) {
+ latch.countDown();
+ setFutureASE(ase);
+ throw ase;
+ }
+ });
+ }
+ // wait for tasks to finish.
+ latch.await();
+ } finally {
+ executor.shutdown();
+ }
+
+ // Check if an Excp or ASE was caught while the test threads were running.
+ maybeReThrowFutureException();
+ maybeReThrowFutureASE();
+
+ }
+
+ private IOStatistics assertThreadStatisticsBytesWrite(FSDataOutputStream out, int writeBytes) {
+ S3ABlockOutputStream s3aOut = (S3ABlockOutputStream) out.getWrappedStream();
+ IOStatisticsContext ioStatisticsContext = s3aOut.getIoStatisticsContext();
Review Comment:
stream should just cache the thread IOStatistics in its constructor, not the context.
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AInputStream.java:
##########
@@ -561,6 +566,12 @@ public synchronized void close() throws IOException {
try {
// close or abort the stream; blocking
awaitFuture(closeStream("close() operation", false, true));
+ // Collect ThreadLevel IOStats
+ if (ioStatisticsContext != null) {
+ ioStatisticsContext.getThreadIOStatistics().aggregate(streamStatistics.getIOStatistics());
+ LOG.debug("IOStatistics of thread: {}\n{}",
+ Thread.currentThread().getId(), ioStatisticsContext.toString());
Review Comment:
too expensive.
Use IOStatisticsLogging for on-demand logging of the thread io stats you'll be caching
--
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
[GitHub] [hadoop] steveloughran commented on a diff in pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
steveloughran commented on code in PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#discussion_r907678976
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContext.java:
##########
@@ -0,0 +1,196 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.statistics.impl;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.impl.WeakReferenceThreadMap;
+import org.apache.hadoop.fs.statistics.IOStatistics;
+import org.apache.hadoop.fs.statistics.IOStatisticsAggregator;
+import org.apache.hadoop.fs.statistics.IOStatisticsSnapshot;
+
+import static org.apache.hadoop.fs.CommonConfigurationKeys.THREAD_LEVEL_IOSTATISTICS_ENABLED;
+import static org.apache.hadoop.fs.CommonConfigurationKeys.THREAD_LEVEL_IOSTATISTICS_ENABLED_DEFAULT;
+
+/**
+ * Implementing the IOStatisticsContext interface.
Review Comment:
javadocs here don't match with latest code.
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContext.java:
##########
@@ -0,0 +1,196 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.statistics.impl;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.impl.WeakReferenceThreadMap;
+import org.apache.hadoop.fs.statistics.IOStatistics;
+import org.apache.hadoop.fs.statistics.IOStatisticsAggregator;
+import org.apache.hadoop.fs.statistics.IOStatisticsSnapshot;
+
+import static org.apache.hadoop.fs.CommonConfigurationKeys.THREAD_LEVEL_IOSTATISTICS_ENABLED;
+import static org.apache.hadoop.fs.CommonConfigurationKeys.THREAD_LEVEL_IOSTATISTICS_ENABLED_DEFAULT;
+
+/**
+ * Implementing the IOStatisticsContext interface.
+ *
+ * A Context defined for IOStatistics collection per thread which captures
+ * each worker thread's work in FS streams and stores it in the form of
+ * IOStatisticsAggregator which could be either IOStatisticsSnapshot or
+ * EmptyIOStatisticsStore if thread level aggregation is enabled or not for
+ * the FS. An active instance of the IOStatisticsContext can be used to
+ * collect the statistics.
+ *
+ * For the current thread the IOStatisticsSnapshot can be used as a way to
+ * move the IOStatistics data between applications using the Serializable
+ * nature of the class.
+ */
+public class IOStatisticsContext {
+ private static final Logger LOG =
+ LoggerFactory.getLogger(IOStatisticsContext.class);
+ private static final boolean IS_THREAD_IOSTATS_ENABLED;
+
+ private static final WeakReferenceThreadMap<IOStatisticsContext>
+ ACTIVE_IOSTATS_CONTEXT = new WeakReferenceThreadMap<>(
+ IOStatisticsContext::createNewInstance,
Review Comment:
nit, indent this and L56
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContext.java:
##########
@@ -0,0 +1,196 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.statistics.impl;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.impl.WeakReferenceThreadMap;
+import org.apache.hadoop.fs.statistics.IOStatistics;
+import org.apache.hadoop.fs.statistics.IOStatisticsAggregator;
+import org.apache.hadoop.fs.statistics.IOStatisticsSnapshot;
+
+import static org.apache.hadoop.fs.CommonConfigurationKeys.THREAD_LEVEL_IOSTATISTICS_ENABLED;
+import static org.apache.hadoop.fs.CommonConfigurationKeys.THREAD_LEVEL_IOSTATISTICS_ENABLED_DEFAULT;
+
+/**
+ * Implementing the IOStatisticsContext interface.
+ *
+ * A Context defined for IOStatistics collection per thread which captures
+ * each worker thread's work in FS streams and stores it in the form of
+ * IOStatisticsAggregator which could be either IOStatisticsSnapshot or
+ * EmptyIOStatisticsStore if thread level aggregation is enabled or not for
+ * the FS. An active instance of the IOStatisticsContext can be used to
+ * collect the statistics.
+ *
+ * For the current thread the IOStatisticsSnapshot can be used as a way to
+ * move the IOStatistics data between applications using the Serializable
+ * nature of the class.
+ */
+public class IOStatisticsContext {
+ private static final Logger LOG =
+ LoggerFactory.getLogger(IOStatisticsContext.class);
+ private static final boolean IS_THREAD_IOSTATS_ENABLED;
+
+ private static final WeakReferenceThreadMap<IOStatisticsContext>
+ ACTIVE_IOSTATS_CONTEXT = new WeakReferenceThreadMap<>(
+ IOStatisticsContext::createNewInstance,
+ IOStatisticsContext::referenceLostContext
+ );
+
+ /**
+ * Collecting IOStatistics per thread.
+ */
+ private final WeakReferenceThreadMap<IOStatisticsAggregator>
+ threadIOStatsContext = new WeakReferenceThreadMap<>(
+ this::getIOStatisticsAggregatorFactory,
+ this::referenceLost);
+
+ static {
+ // Work out if the current context have thread level IOStatistics enabled.
Review Comment:
nit "has"
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContext.java:
##########
@@ -0,0 +1,196 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.statistics.impl;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.impl.WeakReferenceThreadMap;
+import org.apache.hadoop.fs.statistics.IOStatistics;
+import org.apache.hadoop.fs.statistics.IOStatisticsAggregator;
+import org.apache.hadoop.fs.statistics.IOStatisticsSnapshot;
+
+import static org.apache.hadoop.fs.CommonConfigurationKeys.THREAD_LEVEL_IOSTATISTICS_ENABLED;
+import static org.apache.hadoop.fs.CommonConfigurationKeys.THREAD_LEVEL_IOSTATISTICS_ENABLED_DEFAULT;
+
+/**
+ * Implementing the IOStatisticsContext interface.
+ *
+ * A Context defined for IOStatistics collection per thread which captures
+ * each worker thread's work in FS streams and stores it in the form of
+ * IOStatisticsAggregator which could be either IOStatisticsSnapshot or
+ * EmptyIOStatisticsStore if thread level aggregation is enabled or not for
+ * the FS. An active instance of the IOStatisticsContext can be used to
+ * collect the statistics.
+ *
+ * For the current thread the IOStatisticsSnapshot can be used as a way to
+ * move the IOStatistics data between applications using the Serializable
+ * nature of the class.
+ */
+public class IOStatisticsContext {
+ private static final Logger LOG =
+ LoggerFactory.getLogger(IOStatisticsContext.class);
+ private static final boolean IS_THREAD_IOSTATS_ENABLED;
+
+ private static final WeakReferenceThreadMap<IOStatisticsContext>
+ ACTIVE_IOSTATS_CONTEXT = new WeakReferenceThreadMap<>(
+ IOStatisticsContext::createNewInstance,
+ IOStatisticsContext::referenceLostContext
+ );
+
+ /**
+ * Collecting IOStatistics per thread.
+ */
+ private final WeakReferenceThreadMap<IOStatisticsAggregator>
+ threadIOStatsContext = new WeakReferenceThreadMap<>(
+ this::getIOStatisticsAggregatorFactory,
+ this::referenceLost);
+
+ static {
+ // Work out if the current context have thread level IOStatistics enabled.
+ final Configuration configuration = new Configuration();
+ IS_THREAD_IOSTATS_ENABLED =
+ configuration.getBoolean(THREAD_LEVEL_IOSTATISTICS_ENABLED,
+ THREAD_LEVEL_IOSTATISTICS_ENABLED_DEFAULT);
+ }
+
+ /**
+ * Creating a new IOStatisticsContext instance for a FS to be used.
+ *
+ * @param key Thread ID that represents which thread the context belongs to.
+ * @return an instance of IOStatisticsContext.
+ */
+ private static IOStatisticsContext createNewInstance(Long key) {
+ return new IOStatisticsContext();
+ }
+
+ /**
+ * Get the current IOStatisticsContext.
+ *
+ * @return current IOStatisticsContext instance.
+ */
+ public static IOStatisticsContext currentIOStatisticsContext() {
+ return ACTIVE_IOSTATS_CONTEXT.get(Thread.currentThread().getId());
+ }
+
+ /**
+ * A Method to act as an IOStatisticsSnapshot factory, in a
+ * WeakReferenceThreadMap.
+ *
+ * @param key ThreadID.
+ * @return an Instance of IOStatisticsSnapshot.
+ */
+ private IOStatisticsAggregator getIOStatisticsAggregatorFactory(Long key) {
+ return getThreadIOStatistics();
+ }
+
+ /**
+ * In case of reference loss.
+ *
+ * @param key ThreadID.
+ */
+ private void referenceLost(Long key) {
+ LOG.debug("Reference lost for threadID: {}", key);
+ }
+
+ /**
+ * In case of reference loss for IOStatisticsContext.
+ *
+ * @param key ThreadID.
+ */
+ private static void referenceLostContext(Long key) {
+ LOG.debug("Reference lost for threadID for the context: {}", key);
+ }
+
+ /**
+ * A Method to get the IOStatisticsAggregator of the currentThread. This
+ * denotes the aggregated IOStatistics per thread.
+ *
+ * @return the instance of IOStatisticsAggregator for the current thread.
+ */
+ public IOStatisticsAggregator getThreadIOStatistics() {
+ // If Thread IOStats is disabled we return an emptyIOStatistics instance
+ // back.
+ if (!IS_THREAD_IOSTATS_ENABLED) {
+ return EmptyIOStatisticsStore.getInstance();
+ }
+
+ // If the current Thread ID already have an aggregator assigned, return
+ // that.
+ boolean isThreadIOStatsPresent =
+ threadIOStatsContext.containsKey(Thread.currentThread().getId());
+ if (isThreadIOStatsPresent) {
+ return threadIOStatsContext.getForCurrentThread();
+ }
+
+ // If no aggregator is defined to the thread ID, create one and assign it.
+ IOStatisticsSnapshot ioStatisticsSnapshot = new IOStatisticsSnapshot();
+ setThreadIOStatistics(ioStatisticsSnapshot);
+ return ioStatisticsSnapshot;
+ }
+
+ /**
+ * Set the IOStatisticsAggregator for the current context for a specific
+ * thread.
+ *
+ * @param ioStatisticsAggregator IOStatisticsAggregator instance for the
+ * current thread.
+ */
+ public void setThreadIOStatistics(
+ IOStatisticsAggregator ioStatisticsAggregator) {
+ threadIOStatsContext.setForCurrentThread(ioStatisticsAggregator);
+ }
+
+ /**
+ * Returns a snapshot of the current thread's IOStatistics.
+ *
+ * @return IOStatisticsSnapshot of the current thread.
+ */
+ public IOStatisticsSnapshot getThreadIOStatisticsSnapshot() {
+ if (IS_THREAD_IOSTATS_ENABLED) {
+ return (IOStatisticsSnapshot) getThreadIOStatistics();
+ }
+ return new IOStatisticsSnapshot();
+ }
+
+ /**
+ * Get thread ID specific IOStatistics values.
+ *
+ * @param testThreadId thread ID.
+ * @return IOStatistics instance.
+ */
+ @VisibleForTesting
+ public IOStatistics getThreadIOStatistics(long testThreadId) {
+ LOG.info("IOStatsContext thread ID required: {}", testThreadId);
Review Comment:
info? or debug?
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AIOStatisticsContext.java:
##########
@@ -0,0 +1,332 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.s3a;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.net.URI;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FSDataInputStream;
+import org.apache.hadoop.fs.FSDataOutputStream;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.statistics.IOStatisticAssertions;
+import org.apache.hadoop.fs.statistics.IOStatistics;
+import org.apache.hadoop.fs.statistics.IOStatisticsSnapshot;
+import org.apache.hadoop.fs.statistics.StreamStatisticNames;
+import org.apache.hadoop.util.concurrent.HadoopExecutors;
+
+import static org.apache.hadoop.fs.CommonConfigurationKeys.THREAD_LEVEL_IOSTATISTICS_ENABLED;
+import static org.apache.hadoop.fs.contract.ContractTestUtils.dataset;
+import static org.apache.hadoop.fs.contract.ContractTestUtils.writeDataset;
+import static org.apache.hadoop.fs.s3a.S3ATestUtils.removeBaseAndBucketOverrides;
+import static org.apache.hadoop.fs.statistics.impl.IOStatisticsContext.currentIOStatisticsContext;
+
+/**
+ * Tests to verify the Thread-level IOStatistics.
+ */
+public class ITestS3AIOStatisticsContext extends AbstractS3ATestBase {
+
+ private static final int SMALL_THREADS = 2;
+ private static final int BYTES_BIG = 100;
+ private static final int BYTES_SMALL = 50;
+ private static final long TEST_THREAD_ID = Thread.currentThread().getId();
+
+ /**
+ * Run this before the tests once, to note down some work in the
+ * constructor thread to be verified later on in a test.
+ */
+ @BeforeClass
+ public static void beforeClass() throws Exception {
+ // Do some work in constructor thread.
+ S3AFileSystem fs = new S3AFileSystem();
+ Configuration conf = new Configuration();
+ fs.initialize(new URI(conf.get(TEST_FS_S3A_NAME)), conf);
+ Path path = new Path("testConstructor");
+ try (FSDataOutputStream out = fs.create(path)) {
+ out.write('a');
+ }
+ try (FSDataInputStream in = fs.open(path)) {
+ in.read();
+ }
+ }
+
+ @Override
+ protected Configuration createConfiguration() {
+ Configuration configuration = super.createConfiguration();
+ removeBaseAndBucketOverrides(configuration,
+ THREAD_LEVEL_IOSTATISTICS_ENABLED);
+ return configuration;
+ }
+
+ /**
+ * Verify that S3AInputStream aggregates per thread IOStats collection
+ * correctly.
+ */
+ @Test
+ public void testS3AInputStreamIOStatisticsContext()
+ throws Exception {
+ S3AFileSystem fs = getFileSystem();
+ Path path = path(getMethodName());
+ byte[] data = dataset(256, 'a', 'z');
+ byte[] readDataFirst = new byte[BYTES_BIG];
+ byte[] readDataSecond = new byte[BYTES_SMALL];
+ writeDataset(fs, path, data, data.length, 1024, true);
+
+ final ExecutorService executor =
+ HadoopExecutors.newFixedThreadPool(SMALL_THREADS);
+ CountDownLatch latch = new CountDownLatch(SMALL_THREADS);
+
+ try {
+ for (int i = 0; i < SMALL_THREADS; i++) {
+ executor.submit(() -> {
+ try {
+ IOStatistics ioStatisticsFirst;
+ try (FSDataInputStream in = fs.open(path)) {
+ in.seek(50);
+ in.read(readDataFirst);
+ in.close();
+ ioStatisticsFirst = assertThreadStatisticsBytesRead(in,
+ BYTES_BIG);
+ }
+ // Stream is closed for a thread. Re-open and do more operations.
+ IOStatistics ioStatisticsSecond;
+ try (FSDataInputStream in = fs.open(path)) {
+ in.seek(100);
+ in.read(readDataSecond);
+ in.close();
+ ioStatisticsSecond = assertThreadStatisticsBytesRead(in,
+ BYTES_BIG + BYTES_SMALL);
+ }
+ latch.countDown();
+ } catch (Exception e) {
+ latch.countDown();
+ setFutureException(e);
+ LOG.error("An error occurred while doing a task in the thread", e);
+ } catch (AssertionError ase) {
+ latch.countDown();
+ setFutureAse(ase);
+ throw ase;
+ }
+ });
+ }
+ // wait for tasks to finish.
+ latch.await();
+ } finally {
+ executor.shutdown();
+ }
+
+ // Check if an Excp or ASE was caught while the test threads were running.
+ maybeReThrowFutureException();
+ maybeReThrowFutureASE();
+
+ }
+
+ /**
+ * Verify that S3ABlockOutputStream aggregates per thread IOStats collection
+ * correctly.
+ */
+ @Test
+ public void testS3ABlockOutputStreamIOStatisticsContext()
+ throws Exception {
+ S3AFileSystem fs = getFileSystem();
+ Path path = path(getMethodName());
+ byte[] writeDataFirst = new byte[BYTES_BIG];
+ byte[] writeDataSecond = new byte[BYTES_SMALL];
+
+ final ExecutorService executor =
+ HadoopExecutors.newFixedThreadPool(SMALL_THREADS);
+ CountDownLatch latch = new CountDownLatch(SMALL_THREADS);
+
+ try {
+ for (int i = 0; i < SMALL_THREADS; i++) {
+ executor.submit(() -> {
+ try {
+ IOStatistics ioStatisticsFirst;
+ try (FSDataOutputStream out = fs.create(path)) {
+ out.write(writeDataFirst);
+ out.close();
+ ioStatisticsFirst = assertThreadStatisticsBytesWrite(out,
+ BYTES_BIG);
+ }
+ // Stream is closed for a thread. Re-open and do more operations.
+ IOStatistics ioStatisticsSecond;
+ try (FSDataOutputStream out = fs.create(path)) {
+ out.write(writeDataSecond);
+ out.close();
+ ioStatisticsSecond = assertThreadStatisticsBytesWrite(out,
+ BYTES_BIG + BYTES_SMALL);
+ }
+ latch.countDown();
+ } catch (Exception e) {
+ latch.countDown();
+ setFutureException(e);
+ LOG.error("An error occurred while doing a task in the thread", e);
+ } catch (AssertionError ase) {
+ latch.countDown();
+ setFutureAse(ase);
+ throw ase;
+ }
+ });
+ }
+ // wait for tasks to finish.
+ latch.await();
+ } finally {
+ executor.shutdown();
+ }
+
+ // Check if an Excp or ASE was caught while the test threads were running.
+ maybeReThrowFutureException();
+ maybeReThrowFutureASE();
+
+ }
+
+ /**
+ * Verify stats collection and aggregation for constructor thread, Junit
+ * thread and a worker thread.
+ */
+ @Test
+ public void testThreadIOStatisticsForDifferentThreads()
+ throws IOException, InterruptedException {
+ S3AFileSystem fs = getFileSystem();
+ Path path = path(getMethodName());
+ byte[] data = new byte[BYTES_BIG];
+ long threadIdForTest = Thread.currentThread().getId();
+
+ // Write in the Junit thread.
+ try (FSDataOutputStream out = fs.create(path)) {
+ out.write(data);
+ }
+
+ // Read in the Junit thread.
+ try (FSDataInputStream in = fs.open(path)) {
+ in.read(data);
+ }
+
+ // Worker thread work and wait for it to finish.
+ TestWorkerThread workerThread = new TestWorkerThread();
+ long workerThreadID = workerThread.getId();
+ workerThread.start();
+ workerThread.join();
+
+ // Work done in constructor: Wrote and Read 1 byte.
+ // Work done in Junit thread: Wrote and Read BYTES_BIG bytes.
+ // Work done in Junit's worker thread: Wrote and Read BYTES_SMALL bytes.
+ assertThreadStatisticsForThread(TEST_THREAD_ID, 1);
+ assertThreadStatisticsForThread(threadIdForTest, BYTES_BIG);
+ assertThreadStatisticsForThread(workerThreadID, BYTES_SMALL);
+
+ }
+
+ /**
+ * Assert bytes wrote by the current thread.
+ *
+ * @param out OutputStream.
+ * @param writeBytes expected bytes.
+ * @return IOStatistics for this stream.
+ */
+ private IOStatistics assertThreadStatisticsBytesWrite(FSDataOutputStream out,
+ int writeBytes) {
+ S3ABlockOutputStream s3aOut = (S3ABlockOutputStream) out.getWrappedStream();
+ IOStatistics ioStatistics =
+ (IOStatisticsSnapshot) s3aOut.getThreadIOStatistics();
+ IOStatisticAssertions.assertThatStatisticCounter(ioStatistics,
+ StreamStatisticNames.STREAM_WRITE_BYTES)
+ .describedAs("Bytes wrote are not as expected")
+ .isEqualTo(writeBytes);
+
+ return ioStatistics;
+ }
+
+ /**
+ * Assert bytes read by the current thread.
+ *
+ * @param in InputStream.
+ * @param readBytes expected bytes.
+ * @return IOStatistics for this stream.
+ */
+ private IOStatistics assertThreadStatisticsBytesRead(FSDataInputStream in,
+ int readBytes) {
+ S3AInputStream s3AInputStream =
+ (S3AInputStream) in.getWrappedStream();
+ IOStatistics ioStatistics = s3AInputStream.getThreadIOStatistics();
+ IOStatisticAssertions.assertThatStatisticCounter(ioStatistics,
+ StreamStatisticNames.STREAM_READ_BYTES)
+ .describedAs("Bytes read are not as expected")
+ .isEqualTo(readBytes);
+
+ return ioStatistics;
+ }
+
+ /**
+ * Assert fixed bytes wrote and read for a particular thread ID.
+ *
+ * @param testThreadId thread ID.
+ * @param expectedBytesWrittenAndRead expected bytes.
+ */
+ private void assertThreadStatisticsForThread(long testThreadId,
+ int expectedBytesWrittenAndRead) {
+ LOG.info("Thread ID to be asserted: {}", testThreadId);
+ IOStatistics ioStatistics =
+ currentIOStatisticsContext().getThreadIOStatistics(testThreadId);
+
+ IOStatisticAssertions.assertThatStatisticCounter(ioStatistics,
+ StreamStatisticNames.STREAM_WRITE_BYTES)
+ .describedAs("Bytes wrote are not as expected for thread :{}",
+ testThreadId)
+ .isEqualTo(expectedBytesWrittenAndRead);
+
+ IOStatisticAssertions.assertThatStatisticCounter(ioStatistics,
+ StreamStatisticNames.STREAM_READ_BYTES)
+ .describedAs("Bytes read are not as expected for thread :{}",
+ testThreadId)
+ .isEqualTo(expectedBytesWrittenAndRead);
+ }
+
+ /**
+ * Simulating doing some work in a separate thread.
+ */
+ private class TestWorkerThread extends Thread implements Runnable {
+ @Override
+ public void run() {
+ S3AFileSystem fs = getFileSystem();
+ Path path = new Path("workerThread");
Review Comment:
make path a field, pass in the methodpath in its constructor. i want to be able to run multiple hadoop-aws test runs in the same bucket ASAP.
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContext.java:
##########
@@ -0,0 +1,196 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.statistics.impl;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.impl.WeakReferenceThreadMap;
+import org.apache.hadoop.fs.statistics.IOStatistics;
+import org.apache.hadoop.fs.statistics.IOStatisticsAggregator;
+import org.apache.hadoop.fs.statistics.IOStatisticsSnapshot;
+
+import static org.apache.hadoop.fs.CommonConfigurationKeys.THREAD_LEVEL_IOSTATISTICS_ENABLED;
+import static org.apache.hadoop.fs.CommonConfigurationKeys.THREAD_LEVEL_IOSTATISTICS_ENABLED_DEFAULT;
+
+/**
+ * Implementing the IOStatisticsContext interface.
+ *
+ * A Context defined for IOStatistics collection per thread which captures
+ * each worker thread's work in FS streams and stores it in the form of
+ * IOStatisticsAggregator which could be either IOStatisticsSnapshot or
+ * EmptyIOStatisticsStore if thread level aggregation is enabled or not for
+ * the FS. An active instance of the IOStatisticsContext can be used to
+ * collect the statistics.
+ *
+ * For the current thread the IOStatisticsSnapshot can be used as a way to
+ * move the IOStatistics data between applications using the Serializable
+ * nature of the class.
+ */
+public class IOStatisticsContext {
+ private static final Logger LOG =
+ LoggerFactory.getLogger(IOStatisticsContext.class);
+ private static final boolean IS_THREAD_IOSTATS_ENABLED;
+
+ private static final WeakReferenceThreadMap<IOStatisticsContext>
+ ACTIVE_IOSTATS_CONTEXT = new WeakReferenceThreadMap<>(
+ IOStatisticsContext::createNewInstance,
+ IOStatisticsContext::referenceLostContext
+ );
+
+ /**
+ * Collecting IOStatistics per thread.
+ */
+ private final WeakReferenceThreadMap<IOStatisticsAggregator>
+ threadIOStatsContext = new WeakReferenceThreadMap<>(
+ this::getIOStatisticsAggregatorFactory,
Review Comment:
same; indent the args to the constructor
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3ABlockOutputStream.java:
##########
@@ -41,6 +41,8 @@
import com.amazonaws.services.s3.model.UploadPartRequest;
import org.apache.hadoop.fs.s3a.impl.PutObjectOptions;
+import org.apache.hadoop.classification.VisibleForTesting;
Review Comment:
nit, should go above L43
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AInputStream.java:
##########
@@ -37,6 +37,8 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import org.apache.hadoop.fs.statistics.IOStatisticsAggregator;
Review Comment:
move down to the "real" hadoop imports are
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/impl/IOStatisticsContext.java:
##########
@@ -0,0 +1,196 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.statistics.impl;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.impl.WeakReferenceThreadMap;
+import org.apache.hadoop.fs.statistics.IOStatistics;
+import org.apache.hadoop.fs.statistics.IOStatisticsAggregator;
+import org.apache.hadoop.fs.statistics.IOStatisticsSnapshot;
+
+import static org.apache.hadoop.fs.CommonConfigurationKeys.THREAD_LEVEL_IOSTATISTICS_ENABLED;
+import static org.apache.hadoop.fs.CommonConfigurationKeys.THREAD_LEVEL_IOSTATISTICS_ENABLED_DEFAULT;
+
+/**
+ * Implementing the IOStatisticsContext interface.
+ *
+ * A Context defined for IOStatistics collection per thread which captures
+ * each worker thread's work in FS streams and stores it in the form of
+ * IOStatisticsAggregator which could be either IOStatisticsSnapshot or
+ * EmptyIOStatisticsStore if thread level aggregation is enabled or not for
+ * the FS. An active instance of the IOStatisticsContext can be used to
+ * collect the statistics.
+ *
+ * For the current thread the IOStatisticsSnapshot can be used as a way to
+ * move the IOStatistics data between applications using the Serializable
+ * nature of the class.
+ */
+public class IOStatisticsContext {
+ private static final Logger LOG =
+ LoggerFactory.getLogger(IOStatisticsContext.class);
+ private static final boolean IS_THREAD_IOSTATS_ENABLED;
+
+ private static final WeakReferenceThreadMap<IOStatisticsContext>
+ ACTIVE_IOSTATS_CONTEXT = new WeakReferenceThreadMap<>(
+ IOStatisticsContext::createNewInstance,
+ IOStatisticsContext::referenceLostContext
+ );
+
+ /**
+ * Collecting IOStatistics per thread.
+ */
+ private final WeakReferenceThreadMap<IOStatisticsAggregator>
+ threadIOStatsContext = new WeakReferenceThreadMap<>(
+ this::getIOStatisticsAggregatorFactory,
+ this::referenceLost);
+
+ static {
+ // Work out if the current context have thread level IOStatistics enabled.
+ final Configuration configuration = new Configuration();
+ IS_THREAD_IOSTATS_ENABLED =
+ configuration.getBoolean(THREAD_LEVEL_IOSTATISTICS_ENABLED,
+ THREAD_LEVEL_IOSTATISTICS_ENABLED_DEFAULT);
+ }
+
+ /**
+ * Creating a new IOStatisticsContext instance for a FS to be used.
+ *
+ * @param key Thread ID that represents which thread the context belongs to.
+ * @return an instance of IOStatisticsContext.
+ */
+ private static IOStatisticsContext createNewInstance(Long key) {
+ return new IOStatisticsContext();
+ }
+
+ /**
+ * Get the current IOStatisticsContext.
+ *
+ * @return current IOStatisticsContext instance.
+ */
+ public static IOStatisticsContext currentIOStatisticsContext() {
+ return ACTIVE_IOSTATS_CONTEXT.get(Thread.currentThread().getId());
+ }
+
+ /**
+ * A Method to act as an IOStatisticsSnapshot factory, in a
+ * WeakReferenceThreadMap.
+ *
+ * @param key ThreadID.
+ * @return an Instance of IOStatisticsSnapshot.
+ */
+ private IOStatisticsAggregator getIOStatisticsAggregatorFactory(Long key) {
+ return getThreadIOStatistics();
+ }
+
+ /**
+ * In case of reference loss.
+ *
+ * @param key ThreadID.
+ */
+ private void referenceLost(Long key) {
+ LOG.debug("Reference lost for threadID: {}", key);
+ }
+
+ /**
+ * In case of reference loss for IOStatisticsContext.
+ *
+ * @param key ThreadID.
+ */
+ private static void referenceLostContext(Long key) {
+ LOG.debug("Reference lost for threadID for the context: {}", key);
+ }
+
+ /**
+ * A Method to get the IOStatisticsAggregator of the currentThread. This
+ * denotes the aggregated IOStatistics per thread.
+ *
+ * @return the instance of IOStatisticsAggregator for the current thread.
+ */
+ public IOStatisticsAggregator getThreadIOStatistics() {
+ // If Thread IOStats is disabled we return an emptyIOStatistics instance
+ // back.
+ if (!IS_THREAD_IOSTATS_ENABLED) {
+ return EmptyIOStatisticsStore.getInstance();
+ }
+
+ // If the current Thread ID already have an aggregator assigned, return
+ // that.
+ boolean isThreadIOStatsPresent =
+ threadIOStatsContext.containsKey(Thread.currentThread().getId());
+ if (isThreadIOStatsPresent) {
+ return threadIOStatsContext.getForCurrentThread();
+ }
+
+ // If no aggregator is defined to the thread ID, create one and assign it.
+ IOStatisticsSnapshot ioStatisticsSnapshot = new IOStatisticsSnapshot();
+ setThreadIOStatistics(ioStatisticsSnapshot);
+ return ioStatisticsSnapshot;
+ }
+
+ /**
+ * Set the IOStatisticsAggregator for the current context for a specific
+ * thread.
+ *
+ * @param ioStatisticsAggregator IOStatisticsAggregator instance for the
+ * current thread.
+ */
+ public void setThreadIOStatistics(
+ IOStatisticsAggregator ioStatisticsAggregator) {
+ threadIOStatsContext.setForCurrentThread(ioStatisticsAggregator);
+ }
+
+ /**
+ * Returns a snapshot of the current thread's IOStatistics.
+ *
+ * @return IOStatisticsSnapshot of the current thread.
+ */
+ public IOStatisticsSnapshot getThreadIOStatisticsSnapshot() {
Review Comment:
how does this get used. as it is not a snapshot if another thread is actively updating the thread.
if this is to be used to get a copy of the stats, we should call the method `snapshotCurrentThreadIOStatistics()` and always return a snapshot, either an empty one or one off the current thread's stats.
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AIOStatisticsContext.java:
##########
@@ -0,0 +1,332 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.s3a;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.net.URI;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FSDataInputStream;
+import org.apache.hadoop.fs.FSDataOutputStream;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.statistics.IOStatisticAssertions;
+import org.apache.hadoop.fs.statistics.IOStatistics;
+import org.apache.hadoop.fs.statistics.IOStatisticsSnapshot;
+import org.apache.hadoop.fs.statistics.StreamStatisticNames;
+import org.apache.hadoop.util.concurrent.HadoopExecutors;
+
+import static org.apache.hadoop.fs.CommonConfigurationKeys.THREAD_LEVEL_IOSTATISTICS_ENABLED;
+import static org.apache.hadoop.fs.contract.ContractTestUtils.dataset;
+import static org.apache.hadoop.fs.contract.ContractTestUtils.writeDataset;
+import static org.apache.hadoop.fs.s3a.S3ATestUtils.removeBaseAndBucketOverrides;
+import static org.apache.hadoop.fs.statistics.impl.IOStatisticsContext.currentIOStatisticsContext;
+
+/**
+ * Tests to verify the Thread-level IOStatistics.
+ */
+public class ITestS3AIOStatisticsContext extends AbstractS3ATestBase {
+
+ private static final int SMALL_THREADS = 2;
+ private static final int BYTES_BIG = 100;
+ private static final int BYTES_SMALL = 50;
+ private static final long TEST_THREAD_ID = Thread.currentThread().getId();
+
+ /**
+ * Run this before the tests once, to note down some work in the
+ * constructor thread to be verified later on in a test.
+ */
+ @BeforeClass
+ public static void beforeClass() throws Exception {
+ // Do some work in constructor thread.
+ S3AFileSystem fs = new S3AFileSystem();
+ Configuration conf = new Configuration();
+ fs.initialize(new URI(conf.get(TEST_FS_S3A_NAME)), conf);
+ Path path = new Path("testConstructor");
+ try (FSDataOutputStream out = fs.create(path)) {
+ out.write('a');
+ }
+ try (FSDataInputStream in = fs.open(path)) {
+ in.read();
+ }
+ }
+
+ @Override
+ protected Configuration createConfiguration() {
+ Configuration configuration = super.createConfiguration();
+ removeBaseAndBucketOverrides(configuration,
+ THREAD_LEVEL_IOSTATISTICS_ENABLED);
+ return configuration;
+ }
+
+ /**
+ * Verify that S3AInputStream aggregates per thread IOStats collection
+ * correctly.
+ */
+ @Test
+ public void testS3AInputStreamIOStatisticsContext()
+ throws Exception {
+ S3AFileSystem fs = getFileSystem();
+ Path path = path(getMethodName());
+ byte[] data = dataset(256, 'a', 'z');
+ byte[] readDataFirst = new byte[BYTES_BIG];
+ byte[] readDataSecond = new byte[BYTES_SMALL];
+ writeDataset(fs, path, data, data.length, 1024, true);
+
+ final ExecutorService executor =
+ HadoopExecutors.newFixedThreadPool(SMALL_THREADS);
+ CountDownLatch latch = new CountDownLatch(SMALL_THREADS);
+
+ try {
+ for (int i = 0; i < SMALL_THREADS; i++) {
+ executor.submit(() -> {
+ try {
+ IOStatistics ioStatisticsFirst;
+ try (FSDataInputStream in = fs.open(path)) {
+ in.seek(50);
+ in.read(readDataFirst);
+ in.close();
+ ioStatisticsFirst = assertThreadStatisticsBytesRead(in,
+ BYTES_BIG);
+ }
+ // Stream is closed for a thread. Re-open and do more operations.
+ IOStatistics ioStatisticsSecond;
+ try (FSDataInputStream in = fs.open(path)) {
+ in.seek(100);
+ in.read(readDataSecond);
+ in.close();
+ ioStatisticsSecond = assertThreadStatisticsBytesRead(in,
+ BYTES_BIG + BYTES_SMALL);
+ }
+ latch.countDown();
+ } catch (Exception e) {
+ latch.countDown();
+ setFutureException(e);
+ LOG.error("An error occurred while doing a task in the thread", e);
+ } catch (AssertionError ase) {
+ latch.countDown();
+ setFutureAse(ase);
+ throw ase;
+ }
+ });
+ }
+ // wait for tasks to finish.
+ latch.await();
+ } finally {
+ executor.shutdown();
+ }
+
+ // Check if an Excp or ASE was caught while the test threads were running.
+ maybeReThrowFutureException();
+ maybeReThrowFutureASE();
+
+ }
+
+ /**
+ * Verify that S3ABlockOutputStream aggregates per thread IOStats collection
+ * correctly.
+ */
+ @Test
+ public void testS3ABlockOutputStreamIOStatisticsContext()
+ throws Exception {
+ S3AFileSystem fs = getFileSystem();
+ Path path = path(getMethodName());
+ byte[] writeDataFirst = new byte[BYTES_BIG];
+ byte[] writeDataSecond = new byte[BYTES_SMALL];
+
+ final ExecutorService executor =
+ HadoopExecutors.newFixedThreadPool(SMALL_THREADS);
+ CountDownLatch latch = new CountDownLatch(SMALL_THREADS);
+
+ try {
+ for (int i = 0; i < SMALL_THREADS; i++) {
+ executor.submit(() -> {
+ try {
+ IOStatistics ioStatisticsFirst;
+ try (FSDataOutputStream out = fs.create(path)) {
+ out.write(writeDataFirst);
+ out.close();
+ ioStatisticsFirst = assertThreadStatisticsBytesWrite(out,
+ BYTES_BIG);
+ }
+ // Stream is closed for a thread. Re-open and do more operations.
+ IOStatistics ioStatisticsSecond;
+ try (FSDataOutputStream out = fs.create(path)) {
+ out.write(writeDataSecond);
+ out.close();
+ ioStatisticsSecond = assertThreadStatisticsBytesWrite(out,
+ BYTES_BIG + BYTES_SMALL);
+ }
+ latch.countDown();
+ } catch (Exception e) {
+ latch.countDown();
+ setFutureException(e);
+ LOG.error("An error occurred while doing a task in the thread", e);
+ } catch (AssertionError ase) {
+ latch.countDown();
+ setFutureAse(ase);
+ throw ase;
+ }
+ });
+ }
+ // wait for tasks to finish.
+ latch.await();
+ } finally {
+ executor.shutdown();
+ }
+
+ // Check if an Excp or ASE was caught while the test threads were running.
+ maybeReThrowFutureException();
+ maybeReThrowFutureASE();
+
+ }
+
+ /**
+ * Verify stats collection and aggregation for constructor thread, Junit
+ * thread and a worker thread.
+ */
+ @Test
+ public void testThreadIOStatisticsForDifferentThreads()
+ throws IOException, InterruptedException {
+ S3AFileSystem fs = getFileSystem();
+ Path path = path(getMethodName());
+ byte[] data = new byte[BYTES_BIG];
+ long threadIdForTest = Thread.currentThread().getId();
+
+ // Write in the Junit thread.
+ try (FSDataOutputStream out = fs.create(path)) {
+ out.write(data);
+ }
+
+ // Read in the Junit thread.
+ try (FSDataInputStream in = fs.open(path)) {
+ in.read(data);
+ }
+
+ // Worker thread work and wait for it to finish.
+ TestWorkerThread workerThread = new TestWorkerThread();
+ long workerThreadID = workerThread.getId();
+ workerThread.start();
+ workerThread.join();
+
+ // Work done in constructor: Wrote and Read 1 byte.
+ // Work done in Junit thread: Wrote and Read BYTES_BIG bytes.
+ // Work done in Junit's worker thread: Wrote and Read BYTES_SMALL bytes.
+ assertThreadStatisticsForThread(TEST_THREAD_ID, 1);
+ assertThreadStatisticsForThread(threadIdForTest, BYTES_BIG);
+ assertThreadStatisticsForThread(workerThreadID, BYTES_SMALL);
+
+ }
+
+ /**
+ * Assert bytes wrote by the current thread.
+ *
+ * @param out OutputStream.
+ * @param writeBytes expected bytes.
+ * @return IOStatistics for this stream.
+ */
+ private IOStatistics assertThreadStatisticsBytesWrite(FSDataOutputStream out,
+ int writeBytes) {
+ S3ABlockOutputStream s3aOut = (S3ABlockOutputStream) out.getWrappedStream();
+ IOStatistics ioStatistics =
+ (IOStatisticsSnapshot) s3aOut.getThreadIOStatistics();
+ IOStatisticAssertions.assertThatStatisticCounter(ioStatistics,
+ StreamStatisticNames.STREAM_WRITE_BYTES)
+ .describedAs("Bytes wrote are not as expected")
+ .isEqualTo(writeBytes);
+
+ return ioStatistics;
+ }
+
+ /**
+ * Assert bytes read by the current thread.
+ *
+ * @param in InputStream.
+ * @param readBytes expected bytes.
+ * @return IOStatistics for this stream.
+ */
+ private IOStatistics assertThreadStatisticsBytesRead(FSDataInputStream in,
+ int readBytes) {
+ S3AInputStream s3AInputStream =
+ (S3AInputStream) in.getWrappedStream();
+ IOStatistics ioStatistics = s3AInputStream.getThreadIOStatistics();
+ IOStatisticAssertions.assertThatStatisticCounter(ioStatistics,
+ StreamStatisticNames.STREAM_READ_BYTES)
+ .describedAs("Bytes read are not as expected")
+ .isEqualTo(readBytes);
+
+ return ioStatistics;
+ }
+
+ /**
+ * Assert fixed bytes wrote and read for a particular thread ID.
+ *
+ * @param testThreadId thread ID.
+ * @param expectedBytesWrittenAndRead expected bytes.
+ */
+ private void assertThreadStatisticsForThread(long testThreadId,
+ int expectedBytesWrittenAndRead) {
+ LOG.info("Thread ID to be asserted: {}", testThreadId);
+ IOStatistics ioStatistics =
+ currentIOStatisticsContext().getThreadIOStatistics(testThreadId);
+
+ IOStatisticAssertions.assertThatStatisticCounter(ioStatistics,
+ StreamStatisticNames.STREAM_WRITE_BYTES)
+ .describedAs("Bytes wrote are not as expected for thread :{}",
Review Comment:
nit "written"
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java:
##########
@@ -124,13 +124,15 @@
import org.apache.hadoop.fs.s3a.impl.StatusProbeEnum;
import org.apache.hadoop.fs.s3a.impl.StoreContext;
import org.apache.hadoop.fs.s3a.impl.StoreContextBuilder;
+import org.apache.hadoop.fs.statistics.IOStatisticsAggregator;
Review Comment:
nit, move down to L133
--
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
[GitHub] [hadoop] steveloughran commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
steveloughran commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1195545593
+1 pending yetus
--
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
[GitHub] [hadoop] steveloughran merged pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
steveloughran merged PR #4352:
URL: https://github.com/apache/hadoop/pull/4352
--
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
[GitHub] [hadoop] hadoop-yetus commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1188844927
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 41s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 5 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 44m 47s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 27m 25s | | trunk passed |
| +1 :green_heart: | compile | 25m 19s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | compile | 22m 3s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 4m 15s | | trunk passed |
| +1 :green_heart: | mvnsite | 3m 24s | | trunk passed |
| +1 :green_heart: | javadoc | 2m 38s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 2m 23s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 5m 11s | | trunk passed |
| +1 :green_heart: | shadedclient | 24m 41s | | branch has no errors when building and testing our client artifacts. |
| -0 :warning: | patch | 25m 12s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 33s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 54s | | the patch passed |
| +1 :green_heart: | compile | 24m 52s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javac | 24m 52s | | the patch passed |
| +1 :green_heart: | compile | 22m 57s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 22m 57s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 4m 2s | | the patch passed |
| +1 :green_heart: | mvnsite | 3m 35s | | the patch passed |
| +1 :green_heart: | javadoc | 2m 44s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 2m 24s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 5m 25s | | the patch passed |
| +1 :green_heart: | shadedclient | 23m 19s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 19m 30s | | hadoop-common in the patch passed. |
| +1 :green_heart: | unit | 3m 16s | | hadoop-aws in the patch passed. |
| +1 :green_heart: | asflicense | 1m 32s | | The patch does not generate ASF License warnings. |
| | | 283m 38s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/10/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/4352 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 2607ccafc906 4.15.0-169-generic #177-Ubuntu SMP Thu Feb 3 10:50:38 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / f54ad18a9e2230191dc4c0c504d332772bceb23e |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/10/testReport/ |
| Max. process+thread count | 1251 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/10/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hadoop] steveloughran commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
steveloughran commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1182217571
see https://github.com/steveloughran/spark/tree/HADOOP-17461-iostatistics
retrieval
https://github.com/steveloughran/spark/blob/HADOOP-17461-iostatistics/core/src/main/scala/org/apache/spark/deploy/SparkHadoopUtil.scala#L211
task metrics
https://github.com/steveloughran/spark/blob/HADOOP-17461-iostatistics/core/src/main/scala/org/apache/spark/executor/TaskMetrics.scala
and where we'd be doing the collection for the spark RDDs (not the sql code, which is more significant but harder to get started on)
https://github.com/steveloughran/spark/blob/HADOOP-17461-iostatistics/core/src/main/scala/org/apache/spark/rdd/NewHadoopRDD.scala#L172
--
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
[GitHub] [hadoop] hadoop-yetus commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1186951915
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 39s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 1s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 5 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 14m 44s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 25m 3s | | trunk passed |
| +1 :green_heart: | compile | 23m 8s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | compile | 20m 34s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 4m 25s | | trunk passed |
| +1 :green_heart: | mvnsite | 3m 34s | | trunk passed |
| +1 :green_heart: | javadoc | 2m 58s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 2m 43s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 5m 3s | | trunk passed |
| +1 :green_heart: | shadedclient | 22m 9s | | branch has no errors when building and testing our client artifacts. |
| -0 :warning: | patch | 22m 40s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 32s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 45s | | the patch passed |
| +1 :green_heart: | compile | 23m 8s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javac | 23m 8s | | the patch passed |
| +1 :green_heart: | compile | 21m 52s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 21m 52s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 4m 17s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/9/artifact/out/results-checkstyle-root.txt) | root: The patch generated 5 new + 76 unchanged - 0 fixed = 81 total (was 76) |
| +1 :green_heart: | mvnsite | 3m 35s | | the patch passed |
| +1 :green_heart: | javadoc | 2m 51s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 2m 43s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 5m 11s | | the patch passed |
| +1 :green_heart: | shadedclient | 22m 45s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 18m 59s | | hadoop-common in the patch passed. |
| +1 :green_heart: | unit | 3m 19s | | hadoop-aws in the patch passed. |
| +1 :green_heart: | asflicense | 1m 30s | | The patch does not generate ASF License warnings. |
| | | 242m 55s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/9/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/4352 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 6932bdb96253 4.15.0-169-generic #177-Ubuntu SMP Thu Feb 3 10:50:38 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / bf84de68fd815ca9cc2f33f830a81f1c3a0263c0 |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/9/testReport/ |
| Max. process+thread count | 1282 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/9/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hadoop] hadoop-yetus commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1190851834
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 46s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 1s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 6 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 15m 31s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 25m 34s | | trunk passed |
| +1 :green_heart: | compile | 23m 11s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | compile | 20m 51s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 4m 21s | | trunk passed |
| +1 :green_heart: | mvnsite | 3m 42s | | trunk passed |
| +1 :green_heart: | javadoc | 2m 50s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 2m 33s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 5m 2s | | trunk passed |
| +1 :green_heart: | shadedclient | 23m 52s | | branch has no errors when building and testing our client artifacts. |
| -0 :warning: | patch | 24m 24s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 35s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 45s | | the patch passed |
| +1 :green_heart: | compile | 22m 24s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javac | 22m 24s | | the patch passed |
| +1 :green_heart: | compile | 20m 50s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 20m 50s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 4m 19s | | the patch passed |
| +1 :green_heart: | mvnsite | 3m 42s | | the patch passed |
| +1 :green_heart: | javadoc | 2m 44s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 2m 41s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 5m 12s | | the patch passed |
| +1 :green_heart: | shadedclient | 22m 19s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 18m 51s | | hadoop-common in the patch passed. |
| +1 :green_heart: | unit | 3m 20s | | hadoop-aws in the patch passed. |
| +1 :green_heart: | asflicense | 1m 39s | | The patch does not generate ASF License warnings. |
| | | 244m 18s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/12/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/4352 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 747c1495282e 4.15.0-169-generic #177-Ubuntu SMP Thu Feb 3 10:50:38 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 7b3c3356cd52f5378c4d68b6ddfed548d074ddf6 |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/12/testReport/ |
| Max. process+thread count | 3158 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/12/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hadoop] hadoop-yetus commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1136252339
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 44s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 3 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 14m 24s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 27m 11s | | trunk passed |
| +1 :green_heart: | compile | 23m 2s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | compile | 20m 28s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 4m 24s | | trunk passed |
| +1 :green_heart: | mvnsite | 3m 45s | | trunk passed |
| +1 :green_heart: | javadoc | 2m 59s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 2m 42s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 5m 2s | | trunk passed |
| +1 :green_heart: | shadedclient | 22m 5s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 35s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 46s | | the patch passed |
| +1 :green_heart: | compile | 22m 10s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javac | 22m 10s | | the patch passed |
| +1 :green_heart: | compile | 20m 38s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 20m 38s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 4m 15s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/1/artifact/out/results-checkstyle-root.txt) | root: The patch generated 3 new + 6 unchanged - 0 fixed = 9 total (was 6) |
| +1 :green_heart: | mvnsite | 3m 38s | | the patch passed |
| +1 :green_heart: | javadoc | 2m 53s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| -1 :x: | javadoc | 1m 19s | [/results-javadoc-javadoc-hadoop-tools_hadoop-aws-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/1/artifact/out/results-javadoc-javadoc-hadoop-tools_hadoop-aws-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07.txt) | hadoop-tools_hadoop-aws-jdkPrivateBuild-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 generated 2 new + 38 unchanged - 0 fixed = 40 total (was 38) |
| +1 :green_heart: | spotbugs | 5m 12s | | the patch passed |
| +1 :green_heart: | shadedclient | 22m 9s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 18m 30s | | hadoop-common in the patch passed. |
| +1 :green_heart: | unit | 3m 2s | | hadoop-aws in the patch passed. |
| +1 :green_heart: | asflicense | 1m 37s | | The patch does not generate ASF License warnings. |
| | | 241m 16s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/4352 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell |
| uname | Linux 10f5099454c2 4.15.0-169-generic #177-Ubuntu SMP Thu Feb 3 10:50:38 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / cedc0a9b7a47ca9434fc73ffc915ae1209f9499f |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/1/testReport/ |
| Max. process+thread count | 2283 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4352/1/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
This message was automatically generated.
--
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
[GitHub] [hadoop] mehakmeet commented on pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
mehakmeet commented on PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#issuecomment-1157264920
Thanks for the review @steveloughran, sorry couldn't address anything until now(got little ill)
> main comment is that the thread's statistic aggregator should be fetched/stored in constructor, not in close
Got your point, so just one concern on that, should the IOStatisticsContext be a static instance in the S3AFileSystem, and we just pass on the iostatisticsAggregator to the streams, since we would still require the context in the streams to update the WeakReferenceThreadMap after the aggregation, right?
--
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
[GitHub] [hadoop] steveloughran commented on a diff in pull request #4352: HADOOP-17461. Thread-level IOStatistics in S3A
Posted by GitBox <gi...@apache.org>.
steveloughran commented on code in PR #4352:
URL: https://github.com/apache/hadoop/pull/4352#discussion_r900419091
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java:
##########
@@ -374,6 +376,16 @@ public class S3AFileSystem extends FileSystem implements StreamCapabilities,
*/
private ArnResource accessPoint;
+ /**
+ * Does the fs have thread-level IOStats support enabled?
+ */
+ private boolean isThreadLevelIOStatsEnabled;
Review Comment:
as discussed, this should go into hadoop common
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3ABlockOutputStream.java:
##########
@@ -444,6 +450,9 @@ public void close() throws IOException {
*/
private synchronized void cleanupOnClose() {
cleanupWithLogger(LOG, getActiveBlock(), blockFactory);
+ if(ioStatisticsContext != null) {
Review Comment:
nit, add a space
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/test/LambdaTestUtils.java:
##########
@@ -65,6 +66,15 @@ private LambdaTestUtils() {
*/
public static final String NULL_RESULT = "(null)";
+ /**
+ * Atomic references to be used to re-throw an Exception or an ASE
+ * caught inside a lambda function.
+ */
+ public static final AtomicReference<Exception> futureExcp =
Review Comment:
yes
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/statistics/IOStatisticsContext.java:
##########
@@ -0,0 +1,28 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.statistics;
+
+/**
+ * Context used to capture IOStatistics per thread in a task.
+ */
+public interface IOStatisticsContext {
+
+ IOStatisticsSnapshot getThreadIOStatistics();
Review Comment:
nit: javadocs
--
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