From bfdf1c049c6e009f3e2eb29a20128443d27dc2c4 Mon Sep 17 00:00:00 2001 From: Ray Mattingly Date: Thu, 4 May 2023 14:12:11 -0400 Subject: [PATCH] HBASE-27799: RpcThrottlingException wait interval message is misleading between 0-1s (#5192) Signed-off-by: Bryan Beaudreault --- .../hbase/quotas/RpcThrottlingException.java | 61 +++++++++++----- .../quotas/TestRpcThrottlingException.java | 72 +++++++++++++++++++ 2 files changed, 115 insertions(+), 18 deletions(-) create mode 100644 hbase-client/src/test/java/org/apache/hadoop/hbase/quotas/TestRpcThrottlingException.java diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/RpcThrottlingException.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/RpcThrottlingException.java index 1cb8abf490d..2c1f13e94e6 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/RpcThrottlingException.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/quotas/RpcThrottlingException.java @@ -20,7 +20,6 @@ package org.apache.hadoop.hbase.quotas; 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 class RpcThrottlingException extends HBaseIOException { 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); - } - return time; - } + 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; + } + time += Math.round(Float.parseFloat(group) * factors[j - 1]); + } + return time; + } return -1; } } diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/quotas/TestRpcThrottlingException.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/quotas/TestRpcThrottlingException.java new file mode 100644 index 00000000000..21b01ad4764 --- /dev/null +++ b/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 STR_TO_MS_NEW_FORMAT = + ImmutableMap. 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 STR_TO_MS_LEGACY_FORMAT = ImmutableMap. 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 strAndMs : STR_TO_MS_NEW_FORMAT.entrySet()) { + String output = RpcThrottlingException.stringFromMillis(strAndMs.getValue()); + assertEquals(strAndMs.getKey(), output); + } + } + + @Test + public void itConvertsNewStringToMillis() { + for (Map.Entry strAndMs : STR_TO_MS_NEW_FORMAT.entrySet()) { + Long output = RpcThrottlingException.timeFromString(strAndMs.getKey()); + assertEquals(strAndMs.getValue(), output); + } + } + + @Test + public void itConvertsLegacyStringToMillis() { + for (Map.Entry strAndMs : STR_TO_MS_LEGACY_FORMAT.entrySet()) { + Long output = RpcThrottlingException.timeFromString(strAndMs.getKey()); + assertEquals(strAndMs.getValue(), output); + } + } + +}