steveloughran commented on a change in pull request #2604:
URL: https://github.com/apache/hadoop/pull/2604#discussion_r553348624
##########
File path:
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsStatistic.java
##########
@@ -73,7 +73,35 @@
READ_THROTTLES("read_throttles",
"Total number of times a read operation is throttled."),
WRITE_THROTTLES("write_throttles",
- "Total number of times a write operation is throttled.");
+ "Total number of times a write operation is throttled."),
+
+ //OutputStream statistics.
Review comment:
I've been trying to create standard names in hadoop-common; are there
ones you can use. If not lets' create new ones
##########
File path:
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsInputStreamStatisticsImpl.java
##########
@@ -18,23 +18,50 @@
package org.apache.hadoop.fs.azurebfs.services;
+import java.util.concurrent.atomic.AtomicLong;
+
+import com.google.common.annotations.VisibleForTesting;
Review comment:
need to use the shaded ones for trunk PRs
##########
File path:
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsOutputStream.java
##########
@@ -144,6 +151,7 @@ public AbfsOutputStream(
this.completionService = new
ExecutorCompletionService<>(this.threadExecutor);
this.cachedSasToken = new CachedSASToken(
abfsOutputStreamContext.getSasTokenRenewPeriodForStreamsInSeconds());
+ ioStatistics = outputStreamStatistics.getIOStatistics();
Review comment:
1. make a `this.` for consistency
2. Is there ever a risk that outputStreamStatistics is null/has empty
iostats at this point?
##########
File path:
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsOutputStreamStatistics.java
##########
@@ -65,6 +64,12 @@
*/
void writeCurrentBuffer();
+ /**
+ * Get the IOStatisticsStore instance from AbfsOutputStreamStatistics.
+ * @return instance of IOStatisticsStore which extends IOStatistics.
+ */
+ IOStatistics getIOStatistics();
Review comment:
Is this class an instance of IOStatisticsSource? It should be.
##########
File path:
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsInputStreamStatistics.java
##########
@@ -366,6 +365,39 @@ public void testReadAheadCounters() throws IOException {
}
}
+ /**
+ * Testing time taken by AbfsInputStream to complete a GET request.
+ */
+ @Test
+ public void testActionHttpGetRequest() throws IOException {
+ describe("Test to check the correct value of Time taken by http get "
+ + "request in AbfsInputStream");
+ AzureBlobFileSystem fs = getFileSystem();
+ AzureBlobFileSystemStore abfss = fs.getAbfsStore();
+ Path actionHttpGetRequestPath = path(getMethodName());
+ AbfsInputStream abfsInputStream = null;
+ AbfsOutputStream abfsOutputStream = null;
+ try {
+ abfsOutputStream = createAbfsOutputStreamWithFlushEnabled(fs,
+ actionHttpGetRequestPath);
+ abfsOutputStream.write('a');
+ abfsOutputStream.hflush();
+
+ abfsInputStream =
+ abfss.openFileForRead(actionHttpGetRequestPath,
fs.getFsStatistics());
+ abfsInputStream.read();
+ AbfsInputStreamStatisticsImpl abfsInputStreamStatistics =
+ (AbfsInputStreamStatisticsImpl)
abfsInputStream.getStreamStatistics();
Review comment:
be good to log @ info here for the curious
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
