[GitHub] [hbase] virajjasani commented on a change in pull request #1765: HBASE-24396 : RetryCounter#sleepUntilNextRetry and ThrottledInputStre…

[GitBox <gi...@apache.org>]

virajjasani commented on a change in pull request #1765:
URL: https://github.com/apache/hbase/pull/1765#discussion_r429547270



##########
File path: hbase-it/src/test/java/org/apache/hadoop/hbase/RESTApiClusterManager.java
##########
@@ -513,11 +513,7 @@ private boolean hasCommandCompletedSuccessfully(final long commandId) {
           throw new RuntimeException("retries exhausted", e);
         }
       }
-      try {
-        retryCounter.sleepUntilNextRetry();
-      } catch (InterruptedException e) {
-        throw new RuntimeException(e);
-      }
+      retryCounter.sleepUntilNextRetry();

Review comment:
       I believe the purpose of retryCounter.sleepUntilNextRetry() should be uninterrupted sleep because RetryCounter is mainly being used by retries with sleeps and retries with different backoff policies. In such scenario, RetryCounter being a library should not ideally throw InterruptedException even if sleep is interrupted because it is being retried by clients to achieve certain tasks. 

##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/io/hadoopbackport/ThrottledInputStream.java
##########
@@ -143,14 +144,10 @@ static long calSleepTimeMs(long bytesRead, long maxBytesPerSec, long elapsed) {
     }
   }
 
-  private void throttle() throws InterruptedIOException {
+  private void throttle() {
     long sleepTime = calSleepTimeMs();
     totalSleepTime += sleepTime;
-    try {
-      TimeUnit.MILLISECONDS.sleep(sleepTime);
-    } catch (InterruptedException e) {
-      throw new InterruptedIOException("Thread aborted");
-    }
+    Uninterruptibles.sleepUninterruptibly(sleepTime, TimeUnit.MILLISECONDS);

Review comment:
       Same here, throttle()'s purpose should ideally be to throttle without any interruption.




----------------------------------------------------------------
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:
users@infra.apache.org