[
https://issues.apache.org/jira/browse/HADOOP-17471?focusedWorklogId=581051&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-581051
]
ASF GitHub Bot logged work on HADOOP-17471:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 12/Apr/21 14:42
Start Date: 12/Apr/21 14:42
Worklog Time Spent: 10m
Work Description: 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:
us...@infra.apache.org
Issue Time Tracking
-------------------
Worklog Id: (was: 581051)
Time Spent: 1h (was: 50m)
> ABFS to collect IOStatistics
> ----------------------------
>
> Key: HADOOP-17471
> URL: https://issues.apache.org/jira/browse/HADOOP-17471
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs/azure
> Reporter: Steve Loughran
> Assignee: Mehakmeet Singh
> Priority: Major
> Labels: pull-request-available
> Time Spent: 1h
> Remaining Estimate: 0h
>
> Add stats collection to ABFS FS operations, especially
> * create
> * open
> * delete
> * rename
> * getFilesStatus
> * list
> * attribute get/set
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
