Skip to content

Commit 449c446

Browse files
authored
HBASE-28187 NPE when flushing a non-existing column family (#5692)
Signed-off-by: Nick Dimiduk <[email protected]> Signed-off-by: Hui Ruan <[email protected]>
1 parent bd1bee0 commit 449c446

File tree

6 files changed

+215
-3
lines changed

6 files changed

+215
-3
lines changed

hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java

+9-1
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@
8282
import org.apache.hadoop.hbase.quotas.QuotaSettings;
8383
import org.apache.hadoop.hbase.quotas.QuotaTableUtil;
8484
import org.apache.hadoop.hbase.quotas.SpaceQuotaSnapshot;
85+
import org.apache.hadoop.hbase.regionserver.NoSuchColumnFamilyException;
8586
import org.apache.hadoop.hbase.replication.ReplicationException;
8687
import org.apache.hadoop.hbase.replication.ReplicationPeerConfig;
8788
import org.apache.hadoop.hbase.replication.ReplicationPeerDescription;
@@ -967,6 +968,8 @@ public CompletableFuture<Void> flush(TableName tableName) {
967968

968969
@Override
969970
public CompletableFuture<Void> flush(TableName tableName, byte[] columnFamily) {
971+
Preconditions.checkNotNull(columnFamily,
972+
"columnFamily is null, If you don't specify a columnFamily, use flush(TableName) instead.");
970973
return flush(tableName, Collections.singletonList(columnFamily));
971974
}
972975

@@ -976,6 +979,8 @@ public CompletableFuture<Void> flush(TableName tableName, List<byte[]> columnFam
976979
// If the server version is lower than the client version, it's possible that the
977980
// flushTable method is not present in the server side, if so, we need to fall back
978981
// to the old implementation.
982+
Preconditions.checkNotNull(columnFamilyList,
983+
"columnFamily is null, If you don't specify a columnFamily, use flush(TableName) instead.");
979984
List<byte[]> columnFamilies = columnFamilyList.stream()
980985
.filter(cf -> cf != null && cf.length > 0).distinct().collect(Collectors.toList());
981986
FlushTableRequest request = RequestConverter.buildFlushTableRequest(tableName, columnFamilies,
@@ -986,7 +991,10 @@ public CompletableFuture<Void> flush(TableName tableName, List<byte[]> columnFam
986991
CompletableFuture<Void> future = new CompletableFuture<>();
987992
addListener(procFuture, (ret, error) -> {
988993
if (error != null) {
989-
if (error instanceof TableNotFoundException || error instanceof TableNotEnabledException) {
994+
if (
995+
error instanceof TableNotFoundException || error instanceof TableNotEnabledException
996+
|| error instanceof NoSuchColumnFamilyException
997+
) {
990998
future.completeExceptionally(error);
991999
} else if (error instanceof DoNotRetryIOException) {
9921000
// usually this is caused by the method is not present on the server or

hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/FlushTableProcedure.java

+26
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,13 @@
2424
import org.apache.hadoop.hbase.HBaseIOException;
2525
import org.apache.hadoop.hbase.TableName;
2626
import org.apache.hadoop.hbase.client.RegionReplicaUtil;
27+
import org.apache.hadoop.hbase.client.TableDescriptor;
28+
import org.apache.hadoop.hbase.master.MasterServices;
2729
import org.apache.hadoop.hbase.procedure.flush.MasterFlushTableProcedureManager;
2830
import org.apache.hadoop.hbase.procedure2.ProcedureStateSerializer;
2931
import org.apache.hadoop.hbase.procedure2.ProcedureSuspendedException;
3032
import org.apache.hadoop.hbase.procedure2.ProcedureYieldException;
33+
import org.apache.hadoop.hbase.regionserver.NoSuchColumnFamilyException;
3134
import org.apache.hadoop.hbase.util.Bytes;
3235
import org.apache.hadoop.hbase.util.Strings;
3336
import org.apache.yetus.audience.InterfaceAudience;
@@ -111,6 +114,29 @@ protected Flow executeFromState(MasterProcedureEnv env, FlushTableState state)
111114
return Flow.HAS_MORE_STATE;
112115
}
113116

117+
@Override
118+
protected void preflightChecks(MasterProcedureEnv env, Boolean enabled) throws HBaseIOException {
119+
super.preflightChecks(env, enabled);
120+
if (columnFamilies == null) {
121+
return;
122+
}
123+
MasterServices master = env.getMasterServices();
124+
try {
125+
TableDescriptor tableDescriptor = master.getTableDescriptors().get(tableName);
126+
List<String> noSuchFamilies = columnFamilies.stream()
127+
.filter(cf -> !tableDescriptor.hasColumnFamily(cf)).map(Bytes::toString).toList();
128+
if (!noSuchFamilies.isEmpty()) {
129+
throw new NoSuchColumnFamilyException("Column families " + noSuchFamilies
130+
+ " don't exist in table " + tableName.getNameAsString());
131+
}
132+
} catch (IOException ioe) {
133+
if (ioe instanceof HBaseIOException) {
134+
throw (HBaseIOException) ioe;
135+
}
136+
throw new HBaseIOException(ioe);
137+
}
138+
}
139+
114140
@Override
115141
protected void rollbackState(MasterProcedureEnv env, FlushTableState state)
116142
throws IOException, InterruptedException {

hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/flush/MasterFlushTableProcedureManager.java

+20-1
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,14 @@
2424
import java.util.Map;
2525
import java.util.Set;
2626
import java.util.concurrent.ThreadPoolExecutor;
27+
import java.util.stream.StreamSupport;
2728
import org.apache.hadoop.conf.Configuration;
2829
import org.apache.hadoop.hbase.HBaseInterfaceAudience;
2930
import org.apache.hadoop.hbase.HConstants;
3031
import org.apache.hadoop.hbase.ServerName;
3132
import org.apache.hadoop.hbase.TableName;
3233
import org.apache.hadoop.hbase.client.RegionInfo;
34+
import org.apache.hadoop.hbase.client.TableDescriptor;
3335
import org.apache.hadoop.hbase.errorhandling.ForeignException;
3436
import org.apache.hadoop.hbase.errorhandling.ForeignExceptionDispatcher;
3537
import org.apache.hadoop.hbase.master.MasterCoprocessorHost;
@@ -40,9 +42,12 @@
4042
import org.apache.hadoop.hbase.procedure.ProcedureCoordinator;
4143
import org.apache.hadoop.hbase.procedure.ProcedureCoordinatorRpcs;
4244
import org.apache.hadoop.hbase.procedure.ZKProcedureCoordinator;
45+
import org.apache.hadoop.hbase.regionserver.NoSuchColumnFamilyException;
4346
import org.apache.hadoop.hbase.security.User;
4447
import org.apache.hadoop.hbase.security.access.AccessChecker;
48+
import org.apache.hadoop.hbase.util.Bytes;
4549
import org.apache.hadoop.hbase.util.Pair;
50+
import org.apache.hadoop.hbase.util.Strings;
4651
import org.apache.yetus.audience.InterfaceAudience;
4752
import org.apache.zookeeper.KeeperException;
4853
import org.slf4j.Logger;
@@ -152,7 +157,21 @@ public void execProcedure(ProcedureDescription desc) throws IOException {
152157
families = nsp;
153158
}
154159
}
155-
byte[] procArgs = families != null ? families.toByteArray() : new byte[0];
160+
161+
byte[] procArgs;
162+
if (families != null) {
163+
TableDescriptor tableDescriptor = master.getTableDescriptors().get(tableName);
164+
List<String> noSuchFamilies =
165+
StreamSupport.stream(Strings.SPLITTER.split(families.getValue()).spliterator(), false)
166+
.filter(cf -> !tableDescriptor.hasColumnFamily(Bytes.toBytes(cf))).toList();
167+
if (!noSuchFamilies.isEmpty()) {
168+
throw new NoSuchColumnFamilyException("Column families " + noSuchFamilies
169+
+ " don't exist in table " + tableName.getNameAsString());
170+
}
171+
procArgs = families.toByteArray();
172+
} else {
173+
procArgs = new byte[0];
174+
}
156175

157176
// Kick of the global procedure from the master coordinator to the region servers.
158177
// We rely on the existing Distributed Procedure framework to prevent any concurrent

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java

+8-1
Original file line numberDiff line numberDiff line change
@@ -1632,8 +1632,15 @@ public FlushRegionResponse flushRegion(final RpcController controller,
16321632
// Go behind the curtain so we can manage writing of the flush WAL marker
16331633
HRegion.FlushResultImpl flushResult = null;
16341634
if (request.hasFamily()) {
1635-
List families = new ArrayList();
1635+
List<byte[]> families = new ArrayList();
16361636
families.add(request.getFamily().toByteArray());
1637+
TableDescriptor tableDescriptor = region.getTableDescriptor();
1638+
List<String> noSuchFamilies = families.stream()
1639+
.filter(f -> !tableDescriptor.hasColumnFamily(f)).map(Bytes::toString).toList();
1640+
if (!noSuchFamilies.isEmpty()) {
1641+
throw new NoSuchColumnFamilyException("Column families " + noSuchFamilies
1642+
+ " don't exist in table " + tableDescriptor.getTableName().getNameAsString());
1643+
}
16371644
flushResult =
16381645
region.flushcache(families, writeFlushWalMarker, FlushLifeCycleTracker.DUMMY);
16391646
} else {

hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFlushFromClient.java

+31
Original file line numberDiff line numberDiff line change
@@ -19,20 +19,27 @@
1919

2020
import static org.junit.Assert.assertEquals;
2121
import static org.junit.Assert.assertFalse;
22+
import static org.junit.Assert.assertNotNull;
23+
import static org.junit.Assert.assertThrows;
2224
import static org.junit.Assert.assertTrue;
2325

26+
import java.io.IOException;
27+
import java.util.ArrayList;
2428
import java.util.Arrays;
2529
import java.util.List;
30+
import java.util.concurrent.CompletableFuture;
2631
import java.util.concurrent.TimeUnit;
2732
import java.util.stream.Collectors;
2833
import org.apache.hadoop.hbase.HBaseClassTestRule;
2934
import org.apache.hadoop.hbase.HBaseTestingUtil;
3035
import org.apache.hadoop.hbase.TableName;
3136
import org.apache.hadoop.hbase.regionserver.HRegion;
3237
import org.apache.hadoop.hbase.regionserver.HRegionServer;
38+
import org.apache.hadoop.hbase.regionserver.NoSuchColumnFamilyException;
3339
import org.apache.hadoop.hbase.testclassification.ClientTests;
3440
import org.apache.hadoop.hbase.testclassification.MediumTests;
3541
import org.apache.hadoop.hbase.util.Bytes;
42+
import org.apache.hadoop.hbase.util.FutureUtils;
3643
import org.apache.hadoop.hbase.util.JVMClusterUtil;
3744
import org.junit.After;
3845
import org.junit.AfterClass;
@@ -185,6 +192,30 @@ public void testAsyncFlushRegionFamily() throws Exception {
185192
}
186193
}
187194

195+
@Test
196+
public void testAsyncFlushTableWithNonExistingFamilies() throws IOException {
197+
AsyncAdmin admin = asyncConn.getAdmin();
198+
List<byte[]> families = new ArrayList<>();
199+
families.add(FAMILY_1);
200+
families.add(FAMILY_2);
201+
families.add(Bytes.toBytes("non_family01"));
202+
families.add(Bytes.toBytes("non_family02"));
203+
CompletableFuture<Void> future = CompletableFuture.allOf(admin.flush(tableName, families));
204+
assertThrows(NoSuchColumnFamilyException.class, () -> FutureUtils.get(future));
205+
}
206+
207+
@Test
208+
public void testAsyncFlushRegionWithNonExistingFamily() throws IOException {
209+
AsyncAdmin admin = asyncConn.getAdmin();
210+
List<HRegion> regions = getRegionInfo();
211+
assertNotNull(regions);
212+
assertTrue(regions.size() > 0);
213+
HRegion region = regions.get(0);
214+
CompletableFuture<Void> future = CompletableFuture.allOf(admin
215+
.flushRegion(region.getRegionInfo().getEncodedNameAsBytes(), Bytes.toBytes("non_family")));
216+
assertThrows(NoSuchColumnFamilyException.class, () -> FutureUtils.get(future));
217+
}
218+
188219
@Test
189220
public void testFlushRegionServer() throws Exception {
190221
try (Admin admin = TEST_UTIL.getAdmin()) {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
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.client;
19+
20+
import static org.junit.Assert.assertFalse;
21+
import static org.junit.Assert.assertThrows;
22+
23+
import java.util.ArrayList;
24+
import java.util.List;
25+
import java.util.concurrent.CompletableFuture;
26+
import org.apache.hadoop.conf.Configuration;
27+
import org.apache.hadoop.hbase.HBaseClassTestRule;
28+
import org.apache.hadoop.hbase.HBaseTestingUtil;
29+
import org.apache.hadoop.hbase.TableName;
30+
import org.apache.hadoop.hbase.procedure.flush.MasterFlushTableProcedureManager;
31+
import org.apache.hadoop.hbase.regionserver.HRegion;
32+
import org.apache.hadoop.hbase.regionserver.NoSuchColumnFamilyException;
33+
import org.apache.hadoop.hbase.testclassification.ClientTests;
34+
import org.apache.hadoop.hbase.testclassification.MediumTests;
35+
import org.apache.hadoop.hbase.util.Bytes;
36+
import org.apache.hadoop.hbase.util.FutureUtils;
37+
import org.junit.After;
38+
import org.junit.AfterClass;
39+
import org.junit.Before;
40+
import org.junit.BeforeClass;
41+
import org.junit.ClassRule;
42+
import org.junit.Rule;
43+
import org.junit.Test;
44+
import org.junit.experimental.categories.Category;
45+
import org.junit.rules.TestName;
46+
import org.slf4j.Logger;
47+
import org.slf4j.LoggerFactory;
48+
49+
import org.apache.hbase.thirdparty.com.google.common.io.Closeables;
50+
51+
@Category({ MediumTests.class, ClientTests.class })
52+
public class TestFlushFromClientWithDisabledFlushProcedure {
53+
54+
@ClassRule
55+
public static final HBaseClassTestRule CLASS_RULE =
56+
HBaseClassTestRule.forClass(TestFlushFromClientWithDisabledFlushProcedure.class);
57+
58+
private static final Logger LOG =
59+
LoggerFactory.getLogger(TestFlushFromClientWithDisabledFlushProcedure.class);
60+
private final static HBaseTestingUtil TEST_UTIL = new HBaseTestingUtil();
61+
private static AsyncConnection asyncConn;
62+
private static final byte[] FAMILY = Bytes.toBytes("info");
63+
private static final byte[] QUALIFIER = Bytes.toBytes("name");
64+
65+
@Rule
66+
public TestName name = new TestName();
67+
68+
private TableName tableName;
69+
70+
@BeforeClass
71+
public static void setUpBeforeClass() throws Exception {
72+
Configuration configuration = TEST_UTIL.getConfiguration();
73+
configuration.setBoolean(MasterFlushTableProcedureManager.FLUSH_PROCEDURE_ENABLED, false);
74+
TEST_UTIL.startMiniCluster(1);
75+
asyncConn = ConnectionFactory.createAsyncConnection(TEST_UTIL.getConfiguration()).get();
76+
}
77+
78+
@AfterClass
79+
public static void tearDownAfterClass() throws Exception {
80+
Closeables.close(asyncConn, true);
81+
TEST_UTIL.shutdownMiniCluster();
82+
}
83+
84+
@Before
85+
public void setUp() throws Exception {
86+
tableName = TableName.valueOf(name.getMethodName());
87+
try (Table t = TEST_UTIL.createTable(tableName, FAMILY)) {
88+
List<Put> puts = new ArrayList<>();
89+
for (int i = 0; i <= 10; ++i) {
90+
Put put = new Put(Bytes.toBytes(i));
91+
put.addColumn(FAMILY, QUALIFIER, Bytes.toBytes(i));
92+
puts.add(put);
93+
}
94+
t.put(puts);
95+
}
96+
List<HRegion> regions = TEST_UTIL.getHBaseCluster().getRegions(tableName);
97+
assertFalse(regions.isEmpty());
98+
}
99+
100+
@After
101+
public void tearDown() throws Exception {
102+
for (TableDescriptor htd : TEST_UTIL.getAdmin().listTableDescriptors()) {
103+
LOG.info("Tear down, remove table=" + htd.getTableName());
104+
TEST_UTIL.deleteTable(htd.getTableName());
105+
}
106+
}
107+
108+
@Test
109+
public void flushTableWithNonExistingFamily() {
110+
AsyncAdmin admin = asyncConn.getAdmin();
111+
List<byte[]> families = new ArrayList<>();
112+
families.add(FAMILY);
113+
families.add(Bytes.toBytes("non_family01"));
114+
families.add(Bytes.toBytes("non_family02"));
115+
assertFalse(TEST_UTIL.getConfiguration().getBoolean(
116+
MasterFlushTableProcedureManager.FLUSH_PROCEDURE_ENABLED,
117+
MasterFlushTableProcedureManager.FLUSH_PROCEDURE_ENABLED_DEFAULT));
118+
CompletableFuture<Void> future = CompletableFuture.allOf(admin.flush(tableName, families));
119+
assertThrows(NoSuchColumnFamilyException.class, () -> FutureUtils.get(future));
120+
}
121+
}

0 commit comments

Comments
 (0)