steveloughran commented on a change in pull request #2353:
URL: https://github.com/apache/hadoop/pull/2353#discussion_r500464223
##########
File path:
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsInputStream.java
##########
@@ -279,7 +289,15 @@ int readRemote(long position, byte[] b, int offset, int
length) throws IOExcepti
AbfsPerfTracker tracker = client.getAbfsPerfTracker();
try (AbfsPerfInfo perfInfo = new AbfsPerfInfo(tracker, "readRemote",
"read")) {
LOG.trace("Trigger client.read for path={} position={} offset={}
length={}", path, position, offset, length);
- op = client.read(path, position, b, offset, length, tolerateOobAppends ?
"*" : eTag, cachedSasToken.get());
+ if (ioStatistics != null) {
+ op = IOStatisticsBinding.trackDuration((IOStatisticsStore)
ioStatistics,
Review comment:
I'm going to make trackDuration actually apply the operation, have a
different method to create a wrapper callable
##########
File path:
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsInputStreamStatisticsImpl.java
##########
@@ -161,55 +170,92 @@ public void remoteBytesRead(long bytes) {
*/
@Override
public void remoteReadOperation() {
- remoteReadOperations++;
+ ioStatisticsStore.incrementCounter(getStatName(REMOTE_READ_OP));
}
+ /**
+ * Getter for IOStatistics instance used.
+ * @return IOStatisticsStore instance which extends IOStatistics.
+ */
+ @Override
+ public IOStatistics getIOStatistics() {
+ return ioStatisticsStore;
+ }
+
+ @VisibleForTesting
public long getSeekOperations() {
- return seekOperations;
+ return
ioStatisticsStore.counters().get(StreamStatisticNames.STREAM_READ_SEEK_OPERATIONS);
}
+ @VisibleForTesting
public long getForwardSeekOperations() {
- return forwardSeekOperations;
+ return
ioStatisticsStore.counters().get(StreamStatisticNames.STREAM_READ_SEEK_FORWARD_OPERATIONS);
}
+ @VisibleForTesting
public long getBackwardSeekOperations() {
- return backwardSeekOperations;
+ return
ioStatisticsStore.counters().get(StreamStatisticNames.STREAM_READ_SEEK_BACKWARD_OPERATIONS);
}
+ @VisibleForTesting
public long getBytesRead() {
- return bytesRead;
+ return
ioStatisticsStore.counters().get(StreamStatisticNames.STREAM_READ_BYTES);
}
+ @VisibleForTesting
public long getBytesSkippedOnSeek() {
- return bytesSkippedOnSeek;
+ return
ioStatisticsStore.counters().get(StreamStatisticNames.STREAM_READ_SEEK_BYTES_SKIPPED);
}
- public long getBytesBackwardsOnSeek() {
- return bytesBackwardsOnSeek;
+ @VisibleForTesting
+ public long getNegativeBytesBackwardsOnSeek() {
+ return
ioStatisticsStore.counters().get(getStatName(NEGATIVE_SEEK_BYTES_BACKWARDS));
}
+ @VisibleForTesting
public long getSeekInBuffer() {
- return seekInBuffer;
+ return ioStatisticsStore.counters().get(getStatName(SEEK_IN_BUFFER));
+
}
+ @VisibleForTesting
public long getReadOperations() {
- return readOperations;
+ return
ioStatisticsStore.counters().get(StreamStatisticNames.STREAM_READ_OPERATIONS);
Review comment:
consider static imports for the statistic names
##########
File path:
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsInputStreamStatistics.java
##########
@@ -160,7 +159,7 @@ public void testSeekStatistics() throws IOException {
assertEquals("Mismatch in forwardSeekOps value", OPERATIONS,
stats.getForwardSeekOperations());
assertEquals("Mismatch in bytesBackwardsOnSeek value",
- -1 * OPERATIONS * ONE_MB, stats.getBytesBackwardsOnSeek());
+ OPERATIONS * ONE_MB, stats.getNegativeBytesBackwardsOnSeek());
Review comment:
why the name change? What does this statistic mean?
----------------------------------------------------------------
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]
