Skip to content

Commit 05de158

Browse files
rmdmattinglybbeaudreault
authored andcommitted
HBASE-27799: RpcThrottlingException wait interval message is misleading between 0-1s (apache#5192)
Signed-off-by: Bryan Beaudreault <bbeaudreault@apache.org>
1 parent 458d746 commit 05de158

File tree

2 files changed

+114
-17
lines changed

2 files changed

+114
-17
lines changed

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

Lines changed: 42 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import java.util.regex.Matcher;
2121
import java.util.regex.Pattern;
2222
import org.apache.hadoop.hbase.HBaseIOException;
23-
import org.apache.hadoop.util.StringUtils;
2423
import org.apache.yetus.audience.InterfaceAudience;
2524

2625
/**
@@ -130,29 +129,55 @@ public static void throwWriteCapacityUnitExceeded(final long waitInterval)
130129

131130
private static void throwThrottlingException(final Type type, final long waitInterval)
132131
throws RpcThrottlingException {
133-
String msg = MSG_TYPE[type.ordinal()] + MSG_WAIT + StringUtils.formatTime(waitInterval);
132+
String msg = MSG_TYPE[type.ordinal()] + MSG_WAIT + stringFromMillis(waitInterval);
134133
throw new RpcThrottlingException(type, waitInterval, msg);
135134
}
136135

137-
private static long timeFromString(String timeDiff) {
138-
Pattern[] patterns = new Pattern[] { Pattern.compile("^(\\d+\\.\\d\\d)sec"),
139-
Pattern.compile("^(\\d+)mins, (\\d+\\.\\d\\d)sec"),
140-
Pattern.compile("^(\\d+)hrs, (\\d+)mins, (\\d+\\.\\d\\d)sec") };
136+
// Visible for TestRpcThrottlingException
137+
protected static String stringFromMillis(long millis) {
138+
StringBuilder buf = new StringBuilder();
139+
long hours = millis / (60 * 60 * 1000);
140+
long rem = (millis % (60 * 60 * 1000));
141+
long minutes = rem / (60 * 1000);
142+
rem = rem % (60 * 1000);
143+
long seconds = rem / 1000;
144+
long milliseconds = rem % 1000;
141145

142-
for (int i = 0; i < patterns.length; ++i) {
143-
Matcher m = patterns[i].matcher(timeDiff);
144-
if (m.find()) {
145-
long time = Math.round(Float.parseFloat(m.group(1 + i)) * 1000);
146-
if (i > 0) {
147-
time += Long.parseLong(m.group(i)) * (60 * 1000);
148-
}
149-
if (i > 1) {
150-
time += Long.parseLong(m.group(i - 1)) * (60 * 60 * 1000);
146+
if (hours != 0) {
147+
buf.append(hours);
148+
buf.append(hours > 1 ? "hrs, " : "hr, ");
149+
}
150+
if (minutes != 0) {
151+
buf.append(minutes);
152+
buf.append(minutes > 1 ? "mins, " : "min, ");
153+
}
154+
if (seconds != 0) {
155+
buf.append(seconds);
156+
buf.append("sec, ");
157+
}
158+
buf.append(milliseconds);
159+
buf.append("ms");
160+
return buf.toString();
161+
}
162+
163+
// Visible for TestRpcThrottlingException
164+
protected static long timeFromString(String timeDiff) {
165+
Pattern pattern =
166+
Pattern.compile("^(?:(\\d+)hrs?, )?(?:(\\d+)mins?, )?(?:(\\d+)sec[, ]{0,2})?(?:(\\d+)ms)?");
167+
long[] factors = new long[] { 60 * 60 * 1000, 60 * 1000, 1000, 1 };
168+
Matcher m = pattern.matcher(timeDiff);
169+
if (m.find()) {
170+
int numGroups = m.groupCount();
171+
long time = 0;
172+
for (int j = 1; j <= numGroups; j++) {
173+
String group = m.group(j);
174+
if (group == null) {
175+
continue;
151176
}
152-
return time;
177+
time += Math.round(Float.parseFloat(group) * factors[j - 1]);
153178
}
179+
return time;
154180
}
155-
156181
return -1;
157182
}
158183
}
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
package org.apache.hadoop.hbase.quotas;
19+
20+
import static org.junit.Assert.assertEquals;
21+
22+
import java.util.Map;
23+
import org.apache.hadoop.hbase.HBaseClassTestRule;
24+
import org.apache.hadoop.hbase.testclassification.SmallTests;
25+
import org.junit.ClassRule;
26+
import org.junit.Test;
27+
import org.junit.experimental.categories.Category;
28+
29+
import org.apache.hbase.thirdparty.com.google.common.collect.ImmutableMap;
30+
31+
@Category({ SmallTests.class })
32+
public class TestRpcThrottlingException {
33+
34+
@ClassRule
35+
public static final HBaseClassTestRule CLASS_RULE =
36+
HBaseClassTestRule.forClass(TestRpcThrottlingException.class);
37+
38+
private static final Map<String, Long> STR_TO_MS_NEW_FORMAT =
39+
ImmutableMap.<String, Long> builder().put("0ms", 0L).put("50ms", 50L).put("1sec, 1ms", 1001L)
40+
.put("1min, 5sec, 15ms", 65_015L).put("5mins, 2sec, 0ms", 302000L)
41+
.put("1hr, 3mins, 5sec, 1ms", 3785001L).put("1hr, 5sec, 1ms", 3605001L)
42+
.put("1hr, 0ms", 3600000L).put("1hr, 1min, 1ms", 3660001L).build();
43+
private static final Map<String,
44+
Long> STR_TO_MS_LEGACY_FORMAT = ImmutableMap.<String, Long> builder().put("0sec", 0L)
45+
.put("1sec", 1000L).put("2sec", 2000L).put("1mins, 5sec", 65_000L).put("5mins, 2sec", 302000L)
46+
.put("1hrs, 3mins, 5sec", 3785000L).build();
47+
48+
@Test
49+
public void itConvertsMillisToNewString() {
50+
for (Map.Entry<String, Long> strAndMs : STR_TO_MS_NEW_FORMAT.entrySet()) {
51+
String output = RpcThrottlingException.stringFromMillis(strAndMs.getValue());
52+
assertEquals(strAndMs.getKey(), output);
53+
}
54+
}
55+
56+
@Test
57+
public void itConvertsNewStringToMillis() {
58+
for (Map.Entry<String, Long> strAndMs : STR_TO_MS_NEW_FORMAT.entrySet()) {
59+
Long output = RpcThrottlingException.timeFromString(strAndMs.getKey());
60+
assertEquals(strAndMs.getValue(), output);
61+
}
62+
}
63+
64+
@Test
65+
public void itConvertsLegacyStringToMillis() {
66+
for (Map.Entry<String, Long> strAndMs : STR_TO_MS_LEGACY_FORMAT.entrySet()) {
67+
Long output = RpcThrottlingException.timeFromString(strAndMs.getKey());
68+
assertEquals(strAndMs.getValue(), output);
69+
}
70+
}
71+
72+
}

0 commit comments

Comments
 (0)