Skip to content

Commit 13e29cb

Browse files
committed
HBASE-29005 Cannot split split system tables when quota enforcement is enabled
1 parent a735eff commit 13e29cb

2 files changed

Lines changed: 156 additions & 10 deletions

File tree

hbase-server/src/main/java/org/apache/hadoop/hbase/namespace/NamespaceStateManager.java

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,9 @@
4141
class NamespaceStateManager {
4242

4343
private static final Logger LOG = LoggerFactory.getLogger(NamespaceStateManager.class);
44-
private ConcurrentMap<String, NamespaceTableAndRegionInfo> nsStateCache;
45-
private MasterServices master;
44+
45+
private final ConcurrentMap<String, NamespaceTableAndRegionInfo> nsStateCache;
46+
private final MasterServices master;
4647
private volatile boolean initialized = false;
4748

4849
public NamespaceStateManager(MasterServices masterServices) {
@@ -76,6 +77,9 @@ public NamespaceTableAndRegionInfo getState(String name) {
7677
*/
7778
synchronized boolean checkAndUpdateNamespaceRegionCount(TableName name, byte[] regionName,
7879
int incr) throws IOException {
80+
if (name.isSystemTable()) {
81+
return true;
82+
}
7983
String namespace = name.getNamespaceAsString();
8084
NamespaceDescriptor nspdesc = getNamespaceDescriptor(namespace);
8185
if (nspdesc != null) {
@@ -84,17 +88,18 @@ synchronized boolean checkAndUpdateNamespaceRegionCount(TableName name, byte[] r
8488
int regionCount = currentStatus.getRegionCount();
8589
long maxRegionCount = TableNamespaceManager.getMaxRegions(nspdesc);
8690
if (incr > 0 && regionCount >= maxRegionCount) {
87-
LOG.warn("The region " + Bytes.toStringBinary(regionName)
88-
+ " cannot be created. The region count will exceed quota on the namespace. "
89-
+ "This may be transient, please retry later if there are any ongoing split"
90-
+ " operations in the namespace.");
91+
LOG.warn(
92+
"The region {} cannot be created. The region count will exceed quota on the namespace. "
93+
+ "This may be transient, please retry later if there are any ongoing split"
94+
+ " operations in the namespace.",
95+
Bytes.toStringBinary(regionName));
9196
return false;
9297
}
9398
NamespaceTableAndRegionInfo nsInfo = nsStateCache.get(namespace);
9499
if (nsInfo != null) {
95100
nsInfo.incRegionCountForTable(name, incr);
96101
} else {
97-
LOG.warn("Namespace state found null for namespace : " + namespace);
102+
LOG.warn("Namespace state found null for namespace : {}", namespace);
98103
}
99104
}
100105
return true;
@@ -110,6 +115,9 @@ synchronized boolean checkAndUpdateNamespaceRegionCount(TableName name, byte[] r
110115
*/
111116
synchronized void checkAndUpdateNamespaceRegionCount(TableName name, int incr)
112117
throws IOException {
118+
if (name.isSystemTable()) {
119+
return;
120+
}
113121
String namespace = name.getNamespaceAsString();
114122
NamespaceDescriptor nspdesc = getNamespaceDescriptor(namespace);
115123
if (nspdesc != null) {
@@ -119,7 +127,7 @@ synchronized void checkAndUpdateNamespaceRegionCount(TableName name, int incr)
119127
(currentStatus.getRegionCount() - regionCountOfTable + incr)
120128
> TableNamespaceManager.getMaxRegions(nspdesc)
121129
) {
122-
throw new QuotaExceededException("The table " + name.getNameAsString()
130+
throw new QuotaExceededException("The table {}" + name.getNameAsString()
123131
+ " region count cannot be updated as it would exceed maximum number "
124132
+ "of regions allowed in the namespace. The total number of regions permitted is "
125133
+ TableNamespaceManager.getMaxRegions(nspdesc));
@@ -133,13 +141,16 @@ private NamespaceDescriptor getNamespaceDescriptor(String namespaceAsString) {
133141
try {
134142
return this.master.getClusterSchema().getNamespace(namespaceAsString);
135143
} catch (IOException e) {
136-
LOG.error("Error while fetching namespace descriptor for namespace : " + namespaceAsString);
144+
LOG.error("Error while fetching namespace descriptor for namespace : {}", namespaceAsString);
137145
return null;
138146
}
139147
}
140148

141149
synchronized void checkAndUpdateNamespaceTableCount(TableName table, int numRegions)
142150
throws IOException {
151+
if (table.isSystemTable()) {
152+
return;
153+
}
143154
String namespace = table.getNamespaceAsString();
144155
NamespaceDescriptor nspdesc = getNamespaceDescriptor(namespace);
145156
if (nspdesc != null) {
@@ -186,6 +197,9 @@ void deleteNamespace(String namespace) {
186197
}
187198

188199
private void addTable(TableName tableName, int regionCount) throws IOException {
200+
if (tableName.isSystemTable()) {
201+
return;
202+
}
189203
NamespaceTableAndRegionInfo info = nsStateCache.get(tableName.getNamespaceAsString());
190204
if (info != null) {
191205
info.addTable(tableName, regionCount);
@@ -196,6 +210,9 @@ private void addTable(TableName tableName, int regionCount) throws IOException {
196210
}
197211

198212
synchronized void removeTable(TableName tableName) {
213+
if (tableName.isSystemTable()) {
214+
return;
215+
}
199216
NamespaceTableAndRegionInfo info = nsStateCache.get(tableName.getNamespaceAsString());
200217
if (info != null) {
201218
info.removeTable(tableName);
@@ -219,7 +236,7 @@ private void initialize() throws IOException {
219236
addTable(table, regions.size());
220237
}
221238
}
222-
LOG.info("Finished updating state of " + nsStateCache.size() + " namespaces. ");
239+
LOG.info("Finished updating state of {} namespaces.", nsStateCache.size());
223240
initialized = true;
224241
}
225242

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
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;
19+
20+
import static org.junit.Assert.assertEquals;
21+
22+
import java.util.List;
23+
import java.util.Map;
24+
import java.util.concurrent.TimeUnit;
25+
import org.apache.hadoop.conf.Configuration;
26+
import org.apache.hadoop.hbase.client.AsyncAdmin;
27+
import org.apache.hadoop.hbase.client.RegionInfo;
28+
import org.apache.hadoop.hbase.quotas.QuotaTableUtil;
29+
import org.apache.hadoop.hbase.quotas.QuotaUtil;
30+
import org.apache.hadoop.hbase.testclassification.LargeTests;
31+
import org.apache.hadoop.hbase.testclassification.MiscTests;
32+
import org.apache.hadoop.hbase.util.Bytes;
33+
import org.junit.ClassRule;
34+
import org.junit.Rule;
35+
import org.junit.Test;
36+
import org.junit.experimental.categories.Category;
37+
import org.junit.runner.RunWith;
38+
import org.junit.runners.Parameterized;
39+
import org.slf4j.Logger;
40+
import org.slf4j.LoggerFactory;
41+
42+
/**
43+
* Test that we can split and merge select system tables given the presence of various configuration
44+
* settings.
45+
*/
46+
@Category({ MiscTests.class, LargeTests.class })
47+
@RunWith(Parameterized.class)
48+
public class TestSplitMergeSystemTables {
49+
50+
private static final Logger LOG = LoggerFactory.getLogger(TestSplitMergeSystemTables.class);
51+
52+
@ClassRule
53+
public static final HBaseClassTestRule CLASS_RULE =
54+
HBaseClassTestRule.forClass(TestSplitMergeSystemTables.class);
55+
56+
@Parameterized.Parameters(name = "{0}: {1}")
57+
public static Object[][] params() {
58+
return new Object[][] {
59+
// quotas table is only created when the quota system is enabled.
60+
{ QuotaTableUtil.QUOTA_TABLE_NAME, Map.of(QuotaUtil.QUOTA_CONF_KEY, "true") }, };
61+
}
62+
63+
private final TableName tableName;
64+
65+
@Rule
66+
public final MiniClusterRule miniClusterRule;
67+
68+
public TestSplitMergeSystemTables(TableName tableName, Map<String, String> configMap) {
69+
this.tableName = tableName;
70+
this.miniClusterRule = MiniClusterRule.newBuilder().setConfiguration(() -> {
71+
Configuration conf = HBaseConfiguration.create();
72+
conf.setInt(HConstants.HBASE_CLIENT_META_OPERATION_TIMEOUT, 1000);
73+
conf.setInt(HConstants.HBASE_CLIENT_RETRIES_NUMBER, 2);
74+
configMap.forEach(conf::set);
75+
return conf;
76+
}).build();
77+
78+
}
79+
80+
@Test
81+
public void testSplitMergeSystemTable() throws Exception {
82+
HBaseTestingUtil util = miniClusterRule.getTestingUtility();
83+
util.waitTableAvailable(tableName, 30_000);
84+
AsyncAdmin admin = util.getAsyncConnection().getAdmin();
85+
admin.split(tableName, Bytes.toBytes(0x10)).get(30, TimeUnit.SECONDS);
86+
util.waitFor(30_000, new Waiter.ExplainingPredicate<Exception>() {
87+
88+
@Override
89+
public boolean evaluate() throws Exception {
90+
// can potentially observe the parent and both children via this interface.
91+
return admin.getRegions(tableName)
92+
.thenApply(val -> val.stream()
93+
.filter(info -> info.getReplicaId() == RegionInfo.DEFAULT_REPLICA_ID).toList())
94+
.get(30, TimeUnit.SECONDS).size() > 1;
95+
}
96+
97+
@Override
98+
public String explainFailure() {
99+
return "Split has not finished yet";
100+
}
101+
});
102+
util.waitUntilNoRegionsInTransition();
103+
List<RegionInfo> regionInfos = admin.getRegions(tableName)
104+
.thenApply(val -> val.stream()
105+
.filter(info -> info.getReplicaId() == RegionInfo.DEFAULT_REPLICA_ID).toList())
106+
.get(30, TimeUnit.SECONDS);
107+
assertEquals(2, regionInfos.size());
108+
LOG.info("{}", regionInfos);
109+
admin.mergeRegions(regionInfos.stream().map(RegionInfo::getRegionName).toList(), false).get(30,
110+
TimeUnit.SECONDS);
111+
util.waitFor(30000, new Waiter.ExplainingPredicate<Exception>() {
112+
113+
@Override
114+
public boolean evaluate() throws Exception {
115+
// can potentially observe the parent and both children via this interface.
116+
return admin.getRegions(tableName)
117+
.thenApply(val -> val.stream()
118+
.filter(info -> info.getReplicaId() == RegionInfo.DEFAULT_REPLICA_ID).toList())
119+
.get(30, TimeUnit.SECONDS).size() == 1;
120+
}
121+
122+
@Override
123+
public String explainFailure() {
124+
return "Merge has not finished yet";
125+
}
126+
});
127+
assertEquals(1, admin.getRegions(tableName).get(30, TimeUnit.SECONDS).size());
128+
}
129+
}

0 commit comments

Comments
 (0)