steveloughran commented on a change in pull request #2353:
URL: https://github.com/apache/hadoop/pull/2353#discussion_r497636885
##########
File path:
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsOutputStreamStatistics.java
##########
@@ -219,6 +226,31 @@ public void testAbfsOutputStreamWriteBuffer() throws
IOException {
}
}
+ /**
+ * Test to check correct value of time spent on a PUT request in
+ * AbfsOutputStream.
+ */
+ @Test
+ public void testAbfsOutputStreamDurationTrackerPutRequest() throws
IOException {
+ describe("Testing to check if DurationTracker for PUT request is working "
+ + "correctly.");
+ AzureBlobFileSystem fs = getFileSystem();
+ Path pathForPutRequest = path(getMethodName());
+
+ try(AbfsOutputStream outputStream =
+ createAbfsOutputStreamWithFlushEnabled(fs, pathForPutRequest)) {
+ outputStream.write('a');
+ outputStream.hflush();
+
+ AbfsOutputStreamStatisticsImpl abfsOutputStreamStatistics =
+ getAbfsOutputStreamStatistics(outputStream);
+
+
Assertions.assertThat(abfsOutputStreamStatistics.getTimeSpentOnPutRequest())
+ .describedAs("Mismatch in timeTakenForPutRequest DurationTracker")
Review comment:
timeSpentOnPutRequest.
##########
File path:
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsOutputStreamStatisticsImpl.java
##########
@@ -125,31 +120,63 @@ public void queueShrunk() {
*/
@Override
public void writeCurrentBuffer() {
- writeCurrentBufferOperations++;
+
ioStatisticsStore.incrementCounter(getStatName(WRITE_CURRENT_BUFFER_OPERATIONS));
+ }
+
+ /**
+ * {@inheritDoc}
+ *
+ * A getter for IOStatisticsStore instance which extends IOStatistics.
+ *
+ * @return IOStatisticsStore instance.
+ */
+ @Override
+ public IOStatistics getIOStatistics() {
+ return ioStatisticsStore;
}
+ @VisibleForTesting
public long getBytesToUpload() {
- return bytesToUpload;
+ return
ioStatisticsStore.getCounterReference(getStatName(BYTES_TO_UPLOAD)).get();
Review comment:
counters.get(stat-name) will do this too
##########
File path:
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsOutputStream.java
##########
@@ -557,6 +567,11 @@ int getMaxRequestsThatCanBeQueued() {
return maxRequestsThatCanBeQueued;
}
+ @VisibleForTesting
+ public IOStatistics getIoStatistics() {
Review comment:
declare implementation of IOStatisticsSource, implement accessor there
##########
File path:
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsOutputStreamStatisticsImpl.java
##########
@@ -125,31 +120,63 @@ public void queueShrunk() {
*/
@Override
public void writeCurrentBuffer() {
- writeCurrentBufferOperations++;
+
ioStatisticsStore.incrementCounter(getStatName(WRITE_CURRENT_BUFFER_OPERATIONS));
+ }
+
+ /**
+ * {@inheritDoc}
+ *
+ * A getter for IOStatisticsStore instance which extends IOStatistics.
+ *
+ * @return IOStatisticsStore instance.
+ */
+ @Override
+ public IOStatistics getIOStatistics() {
+ return ioStatisticsStore;
}
+ @VisibleForTesting
public long getBytesToUpload() {
- return bytesToUpload;
+ return
ioStatisticsStore.getCounterReference(getStatName(BYTES_TO_UPLOAD)).get();
}
+ @VisibleForTesting
public long getBytesUploadSuccessful() {
- return bytesUploadSuccessful;
+ return
ioStatisticsStore.getCounterReference(getStatName(BYTES_UPLOAD_SUCCESSFUL)).get();
}
+ @VisibleForTesting
public long getBytesUploadFailed() {
- return bytesUploadFailed;
+ return
ioStatisticsStore.getCounterReference(getStatName(BYTES_UPLOAD_FAILED)).get();
}
+ @VisibleForTesting
public long getTimeSpentOnTaskWait() {
- return timeSpentOnTaskWait;
+ return
ioStatisticsStore.getCounterReference(getStatName(TIME_SPENT_ON_TASK_WAIT)).get();
}
+ @VisibleForTesting
public long getQueueShrunkOps() {
- return queueShrunkOps;
+ return
ioStatisticsStore.getCounterReference(getStatName(QUEUE_SHRUNK_OPS)).get();
}
+ @VisibleForTesting
public long getWriteCurrentBufferOperations() {
- return writeCurrentBufferOperations;
+ return
ioStatisticsStore.getCounterReference(getStatName(WRITE_CURRENT_BUFFER_OPERATIONS)).get();
+ }
+
+ @VisibleForTesting
+ public double getTimeSpentOnPutRequest() {
+ return
ioStatisticsStore.getMeanStatistic(getStatName(TIME_TAKEN_TO_PUT_REQUEST) +
".mean").mean();
Review comment:
StoreStatisticsNames.SUFFIX_MEAN has that string value there
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
