HBASE-27799: RpcThrottlingException wait interval message is misleadi…

…ng between 0-1s (apache#5192) (#52)

Signed-off-by: Bryan Beaudreault <[email protected]>

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(hours > 1 ? "hrs, " : "hr, ");
+    }
+    if (minutes != 0) {
+      buf.append(minutes);
+      buf.append(minutes > 1 ? "mins, " : "min, ");
+    }
+    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,72 @@
+/*
+ * 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 java.util.Map;
+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;
+
+import org.apache.hbase.thirdparty.com.google.common.collect.ImmutableMap;
+
+@Category({ SmallTests.class })
+public class TestRpcThrottlingException {
+
+  @ClassRule
+  public static final HBaseClassTestRule CLASS_RULE =
+    HBaseClassTestRule.forClass(TestRpcThrottlingException.class);
+
+  private static final Map<String, Long> STR_TO_MS_NEW_FORMAT =
+    ImmutableMap.<String, Long> builder().put("0ms", 0L).put("50ms", 50L).put("1sec, 1ms", 1001L)
+      .put("1min, 5sec, 15ms", 65_015L).put("5mins, 2sec, 0ms", 302000L)
+      .put("1hr, 3mins, 5sec, 1ms", 3785001L).put("1hr, 5sec, 1ms", 3605001L)
+      .put("1hr, 0ms", 3600000L).put("1hr, 1min, 1ms", 3660001L).build();
+  private static final Map<String,
+    Long> STR_TO_MS_LEGACY_FORMAT = ImmutableMap.<String, Long> builder().put("0sec", 0L)
+      .put("1sec", 1000L).put("2sec", 2000L).put("1mins, 5sec", 65_000L).put("5mins, 2sec", 302000L)
+      .put("1hrs, 3mins, 5sec", 3785000L).build();
+
+  @Test
+  public void itConvertsMillisToNewString() {
+    for (Map.Entry<String, Long> strAndMs : STR_TO_MS_NEW_FORMAT.entrySet()) {
+      String output = RpcThrottlingException.stringFromMillis(strAndMs.getValue());
+      assertEquals(strAndMs.getKey(), output);
+    }
+  }
+
+  @Test
+  public void itConvertsNewStringToMillis() {
+    for (Map.Entry<String, Long> strAndMs : STR_TO_MS_NEW_FORMAT.entrySet()) {
+      Long output = RpcThrottlingException.timeFromString(strAndMs.getKey());
+      assertEquals(strAndMs.getValue(), output);
+    }
+  }
+
+  @Test
+  public void itConvertsLegacyStringToMillis() {
+    for (Map.Entry<String, Long> strAndMs : STR_TO_MS_LEGACY_FORMAT.entrySet()) {
+      Long output = RpcThrottlingException.timeFromString(strAndMs.getKey());
+      assertEquals(strAndMs.getValue(), output);
+    }
+  }
+
+}