HBASE-27799: RpcThrottlingException wait interval message is misleading between 0-1s

hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/RpcThrottlingException.java

@@ -20,7 +20,6 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import org.apache.hadoop.hbase.HBaseIOException;
-import org.apache.hadoop.util.StringUtils;
 import org.apache.yetus.audience.InterfaceAudience;
 
 /**
@@ -130,29 +129,55 @@ public static void throwWriteCapacityUnitExceeded(final long waitInterval)
 
   private static void throwThrottlingException(final Type type, final long waitInterval)
     throws RpcThrottlingException {
-    String msg = MSG_TYPE[type.ordinal()] + MSG_WAIT + StringUtils.formatTime(waitInterval);
+    String msg = MSG_TYPE[type.ordinal()] + MSG_WAIT + stringFromMillis(waitInterval);
     throw new RpcThrottlingException(type, waitInterval, msg);
   }
 
-  private static long timeFromString(String timeDiff) {
-    Pattern[] patterns = new Pattern[] { Pattern.compile("^(\\d+\\.\\d\\d)sec"),
-      Pattern.compile("^(\\d+)mins, (\\d+\\.\\d\\d)sec"),
-      Pattern.compile("^(\\d+)hrs, (\\d+)mins, (\\d+\\.\\d\\d)sec") };
+  // Visible for TestRpcThrottlingException
+  protected static String stringFromMillis(long millis) {
+    StringBuilder buf = new StringBuilder();
+    long hours = millis / (60 * 60 * 1000);
+    long rem = (millis % (60 * 60 * 1000));
+    long minutes = rem / (60 * 1000);
+    rem = rem % (60 * 1000);
+    long seconds = rem / 1000;
+    long milliseconds = rem % 1000;
 
-    for (int i = 0; i < patterns.length; ++i) {
-      Matcher m = patterns[i].matcher(timeDiff);
-      if (m.find()) {
-        long time = Math.round(Float.parseFloat(m.group(1 + i)) * 1000);
-        if (i > 0) {
-          time += Long.parseLong(m.group(i)) * (60 * 1000);
-        }
-        if (i > 1) {
-          time += Long.parseLong(m.group(i - 1)) * (60 * 60 * 1000);
+    if (hours != 0) {
+      buf.append(hours);
+      buf.append("hrs, ");
+    }
+    if (minutes != 0) {
+      buf.append(minutes);
+      buf.append("mins, ");
+    }
+    if (seconds != 0) {
+      buf.append(seconds);
+      buf.append("sec, ");
+    }
+    buf.append(milliseconds);
+    buf.append("ms");
+    return buf.toString();
+  }
+
+  // Visible for TestRpcThrottlingException
+  protected static long timeFromString(String timeDiff) {
+    Pattern pattern =
+      Pattern.compile("^(?:(\\d+)hrs, )?(?:(\\d+)mins, )?(?:(\\d+)sec.{0,2})?(?:(\\d+)ms)?");
+    long[] factors = new long[] { 60 * 60 * 1000, 60 * 1000, 1000, 1 };
+    Matcher m = pattern.matcher(timeDiff);
+    if (m.find()) {
+      int numGroups = m.groupCount();
+      long time = 0;
+      for (int j = 1; j <= numGroups; j++) {
+        String group = m.group(j);
+        if (group == null) {
+          continue;
         }
-        return time;
+        time += Math.round(Float.parseFloat(group) * factors[j - 1]);
       }
+      return time;
     }
-
     return -1;
   }
 }

hbase-client/src/test/java/org/apache/hadoop/hbase/quotas/TestRpcThrottlingException.java

@@ -0,0 +1,91 @@
+/*
+ * 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.hbase.quotas;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+@Category({ SmallTests.class })
+public class TestRpcThrottlingException {
+
+  @ClassRule
+  public static final HBaseClassTestRule CLASS_RULE =
+    HBaseClassTestRule.forClass(TestRpcThrottlingException.class);
+
+  @Test
+  public void itHandlesSecWaitIntervalMessage() {
+    try {
+      RpcThrottlingException.throwNumReadRequestsExceeded(1001);
+      fail();
+    } catch (RpcThrottlingException e) {
+      assertTrue(e.getMessage().contains("wait 1sec, 1ms"));
+    }
+  }
+
+  @Test
+  public void itHandlesMsWaitIntervalMessage() {
+    try {
+      RpcThrottlingException.throwNumReadRequestsExceeded(50);
+      fail();
+    } catch (RpcThrottlingException e) {
+      assertTrue(e.getMessage().contains("wait 50ms"));
+    }
+  }
+
+  @Test
+  public void itHandlesMinWaitIntervalMessage() {
+    try {
+      RpcThrottlingException.throwNumReadRequestsExceeded(65_015);
+      fail();
+    } catch (RpcThrottlingException e) {
+      assertTrue(e.getMessage().contains("wait 1mins, 5sec, 15ms"));
+    }
+  }
+
+  @Test
+  public void itConvertsMillisToString() {
+    String output = RpcThrottlingException.stringFromMillis(6500);
+    assertEquals("6sec, 500ms", output);
+  }
+
+  @Test
+  public void itConvertsStringToMillis() {
+    long millis = RpcThrottlingException.timeFromString("5mins, 2sec, 0ms");
+    assertEquals(millis, 302000);
+  }
+
+  @Test
+  public void itConvertsLegacyStringSecToMillis() {
+    long millis = RpcThrottlingException.timeFromString("5sec");
+    assertEquals(millis, 5000);
+  }
+
+  @Test
+  public void itConvertsLegacyStringHourMinSecToMillis2() {
+    long millis = RpcThrottlingException.timeFromString("1hrs, 3mins, 5sec");
+    assertEquals(millis, 3785000);
+  }
+
+}