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 2021/03/01 14:13:20 UTC

[GitHub] [hadoop] mehakmeet opened a new pull request #2731: HADOOP-17471. ABFS to collect IOStatistics

mehakmeet opened a new pull request #2731:
URL: https://github.com/apache/hadoop/pull/2731


   Tested by: mvn -T 16 -Dparallel-tests=abfs clean verify
   Region: East US
   
   ```
   [INFO] Results:
   [INFO]
   [INFO] Tests run: 95, Failures: 0, Errors: 0, Skipped: 0
   ```
   
   ```
   [INFO] Results:
   [INFO]
   [ERROR] Errors:
   [ERROR]   ITestAzureBlobFileSystemCheckAccess.testCheckAccessForNonExistentFile »  Unexp...
   [ERROR]   ITestAzureBlobFileSystemCheckAccess.testFsActionALL:293->checkPrerequisites:306->setTestUserFs:88->checkIfConfigIsSet:319 » IllegalArgument
   [ERROR]   ITestAzureBlobFileSystemCheckAccess.testFsActionEXECUTE:218->checkPrerequisites:306->setTestUserFs:88->checkIfConfigIsSet:319 » IllegalArgument
   [ERROR]   ITestAzureBlobFileSystemCheckAccess.testFsActionNONE:204->checkPrerequisites:306->setTestUserFs:88->checkIfConfigIsSet:319 » IllegalArgument
   [ERROR]   ITestAzureBlobFileSystemCheckAccess.testFsActionREAD:233->checkPrerequisites:306->setTestUserFs:88->checkIfConfigIsSet:319 » IllegalArgument
   [ERROR]   ITestAzureBlobFileSystemCheckAccess.testFsActionREADEXECUTE:263->checkPrerequisites:306->setTestUserFs:88->checkIfConfigIsSet:319 » IllegalArgument
   [ERROR]   ITestAzureBlobFileSystemCheckAccess.testFsActionWRITE:248->checkPrerequisites:306->setTestUserFs:88->checkIfConfigIsSet:319 » IllegalArgument
   [ERROR]   ITestAzureBlobFileSystemCheckAccess.testFsActionWRITEEXECUTE:278->checkPrerequisites:306->setTestUserFs:88->checkIfConfigIsSet:319 » IllegalArgument
   [INFO]
   [ERROR] Tests run: 512, Failures: 0, Errors: 8, Skipped: 67
   ```
   Illegal argument due to not using oauth config for these tests.
   ```
   [INFO] Results:
   [INFO]
   [WARNING] Tests run: 256, Failures: 0, Errors: 0, Skipped: 77
   ```


----------------------------------------------------------------
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.

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 #2731: HADOOP-17471. ABFS to collect IOStatistics

Posted by GitBox <gi...@apache.org>.
mehakmeet commented on pull request #2731:
URL: https://github.com/apache/hadoop/pull/2731#issuecomment-817700378


   My mistake. I was translating IOException into RuntimeException when it was expected to throw IOException while setup. 


-- 
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.

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 change in pull request #2731: HADOOP-17471. ABFS to collect IOStatistics

Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #2731:
URL: https://github.com/apache/hadoop/pull/2731#discussion_r618495982



##########
File path: hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java
##########
@@ -167,12 +167,30 @@ String getSasToken() {
     this.abfsCounters = client.getAbfsCounters();
   }
 
+  /**
+   * Execute a AbfsRestOperation. Track the Duration of a request if
+   * abfsCounters isn't null.
+   *
+   */
+  public void execute() throws AzureBlobFileSystemException {
+
+    try {
+      IOStatisticsBinding.trackDurationOfInvocation(abfsCounters,
+          AbfsStatistic.getStatNameFromHttpCall(method),
+          () -> completeExecute());
+    } catch (AzureBlobFileSystemException aze) {
+      throw aze;
+    } catch (IOException e) {
+      throw new RuntimeException("Error while tracking Duration of an "

Review comment:
       change to UncheckedIOException; there's stuff in our functional package which can unwrap that down to an IOE as we always know the inner fault is a non-null IOE




-- 
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.

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 change in pull request #2731: HADOOP-17471. ABFS to collect IOStatistics

Posted by GitBox <gi...@apache.org>.
mehakmeet commented on a change in pull request #2731:
URL: https://github.com/apache/hadoop/pull/2731#discussion_r611715063



##########
File path: hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsOutputStreamStatistics.java
##########
@@ -241,10 +247,13 @@ public void testAbfsOutputStreamDurationTrackerPutRequest() throws IOException {
       outputStream.write('a');
       outputStream.hflush();
 
-      AbfsOutputStreamStatisticsImpl abfsOutputStreamStatistics =
-          getAbfsOutputStreamStatistics(outputStream);
-      LOG.info("AbfsOutputStreamStats info: {}", abfsOutputStreamStatistics.toString());
-      Assertions.assertThat(abfsOutputStreamStatistics.getTimeSpentOnPutRequest())
+      IOStatistics ioStatistics = extractStatistics(fs);
+      LOG.info("AbfsOutputStreamStats info: {}",
+          ioStatisticsToPrettyString(ioStatistics));
+      Assertions.assertThat(

Review comment:
       I am not able to add ```.isGreatherThanOrEqualTo(<SOME_DOUBLE>)``` after assertThatStatisticMean(), I guess because this is an ```ObjectAssert<MeanStatistic>``` rather than ```DoubleAssert```?




-- 
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.

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 change in pull request #2731: HADOOP-17471. ABFS to collect IOStatistics

Posted by GitBox <gi...@apache.org>.
mehakmeet commented on a change in pull request #2731:
URL: https://github.com/apache/hadoop/pull/2731#discussion_r611715063



##########
File path: hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsOutputStreamStatistics.java
##########
@@ -241,10 +247,13 @@ public void testAbfsOutputStreamDurationTrackerPutRequest() throws IOException {
       outputStream.write('a');
       outputStream.hflush();
 
-      AbfsOutputStreamStatisticsImpl abfsOutputStreamStatistics =
-          getAbfsOutputStreamStatistics(outputStream);
-      LOG.info("AbfsOutputStreamStats info: {}", abfsOutputStreamStatistics.toString());
-      Assertions.assertThat(abfsOutputStreamStatistics.getTimeSpentOnPutRequest())
+      IOStatistics ioStatistics = extractStatistics(fs);
+      LOG.info("AbfsOutputStreamStats info: {}",
+          ioStatisticsToPrettyString(ioStatistics));
+      Assertions.assertThat(

Review comment:
       I am not able to add ```.isGreatherThanOrEqualTo(<SOME_DOUBLE>)``` after assertThatStatisticMean(), I guess because this is an ObjectAssert<MeanStatistic> rather than DoubleAssert?




-- 
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.

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 #2731: HADOOP-17471. ABFS to collect IOStatistics

Posted by GitBox <gi...@apache.org>.
steveloughran commented on pull request #2731:
URL: https://github.com/apache/hadoop/pull/2731#issuecomment-825527782


   +1, merging. Thanks!


-- 
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.

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 #2731: HADOOP-17471. ABFS to collect IOStatistics

Posted by GitBox <gi...@apache.org>.
steveloughran merged pull request #2731:
URL: https://github.com/apache/hadoop/pull/2731


   


-- 
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.

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 change in pull request #2731: HADOOP-17471. ABFS to collect IOStatistics

Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #2731:
URL: https://github.com/apache/hadoop/pull/2731#discussion_r599105729



##########
File path: hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsOutputStream.java
##########
@@ -433,32 +430,29 @@ private synchronized void writeCurrentBufferToService(boolean isFlush, boolean i
       }
     }
     final Future<Void> job =
-        completionService.submit(IOStatisticsBinding
-            .trackDurationOfCallable((IOStatisticsStore) ioStatistics,
-                StreamStatisticNames.TIME_SPENT_ON_PUT_REQUEST,
-                () -> {
-                  AbfsPerfTracker tracker = client.getAbfsPerfTracker();
-                  try (AbfsPerfInfo perfInfo = new AbfsPerfInfo(tracker,
-                      "writeCurrentBufferToService", "append")) {
-                    AppendRequestParameters.Mode
-                        mode = APPEND_MODE;
-                    if (isFlush & isClose) {
-                      mode = FLUSH_CLOSE_MODE;
-                    } else if (isFlush) {
-                      mode = FLUSH_MODE;
-                    }
-                    AppendRequestParameters reqParams = new AppendRequestParameters(
-                        offset, 0, bytesLength, mode, false);
-                    AbfsRestOperation op = client.append(path, bytes, reqParams,
-                        cachedSasToken.get());
-                    cachedSasToken.update(op.getSasToken());
-                    perfInfo.registerResult(op.getResult());
-                    byteBufferPool.putBuffer(ByteBuffer.wrap(bytes));
-                    perfInfo.registerSuccess(true);
-                    return null;
-                  }
-                })
-        );
+        completionService.submit(() -> {

Review comment:
       It's not in use here, but org.apache.hadoop.util.SemaphoredDelegatingExecutor now takes a DurationTrackerFactory and measures the time between submission and execution -how much time we have to wait for space to actually launch the callback. 
   Not sure it would go in here right now, but it's why a standard DurationTrackerFactory API offers opportunities in future

##########
File path: hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/TestAbfsNetworkStatistics.java
##########
@@ -64,4 +82,60 @@ public void testAbfsThrottlingStatistics() throws IOException {
     assertAbfsStatistics(AbfsStatistic.WRITE_THROTTLES, LARGE_OPERATIONS,
         metricMap);
   }
+
+  /**
+   * Test to check if the DurationTrackers are tracking as expected whilst
+   * doing some work.
+   */
+  @Test
+  public void testAbfsNetworkDurationTrackers() throws IOException {
+    describe("Test to verify the actual values of DurationTrackers are "
+        + "greater than 0.0 while tracking some work.");
+
+    AbfsCounters abfsCounters = new AbfsCountersImpl(getFileSystem().getUri());
+    // Start dummy work for the DurationTrackers and start tracking.
+    try (DurationTracker ignoredPatch =
+        abfsCounters.startRequest(AbfsHttpConstants.HTTP_METHOD_PATCH);
+        DurationTracker ignoredPost =
+            abfsCounters.startRequest(AbfsHttpConstants.HTTP_METHOD_POST)
+    ) {
+      // Emulates doing some work.
+      Thread.sleep(10);
+      LOG.info("Execute some Http requests...");
+    } catch (InterruptedException e) {
+      throw new RuntimeException(
+          "Exception encountered while Thread tried to sleep", e);
+    }
+
+    // Extract the iostats from the abfsCounters instance.
+    IOStatistics ioStatistics = extractStatistics(abfsCounters);
+    // Asserting that the durationTrackers have mean > 0.0.
+    for (AbfsStatistic abfsStatistic : HTTP_DURATION_TRACKER_LIST) {
+      Assertions.assertThat(lookupMeanStatistic(ioStatistics,

Review comment:
       assertThatStatisticMean

##########
File path: hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsDurationTrackers.java
##########
@@ -0,0 +1,110 @@
+/**
+ * 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.azurebfs;
+
+import java.io.IOException;
+
+import org.assertj.core.api.Assertions;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.azurebfs.services.AbfsInputStream;
+import org.apache.hadoop.fs.azurebfs.services.AbfsOutputStream;
+import org.apache.hadoop.fs.statistics.IOStatistics;
+import org.apache.hadoop.fs.statistics.StoreStatisticNames;
+import org.apache.hadoop.io.IOUtils;
+
+import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.HTTP_DELETE_REQUEST;
+import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.HTTP_GET_REQUEST;
+import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.HTTP_HEAD_REQUEST;
+import static org.apache.hadoop.fs.azurebfs.AbfsStatistic.HTTP_PUT_REQUEST;
+import static org.apache.hadoop.fs.statistics.IOStatisticAssertions.extractStatistics;
+import static org.apache.hadoop.fs.statistics.IOStatisticAssertions.lookupMeanStatistic;
+import static org.apache.hadoop.fs.statistics.IOStatisticsLogging.ioStatisticsToPrettyString;
+
+public class ITestAbfsDurationTrackers extends AbstractAbfsIntegrationTest {
+
+  private static final Logger LOG =
+      LoggerFactory.getLogger(ITestAbfsDurationTrackers.class);
+  private static final AbfsStatistic[] HTTP_DURATION_TRACKER_LIST = {
+      HTTP_HEAD_REQUEST,
+      HTTP_GET_REQUEST,
+      HTTP_DELETE_REQUEST,
+      HTTP_PUT_REQUEST,
+  };
+
+  public ITestAbfsDurationTrackers() throws Exception {
+  }
+
+  /**
+   * Test to check if DurationTrackers for Abfs HTTP calls work correctly and
+   * track the duration of the http calls.
+   */
+  @Test
+  public void testAbfsHttpCallsDurations() throws IOException {
+    describe("test to verify if the DurationTrackers for abfs http calls "
+        + "work as expected.");
+
+    AzureBlobFileSystem fs = getFileSystem();
+    Path testFilePath = path(getMethodName());
+
+    // Declaring output and input stream.
+    AbfsOutputStream out = null;
+    AbfsInputStream in = null;
+    try {
+      // PUT the file.
+      out = createAbfsOutputStreamWithFlushEnabled(fs, testFilePath);
+      out.write('a');
+      out.hflush();
+
+      // GET the file.
+      in = fs.getAbfsStore().openFileForRead(testFilePath, fs.getFsStatistics());
+      int res = in.read();
+      LOG.info("Result of Read: {}", res);
+
+      // DELETE the file.
+      fs.delete(testFilePath, false);
+
+      // extract the IOStatistics from the filesystem.
+      IOStatistics ioStatistics = extractStatistics(fs);
+      LOG.info(ioStatisticsToPrettyString(ioStatistics));
+      assertDurationTracker(ioStatistics);
+    } finally {
+      IOUtils.cleanupWithLogger(LOG, out, in);
+    }
+  }
+
+  /**
+   * A method to assert that all the DurationTrackers for the http calls are
+   * working correctly.
+   *
+   * @param ioStatistics the IOStatisticsSource in use.
+   */
+  private void assertDurationTracker(IOStatistics ioStatistics) {

Review comment:
       nice test strategy.

##########
File path: hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsRestOperation.java
##########
@@ -167,12 +167,26 @@ String getSasToken() {
     this.abfsCounters = client.getAbfsCounters();
   }
 
+  /**
+   * Execute a AbfsRestOperation. Track the Duration of a request if
+   * abfsCounters isn't null.
+   *
+   */
+  public void execute() throws AzureBlobFileSystemException {
+    if (abfsCounters != null) {
+      try (DurationTracker ignored = abfsCounters.startRequest(method)) {

Review comment:
       Prefer IOStatisticsBinding.trackDuration as it records different counter/duration for success and failures. Helps split up the two outcomes which may have very different fail-fast/fail-slow performance.

##########
File path: hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsOutputStreamStatistics.java
##########
@@ -241,10 +247,13 @@ public void testAbfsOutputStreamDurationTrackerPutRequest() throws IOException {
       outputStream.write('a');
       outputStream.hflush();
 
-      AbfsOutputStreamStatisticsImpl abfsOutputStreamStatistics =
-          getAbfsOutputStreamStatistics(outputStream);
-      LOG.info("AbfsOutputStreamStats info: {}", abfsOutputStreamStatistics.toString());
-      Assertions.assertThat(abfsOutputStreamStatistics.getTimeSpentOnPutRequest())
+      IOStatistics ioStatistics = extractStatistics(fs);
+      LOG.info("AbfsOutputStreamStats info: {}",
+          ioStatisticsToPrettyString(ioStatistics));
+      Assertions.assertThat(

Review comment:
       again, can we use assertThatStatisticMean()?

##########
File path: hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsCounters.java
##########
@@ -25,13 +25,15 @@
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.classification.InterfaceStability;
 import org.apache.hadoop.fs.azurebfs.AbfsStatistic;
+import org.apache.hadoop.fs.statistics.DurationTracker;
+import org.apache.hadoop.fs.statistics.IOStatisticsSource;
 
 /**
  * An interface for Abfs counters.
  */
 @InterfaceAudience.Private
 @InterfaceStability.Unstable
-public interface AbfsCounters {
+public interface AbfsCounters extends IOStatisticsSource {

Review comment:
       If you implement DurationTrackerFactory then we've got a standard API which we can feed in to other things in future, including hadoop common stuff

##########
File path: hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/TestAbfsNetworkStatistics.java
##########
@@ -64,4 +82,60 @@ public void testAbfsThrottlingStatistics() throws IOException {
     assertAbfsStatistics(AbfsStatistic.WRITE_THROTTLES, LARGE_OPERATIONS,
         metricMap);
   }
+
+  /**
+   * Test to check if the DurationTrackers are tracking as expected whilst
+   * doing some work.
+   */
+  @Test
+  public void testAbfsNetworkDurationTrackers() throws IOException {
+    describe("Test to verify the actual values of DurationTrackers are "
+        + "greater than 0.0 while tracking some work.");
+
+    AbfsCounters abfsCounters = new AbfsCountersImpl(getFileSystem().getUri());
+    // Start dummy work for the DurationTrackers and start tracking.
+    try (DurationTracker ignoredPatch =
+        abfsCounters.startRequest(AbfsHttpConstants.HTTP_METHOD_PATCH);
+        DurationTracker ignoredPost =
+            abfsCounters.startRequest(AbfsHttpConstants.HTTP_METHOD_POST)
+    ) {
+      // Emulates doing some work.
+      Thread.sleep(10);
+      LOG.info("Execute some Http requests...");
+    } catch (InterruptedException e) {

Review comment:
       just add InterruptedException to the list of exceptions the test can throw

##########
File path: hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsInputStreamStatistics.java
##########
@@ -386,12 +392,13 @@ public void testActionHttpGetRequest() throws IOException {
       abfsInputStream =
           abfss.openFileForRead(actionHttpGetRequestPath, fs.getFsStatistics());
       abfsInputStream.read();
-      AbfsInputStreamStatisticsImpl abfsInputStreamStatistics =
-          (AbfsInputStreamStatisticsImpl) abfsInputStream.getStreamStatistics();
-
-      LOG.info("AbfsInputStreamStats info: {}", abfsInputStreamStatistics.toString());
+      IOStatistics ioStatistics = extractStatistics(fs);
+      LOG.info("AbfsInputStreamStats info: {}",
+          ioStatisticsToPrettyString(ioStatistics));
       Assertions.assertThat(
-          abfsInputStreamStatistics.getActionHttpGetRequest())
+          lookupMeanStatistic(ioStatistics,

Review comment:
       Can IOStatisticAssertions.assertThatStatisticMean help here? If not, that's something to improve...I want to make it easy to do effective assertions over stats




-- 
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.

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 change in pull request #2731: HADOOP-17471. ABFS to collect IOStatistics

Posted by GitBox <gi...@apache.org>.
mehakmeet commented on a change in pull request #2731:
URL: https://github.com/apache/hadoop/pull/2731#discussion_r611715063



##########
File path: hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsOutputStreamStatistics.java
##########
@@ -241,10 +247,13 @@ public void testAbfsOutputStreamDurationTrackerPutRequest() throws IOException {
       outputStream.write('a');
       outputStream.hflush();
 
-      AbfsOutputStreamStatisticsImpl abfsOutputStreamStatistics =
-          getAbfsOutputStreamStatistics(outputStream);
-      LOG.info("AbfsOutputStreamStats info: {}", abfsOutputStreamStatistics.toString());
-      Assertions.assertThat(abfsOutputStreamStatistics.getTimeSpentOnPutRequest())
+      IOStatistics ioStatistics = extractStatistics(fs);
+      LOG.info("AbfsOutputStreamStats info: {}",
+          ioStatisticsToPrettyString(ioStatistics));
+      Assertions.assertThat(

Review comment:
       I am not able to add ```.isGreatherThan(<SOME_DOUBLE>)``` after assertThatStatisticMean(), I guess because this is an ```ObjectAssert<MeanStatistic>``` rather than ```DoubleAssert```?




-- 
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.

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 #2731: HADOOP-17471. ABFS to collect IOStatistics

Posted by GitBox <gi...@apache.org>.
mehakmeet commented on pull request #2731:
URL: https://github.com/apache/hadoop/pull/2731#issuecomment-787996700


   Need to remove DurationTrackers from streams, since now I am collecting them from AbfsRestOperation.


----------------------------------------------------------------
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.

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 change in pull request #2731: HADOOP-17471. ABFS to collect IOStatistics

Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #2731:
URL: https://github.com/apache/hadoop/pull/2731#discussion_r618493561



##########
File path: hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsOutputStreamStatistics.java
##########
@@ -241,10 +247,13 @@ public void testAbfsOutputStreamDurationTrackerPutRequest() throws IOException {
       outputStream.write('a');
       outputStream.hflush();
 
-      AbfsOutputStreamStatisticsImpl abfsOutputStreamStatistics =
-          getAbfsOutputStreamStatistics(outputStream);
-      LOG.info("AbfsOutputStreamStats info: {}", abfsOutputStreamStatistics.toString());
-      Assertions.assertThat(abfsOutputStreamStatistics.getTimeSpentOnPutRequest())
+      IOStatistics ioStatistics = extractStatistics(fs);
+      LOG.info("AbfsOutputStreamStats info: {}",
+          ioStatisticsToPrettyString(ioStatistics));
+      Assertions.assertThat(

Review comment:
       yes, I guess so. We'd probably need some conversion to get the mean/sum/count value. Let's not worry about that 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.

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 #2731: HADOOP-17471. ABFS to collect IOStatistics

Posted by GitBox <gi...@apache.org>.
mehakmeet commented on pull request #2731:
URL: https://github.com/apache/hadoop/pull/2731#issuecomment-811630475


   Extended ```AbfsCounters.java``` interface with ```DurationTrackerFactory.java```, and converted the try-with-resources DurationTracking into 
   ```
   try {
         IOStatisticsBinding.trackDurationOfInvocation(abfsCounters,
             AbfsStatistic.getStatNameFromHttpCall(method),
             () -> completeExecute());
       } catch (IOException e) {
         throw new RuntimeException("Error while tracking Duration of an "
             + "AbfsRestOperation call", e);
       }
   ```
   in the execute() method of ```AbfsRestOperation.java``` class, but I am facing this exception now:
   
   ```
   java.lang.RuntimeException: Error while tracking Duration of an AbfsRestOperation call
   
   	at org.apache.hadoop.fs.azurebfs.services.AbfsRestOperation.execute(AbfsRestOperation.java:192)
   	at org.apache.hadoop.fs.azurebfs.services.AbfsClient.getAclStatus(AbfsClient.java:767)
   	at org.apache.hadoop.fs.azurebfs.services.AbfsClient.getAclStatus(AbfsClient.java:749)
   	at org.apache.hadoop.fs.azurebfs.AzureBlobFileSystemStore.getIsNamespaceEnabled(AzureBlobFileSystemStore.java:295)
   	at org.apache.hadoop.fs.azurebfs.AzureBlobFileSystemStore.getFileStatus(AzureBlobFileSystemStore.java:786)
   	at org.apache.hadoop.fs.azurebfs.AzureBlobFileSystem.getFileStatus(AzureBlobFileSystem.java:506)
   	at org.apache.hadoop.fs.azurebfs.AzureBlobFileSystem.tryGetFileStatus(AzureBlobFileSystem.java:1035)
   	at org.apache.hadoop.fs.azurebfs.AzureBlobFileSystem.initialize(AzureBlobFileSystem.java:135)
   	at org.apache.hadoop.fs.FileSystem.createFileSystem(FileSystem.java:3460)
   	at org.apache.hadoop.fs.FileSystem.access$300(FileSystem.java:172)
   	at org.apache.hadoop.fs.FileSystem$Cache.getInternal(FileSystem.java:3565)
   	at org.apache.hadoop.fs.FileSystem$Cache.getUnique(FileSystem.java:3518)
   	at org.apache.hadoop.fs.FileSystem.newInstance(FileSystem.java:592)
   	at org.apache.hadoop.fs.FileSystem.newInstance(FileSystem.java:604)
   	at org.apache.hadoop.fs.azurebfs.AbstractAbfsIntegrationTest.createFileSystem(AbstractAbfsIntegrationTest.java:271)
   	at org.apache.hadoop.fs.azurebfs.AbstractAbfsIntegrationTest.setup(AbstractAbfsIntegrationTest.java:146)
   	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
   	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
   	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
   	at java.lang.reflect.Method.invoke(Method.java:498)
   	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
   	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
   	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
   	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:24)
   	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
   	at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:55)
   	at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:298)
   	at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:292)
   	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
   	at java.lang.Thread.run(Thread.java:748)
   Caused by: Operation failed: "The specified filesystem does not exist.", 404, HEAD, https://abfstorageacc.dfs.core.windows.net/abfs-testcontainer-72fd54ec-fe7b-42bc-a21c-61a175de9286/?upn=false&action=getAccessControl&timeout=90
   	at org.apache.hadoop.fs.azurebfs.services.AbfsRestOperation.completeExecute(AbfsRestOperation.java:225)
   	at org.apache.hadoop.fs.azurebfs.services.AbfsRestOperation.lambda$execute$0(AbfsRestOperation.java:190)
   	at org.apache.hadoop.fs.statistics.impl.IOStatisticsBinding.trackDurationOfInvocation(IOStatisticsBinding.java:454)
   	at org.apache.hadoop.fs.azurebfs.services.AbfsRestOperation.execute(AbfsRestOperation.java:188)
   	... 29 more
   ```
   Requests seem to return 404 with FileSystem not found exception. Tried to debug didn't get much info with that as well. Have you faced this issue before with DurationTrackingFactory @steveloughran ?


-- 
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.

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 #2731: HADOOP-17471. ABFS to collect IOStatistics

Posted by GitBox <gi...@apache.org>.
steveloughran commented on pull request #2731:
URL: https://github.com/apache/hadoop/pull/2731#issuecomment-804430014


   Looks great!
   My main production suggestion is to make the counters a DurationTrackerFactory, giving us the option of feeding it in to anything we add in Hadoop common &c to take one. 
   test code: can you use assertThatStatisticMean() for your stats asserts -and if not, are changes needed?


-- 
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.

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 #2731: HADOOP-17471. ABFS to collect IOStatistics

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2731:
URL: https://github.com/apache/hadoop/pull/2731#issuecomment-825540804


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 39s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  1s |  |  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 6 new or modified test files.  |
   |||| _ trunk Compile Tests _ |
   | +0 :ok: |  mvndep  |  15m 38s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  20m 22s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  21m 35s |  |  trunk passed with JDK Ubuntu-11.0.10+9-Ubuntu-0ubuntu1.20.04  |
   | +1 :green_heart: |  compile  |  17m 58s |  |  trunk passed with JDK Private Build-1.8.0_282-8u282-b08-0ubuntu1~20.04-b08  |
   | +1 :green_heart: |  checkstyle  |   3m 52s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   2m 21s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   1m 48s |  |  trunk passed with JDK Ubuntu-11.0.10+9-Ubuntu-0ubuntu1.20.04  |
   | +1 :green_heart: |  javadoc  |   2m 16s |  |  trunk passed with JDK Private Build-1.8.0_282-8u282-b08-0ubuntu1~20.04-b08  |
   | +1 :green_heart: |  spotbugs  |   3m 31s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  14m 22s |  |  branch has no errors when building and testing our client artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 27s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   1m 21s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  20m  4s |  |  the patch passed with JDK Ubuntu-11.0.10+9-Ubuntu-0ubuntu1.20.04  |
   | +1 :green_heart: |  javac  |  20m  4s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  18m  5s |  |  the patch passed with JDK Private Build-1.8.0_282-8u282-b08-0ubuntu1~20.04-b08  |
   | +1 :green_heart: |  javac  |  18m  5s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | +1 :green_heart: |  checkstyle  |   3m 44s |  |  the patch passed  |
   | +1 :green_heart: |  mvnsite  |   2m 21s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   1m 47s |  |  the patch passed with JDK Ubuntu-11.0.10+9-Ubuntu-0ubuntu1.20.04  |
   | +1 :green_heart: |  javadoc  |   2m 16s |  |  the patch passed with JDK Private Build-1.8.0_282-8u282-b08-0ubuntu1~20.04-b08  |
   | +1 :green_heart: |  spotbugs  |   3m 54s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  14m 42s |  |  patch has no errors when building and testing our client artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  17m 26s |  |  hadoop-common in the patch passed.  |
   | +1 :green_heart: |  unit  |   2m 12s |  |  hadoop-azure in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 57s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 197m 28s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2731/6/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/2731 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell |
   | uname | Linux 22858ad1a3f8 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / 665a0af1fae0fb664a9cbc088bced0a30bdb4d45 |
   | Default Java | Private Build-1.8.0_282-8u282-b08-0ubuntu1~20.04-b08 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.10+9-Ubuntu-0ubuntu1.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_282-8u282-b08-0ubuntu1~20.04-b08 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2731/6/testReport/ |
   | Max. process+thread count | 3158 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-azure U: . |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2731/6/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.

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 #2731: HADOOP-17471. ABFS to collect IOStatistics

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2731:
URL: https://github.com/apache/hadoop/pull/2731#issuecomment-819633848


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 58s |  |  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 6 new or modified test files.  |
   |||| _ trunk Compile Tests _ |
   | +0 :ok: |  mvndep  |  15m 36s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  23m  9s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  22m 38s |  |  trunk passed with JDK Ubuntu-11.0.10+9-Ubuntu-0ubuntu1.20.04  |
   | +1 :green_heart: |  compile  |  19m 18s |  |  trunk passed with JDK Private Build-1.8.0_282-8u282-b08-0ubuntu1~20.04-b08  |
   | +1 :green_heart: |  checkstyle  |   4m  1s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   2m  9s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   1m 35s |  |  trunk passed with JDK Ubuntu-11.0.10+9-Ubuntu-0ubuntu1.20.04  |
   | +1 :green_heart: |  javadoc  |   2m  4s |  |  trunk passed with JDK Private Build-1.8.0_282-8u282-b08-0ubuntu1~20.04-b08  |
   | +1 :green_heart: |  spotbugs  |   3m 24s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  16m 47s |  |  branch has no errors when building and testing our client artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 21s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   1m 23s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  21m 55s |  |  the patch passed with JDK Ubuntu-11.0.10+9-Ubuntu-0ubuntu1.20.04  |
   | +1 :green_heart: |  javac  |  21m 55s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  19m 16s |  |  the patch passed with JDK Private Build-1.8.0_282-8u282-b08-0ubuntu1~20.04-b08  |
   | +1 :green_heart: |  javac  |  19m 16s |  |  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  |   2m 10s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   1m 33s |  |  the patch passed with JDK Ubuntu-11.0.10+9-Ubuntu-0ubuntu1.20.04  |
   | +1 :green_heart: |  javadoc  |   2m  3s |  |  the patch passed with JDK Private Build-1.8.0_282-8u282-b08-0ubuntu1~20.04-b08  |
   | +1 :green_heart: |  spotbugs  |   3m 47s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  17m  5s |  |  patch has no errors when building and testing our client artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |  17m 36s |  |  hadoop-common in the patch passed.  |
   | +1 :green_heart: |  unit  |   2m  2s |  |  hadoop-azure in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 45s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 208m 38s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2731/5/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/2731 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell |
   | uname | Linux 8b5358ce85a7 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 / 16d11277eeb86d0dbe6a423f9464f7e09227bf93 |
   | Default Java | Private Build-1.8.0_282-8u282-b08-0ubuntu1~20.04-b08 |
   | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.10+9-Ubuntu-0ubuntu1.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_282-8u282-b08-0ubuntu1~20.04-b08 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2731/5/testReport/ |
   | Max. process+thread count | 1274 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-azure U: . |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2731/5/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.

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