[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-04-23 Thread GitBox


steveloughran commented on a change in pull request #1899:
URL: https://github.com/apache/hadoop/pull/1899#discussion_r413708902



##
File path: 
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java
##
@@ -51,6 +51,7 @@
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
+import org.apache.hadoop.fs.azurebfs.services.AbfsOutputStreamStatisticsImpl;

Review comment:
   needs to go in with the org.apache block





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



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-04-20 Thread GitBox


steveloughran commented on a change in pull request #1899:
URL: https://github.com/apache/hadoop/pull/1899#discussion_r411474545



##
File path: 
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsOutputStream.java
##
@@ -436,4 +453,28 @@ private void waitForTaskToComplete() throws IOException {
   public synchronized void waitForPendingUploads() throws IOException {
 waitForTaskToComplete();
   }
+
+  /**
+   * Getter method for AbfsOutputStream Statistics.
+   *
+   * @return statistics for AbfsOutputStream.
+   */
+  @VisibleForTesting
+  public AbfsOutputStreamStatisticsImpl getOutputStreamStatistics() {

Review comment:
   this would be coding into the implementation/semi-public API something 
only relevant for testing -and make it very hard to ever change to a different 
implementation.
   
   Just add some static method in the test suite
   ```
   AbfsOutputStreamStatisticsImp getStreamStatistics(AbfsOutputStream)
   ```
   and you could do the casting in just one place.
   
   





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



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-04-20 Thread GitBox


steveloughran commented on a change in pull request #1899:
URL: https://github.com/apache/hadoop/pull/1899#discussion_r411472091



##
File path: 
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsOutputStreamStatistics.java
##
@@ -0,0 +1,233 @@
+/**
+ * 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.junit.Test;
+
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.azurebfs.services.AbfsOutputStream;
+import org.apache.hadoop.fs.azurebfs.services.AbfsOutputStreamStatisticsImpl;
+
+/**
+ * Test AbfsOutputStream statistics.
+ */
+public class ITestAbfsOutputStreamStatistics
+extends AbstractAbfsIntegrationTest {
+  private static final int LARGE_OPERATIONS = 10;
+
+  public ITestAbfsOutputStreamStatistics() throws Exception {

Review comment:
   test suites can throw exceptions, but the constructor shouldn't have to. 
But if you want to keep it, then go ahead and keep it. It's not important





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



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-04-14 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r408179091
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsOutputStreamStatisticsImpl.java
 ##
 @@ -0,0 +1,177 @@
+/**
+ * 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.services;
+
+/**
+ * OutputStream Statistics Implementation for Abfs.
+ */
+public class AbfsOutputStreamStatisticsImpl
+implements AbfsOutputStreamStatistics {
+  private long bytesToUpload;
+  private long bytesUploadSuccessful;
+  private long bytesUploadFailed;
+  /**
+   * counter to get the total time spent while waiting for tasks to complete
+   * in the Blocking queue inside the thread executor.
+   */
+  private long timeSpendOnTaskWait;
+  /**
+   * counter to get the total number of queue shrink operations done{@code
+   * AbfsOutputStream#shrinkWriteOperationQueue()} by
+   * AbfsOutputStream to remove the write operations which were successfully
+   * done by AbfsOutputStream from the Blocking Queue.
+   */
+  private long queueShrunkOps;
+  /**
+   * counter to get the total number of times the current buffer is written
+   * to the service{@code AbfsOutputStream#writeCurrentBufferToService()} via
+   * AbfsClient and appended to the
+   * Data store by
+   * AbfsResOperation.
+   */
+  private long writeCurrentBufferOperations;
+
+  /**
+   * Records the need to upload bytes and increments the total bytes that
+   * needs to be uploaded.
+   *
+   * @param bytes Total bytes to upload. Negative bytes are ignored.
+   */
+  @Override
+  public void bytesToUpload(long bytes) {
+if (bytes > 0) {
+  bytesToUpload += bytes;
+}
+  }
+
+  /**
+   * Records the total bytes successfully uploaded through AbfsOutputStream.
+   *
+   * @param bytes number of bytes that were successfully uploaded. Negative
+   *  bytes are ignored.
+   */
+  @Override
+  public void uploadSuccessful(long bytes) {
+if (bytes > 0) {
+  bytesUploadSuccessful += bytes;
+}
+  }
+
+  /**
+   * Records the total bytes failed to upload through AbfsOutputStream.
+   *
+   * @param bytes number of bytes failed to upload. Negative bytes are ignored.
+   */
+  @Override
+  public void uploadFailed(long bytes) {
+if (bytes > 0) {
+  bytesUploadFailed += bytes;
+}
+  }
+
+  /**
+   * {@inheritDoc}
+   *
+   * Records the total time spent waiting for a task.
+   * When the thread executor has a task
+   * queue{@link java.util.concurrent.BlockingQueue} of size greater than or 
equal to 2
+   * times the maxConcurrentRequestCounts then, it waits for a task in that
+   * queue to finish, then do the next task in the queue.
+   *
+   * This time spent while waiting for the task to be completed is being
+   * recorded in this counter.
+   *
+   * @param startTime time(in milliseconds) before the wait for task to be
+   *  completed is begin.
+   * @param endTime   time(in milliseconds) after the wait for the task to be
+   *  completed is done.
+   */
+  @Override
+  public void timeSpentTaskWait(long startTime, long endTime) {
+timeSpendOnTaskWait += endTime - startTime;
+  }
+
+  /**
+   * {@inheritDoc}
+   *
+   * Records the number of times AbfsOutputStream try to remove the completed
+   * write operations from the beginning of write operation FIFO queue.
+   */
+  @Override
+  public void queueShrunk() {
+queueShrunkOps++;
+  }
+
+  /**
+   * {@inheritDoc}
+   *
+   * Records the number of times AbfsOutputStream writes the buffer to the
+   * service via the AbfsClient and appends the buffer to the service.
+   */
+  @Override
+  public void writeCurrentBuffer() {
+writeCurrentBufferOperations++;
+  }
+
+  public long getBytesToUpload() {
+return bytesToUpload;
+  }
+
+  public long getBytesUploadSuccessful() {
+return bytesUploadSuccessful;
+  }
+
+  public long getBytesUploadFailed() {
+return bytesUploadFailed;
+  }
+
+  public long getTimeSpendOnTaskWait() {
+return timeSpendOnTaskWait;
+  }
+
+  public long 

[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-04-14 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r408175364
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsOutputStreamStatistics.java
 ##
 @@ -0,0 +1,233 @@
+/**
+ * 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.junit.Test;
+
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.azurebfs.services.AbfsOutputStream;
+import org.apache.hadoop.fs.azurebfs.services.AbfsOutputStreamStatisticsImpl;
+
+/**
+ * Test AbfsOutputStream statistics.
+ */
+public class ITestAbfsOutputStreamStatistics
+extends AbstractAbfsIntegrationTest {
+  private static final int LARGE_OPERATIONS = 10;
+
+  public ITestAbfsOutputStreamStatistics() throws Exception {
+  }
+
+  /**
+   * Tests to check bytes Uploaded successfully in {@link AbfsOutputStream}.
+   *
+   * @throws IOException
+   */
+  @Test
+  public void testAbfsOutputStreamUploadingBytes() throws IOException {
+describe("Testing Bytes uploaded successfully in AbfsOutputSteam");
+final AzureBlobFileSystem fs = getFileSystem();
+Path uploadBytesFilePath = path(getMethodName());
+String testBytesToUpload = "bytes";
+
+try (
+AbfsOutputStream outForSomeBytes = 
createAbfsOutputStreamWithFlushEnabled(
+fs,
+uploadBytesFilePath)
+) {
+
+  AbfsOutputStreamStatisticsImpl abfsOutputStreamStatisticsForUploadBytes =
+  outForSomeBytes.getOutputStreamStatistics();
+
+  //Test for zero bytes To upload.
+  assertValues("bytes to upload", 0,
+  abfsOutputStreamStatisticsForUploadBytes.getBytesToUpload());
+
+  outForSomeBytes.write(testBytesToUpload.getBytes());
+  outForSomeBytes.flush();
+  abfsOutputStreamStatisticsForUploadBytes =
+  outForSomeBytes.getOutputStreamStatistics();
+
+  //Test for bytes to upload.
+  assertValues("bytes to upload", testBytesToUpload.getBytes().length,
+  abfsOutputStreamStatisticsForUploadBytes.getBytesToUpload());
+
+  //Test for successful bytes uploaded.
+  assertValues("successful bytes uploaded",
+  testBytesToUpload.getBytes().length,
+  abfsOutputStreamStatisticsForUploadBytes.getBytesUploadSuccessful());
+
+}
+
+try (
+AbfsOutputStream outForLargeBytes = 
createAbfsOutputStreamWithFlushEnabled(
+fs,
+uploadBytesFilePath)) {
+
+  for (int i = 0; i < LARGE_OPERATIONS; i++) {
+outForLargeBytes.write(testBytesToUpload.getBytes());
+  }
+  outForLargeBytes.flush();
+  AbfsOutputStreamStatisticsImpl abfsOutputStreamStatistics =
+  outForLargeBytes.getOutputStreamStatistics();
+
+  //Test for bytes to upload.
+  assertValues("bytes to upload",
+  LARGE_OPERATIONS * (testBytesToUpload.getBytes().length),
+  abfsOutputStreamStatistics.getBytesToUpload());
+
+  //Test for successful bytes uploaded.
+  assertValues("successful bytes uploaded",
+  LARGE_OPERATIONS * (testBytesToUpload.getBytes().length),
+  abfsOutputStreamStatistics.getBytesUploadSuccessful());
+
+}
+  }
+
+  /**
+   * Tests to check number of {@code
+   * AbfsOutputStream#shrinkWriteOperationQueue()} calls.
+   * After writing data, AbfsOutputStream doesn't upload the data until
+   * Flushed. Hence, flush() method is called after write() to test Queue
+   * shrink calls.
+   *
+   * @throws IOException
 
 Review comment:
   nit: just cut these lines from the javadoc. Nice to see the rest of the 
detail


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: 

[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-04-14 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r408178498
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/TestAbfsOutputStreamStatistics.java
 ##
 @@ -0,0 +1,120 @@
+/**
+ * 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 java.util.Random;
+
+import org.junit.Test;
+
+import org.apache.hadoop.fs.azurebfs.services.AbfsOutputStream;
+import org.apache.hadoop.fs.azurebfs.services.AbfsOutputStreamStatisticsImpl;
+
+/**
+ * Unit Tests for AbfsOutputStream Statistics.
+ */
+public class TestAbfsOutputStreamStatistics
+extends AbstractAbfsIntegrationTest {
+
+  private static final int LOW_RANGE_FOR_RANDOM_VALUE = 49;
+  private static final int HIGH_RANGE_FOR_RANDOM_VALUE = ;
+
+  public TestAbfsOutputStreamStatistics() throws Exception {
+  }
+
+  /**
+   * Tests to check bytes failed to Upload in {@link AbfsOutputStream}.
+   *
+   * @throws IOException
+   */
+  @Test
+  public void testAbfsOutputStreamBytesFailed() {
+describe("Testing Bytes Failed during uploading in AbfsOutputSteam");
+
+AbfsOutputStreamStatisticsImpl abfsOutputStreamStatistics =
+new AbfsOutputStreamStatisticsImpl();
+
+//Test for zero bytes uploaded.
+assertValues("number fo bytes failed to upload", 0,
+abfsOutputStreamStatistics.getBytesUploadFailed());
+
+//Populating small random value for bytesFailed.
+int randomBytesFailed = new Random().nextInt(LOW_RANGE_FOR_RANDOM_VALUE);
+abfsOutputStreamStatistics.uploadFailed(randomBytesFailed);
+//Test for bytes failed to upload.
+assertValues("number fo bytes failed to upload", randomBytesFailed,
+abfsOutputStreamStatistics.getBytesUploadFailed());
+
+//Initializing again to reset the statistics.
+abfsOutputStreamStatistics = new AbfsOutputStreamStatisticsImpl();
+
+//Populating large random values for bytesFailed.
+randomBytesFailed = new Random().nextInt(HIGH_RANGE_FOR_RANDOM_VALUE);
+abfsOutputStreamStatistics.uploadFailed(randomBytesFailed);
+//Test for bytes failed to upload.
+assertValues("number fo bytes failed to upload", randomBytesFailed,
+abfsOutputStreamStatistics.getBytesUploadFailed());
+  }
+
+  /**
+   * Tests to check time spent on waiting for tasks to be complete on a
+   * blocking queue in {@link AbfsOutputStream}.
+   *
+   * @throws IOException
+   */
+  @Test
+  public void testAbfsOutputStreamTimeSpentOnWaitTask() {
+describe("Testing Time Spend on Waiting for Task to be complete");
+
+AbfsOutputStreamStatisticsImpl abfsOutputStreamStatistics =
+new AbfsOutputStreamStatisticsImpl();
+
+//Test for initial value of timeSpentWaitTask.
+assertValues("Time spend on waiting for tasks to complete", 0,
+abfsOutputStreamStatistics.getTimeSpendOnTaskWait());
+
+int smallRandomStartTime =
+new Random().nextInt(LOW_RANGE_FOR_RANDOM_VALUE);
+int smallRandomEndTime =
+new Random().nextInt(LOW_RANGE_FOR_RANDOM_VALUE)
++ smallRandomStartTime;
+int smallDiff = smallRandomEndTime - smallRandomStartTime;
+abfsOutputStreamStatistics
+.timeSpentTaskWait(smallRandomStartTime, smallRandomEndTime);
+//Test for small random value of timeSpentWaitTask.
+assertValues("Time spend on waiting for tasks to complete", smallDiff,
+abfsOutputStreamStatistics.getTimeSpendOnTaskWait());
+
+int largeRandomStartTime =
+new Random().nextInt(HIGH_RANGE_FOR_RANDOM_VALUE);
+int largeRandomEndTime = new Random().nextInt(HIGH_RANGE_FOR_RANDOM_VALUE)
++ largeRandomStartTime;
+int randomDiff = largeRandomEndTime - largeRandomStartTime;
+abfsOutputStreamStatistics
+.timeSpentTaskWait(largeRandomStartTime, largeRandomEndTime);
+  /*
+  Test for large random value of timeSpentWaitTask plus the time spent
+  waiting in previous test.
+   */
+assertValues("Time spend on waiting for tasks to complete",
+smallDiff + randomDiff,
+

[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-04-14 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r408176558
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsOutputStream.java
 ##
 @@ -436,4 +453,28 @@ private void waitForTaskToComplete() throws IOException {
   public synchronized void waitForPendingUploads() throws IOException {
 waitForTaskToComplete();
   }
+
+  /**
+   * Getter method for AbfsOutputStream Statistics.
+   *
+   * @return statistics for AbfsOutputStream.
+   */
+  @VisibleForTesting
+  public AbfsOutputStreamStatisticsImpl getOutputStreamStatistics() {
 
 Review comment:
   why is this being cast rather than returned as is?


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-04-14 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r408173830
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsOutputStreamStatisticsImpl.java
 ##
 @@ -0,0 +1,177 @@
+/**
+ * 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.services;
+
+/**
+ * OutputStream Statistics Implementation for Abfs.
+ */
+public class AbfsOutputStreamStatisticsImpl
+implements AbfsOutputStreamStatistics {
+  private long bytesToUpload;
 
 Review comment:
   remember that discussion we had about volatile vs long and you concluded 
that we could shut yetus up by going non volatile?
   In #1820 I've moved s3a input stream stats to volatile so that the 
IOStatistics gets the latest values without blocking...and then turned off 
findbugs warnings (or at least, I'm trying to)


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-04-14 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r408178312
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/TestAbfsOutputStreamStatistics.java
 ##
 @@ -0,0 +1,120 @@
+/**
+ * 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 java.util.Random;
+
+import org.junit.Test;
+
+import org.apache.hadoop.fs.azurebfs.services.AbfsOutputStream;
+import org.apache.hadoop.fs.azurebfs.services.AbfsOutputStreamStatisticsImpl;
+
+/**
+ * Unit Tests for AbfsOutputStream Statistics.
+ */
+public class TestAbfsOutputStreamStatistics
+extends AbstractAbfsIntegrationTest {
+
+  private static final int LOW_RANGE_FOR_RANDOM_VALUE = 49;
+  private static final int HIGH_RANGE_FOR_RANDOM_VALUE = ;
+
+  public TestAbfsOutputStreamStatistics() throws Exception {
+  }
+
+  /**
+   * Tests to check bytes failed to Upload in {@link AbfsOutputStream}.
+   *
+   * @throws IOException
+   */
+  @Test
+  public void testAbfsOutputStreamBytesFailed() {
+describe("Testing Bytes Failed during uploading in AbfsOutputSteam");
+
+AbfsOutputStreamStatisticsImpl abfsOutputStreamStatistics =
+new AbfsOutputStreamStatisticsImpl();
+
+//Test for zero bytes uploaded.
+assertValues("number fo bytes failed to upload", 0,
 
 Review comment:
   typo


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-04-14 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r408174898
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsOutputStreamStatistics.java
 ##
 @@ -0,0 +1,233 @@
+/**
+ * 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.junit.Test;
+
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.azurebfs.services.AbfsOutputStream;
+import org.apache.hadoop.fs.azurebfs.services.AbfsOutputStreamStatisticsImpl;
+
+/**
+ * Test AbfsOutputStream statistics.
+ */
+public class ITestAbfsOutputStreamStatistics
+extends AbstractAbfsIntegrationTest {
+  private static final int LARGE_OPERATIONS = 10;
+
+  public ITestAbfsOutputStreamStatistics() throws Exception {
+  }
+
+  /**
+   * Tests to check bytes Uploaded successfully in {@link AbfsOutputStream}.
+   *
+   * @throws IOException
+   */
+  @Test
+  public void testAbfsOutputStreamUploadingBytes() throws IOException {
+describe("Testing Bytes uploaded successfully in AbfsOutputSteam");
+final AzureBlobFileSystem fs = getFileSystem();
+Path uploadBytesFilePath = path(getMethodName());
+String testBytesToUpload = "bytes";
+
+try (
+AbfsOutputStream outForSomeBytes = 
createAbfsOutputStreamWithFlushEnabled(
+fs,
+uploadBytesFilePath)
+) {
 
 Review comment:
   nit: pull up to previous line


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-04-14 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r408179593
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsOutputStreamStatisticsImpl.java
 ##
 @@ -0,0 +1,177 @@
+/**
+ * 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.services;
+
+/**
+ * OutputStream Statistics Implementation for Abfs.
+ */
+public class AbfsOutputStreamStatisticsImpl
+implements AbfsOutputStreamStatistics {
+  private long bytesToUpload;
+  private long bytesUploadSuccessful;
+  private long bytesUploadFailed;
+  /**
+   * counter to get the total time spent while waiting for tasks to complete
+   * in the Blocking queue inside the thread executor.
+   */
+  private long timeSpendOnTaskWait;
 
 Review comment:
   timeSpent


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-04-14 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r408177932
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/AbstractAbfsIntegrationTest.java
 ##
 @@ -383,4 +391,34 @@ protected AbfsDelegationTokenManager 
getDelegationTokenManager()
   throws IOException {
 return getFileSystem().getDelegationTokenManager();
   }
+
+  /**
+   * Generic assert method.
+   *
+   * @param operation operation being asserted.
+   * @param expectedValue value that is expected.
+   * @param actualValue   value that is actual.
+   */
+  protected void assertValues(String operation, long expectedValue,
 
 Review comment:
   too vague a name and no obvious difference with assertEquals. 
   
   Propose:
   *  leave existing test alone, 
   and then one of
   * use Junit assertEquals()
   * or Assertions.assertThat(object).describedAs().equals().
   


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-04-14 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r408174325
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsOutputStreamStatistics.java
 ##
 @@ -0,0 +1,233 @@
+/**
+ * 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.junit.Test;
+
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.azurebfs.services.AbfsOutputStream;
+import org.apache.hadoop.fs.azurebfs.services.AbfsOutputStreamStatisticsImpl;
+
+/**
+ * Test AbfsOutputStream statistics.
+ */
+public class ITestAbfsOutputStreamStatistics
+extends AbstractAbfsIntegrationTest {
+  private static final int LARGE_OPERATIONS = 10;
+
+  public ITestAbfsOutputStreamStatistics() throws Exception {
 
 Review comment:
   not needed; just cut it


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-04-14 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r408172102
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsOutputStream.java
 ##
 @@ -279,6 +283,9 @@ public synchronized void close() throws IOException {
 threadExecutor.shutdownNow();
   }
 }
+if (LOG.isDebugEnabled()) {
 
 Review comment:
   just create your own static LOG


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-04-14 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r408178198
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/TestAbfsOutputStreamStatistics.java
 ##
 @@ -0,0 +1,120 @@
+/**
+ * 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 java.util.Random;
+
+import org.junit.Test;
+
+import org.apache.hadoop.fs.azurebfs.services.AbfsOutputStream;
+import org.apache.hadoop.fs.azurebfs.services.AbfsOutputStreamStatisticsImpl;
+
+/**
+ * Unit Tests for AbfsOutputStream Statistics.
+ */
+public class TestAbfsOutputStreamStatistics
+extends AbstractAbfsIntegrationTest {
+
+  private static final int LOW_RANGE_FOR_RANDOM_VALUE = 49;
+  private static final int HIGH_RANGE_FOR_RANDOM_VALUE = ;
+
+  public TestAbfsOutputStreamStatistics() throws Exception {
+  }
+
+  /**
+   * Tests to check bytes failed to Upload in {@link AbfsOutputStream}.
+   *
+   * @throws IOException
+   */
+  @Test
+  public void testAbfsOutputStreamBytesFailed() {
+describe("Testing Bytes Failed during uploading in AbfsOutputSteam");
+
+AbfsOutputStreamStatisticsImpl abfsOutputStreamStatistics =
+new AbfsOutputStreamStatisticsImpl();
+
+//Test for zero bytes uploaded.
+assertValues("number fo bytes failed to upload", 0,
+abfsOutputStreamStatistics.getBytesUploadFailed());
+
+//Populating small random value for bytesFailed.
+int randomBytesFailed = new Random().nextInt(LOW_RANGE_FOR_RANDOM_VALUE);
+abfsOutputStreamStatistics.uploadFailed(randomBytesFailed);
+//Test for bytes failed to upload.
+assertValues("number fo bytes failed to upload", randomBytesFailed,
+abfsOutputStreamStatistics.getBytesUploadFailed());
+
+//Initializing again to reset the statistics.
+abfsOutputStreamStatistics = new AbfsOutputStreamStatisticsImpl();
+
+//Populating large random values for bytesFailed.
+randomBytesFailed = new Random().nextInt(HIGH_RANGE_FOR_RANDOM_VALUE);
+abfsOutputStreamStatistics.uploadFailed(randomBytesFailed);
+//Test for bytes failed to upload.
+assertValues("number fo bytes failed to upload", randomBytesFailed,
 
 Review comment:
   nit: typo


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-04-14 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r408179735
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsOutputStreamStatisticsImpl.java
 ##
 @@ -0,0 +1,177 @@
+/**
+ * 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.services;
+
+/**
+ * OutputStream Statistics Implementation for Abfs.
+ */
+public class AbfsOutputStreamStatisticsImpl
+implements AbfsOutputStreamStatistics {
+  private long bytesToUpload;
+  private long bytesUploadSuccessful;
+  private long bytesUploadFailed;
+  /**
+   * counter to get the total time spent while waiting for tasks to complete
+   * in the Blocking queue inside the thread executor.
+   */
+  private long timeSpendOnTaskWait;
+  /**
+   * counter to get the total number of queue shrink operations done{@code
+   * AbfsOutputStream#shrinkWriteOperationQueue()} by
+   * AbfsOutputStream to remove the write operations which were successfully
+   * done by AbfsOutputStream from the Blocking Queue.
+   */
+  private long queueShrunkOps;
+  /**
+   * counter to get the total number of times the current buffer is written
+   * to the service{@code AbfsOutputStream#writeCurrentBufferToService()} via
+   * AbfsClient and appended to the
+   * Data store by
+   * AbfsResOperation.
 
 Review comment:
   typo


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-04-07 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r405023294
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsOutputStream.java
 ##
 @@ -384,6 +398,7 @@ private synchronized void 
flushWrittenBytesToServiceInternal(final long offset,
* operation FIFO queue.
*/
   private synchronized void shrinkWriteOperationQueue() throws IOException {
+outputStreamStatistics.queueShrinked();
 
 Review comment:
   it would also be "queueShrunk" as a name


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-04-07 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r405022845
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsOutputStream.java
 ##
 @@ -294,19 +299,23 @@ private synchronized void flushInternalAsync() throws 
IOException {
   }
 
   private synchronized void writeCurrentBufferToService() throws IOException {
+outputStreamStatistics.writeCurrentBuffer();
 
 Review comment:
   yes, let's do that


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-04-07 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r405022610
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsOutputStream.java
 ##
 @@ -279,6 +283,7 @@ public synchronized void close() throws IOException {
 threadExecutor.shutdownNow();
   }
 }
+LOG.debug("Closing AbfsOutputStream ", toString());
 
 Review comment:
   let's guard this with a LOG.isDebugEnabled(), because that toString() 
operation is doing enough work. Or, use `this` as the argument and have SLF4J 
Call this.toString() only if it is printing the log entry


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-04-06 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r404060217
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsOutputStream.java
 ##
 @@ -80,6 +82,7 @@
   = new ElasticByteBufferPool();
 
   private final Statistics statistics;
+  private final AbfsOutputStreamStatisticsImpl outputStreamStatistics;
 
 Review comment:
   yes. Use the interface in reference/args and pass the instance in


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-03-20 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r395754519
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemOauth.java
 ##
 @@ -143,7 +143,7 @@ public void testBlobDataReader() throws Exception {
 
 // TEST WRITE FILE
 try {
-  abfsStore.openFileForWrite(EXISTED_FILE_PATH, true);
+  abfsStore.openFileForWrite(EXISTED_FILE_PATH, fs.getFsStatistics(), 
true);
 
 Review comment:
   add a .close(), even if the original code didn't. Always good to improve a 
test


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-03-20 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r395751202
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsOutputStream.java
 ##
 @@ -133,58 +133,60 @@ public void testAbfsOutputStreamTimeSpendOnWaitTask() 
throws IOException {
   public void testAbfsOutputStreamQueueShrink() throws IOException {
 describe("Testing Queue Shrink calls in AbfsOutputStream");
 final AzureBlobFileSystem fs = getFileSystem();
-Path TEST_PATH = new Path("AbfsOutputStreamStatsPath");
+Path queueShrinkFilePath = new Path("AbfsOutputStreamStatsPath");
 AzureBlobFileSystemStore abfss = fs.getAbfsStore();
 abfss.getAbfsConfiguration().setDisableOutputStreamFlush(false);
 FileSystem.Statistics statistics = fs.getFsStatistics();
 String testQueueShrink = "testQueue";
 
-
 AbfsOutputStream outForOneOp = null;
 
 try {
-  outForOneOp = (AbfsOutputStream) abfss.createFile(TEST_PATH, statistics,
-true,
-  FsPermission.getDefault(), FsPermission.getUMask(fs.getConf()));
+  outForOneOp =
+  (AbfsOutputStream) abfss.createFile(queueShrinkFilePath, statistics,
+  true,
+  FsPermission.getDefault(), FsPermission.getUMask(fs.getConf()));
 
   //Test for shrinking Queue zero time
-  Assert.assertEquals("Mismatch in number of queueShrink() Calls", 0,
+  assertValues("number of queueShrink() Calls", 0,
   outForOneOp.getOutputStreamStatistics().queueShrink);
 
   outForOneOp.write(testQueueShrink.getBytes());
   // Queue is shrunk 2 times when outStream is flushed
   outForOneOp.flush();
 
   //Test for shrinking Queue 2 times
-  Assert.assertEquals("Mismatch in number of queueShrink() Calls", 2,
+  assertValues("number of queueShrink() Calls", 2,
   outForOneOp.getOutputStreamStatistics().queueShrink);
 
 } finally {
-  if(outForOneOp != null){
+  if (outForOneOp != null) {
 outForOneOp.close();
   }
 }
 
 AbfsOutputStream outForLargeOps = null;
 
 try {
-  outForLargeOps = (AbfsOutputStream) abfss.createFile(TEST_PATH,
+  outForLargeOps = (AbfsOutputStream) abfss.createFile(queueShrinkFilePath,
   statistics, true,
   FsPermission.getDefault(), FsPermission.getUMask(fs.getConf()));
 
+  int largeValue = 1000;
   //QueueShrink is called 2 times in 1 flush(), hence 1000 flushes must
   // give 2000 QueueShrink calls
-  for (int i = 0; i < 1000; i++) {
+  for (int i = 0; i < largeValue; i++) {
 outForLargeOps.write(testQueueShrink.getBytes());
 //Flush is quite expensive so 1000 calls only which takes 1 min+
 outForLargeOps.flush();
 
 Review comment:
   do you have to call it so many times?


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-03-20 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r395753205
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsStreamStatistics.java
 ##
 @@ -0,0 +1,147 @@
+/**
+ * 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 org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.hadoop.fs.FSDataOutputStream;
+import org.apache.hadoop.fs.FSDataInputStream;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+
+/**
+ * Test Abfs Stream.
+ */
+
+public class ITestAbfsStreamStatistics extends AbstractAbfsIntegrationTest {
+  public ITestAbfsStreamStatistics() throws Exception {
+  }
+
+  /***
+   * Testing {@code incrementReadOps()} in class {@code AbfsInputStream} and
+   * {@code incrementWriteOps()} in class {@code AbfsOutputStream}.
+   *
+   * @throws Exception
+   */
+  @Test
+  public void testAbfsStreamOps() throws Exception {
+describe("Test to see correct population of read and write operations in "
++ "Abfs");
+
+final AzureBlobFileSystem fs = getFileSystem();
+Path smallOperationsFile = new Path("testOneReadWriteOps");
+Path largeOperationsFile = new Path("testLargeReadWriteOps");
+FileSystem.Statistics statistics = fs.getFsStatistics();
+String testReadWriteOps = "test this";
+statistics.reset();
+
+//Test for zero write operation
+assertReadWriteOps("write", 0, statistics.getWriteOps());
+
+//Test for zero read operation
+assertReadWriteOps("read", 0, statistics.getReadOps());
+
+FSDataOutputStream outForOneOperation = null;
+FSDataInputStream inForOneOperation = null;
+try {
+  outForOneOperation = fs.create(smallOperationsFile);
+  statistics.reset();
+  outForOneOperation.write(testReadWriteOps.getBytes());
+
+  //Test for a single write operation
+  assertReadWriteOps("write", 1, statistics.getWriteOps());
+
+  inForOneOperation = fs.open(smallOperationsFile);
+  inForOneOperation.read(testReadWriteOps.getBytes(), 0,
+  testReadWriteOps.getBytes().length);
+
+  //Test for a single read operation
+  assertReadWriteOps("read", 1, statistics.getReadOps());
+
+} finally {
+  if (inForOneOperation != null) {
 
 Review comment:
   IOUtils.closeQuietly


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-03-20 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r395749601
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsOutputStream.java
 ##
 @@ -0,0 +1,278 @@
+package org.apache.hadoop.fs.azurebfs;
+
+import java.io.IOException;
+
+import org.junit.Test;
+
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.azurebfs.services.AbfsOutputStream;
+import org.apache.hadoop.fs.permission.FsPermission;
+
+/**
+ * Test AbfsOutputStream statistics.
+ */
+public class ITestAbfsOutputStream extends AbstractAbfsIntegrationTest {
+
+  public ITestAbfsOutputStream() throws Exception {
+  }
+
+  /**
+   * Tests to check bytes Uploading in {@link AbfsOutputStream}.
+   *
+   * @throws IOException
+   */
+  @Test
+  public void testAbfsOutputStreamUploadingBytes() throws IOException {
+describe("Testing Bytes uploaded in AbfsOutputSteam");
+final AzureBlobFileSystem fs = getFileSystem();
+Path uploadBytesFilePath = new Path("AbfsOutputStreamStatsPath");
+AzureBlobFileSystemStore abfss = fs.getAbfsStore();
+FileSystem.Statistics statistics = fs.getFsStatistics();
+abfss.getAbfsConfiguration().setDisableOutputStreamFlush(false);
+String testBytesToUpload = "bytes";
+
+AbfsOutputStream outForSomeBytes = null;
+try {
+  outForSomeBytes = (AbfsOutputStream) 
abfss.createFile(uploadBytesFilePath,
+  statistics,
+  true,
+  FsPermission.getDefault(), FsPermission.getUMask(fs.getConf()));
+
+  //Test for zero bytes To upload
+  assertValues("bytes to upload", 0,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload);
+
+  outForSomeBytes.write(testBytesToUpload.getBytes());
+  outForSomeBytes.flush();
+
+  //Test for some bytes to upload
+  assertValues("bytes to upload", testBytesToUpload.getBytes().length,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload);
+
+  //Test for relation between bytesUploadSuccessful, bytesUploadFailed
+  // and bytesToUpload
+  assertValues("bytesUploadSuccessful equal to difference between "
+  + "bytesToUpload and bytesUploadFailed",
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadSuccessful,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload -
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadFailed);
+
+} finally {
+  if (outForSomeBytes != null) {
+outForSomeBytes.close();
+  }
+}
+
+AbfsOutputStream outForLargeBytes = null;
+try {
+  outForLargeBytes =
+  (AbfsOutputStream) abfss.createFile(uploadBytesFilePath,
+  statistics
+  , true, FsPermission.getDefault(),
+  FsPermission.getUMask(fs.getConf()));
+
+  int largeValue = 10;
+  for (int i = 0; i < largeValue; i++) {
+outForLargeBytes.write(testBytesToUpload.getBytes());
+  }
+  outForLargeBytes.flush();
+
+  //Test for large bytes to upload
+  assertValues("bytes to upload",
+  largeValue * (testBytesToUpload.getBytes().length),
+  outForLargeBytes.getOutputStreamStatistics().bytesToUpload);
+
+  //Test for relation between bytesUploadSuccessful, bytesUploadFailed
+  // and bytesToUpload
+  assertValues("bytesUploadSuccessful equal to difference between "
+  + "bytesToUpload and bytesUploadFailed",
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadSuccessful,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload -
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadFailed);
+
+} finally {
+  if (outForLargeBytes != null) {
+outForLargeBytes.close();
+  }
+}
+
+  }
+
+  /**
+   * Tests to check time spend on waiting for tasks to be complete on a
+   * blocking queue in {@link AbfsOutputStream}.
+   *
+   * @throws IOException
+   */
+  @Test
+  public void testAbfsOutputStreamTimeSpendOnWaitTask() throws IOException {
 
 Review comment:
   I don't see any easy way except to assert that it is > 0


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-03-20 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r395750400
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsOutputStream.java
 ##
 @@ -0,0 +1,278 @@
+package org.apache.hadoop.fs.azurebfs;
+
+import java.io.IOException;
+
+import org.junit.Test;
+
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.azurebfs.services.AbfsOutputStream;
+import org.apache.hadoop.fs.permission.FsPermission;
+
+/**
+ * Test AbfsOutputStream statistics.
+ */
+public class ITestAbfsOutputStream extends AbstractAbfsIntegrationTest {
+
+  public ITestAbfsOutputStream() throws Exception {
+  }
+
+  /**
+   * Tests to check bytes Uploading in {@link AbfsOutputStream}.
+   *
+   * @throws IOException
+   */
+  @Test
+  public void testAbfsOutputStreamUploadingBytes() throws IOException {
+describe("Testing Bytes uploaded in AbfsOutputSteam");
+final AzureBlobFileSystem fs = getFileSystem();
+Path uploadBytesFilePath = new Path("AbfsOutputStreamStatsPath");
+AzureBlobFileSystemStore abfss = fs.getAbfsStore();
+FileSystem.Statistics statistics = fs.getFsStatistics();
+abfss.getAbfsConfiguration().setDisableOutputStreamFlush(false);
+String testBytesToUpload = "bytes";
+
+AbfsOutputStream outForSomeBytes = null;
+try {
+  outForSomeBytes = (AbfsOutputStream) 
abfss.createFile(uploadBytesFilePath,
+  statistics,
+  true,
+  FsPermission.getDefault(), FsPermission.getUMask(fs.getConf()));
+
+  //Test for zero bytes To upload
+  assertValues("bytes to upload", 0,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload);
+
+  outForSomeBytes.write(testBytesToUpload.getBytes());
+  outForSomeBytes.flush();
+
+  //Test for some bytes to upload
+  assertValues("bytes to upload", testBytesToUpload.getBytes().length,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload);
+
+  //Test for relation between bytesUploadSuccessful, bytesUploadFailed
+  // and bytesToUpload
+  assertValues("bytesUploadSuccessful equal to difference between "
+  + "bytesToUpload and bytesUploadFailed",
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadSuccessful,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload -
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadFailed);
+
+} finally {
+  if (outForSomeBytes != null) {
+outForSomeBytes.close();
+  }
+}
+
+AbfsOutputStream outForLargeBytes = null;
+try {
+  outForLargeBytes =
+  (AbfsOutputStream) abfss.createFile(uploadBytesFilePath,
+  statistics
+  , true, FsPermission.getDefault(),
+  FsPermission.getUMask(fs.getConf()));
+
+  int largeValue = 10;
+  for (int i = 0; i < largeValue; i++) {
+outForLargeBytes.write(testBytesToUpload.getBytes());
+  }
+  outForLargeBytes.flush();
+
+  //Test for large bytes to upload
+  assertValues("bytes to upload",
+  largeValue * (testBytesToUpload.getBytes().length),
+  outForLargeBytes.getOutputStreamStatistics().bytesToUpload);
+
+  //Test for relation between bytesUploadSuccessful, bytesUploadFailed
+  // and bytesToUpload
+  assertValues("bytesUploadSuccessful equal to difference between "
+  + "bytesToUpload and bytesUploadFailed",
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadSuccessful,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload -
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadFailed);
+
+} finally {
+  if (outForLargeBytes != null) {
+outForLargeBytes.close();
+  }
+}
+
+  }
+
+  /**
+   * Tests to check time spend on waiting for tasks to be complete on a
+   * blocking queue in {@link AbfsOutputStream}.
+   *
+   * @throws IOException
+   */
+  @Test
+  public void testAbfsOutputStreamTimeSpendOnWaitTask() throws IOException {
+describe("Testing Time Spend on Waiting for Task to be complete");
+final AzureBlobFileSystem fs = getFileSystem();
+Path timeSpendFilePath = new Path("AbfsOutputStreamStatsPath");
+AzureBlobFileSystemStore abfss = fs.getAbfsStore();
+FileSystem.Statistics statistics = fs.getFsStatistics();
+
+AbfsOutputStream out =
+(AbfsOutputStream) abfss.createFile(timeSpendFilePath,
+statistics, true,
+FsPermission.getDefault(), FsPermission.getUMask(fs.getConf()));
+
+  }
+
+  /**
+   * Tests to check number of {@codes shrinkWriteOperationQueue()}
+   * calls.
+   * After writing data, AbfsOutputStream doesn't upload the data until
+   * Flushed. Hence, flush() method is called after write() to test Queue
+   * shrink calls.
+   *

[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-03-20 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r395749213
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsOutputStream.java
 ##
 @@ -0,0 +1,278 @@
+package org.apache.hadoop.fs.azurebfs;
+
+import java.io.IOException;
+
+import org.junit.Test;
+
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.azurebfs.services.AbfsOutputStream;
+import org.apache.hadoop.fs.permission.FsPermission;
+
+/**
+ * Test AbfsOutputStream statistics.
+ */
+public class ITestAbfsOutputStream extends AbstractAbfsIntegrationTest {
+
+  public ITestAbfsOutputStream() throws Exception {
+  }
+
+  /**
+   * Tests to check bytes Uploading in {@link AbfsOutputStream}.
+   *
+   * @throws IOException
+   */
+  @Test
+  public void testAbfsOutputStreamUploadingBytes() throws IOException {
+describe("Testing Bytes uploaded in AbfsOutputSteam");
+final AzureBlobFileSystem fs = getFileSystem();
+Path uploadBytesFilePath = new Path("AbfsOutputStreamStatsPath");
+AzureBlobFileSystemStore abfss = fs.getAbfsStore();
+FileSystem.Statistics statistics = fs.getFsStatistics();
+abfss.getAbfsConfiguration().setDisableOutputStreamFlush(false);
+String testBytesToUpload = "bytes";
+
+AbfsOutputStream outForSomeBytes = null;
+try {
+  outForSomeBytes = (AbfsOutputStream) 
abfss.createFile(uploadBytesFilePath,
+  statistics,
+  true,
+  FsPermission.getDefault(), FsPermission.getUMask(fs.getConf()));
+
+  //Test for zero bytes To upload
+  assertValues("bytes to upload", 0,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload);
+
+  outForSomeBytes.write(testBytesToUpload.getBytes());
+  outForSomeBytes.flush();
+
+  //Test for some bytes to upload
+  assertValues("bytes to upload", testBytesToUpload.getBytes().length,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload);
+
+  //Test for relation between bytesUploadSuccessful, bytesUploadFailed
+  // and bytesToUpload
+  assertValues("bytesUploadSuccessful equal to difference between "
+  + "bytesToUpload and bytesUploadFailed",
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadSuccessful,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload -
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadFailed);
+
+} finally {
+  if (outForSomeBytes != null) {
+outForSomeBytes.close();
+  }
+}
+
+AbfsOutputStream outForLargeBytes = null;
+try {
 
 Review comment:
   same


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-03-20 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r395751750
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsOutputStream.java
 ##
 @@ -0,0 +1,278 @@
+package org.apache.hadoop.fs.azurebfs;
+
+import java.io.IOException;
+
+import org.junit.Test;
+
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.azurebfs.services.AbfsOutputStream;
+import org.apache.hadoop.fs.permission.FsPermission;
+
+/**
+ * Test AbfsOutputStream statistics.
+ */
+public class ITestAbfsOutputStream extends AbstractAbfsIntegrationTest {
+
+  public ITestAbfsOutputStream() throws Exception {
+  }
+
+  /**
+   * Tests to check bytes Uploading in {@link AbfsOutputStream}.
+   *
+   * @throws IOException
+   */
+  @Test
+  public void testAbfsOutputStreamUploadingBytes() throws IOException {
+describe("Testing Bytes uploaded in AbfsOutputSteam");
+final AzureBlobFileSystem fs = getFileSystem();
+Path uploadBytesFilePath = new Path("AbfsOutputStreamStatsPath");
+AzureBlobFileSystemStore abfss = fs.getAbfsStore();
+FileSystem.Statistics statistics = fs.getFsStatistics();
+abfss.getAbfsConfiguration().setDisableOutputStreamFlush(false);
+String testBytesToUpload = "bytes";
+
+AbfsOutputStream outForSomeBytes = null;
+try {
+  outForSomeBytes = (AbfsOutputStream) 
abfss.createFile(uploadBytesFilePath,
+  statistics,
+  true,
+  FsPermission.getDefault(), FsPermission.getUMask(fs.getConf()));
+
+  //Test for zero bytes To upload
+  assertValues("bytes to upload", 0,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload);
+
+  outForSomeBytes.write(testBytesToUpload.getBytes());
+  outForSomeBytes.flush();
+
+  //Test for some bytes to upload
+  assertValues("bytes to upload", testBytesToUpload.getBytes().length,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload);
+
+  //Test for relation between bytesUploadSuccessful, bytesUploadFailed
+  // and bytesToUpload
+  assertValues("bytesUploadSuccessful equal to difference between "
+  + "bytesToUpload and bytesUploadFailed",
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadSuccessful,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload -
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadFailed);
+
+} finally {
+  if (outForSomeBytes != null) {
+outForSomeBytes.close();
+  }
+}
+
+AbfsOutputStream outForLargeBytes = null;
+try {
+  outForLargeBytes =
+  (AbfsOutputStream) abfss.createFile(uploadBytesFilePath,
+  statistics
+  , true, FsPermission.getDefault(),
+  FsPermission.getUMask(fs.getConf()));
+
+  int largeValue = 10;
+  for (int i = 0; i < largeValue; i++) {
+outForLargeBytes.write(testBytesToUpload.getBytes());
+  }
+  outForLargeBytes.flush();
+
+  //Test for large bytes to upload
+  assertValues("bytes to upload",
+  largeValue * (testBytesToUpload.getBytes().length),
+  outForLargeBytes.getOutputStreamStatistics().bytesToUpload);
+
+  //Test for relation between bytesUploadSuccessful, bytesUploadFailed
+  // and bytesToUpload
+  assertValues("bytesUploadSuccessful equal to difference between "
+  + "bytesToUpload and bytesUploadFailed",
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadSuccessful,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload -
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadFailed);
+
+} finally {
+  if (outForLargeBytes != null) {
+outForLargeBytes.close();
+  }
+}
+
+  }
+
+  /**
+   * Tests to check time spend on waiting for tasks to be complete on a
+   * blocking queue in {@link AbfsOutputStream}.
+   *
+   * @throws IOException
+   */
+  @Test
+  public void testAbfsOutputStreamTimeSpendOnWaitTask() throws IOException {
+describe("Testing Time Spend on Waiting for Task to be complete");
+final AzureBlobFileSystem fs = getFileSystem();
+Path timeSpendFilePath = new Path("AbfsOutputStreamStatsPath");
+AzureBlobFileSystemStore abfss = fs.getAbfsStore();
+FileSystem.Statistics statistics = fs.getFsStatistics();
+
+AbfsOutputStream out =
+(AbfsOutputStream) abfss.createFile(timeSpendFilePath,
+statistics, true,
+FsPermission.getDefault(), FsPermission.getUMask(fs.getConf()));
+
+  }
+
+  /**
+   * Tests to check number of {@codes shrinkWriteOperationQueue()}
+   * calls.
+   * After writing data, AbfsOutputStream doesn't upload the data until
+   * Flushed. Hence, flush() method is called after write() to test Queue
+   * shrink calls.
+   *

[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-03-20 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r395750967
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsOutputStream.java
 ##
 @@ -0,0 +1,278 @@
+package org.apache.hadoop.fs.azurebfs;
+
+import java.io.IOException;
+
+import org.junit.Test;
+
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.azurebfs.services.AbfsOutputStream;
+import org.apache.hadoop.fs.permission.FsPermission;
+
+/**
+ * Test AbfsOutputStream statistics.
+ */
+public class ITestAbfsOutputStream extends AbstractAbfsIntegrationTest {
+
+  public ITestAbfsOutputStream() throws Exception {
+  }
+
+  /**
+   * Tests to check bytes Uploading in {@link AbfsOutputStream}.
+   *
+   * @throws IOException
+   */
+  @Test
+  public void testAbfsOutputStreamUploadingBytes() throws IOException {
+describe("Testing Bytes uploaded in AbfsOutputSteam");
+final AzureBlobFileSystem fs = getFileSystem();
+Path uploadBytesFilePath = new Path("AbfsOutputStreamStatsPath");
+AzureBlobFileSystemStore abfss = fs.getAbfsStore();
+FileSystem.Statistics statistics = fs.getFsStatistics();
+abfss.getAbfsConfiguration().setDisableOutputStreamFlush(false);
+String testBytesToUpload = "bytes";
+
+AbfsOutputStream outForSomeBytes = null;
+try {
+  outForSomeBytes = (AbfsOutputStream) 
abfss.createFile(uploadBytesFilePath,
+  statistics,
+  true,
+  FsPermission.getDefault(), FsPermission.getUMask(fs.getConf()));
+
+  //Test for zero bytes To upload
+  assertValues("bytes to upload", 0,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload);
+
+  outForSomeBytes.write(testBytesToUpload.getBytes());
+  outForSomeBytes.flush();
+
+  //Test for some bytes to upload
+  assertValues("bytes to upload", testBytesToUpload.getBytes().length,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload);
+
+  //Test for relation between bytesUploadSuccessful, bytesUploadFailed
+  // and bytesToUpload
+  assertValues("bytesUploadSuccessful equal to difference between "
+  + "bytesToUpload and bytesUploadFailed",
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadSuccessful,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload -
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadFailed);
+
+} finally {
+  if (outForSomeBytes != null) {
+outForSomeBytes.close();
+  }
+}
+
+AbfsOutputStream outForLargeBytes = null;
+try {
+  outForLargeBytes =
+  (AbfsOutputStream) abfss.createFile(uploadBytesFilePath,
+  statistics
+  , true, FsPermission.getDefault(),
+  FsPermission.getUMask(fs.getConf()));
+
+  int largeValue = 10;
+  for (int i = 0; i < largeValue; i++) {
+outForLargeBytes.write(testBytesToUpload.getBytes());
+  }
+  outForLargeBytes.flush();
+
+  //Test for large bytes to upload
+  assertValues("bytes to upload",
+  largeValue * (testBytesToUpload.getBytes().length),
+  outForLargeBytes.getOutputStreamStatistics().bytesToUpload);
+
+  //Test for relation between bytesUploadSuccessful, bytesUploadFailed
+  // and bytesToUpload
+  assertValues("bytesUploadSuccessful equal to difference between "
+  + "bytesToUpload and bytesUploadFailed",
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadSuccessful,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload -
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadFailed);
+
+} finally {
+  if (outForLargeBytes != null) {
+outForLargeBytes.close();
+  }
+}
+
+  }
+
+  /**
+   * Tests to check time spend on waiting for tasks to be complete on a
+   * blocking queue in {@link AbfsOutputStream}.
+   *
+   * @throws IOException
+   */
+  @Test
+  public void testAbfsOutputStreamTimeSpendOnWaitTask() throws IOException {
+describe("Testing Time Spend on Waiting for Task to be complete");
+final AzureBlobFileSystem fs = getFileSystem();
+Path timeSpendFilePath = new Path("AbfsOutputStreamStatsPath");
+AzureBlobFileSystemStore abfss = fs.getAbfsStore();
+FileSystem.Statistics statistics = fs.getFsStatistics();
+
+AbfsOutputStream out =
+(AbfsOutputStream) abfss.createFile(timeSpendFilePath,
+statistics, true,
+FsPermission.getDefault(), FsPermission.getUMask(fs.getConf()));
+
+  }
+
+  /**
+   * Tests to check number of {@codes shrinkWriteOperationQueue()}
+   * calls.
+   * After writing data, AbfsOutputStream doesn't upload the data until
+   * Flushed. Hence, flush() method is called after write() to test Queue
+   * shrink calls.
+   *

[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-03-20 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r395755381
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsOutputStreamStatisticsImpl.java
 ##
 @@ -0,0 +1,102 @@
+package org.apache.hadoop.fs.azurebfs.services;
+
+/**
+ * OutputStream Statistics Implementation for Abfs.
+ * timeSpendOnTaskWait - Time spend on waiting for tasks to be complete on
+ * Blocking Queue in AbfsOutputStream.
+ *
+ * queueShrink - Number of times Blocking Queue was shrunk after writing
+ * data.
+ *
+ * WriteCurrentBufferOperations - Number of times
+ * {@codes writeCurrentBufferToService()} calls were made.
+ */
+public class AbfsOutputStreamStatisticsImpl
+implements AbfsOutputStreamStatistics {
+  public volatile long bytesToUpload;
 
 Review comment:
   let's make these private and have getters


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-03-20 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r395750011
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsOutputStream.java
 ##
 @@ -0,0 +1,278 @@
+package org.apache.hadoop.fs.azurebfs;
+
+import java.io.IOException;
+
+import org.junit.Test;
+
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.azurebfs.services.AbfsOutputStream;
+import org.apache.hadoop.fs.permission.FsPermission;
+
+/**
+ * Test AbfsOutputStream statistics.
+ */
+public class ITestAbfsOutputStream extends AbstractAbfsIntegrationTest {
+
+  public ITestAbfsOutputStream() throws Exception {
+  }
+
+  /**
+   * Tests to check bytes Uploading in {@link AbfsOutputStream}.
+   *
+   * @throws IOException
+   */
+  @Test
+  public void testAbfsOutputStreamUploadingBytes() throws IOException {
+describe("Testing Bytes uploaded in AbfsOutputSteam");
+final AzureBlobFileSystem fs = getFileSystem();
+Path uploadBytesFilePath = new Path("AbfsOutputStreamStatsPath");
+AzureBlobFileSystemStore abfss = fs.getAbfsStore();
+FileSystem.Statistics statistics = fs.getFsStatistics();
+abfss.getAbfsConfiguration().setDisableOutputStreamFlush(false);
+String testBytesToUpload = "bytes";
+
+AbfsOutputStream outForSomeBytes = null;
+try {
+  outForSomeBytes = (AbfsOutputStream) 
abfss.createFile(uploadBytesFilePath,
+  statistics,
+  true,
+  FsPermission.getDefault(), FsPermission.getUMask(fs.getConf()));
+
+  //Test for zero bytes To upload
+  assertValues("bytes to upload", 0,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload);
+
+  outForSomeBytes.write(testBytesToUpload.getBytes());
+  outForSomeBytes.flush();
+
+  //Test for some bytes to upload
+  assertValues("bytes to upload", testBytesToUpload.getBytes().length,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload);
+
+  //Test for relation between bytesUploadSuccessful, bytesUploadFailed
+  // and bytesToUpload
+  assertValues("bytesUploadSuccessful equal to difference between "
+  + "bytesToUpload and bytesUploadFailed",
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadSuccessful,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload -
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadFailed);
+
+} finally {
+  if (outForSomeBytes != null) {
+outForSomeBytes.close();
+  }
+}
+
+AbfsOutputStream outForLargeBytes = null;
+try {
+  outForLargeBytes =
+  (AbfsOutputStream) abfss.createFile(uploadBytesFilePath,
+  statistics
+  , true, FsPermission.getDefault(),
+  FsPermission.getUMask(fs.getConf()));
+
+  int largeValue = 10;
+  for (int i = 0; i < largeValue; i++) {
+outForLargeBytes.write(testBytesToUpload.getBytes());
+  }
+  outForLargeBytes.flush();
+
+  //Test for large bytes to upload
+  assertValues("bytes to upload",
+  largeValue * (testBytesToUpload.getBytes().length),
+  outForLargeBytes.getOutputStreamStatistics().bytesToUpload);
+
+  //Test for relation between bytesUploadSuccessful, bytesUploadFailed
+  // and bytesToUpload
+  assertValues("bytesUploadSuccessful equal to difference between "
+  + "bytesToUpload and bytesUploadFailed",
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadSuccessful,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload -
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadFailed);
+
+} finally {
+  if (outForLargeBytes != null) {
+outForLargeBytes.close();
+  }
+}
+
+  }
+
+  /**
+   * Tests to check time spend on waiting for tasks to be complete on a
+   * blocking queue in {@link AbfsOutputStream}.
+   *
+   * @throws IOException
+   */
+  @Test
+  public void testAbfsOutputStreamTimeSpendOnWaitTask() throws IOException {
+describe("Testing Time Spend on Waiting for Task to be complete");
+final AzureBlobFileSystem fs = getFileSystem();
+Path timeSpendFilePath = new Path("AbfsOutputStreamStatsPath");
+AzureBlobFileSystemStore abfss = fs.getAbfsStore();
+FileSystem.Statistics statistics = fs.getFsStatistics();
+
+AbfsOutputStream out =
 
 Review comment:
   will need to be closed


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


With regards,
Apache Git Services


[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-03-20 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r395754102
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsStreamStatistics.java
 ##
 @@ -0,0 +1,147 @@
+/**
+ * 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 org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.hadoop.fs.FSDataOutputStream;
+import org.apache.hadoop.fs.FSDataInputStream;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+
+/**
+ * Test Abfs Stream.
+ */
+
+public class ITestAbfsStreamStatistics extends AbstractAbfsIntegrationTest {
+  public ITestAbfsStreamStatistics() throws Exception {
+  }
+
+  /***
+   * Testing {@code incrementReadOps()} in class {@code AbfsInputStream} and
+   * {@code incrementWriteOps()} in class {@code AbfsOutputStream}.
+   *
+   * @throws Exception
+   */
+  @Test
+  public void testAbfsStreamOps() throws Exception {
+describe("Test to see correct population of read and write operations in "
++ "Abfs");
+
+final AzureBlobFileSystem fs = getFileSystem();
+Path smallOperationsFile = new Path("testOneReadWriteOps");
+Path largeOperationsFile = new Path("testLargeReadWriteOps");
+FileSystem.Statistics statistics = fs.getFsStatistics();
+String testReadWriteOps = "test this";
+statistics.reset();
+
+//Test for zero write operation
+assertReadWriteOps("write", 0, statistics.getWriteOps());
+
+//Test for zero read operation
+assertReadWriteOps("read", 0, statistics.getReadOps());
+
+FSDataOutputStream outForOneOperation = null;
+FSDataInputStream inForOneOperation = null;
+try {
+  outForOneOperation = fs.create(smallOperationsFile);
+  statistics.reset();
+  outForOneOperation.write(testReadWriteOps.getBytes());
+
+  //Test for a single write operation
+  assertReadWriteOps("write", 1, statistics.getWriteOps());
+
+  inForOneOperation = fs.open(smallOperationsFile);
+  inForOneOperation.read(testReadWriteOps.getBytes(), 0,
+  testReadWriteOps.getBytes().length);
+
+  //Test for a single read operation
+  assertReadWriteOps("read", 1, statistics.getReadOps());
+
+} finally {
+  if (inForOneOperation != null) {
+inForOneOperation.close();
+  }
+  if (outForOneOperation != null) {
+outForOneOperation.close();
+  }
+}
+
+//Validating if content is being written in the smallOperationsFile
+Assert.assertTrue("Mismatch in content validation",
+validateContent(fs, smallOperationsFile,
+testReadWriteOps.getBytes()));
+
+FSDataOutputStream outForLargeOperations = null;
+FSDataInputStream inForLargeOperations = null;
+StringBuilder largeOperationsValidationString = new StringBuilder();
+try {
+  outForLargeOperations = fs.create(largeOperationsFile);
+  statistics.reset();
+  int largeValue = 100;
+  for (int i = 0; i < largeValue; i++) {
+outForLargeOperations.write(testReadWriteOps.getBytes());
+
+//Creating the String for content Validation
+largeOperationsValidationString.append(testReadWriteOps);
+  }
+
+  //Test for 100 write operations
+  assertReadWriteOps("write", largeValue, statistics.getWriteOps());
+
+  inForLargeOperations = fs.open(largeOperationsFile);
+  for (int i = 0; i < largeValue; i++)
+inForLargeOperations
+.read(testReadWriteOps.getBytes(), 0,
+testReadWriteOps.getBytes().length);
+
+  //Test for 100 read operations
+  assertReadWriteOps("read", largeValue, statistics.getReadOps());
+
+} finally {
+  if (inForLargeOperations != null) {
+inForLargeOperations.close();
+  }
+  if (outForLargeOperations != null) {
+outForLargeOperations.close();
+  }
+}
+
+//Validating if content is being written in largeOperationsFile
+Assert.assertTrue("Mismatch in content validation",
 
 Review comment:
   again, superflous with validateContent 

[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-03-20 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r395753442
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsStreamStatistics.java
 ##
 @@ -0,0 +1,147 @@
+/**
+ * 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 org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.hadoop.fs.FSDataOutputStream;
+import org.apache.hadoop.fs.FSDataInputStream;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+
+/**
+ * Test Abfs Stream.
+ */
+
+public class ITestAbfsStreamStatistics extends AbstractAbfsIntegrationTest {
+  public ITestAbfsStreamStatistics() throws Exception {
+  }
+
+  /***
+   * Testing {@code incrementReadOps()} in class {@code AbfsInputStream} and
+   * {@code incrementWriteOps()} in class {@code AbfsOutputStream}.
+   *
+   * @throws Exception
+   */
+  @Test
+  public void testAbfsStreamOps() throws Exception {
+describe("Test to see correct population of read and write operations in "
++ "Abfs");
+
+final AzureBlobFileSystem fs = getFileSystem();
+Path smallOperationsFile = new Path("testOneReadWriteOps");
+Path largeOperationsFile = new Path("testLargeReadWriteOps");
+FileSystem.Statistics statistics = fs.getFsStatistics();
+String testReadWriteOps = "test this";
+statistics.reset();
+
+//Test for zero write operation
+assertReadWriteOps("write", 0, statistics.getWriteOps());
+
+//Test for zero read operation
+assertReadWriteOps("read", 0, statistics.getReadOps());
+
+FSDataOutputStream outForOneOperation = null;
+FSDataInputStream inForOneOperation = null;
+try {
+  outForOneOperation = fs.create(smallOperationsFile);
+  statistics.reset();
+  outForOneOperation.write(testReadWriteOps.getBytes());
+
+  //Test for a single write operation
+  assertReadWriteOps("write", 1, statistics.getWriteOps());
+
+  inForOneOperation = fs.open(smallOperationsFile);
+  inForOneOperation.read(testReadWriteOps.getBytes(), 0,
+  testReadWriteOps.getBytes().length);
+
+  //Test for a single read operation
+  assertReadWriteOps("read", 1, statistics.getReadOps());
+
+} finally {
+  if (inForOneOperation != null) {
+inForOneOperation.close();
+  }
+  if (outForOneOperation != null) {
+outForOneOperation.close();
+  }
+}
+
+//Validating if content is being written in the smallOperationsFile
+Assert.assertTrue("Mismatch in content validation",
 
 Review comment:
   once validateContent raises exceptions, you don't need to wrap in an assert


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-03-20 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r395751870
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsOutputStream.java
 ##
 @@ -0,0 +1,278 @@
+package org.apache.hadoop.fs.azurebfs;
+
+import java.io.IOException;
+
+import org.junit.Test;
+
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.azurebfs.services.AbfsOutputStream;
+import org.apache.hadoop.fs.permission.FsPermission;
+
+/**
+ * Test AbfsOutputStream statistics.
+ */
+public class ITestAbfsOutputStream extends AbstractAbfsIntegrationTest {
+
+  public ITestAbfsOutputStream() throws Exception {
+  }
+
+  /**
+   * Tests to check bytes Uploading in {@link AbfsOutputStream}.
+   *
+   * @throws IOException
+   */
+  @Test
+  public void testAbfsOutputStreamUploadingBytes() throws IOException {
+describe("Testing Bytes uploaded in AbfsOutputSteam");
+final AzureBlobFileSystem fs = getFileSystem();
+Path uploadBytesFilePath = new Path("AbfsOutputStreamStatsPath");
+AzureBlobFileSystemStore abfss = fs.getAbfsStore();
+FileSystem.Statistics statistics = fs.getFsStatistics();
+abfss.getAbfsConfiguration().setDisableOutputStreamFlush(false);
+String testBytesToUpload = "bytes";
+
+AbfsOutputStream outForSomeBytes = null;
+try {
+  outForSomeBytes = (AbfsOutputStream) 
abfss.createFile(uploadBytesFilePath,
+  statistics,
+  true,
+  FsPermission.getDefault(), FsPermission.getUMask(fs.getConf()));
+
+  //Test for zero bytes To upload
+  assertValues("bytes to upload", 0,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload);
+
+  outForSomeBytes.write(testBytesToUpload.getBytes());
+  outForSomeBytes.flush();
+
+  //Test for some bytes to upload
+  assertValues("bytes to upload", testBytesToUpload.getBytes().length,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload);
+
+  //Test for relation between bytesUploadSuccessful, bytesUploadFailed
+  // and bytesToUpload
+  assertValues("bytesUploadSuccessful equal to difference between "
+  + "bytesToUpload and bytesUploadFailed",
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadSuccessful,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload -
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadFailed);
+
+} finally {
+  if (outForSomeBytes != null) {
+outForSomeBytes.close();
+  }
+}
+
+AbfsOutputStream outForLargeBytes = null;
+try {
+  outForLargeBytes =
+  (AbfsOutputStream) abfss.createFile(uploadBytesFilePath,
+  statistics
+  , true, FsPermission.getDefault(),
+  FsPermission.getUMask(fs.getConf()));
+
+  int largeValue = 10;
+  for (int i = 0; i < largeValue; i++) {
+outForLargeBytes.write(testBytesToUpload.getBytes());
+  }
+  outForLargeBytes.flush();
+
+  //Test for large bytes to upload
+  assertValues("bytes to upload",
+  largeValue * (testBytesToUpload.getBytes().length),
+  outForLargeBytes.getOutputStreamStatistics().bytesToUpload);
+
+  //Test for relation between bytesUploadSuccessful, bytesUploadFailed
+  // and bytesToUpload
+  assertValues("bytesUploadSuccessful equal to difference between "
+  + "bytesToUpload and bytesUploadFailed",
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadSuccessful,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload -
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadFailed);
+
+} finally {
+  if (outForLargeBytes != null) {
+outForLargeBytes.close();
+  }
+}
+
+  }
+
+  /**
+   * Tests to check time spend on waiting for tasks to be complete on a
+   * blocking queue in {@link AbfsOutputStream}.
+   *
+   * @throws IOException
+   */
+  @Test
+  public void testAbfsOutputStreamTimeSpendOnWaitTask() throws IOException {
+describe("Testing Time Spend on Waiting for Task to be complete");
+final AzureBlobFileSystem fs = getFileSystem();
+Path timeSpendFilePath = new Path("AbfsOutputStreamStatsPath");
+AzureBlobFileSystemStore abfss = fs.getAbfsStore();
+FileSystem.Statistics statistics = fs.getFsStatistics();
+
+AbfsOutputStream out =
+(AbfsOutputStream) abfss.createFile(timeSpendFilePath,
+statistics, true,
+FsPermission.getDefault(), FsPermission.getUMask(fs.getConf()));
+
+  }
+
+  /**
+   * Tests to check number of {@codes shrinkWriteOperationQueue()}
+   * calls.
+   * After writing data, AbfsOutputStream doesn't upload the data until
+   * Flushed. Hence, flush() method is called after write() to test Queue
+   * shrink calls.
+   *

[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-03-20 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r395752947
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsStreamStatistics.java
 ##
 @@ -0,0 +1,147 @@
+/**
+ * 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 org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.hadoop.fs.FSDataOutputStream;
+import org.apache.hadoop.fs.FSDataInputStream;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+
+/**
+ * Test Abfs Stream.
+ */
+
+public class ITestAbfsStreamStatistics extends AbstractAbfsIntegrationTest {
+  public ITestAbfsStreamStatistics() throws Exception {
 
 Review comment:
   not 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:
us...@infra.apache.org


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-03-20 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r395752609
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsOutputStream.java
 ##
 @@ -0,0 +1,278 @@
+package org.apache.hadoop.fs.azurebfs;
+
+import java.io.IOException;
+
+import org.junit.Test;
+
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.azurebfs.services.AbfsOutputStream;
+import org.apache.hadoop.fs.permission.FsPermission;
+
+/**
+ * Test AbfsOutputStream statistics.
+ */
+public class ITestAbfsOutputStream extends AbstractAbfsIntegrationTest {
 
 Review comment:
   This is sounds like a slow test.
   
   1. Use smaller values than 1000, e.g. "10"
   2. make the value a constant used across all tests. 


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-03-20 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r395743922
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsOutputStream.java
 ##
 @@ -36,20 +36,25 @@
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 
+import org.apache.hadoop.fs.FileSystem.Statistics;
 
 Review comment:
   move down to under ElasticByteBufferPool


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-03-20 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r395756248
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsOutputStream.java
 ##
 @@ -28,28 +26,38 @@ public ITestAbfsOutputStream() throws Exception {
   public void testAbfsOutputStreamUploadingBytes() throws IOException {
 
 Review comment:
   I can't think of any. Maybe just have a unit test to take an 
AbfsOutputStreamsImpl and verify that when the method is called, the counter is 
updated.
   
   (Actually, mocking could simulate failure, ...)


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-03-20 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r395743551
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsInputStream.java
 ##
 @@ -101,6 +101,7 @@ public synchronized int read(final byte[] b, final int 
off, final int len) throw
 int currentLen = len;
 int lastReadBytes;
 int totalReadBytes = 0;
+incrementReadOps();
 
 Review comment:
   this is input stream; presumably it's come in from somewhere else


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-03-20 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r395753822
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsStreamStatistics.java
 ##
 @@ -0,0 +1,147 @@
+/**
+ * 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 org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.hadoop.fs.FSDataOutputStream;
+import org.apache.hadoop.fs.FSDataInputStream;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+
+/**
+ * Test Abfs Stream.
+ */
+
+public class ITestAbfsStreamStatistics extends AbstractAbfsIntegrationTest {
+  public ITestAbfsStreamStatistics() throws Exception {
+  }
+
+  /***
+   * Testing {@code incrementReadOps()} in class {@code AbfsInputStream} and
+   * {@code incrementWriteOps()} in class {@code AbfsOutputStream}.
+   *
+   * @throws Exception
+   */
+  @Test
+  public void testAbfsStreamOps() throws Exception {
+describe("Test to see correct population of read and write operations in "
++ "Abfs");
+
+final AzureBlobFileSystem fs = getFileSystem();
+Path smallOperationsFile = new Path("testOneReadWriteOps");
+Path largeOperationsFile = new Path("testLargeReadWriteOps");
+FileSystem.Statistics statistics = fs.getFsStatistics();
+String testReadWriteOps = "test this";
+statistics.reset();
+
+//Test for zero write operation
+assertReadWriteOps("write", 0, statistics.getWriteOps());
+
+//Test for zero read operation
+assertReadWriteOps("read", 0, statistics.getReadOps());
+
+FSDataOutputStream outForOneOperation = null;
+FSDataInputStream inForOneOperation = null;
+try {
+  outForOneOperation = fs.create(smallOperationsFile);
+  statistics.reset();
+  outForOneOperation.write(testReadWriteOps.getBytes());
+
+  //Test for a single write operation
+  assertReadWriteOps("write", 1, statistics.getWriteOps());
+
+  inForOneOperation = fs.open(smallOperationsFile);
+  inForOneOperation.read(testReadWriteOps.getBytes(), 0,
+  testReadWriteOps.getBytes().length);
+
+  //Test for a single read operation
+  assertReadWriteOps("read", 1, statistics.getReadOps());
+
+} finally {
+  if (inForOneOperation != null) {
+inForOneOperation.close();
+  }
+  if (outForOneOperation != null) {
+outForOneOperation.close();
+  }
+}
+
+//Validating if content is being written in the smallOperationsFile
+Assert.assertTrue("Mismatch in content validation",
+validateContent(fs, smallOperationsFile,
+testReadWriteOps.getBytes()));
+
+FSDataOutputStream outForLargeOperations = null;
+FSDataInputStream inForLargeOperations = null;
+StringBuilder largeOperationsValidationString = new StringBuilder();
+try {
+  outForLargeOperations = fs.create(largeOperationsFile);
+  statistics.reset();
+  int largeValue = 100;
+  for (int i = 0; i < largeValue; i++) {
+outForLargeOperations.write(testReadWriteOps.getBytes());
+
+//Creating the String for content Validation
+largeOperationsValidationString.append(testReadWriteOps);
+  }
+
+  //Test for 100 write operations
+  assertReadWriteOps("write", largeValue, statistics.getWriteOps());
+
+  inForLargeOperations = fs.open(largeOperationsFile);
+  for (int i = 0; i < largeValue; i++)
+inForLargeOperations
+.read(testReadWriteOps.getBytes(), 0,
+testReadWriteOps.getBytes().length);
+
+  //Test for 100 read operations
+  assertReadWriteOps("read", largeValue, statistics.getReadOps());
+
+} finally {
+  if (inForLargeOperations != null) {
 
 Review comment:
   IOUtils.closeQuietly


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:

[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-03-20 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r395746300
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsOutputStreamStatisticsImpl.java
 ##
 @@ -0,0 +1,102 @@
+package org.apache.hadoop.fs.azurebfs.services;
+
+/**
+ * OutputStream Statistics Implementation for Abfs.
+ * timeSpendOnTaskWait - Time spend on waiting for tasks to be complete on
+ * Blocking Queue in AbfsOutputStream.
+ *
+ * queueShrink - Number of times Blocking Queue was shrunk after writing
+ * data.
+ *
+ * WriteCurrentBufferOperations - Number of times
+ * {@codes writeCurrentBufferToService()} calls were made.
+ */
+public class AbfsOutputStreamStatisticsImpl
+implements AbfsOutputStreamStatistics {
+  public volatile long bytesToUpload;
+  public volatile long bytesUploadSuccessful;
+  public volatile long bytesUploadFailed;
+  public volatile long timeSpendOnTaskWait;
+  public volatile long queueShrink;
+  public volatile long writeCurrentBufferOperations;
+
+  /**
+   * Number of bytes uploaded only when bytes passed are positive.
+   *
+   * @param bytes negative values are ignored
+   */
+  @Override
+  public void bytesToUpload(long bytes) {
+if (bytes > 0) {
+  bytesToUpload += bytes;
+}
+  }
+
+  @Override
+  public void bytesUploadedSuccessfully(long bytes) {
+if (bytes > 0) {
+  bytesUploadSuccessful += bytes;
+}
+  }
+
+  /**
+   * Number of bytes that weren't uploaded.
+   *
+   * @param bytes negative values are ignored
+   */
+  @Override
+  public void bytesFailed(long bytes) {
+if (bytes > 0) {
+  bytesUploadFailed += bytes;
+}
+  }
+
+  /**
+   * Time spend for waiting a task to be completed.
+   *
+   * @param startTime on calling {@link 
AbfsOutputStream#waitForTaskToComplete()}
+   * @param endTime   on method completing
+   */
+  @Override
+  public void timeSpendTaskWait(long startTime, long endTime) {
+timeSpendOnTaskWait += endTime - startTime;
+  }
+
+  /**
+   * Number of calls to {@link AbfsOutputStream#shrinkWriteOperationQueue()}.
+   */
+  @Override
+  public void queueShrinked() {
+queueShrink++;
+  }
+
+  /**
+   * Number of calls to {@link AbfsOutputStream#writeCurrentBufferToService()}.
 
 Review comment:
   see above comment about javadocs


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-03-20 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r395746252
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsOutputStreamStatisticsImpl.java
 ##
 @@ -0,0 +1,102 @@
+package org.apache.hadoop.fs.azurebfs.services;
+
+/**
+ * OutputStream Statistics Implementation for Abfs.
+ * timeSpendOnTaskWait - Time spend on waiting for tasks to be complete on
+ * Blocking Queue in AbfsOutputStream.
+ *
+ * queueShrink - Number of times Blocking Queue was shrunk after writing
+ * data.
+ *
+ * WriteCurrentBufferOperations - Number of times
+ * {@codes writeCurrentBufferToService()} calls were made.
+ */
+public class AbfsOutputStreamStatisticsImpl
+implements AbfsOutputStreamStatistics {
+  public volatile long bytesToUpload;
+  public volatile long bytesUploadSuccessful;
+  public volatile long bytesUploadFailed;
+  public volatile long timeSpendOnTaskWait;
+  public volatile long queueShrink;
+  public volatile long writeCurrentBufferOperations;
+
+  /**
+   * Number of bytes uploaded only when bytes passed are positive.
+   *
+   * @param bytes negative values are ignored
+   */
+  @Override
+  public void bytesToUpload(long bytes) {
+if (bytes > 0) {
+  bytesToUpload += bytes;
+}
+  }
+
+  @Override
+  public void bytesUploadedSuccessfully(long bytes) {
+if (bytes > 0) {
+  bytesUploadSuccessful += bytes;
+}
+  }
+
+  /**
+   * Number of bytes that weren't uploaded.
+   *
+   * @param bytes negative values are ignored
+   */
+  @Override
+  public void bytesFailed(long bytes) {
+if (bytes > 0) {
+  bytesUploadFailed += bytes;
+}
+  }
+
+  /**
+   * Time spend for waiting a task to be completed.
+   *
+   * @param startTime on calling {@link 
AbfsOutputStream#waitForTaskToComplete()}
+   * @param endTime   on method completing
+   */
+  @Override
+  public void timeSpendTaskWait(long startTime, long endTime) {
+timeSpendOnTaskWait += endTime - startTime;
+  }
+
+  /**
+   * Number of calls to {@link AbfsOutputStream#shrinkWriteOperationQueue()}.
 
 Review comment:
   see above comment about javadocs


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-03-20 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r395744474
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsOutputStream.java
 ##
 @@ -36,20 +36,25 @@
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 
+import org.apache.hadoop.fs.FileSystem.Statistics;
 import 
org.apache.hadoop.fs.azurebfs.contracts.exceptions.AbfsRestOperationException;
 import 
org.apache.hadoop.fs.azurebfs.contracts.exceptions.AzureBlobFileSystemException;
 import org.apache.hadoop.io.ElasticByteBufferPool;
 import org.apache.hadoop.fs.FSExceptionMessages;
 import org.apache.hadoop.fs.StreamCapabilities;
 import org.apache.hadoop.fs.Syncable;
 
+import static org.apache.hadoop.io.IOUtils.LOG;
 import static org.apache.hadoop.io.IOUtils.wrapException;
 
 /**
  * The BlobFsOutputStream for Rest AbfsClient.
  */
 public class AbfsOutputStream extends OutputStream implements Syncable, 
StreamCapabilities {
+
 
 Review comment:
   add both new fields at the bottom of the other fields, e.g Line 85, and keep 
togeher. 


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-03-20 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r395746011
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsOutputStreamStatisticsImpl.java
 ##
 @@ -0,0 +1,102 @@
+package org.apache.hadoop.fs.azurebfs.services;
+
+/**
+ * OutputStream Statistics Implementation for Abfs.
+ * timeSpendOnTaskWait - Time spend on waiting for tasks to be complete on
+ * Blocking Queue in AbfsOutputStream.
+ *
+ * queueShrink - Number of times Blocking Queue was shrunk after writing
+ * data.
+ *
+ * WriteCurrentBufferOperations - Number of times
+ * {@codes writeCurrentBufferToService()} calls were made.
+ */
+public class AbfsOutputStreamStatisticsImpl
+implements AbfsOutputStreamStatistics {
+  public volatile long bytesToUpload;
+  public volatile long bytesUploadSuccessful;
+  public volatile long bytesUploadFailed;
+  public volatile long timeSpendOnTaskWait;
+  public volatile long queueShrink;
+  public volatile long writeCurrentBufferOperations;
+
+  /**
+   * Number of bytes uploaded only when bytes passed are positive.
+   *
+   * @param bytes negative values are ignored
+   */
+  @Override
+  public void bytesToUpload(long bytes) {
+if (bytes > 0) {
+  bytesToUpload += bytes;
+}
+  }
+
+  @Override
+  public void bytesUploadedSuccessfully(long bytes) {
+if (bytes > 0) {
+  bytesUploadSuccessful += bytes;
+}
+  }
+
+  /**
+   * Number of bytes that weren't uploaded.
+   *
+   * @param bytes negative values are ignored
+   */
+  @Override
+  public void bytesFailed(long bytes) {
+if (bytes > 0) {
+  bytesUploadFailed += bytes;
+}
+  }
+
+  /**
+   * Time spend for waiting a task to be completed.
+   *
+   * @param startTime on calling {@link 
AbfsOutputStream#waitForTaskToComplete()}
 
 Review comment:
   MUST NOT use @link to private/package-private/protected methods. Javadoc 
will fail


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-03-20 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r395748307
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/AbstractAbfsTestWithTimeout.java
 ##
 @@ -67,4 +77,46 @@ public void nameThread() {
   protected int getTestTimeoutMillis() {
 return TEST_TIMEOUT;
   }
+
+  /**
+   * Describe a test in the logs.
+   *
+   * @param text text to print
+   * @param args arguments to format in the printing
+   */
+  protected void describe(String text, Object... args) {
+LOG.info("\n\n{}: {}\n",
+methodName.getMethodName(),
+String.format(text, args));
+  }
+
+  /**
+   * Validate Contents written on a file in Abfs.
+   *
+   * @param fsAzureBlobFileSystem
+   * @param path  Path of the file
+   * @param originalByteArray original byte array
+   * @return if content is validated true else, false
+   * @throws IOException
+   */
+  protected boolean validateContent(AzureBlobFileSystem fs, Path path,
+  byte[] originalByteArray)
+  throws IOException {
+FSDataInputStream in = fs.open(path);
+
+int pos = 0;
+int lenOfOriginalByteArray = originalByteArray.length;
+byte valueOfContentAtPos = (byte) in.read();
+
+while (valueOfContentAtPos != -1 && pos < lenOfOriginalByteArray) {
 
 Review comment:
   1. MUST use { } in all if () clauses.
   2. If there's a mismatch, use AssertEquals and include the pos where the 
problem occurred
   
   Imagine: "A remote test run failed -what information should be in the test 
report to begin debugging this?"


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-03-20 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r395749763
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsOutputStream.java
 ##
 @@ -0,0 +1,278 @@
+package org.apache.hadoop.fs.azurebfs;
+
+import java.io.IOException;
+
+import org.junit.Test;
+
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.azurebfs.services.AbfsOutputStream;
+import org.apache.hadoop.fs.permission.FsPermission;
+
+/**
+ * Test AbfsOutputStream statistics.
+ */
+public class ITestAbfsOutputStream extends AbstractAbfsIntegrationTest {
+
+  public ITestAbfsOutputStream() throws Exception {
+  }
+
+  /**
+   * Tests to check bytes Uploading in {@link AbfsOutputStream}.
+   *
+   * @throws IOException
+   */
+  @Test
+  public void testAbfsOutputStreamUploadingBytes() throws IOException {
+describe("Testing Bytes uploaded in AbfsOutputSteam");
+final AzureBlobFileSystem fs = getFileSystem();
+Path uploadBytesFilePath = new Path("AbfsOutputStreamStatsPath");
+AzureBlobFileSystemStore abfss = fs.getAbfsStore();
+FileSystem.Statistics statistics = fs.getFsStatistics();
+abfss.getAbfsConfiguration().setDisableOutputStreamFlush(false);
+String testBytesToUpload = "bytes";
+
+AbfsOutputStream outForSomeBytes = null;
+try {
+  outForSomeBytes = (AbfsOutputStream) 
abfss.createFile(uploadBytesFilePath,
+  statistics,
+  true,
+  FsPermission.getDefault(), FsPermission.getUMask(fs.getConf()));
+
+  //Test for zero bytes To upload
+  assertValues("bytes to upload", 0,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload);
+
+  outForSomeBytes.write(testBytesToUpload.getBytes());
+  outForSomeBytes.flush();
+
+  //Test for some bytes to upload
+  assertValues("bytes to upload", testBytesToUpload.getBytes().length,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload);
+
+  //Test for relation between bytesUploadSuccessful, bytesUploadFailed
+  // and bytesToUpload
+  assertValues("bytesUploadSuccessful equal to difference between "
+  + "bytesToUpload and bytesUploadFailed",
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadSuccessful,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload -
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadFailed);
+
+} finally {
+  if (outForSomeBytes != null) {
+outForSomeBytes.close();
+  }
+}
+
+AbfsOutputStream outForLargeBytes = null;
+try {
+  outForLargeBytes =
+  (AbfsOutputStream) abfss.createFile(uploadBytesFilePath,
+  statistics
+  , true, FsPermission.getDefault(),
+  FsPermission.getUMask(fs.getConf()));
+
+  int largeValue = 10;
+  for (int i = 0; i < largeValue; i++) {
+outForLargeBytes.write(testBytesToUpload.getBytes());
+  }
+  outForLargeBytes.flush();
+
+  //Test for large bytes to upload
+  assertValues("bytes to upload",
+  largeValue * (testBytesToUpload.getBytes().length),
+  outForLargeBytes.getOutputStreamStatistics().bytesToUpload);
+
+  //Test for relation between bytesUploadSuccessful, bytesUploadFailed
+  // and bytesToUpload
+  assertValues("bytesUploadSuccessful equal to difference between "
+  + "bytesToUpload and bytesUploadFailed",
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadSuccessful,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload -
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadFailed);
+
+} finally {
+  if (outForLargeBytes != null) {
+outForLargeBytes.close();
+  }
+}
+
+  }
+
+  /**
+   * Tests to check time spend on waiting for tasks to be complete on a
+   * blocking queue in {@link AbfsOutputStream}.
+   *
+   * @throws IOException
+   */
+  @Test
+  public void testAbfsOutputStreamTimeSpendOnWaitTask() throws IOException {
 
 Review comment:
   Also, "time spent"


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-03-20 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r395745249
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsOutputStreamStatistics.java
 ##
 @@ -0,0 +1,60 @@
+package org.apache.hadoop.fs.azurebfs.services;
+
+import org.apache.hadoop.classification.InterfaceStability;
+
+/**
+ * Interface for {@link AbfsOutputStream} statistics.
+ */
+@InterfaceStability.Unstable
+public interface AbfsOutputStreamStatistics {
+
+  /**
+   * Number of bytes to be uploaded.
+   *
+   * @param bytes number of bytes to upload
+   */
+  void bytesToUpload(long bytes);
+
+  /**
+   * Number of bytes uploaded Successfully.
+   *
+   * @param bytes number of bytes that were successfully uploaded
+   */
+  void bytesUploadedSuccessfully(long bytes);
+
+  /**
+   * Number of bytes failed to upload.
+   *
+   * @param bytes number of bytes that failed to upload
+   */
+  void bytesFailed(long bytes);
 
 Review comment:
   prefer a more detailed description like uploadFailed(long). It's recording 
that an upload failed and the number of bytes


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-03-20 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r395751470
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsOutputStream.java
 ##
 @@ -0,0 +1,278 @@
+package org.apache.hadoop.fs.azurebfs;
+
+import java.io.IOException;
+
+import org.junit.Test;
+
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.azurebfs.services.AbfsOutputStream;
+import org.apache.hadoop.fs.permission.FsPermission;
+
+/**
+ * Test AbfsOutputStream statistics.
+ */
+public class ITestAbfsOutputStream extends AbstractAbfsIntegrationTest {
+
+  public ITestAbfsOutputStream() throws Exception {
+  }
+
+  /**
+   * Tests to check bytes Uploading in {@link AbfsOutputStream}.
+   *
+   * @throws IOException
+   */
+  @Test
+  public void testAbfsOutputStreamUploadingBytes() throws IOException {
+describe("Testing Bytes uploaded in AbfsOutputSteam");
+final AzureBlobFileSystem fs = getFileSystem();
+Path uploadBytesFilePath = new Path("AbfsOutputStreamStatsPath");
+AzureBlobFileSystemStore abfss = fs.getAbfsStore();
+FileSystem.Statistics statistics = fs.getFsStatistics();
+abfss.getAbfsConfiguration().setDisableOutputStreamFlush(false);
+String testBytesToUpload = "bytes";
+
+AbfsOutputStream outForSomeBytes = null;
+try {
+  outForSomeBytes = (AbfsOutputStream) 
abfss.createFile(uploadBytesFilePath,
+  statistics,
+  true,
+  FsPermission.getDefault(), FsPermission.getUMask(fs.getConf()));
+
+  //Test for zero bytes To upload
+  assertValues("bytes to upload", 0,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload);
+
+  outForSomeBytes.write(testBytesToUpload.getBytes());
+  outForSomeBytes.flush();
+
+  //Test for some bytes to upload
+  assertValues("bytes to upload", testBytesToUpload.getBytes().length,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload);
+
+  //Test for relation between bytesUploadSuccessful, bytesUploadFailed
+  // and bytesToUpload
+  assertValues("bytesUploadSuccessful equal to difference between "
+  + "bytesToUpload and bytesUploadFailed",
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadSuccessful,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload -
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadFailed);
+
+} finally {
+  if (outForSomeBytes != null) {
+outForSomeBytes.close();
+  }
+}
+
+AbfsOutputStream outForLargeBytes = null;
+try {
+  outForLargeBytes =
+  (AbfsOutputStream) abfss.createFile(uploadBytesFilePath,
+  statistics
+  , true, FsPermission.getDefault(),
+  FsPermission.getUMask(fs.getConf()));
+
+  int largeValue = 10;
+  for (int i = 0; i < largeValue; i++) {
+outForLargeBytes.write(testBytesToUpload.getBytes());
+  }
+  outForLargeBytes.flush();
+
+  //Test for large bytes to upload
+  assertValues("bytes to upload",
+  largeValue * (testBytesToUpload.getBytes().length),
+  outForLargeBytes.getOutputStreamStatistics().bytesToUpload);
+
+  //Test for relation between bytesUploadSuccessful, bytesUploadFailed
+  // and bytesToUpload
+  assertValues("bytesUploadSuccessful equal to difference between "
+  + "bytesToUpload and bytesUploadFailed",
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadSuccessful,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload -
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadFailed);
+
+} finally {
+  if (outForLargeBytes != null) {
+outForLargeBytes.close();
+  }
+}
+
+  }
+
+  /**
+   * Tests to check time spend on waiting for tasks to be complete on a
+   * blocking queue in {@link AbfsOutputStream}.
+   *
+   * @throws IOException
+   */
+  @Test
+  public void testAbfsOutputStreamTimeSpendOnWaitTask() throws IOException {
+describe("Testing Time Spend on Waiting for Task to be complete");
+final AzureBlobFileSystem fs = getFileSystem();
+Path timeSpendFilePath = new Path("AbfsOutputStreamStatsPath");
+AzureBlobFileSystemStore abfss = fs.getAbfsStore();
+FileSystem.Statistics statistics = fs.getFsStatistics();
+
+AbfsOutputStream out =
+(AbfsOutputStream) abfss.createFile(timeSpendFilePath,
+statistics, true,
+FsPermission.getDefault(), FsPermission.getUMask(fs.getConf()));
+
+  }
+
+  /**
+   * Tests to check number of {@codes shrinkWriteOperationQueue()}
+   * calls.
+   * After writing data, AbfsOutputStream doesn't upload the data until
+   * Flushed. Hence, flush() method is called after write() to test Queue
+   * shrink calls.
+   *

[GitHub] [hadoop] steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding Output Stream Counters in ABFS

2020-03-20 Thread GitBox
steveloughran commented on a change in pull request #1899: HADOOP-16914 Adding 
Output Stream Counters in ABFS
URL: https://github.com/apache/hadoop/pull/1899#discussion_r395749023
 
 

 ##
 File path: 
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAbfsOutputStream.java
 ##
 @@ -0,0 +1,278 @@
+package org.apache.hadoop.fs.azurebfs;
+
+import java.io.IOException;
+
+import org.junit.Test;
+
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.azurebfs.services.AbfsOutputStream;
+import org.apache.hadoop.fs.permission.FsPermission;
+
+/**
+ * Test AbfsOutputStream statistics.
+ */
+public class ITestAbfsOutputStream extends AbstractAbfsIntegrationTest {
+
+  public ITestAbfsOutputStream() throws Exception {
+  }
+
+  /**
+   * Tests to check bytes Uploading in {@link AbfsOutputStream}.
+   *
+   * @throws IOException
+   */
+  @Test
+  public void testAbfsOutputStreamUploadingBytes() throws IOException {
+describe("Testing Bytes uploaded in AbfsOutputSteam");
+final AzureBlobFileSystem fs = getFileSystem();
+Path uploadBytesFilePath = new Path("AbfsOutputStreamStatsPath");
+AzureBlobFileSystemStore abfss = fs.getAbfsStore();
+FileSystem.Statistics statistics = fs.getFsStatistics();
+abfss.getAbfsConfiguration().setDisableOutputStreamFlush(false);
+String testBytesToUpload = "bytes";
+
+AbfsOutputStream outForSomeBytes = null;
+try {
+  outForSomeBytes = (AbfsOutputStream) 
abfss.createFile(uploadBytesFilePath,
+  statistics,
+  true,
+  FsPermission.getDefault(), FsPermission.getUMask(fs.getConf()));
+
+  //Test for zero bytes To upload
+  assertValues("bytes to upload", 0,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload);
+
+  outForSomeBytes.write(testBytesToUpload.getBytes());
+  outForSomeBytes.flush();
+
+  //Test for some bytes to upload
+  assertValues("bytes to upload", testBytesToUpload.getBytes().length,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload);
+
+  //Test for relation between bytesUploadSuccessful, bytesUploadFailed
+  // and bytesToUpload
+  assertValues("bytesUploadSuccessful equal to difference between "
+  + "bytesToUpload and bytesUploadFailed",
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadSuccessful,
+  outForSomeBytes.getOutputStreamStatistics().bytesToUpload -
+  outForSomeBytes.getOutputStreamStatistics().bytesUploadFailed);
+
+} finally {
+  if (outForSomeBytes != null) {
 
 Review comment:
   IOUtils.closeQuietly(LOG, ...), or try-with-resources


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org