Skip to content

Commit 23c8262

Browse files
infraioinfraio
committed
HBASE-22624 Should sanity check table configuration when clone snapshot to a new table
1 parent 0198868 commit 23c8262

18 files changed

+334
-291
lines changed

‎hbase-it/src/test/java/org/apache/hadoop/hbase/IntegrationTestRegionReplicaReplication.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.apache.hadoop.hbase.util.MultiThreadedUpdater;
3030
import org.apache.hadoop.hbase.util.MultiThreadedWriter;
3131
import org.apache.hadoop.hbase.util.ServerRegionReplicaUtil;
32+
import org.apache.hadoop.hbase.util.TableDescriptorChecker;
3233
import org.apache.hadoop.hbase.util.Threads;
3334
import org.apache.hadoop.hbase.util.test.LoadTestDataGenerator;
3435
import org.apache.hadoop.util.StringUtils;
@@ -94,7 +95,7 @@ public void setConf(Configuration conf) {
9495
String.format("%s.%s", TEST_NAME, LoadTestTool.OPT_COLUMN_FAMILIES),
9596
StringUtils.join(",", DEFAULT_COLUMN_FAMILIES));
9697

97-
conf.setBoolean("hbase.table.sanity.checks", true);
98+
conf.setBoolean(TableDescriptorChecker.TABLE_SANITY_CHECKS, true);
9899

99100
// enable async wal replication to region replicas for unit tests
100101
conf.setBoolean(ServerRegionReplicaUtil.REGION_REPLICA_REPLICATION_CONF_KEY, true);

‎hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java

Lines changed: 12 additions & 269 deletions
Large diffs are not rendered by default.

‎hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@
8585
import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
8686
import org.apache.hadoop.hbase.util.FSUtils;
8787
import org.apache.hadoop.hbase.util.NonceKey;
88+
import org.apache.hadoop.hbase.util.TableDescriptorChecker;
8889
import org.apache.yetus.audience.InterfaceAudience;
8990
import org.apache.yetus.audience.InterfaceStability;
9091
import org.apache.zookeeper.KeeperException;
@@ -826,6 +827,9 @@ public long restoreOrCloneSnapshot(final SnapshotDescription reqSnapshot, final
826827
TableDescriptor snapshotTableDesc = manifest.getTableDescriptor();
827828
TableName tableName = TableName.valueOf(reqSnapshot.getTable());
828829

830+
// sanity check the new table descriptor
831+
TableDescriptorChecker.sanityCheck(master.getConfiguration(), snapshotTableDesc);
832+
829833
// stop tracking "abandoned" handlers
830834
cleanupSentinels();
831835

‎hbase-server/src/main/java/org/apache/hadoop/hbase/util/TableDescriptorChecker.java

Lines changed: 300 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,300 @@
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.util;
19+
20+
import java.io.IOException;
21+
22+
import org.apache.hadoop.conf.Configuration;
23+
import org.apache.hadoop.hbase.CompoundConfiguration;
24+
import org.apache.hadoop.hbase.DoNotRetryIOException;
25+
import org.apache.hadoop.hbase.HConstants;
26+
import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
27+
import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder;
28+
import org.apache.hadoop.hbase.client.TableDescriptor;
29+
import org.apache.hadoop.hbase.master.HMaster;
30+
import org.apache.hadoop.hbase.regionserver.DefaultStoreEngine;
31+
import org.apache.hadoop.hbase.regionserver.HStore;
32+
import org.apache.hadoop.hbase.regionserver.RegionCoprocessorHost;
33+
import org.apache.hadoop.hbase.regionserver.RegionSplitPolicy;
34+
import org.apache.hadoop.hbase.regionserver.compactions.ExploringCompactionPolicy;
35+
import org.apache.hadoop.hbase.regionserver.compactions.FIFOCompactionPolicy;
36+
import org.apache.hadoop.hbase.shaded.protobuf.generated.WALProtos;
37+
import org.apache.yetus.audience.InterfaceAudience;
38+
import org.slf4j.Logger;
39+
import org.slf4j.LoggerFactory;
40+
41+
/**
42+
* Used for master to sanity check {@link org.apache.hadoop.hbase.client.TableDescriptor}.
43+
*/
44+
@InterfaceAudience.Private
45+
public class TableDescriptorChecker {
46+
private static Logger LOG = LoggerFactory.getLogger(TableDescriptorChecker.class);
47+
48+
public static final String TABLE_SANITY_CHECKS = "hbase.table.sanity.checks";
49+
public static final boolean DEFAULT_TABLE_SANITY_CHECKS = true;
50+
51+
/**
52+
* Checks whether the table conforms to some sane limits, and configured
53+
* values (compression, etc) work. Throws an exception if something is wrong.
54+
*/
55+
public static void sanityCheck(final Configuration conf, final TableDescriptor td)
56+
throws IOException {
57+
sanityCheck(conf, td,
58+
conf.getBoolean(HMaster.MASTER_CHECK_COMPRESSION, HMaster.DEFAULT_MASTER_CHECK_COMPRESSION),
59+
conf.getBoolean(HMaster.MASTER_CHECK_ENCRYPTION, HMaster.DEFAULT_MASTER_CHECK_ENCRYPTION));
60+
}
61+
62+
/**
63+
* Checks whether the table conforms to some sane limits, and configured
64+
* values (compression, etc) work. Throws an exception if something is wrong.
65+
*/
66+
public static void sanityCheck(final Configuration conf, final TableDescriptor td,
67+
boolean checkCompression, boolean checkEncryption) throws IOException {
68+
boolean logWarn = false;
69+
if (!conf.getBoolean(TABLE_SANITY_CHECKS, DEFAULT_TABLE_SANITY_CHECKS)) {
70+
logWarn = true;
71+
}
72+
String tableVal = td.getValue(TABLE_SANITY_CHECKS);
73+
if (tableVal != null && !Boolean.valueOf(tableVal)) {
74+
logWarn = true;
75+
}
76+
77+
// check max file size
78+
long maxFileSizeLowerLimit = 2 * 1024 * 1024L; // 2M is the default lower limit
79+
long maxFileSize = td.getMaxFileSize();
80+
if (maxFileSize < 0) {
81+
maxFileSize = conf.getLong(HConstants.HREGION_MAX_FILESIZE, maxFileSizeLowerLimit);
82+
}
83+
if (maxFileSize < conf.getLong("hbase.hregion.max.filesize.limit", maxFileSizeLowerLimit)) {
84+
String message =
85+
"MAX_FILESIZE for table descriptor or " + "\"hbase.hregion.max.filesize\" (" +
86+
maxFileSize + ") is too small, which might cause over splitting into unmanageable " +
87+
"number of regions.";
88+
warnOrThrowExceptionForFailure(logWarn, message, null);
89+
}
90+
91+
// check flush size
92+
long flushSizeLowerLimit = 1024 * 1024L; // 1M is the default lower limit
93+
long flushSize = td.getMemStoreFlushSize();
94+
if (flushSize < 0) {
95+
flushSize = conf.getLong(HConstants.HREGION_MEMSTORE_FLUSH_SIZE, flushSizeLowerLimit);
96+
}
97+
if (flushSize < conf.getLong("hbase.hregion.memstore.flush.size.limit", flushSizeLowerLimit)) {
98+
String message = "MEMSTORE_FLUSHSIZE for table descriptor or " +
99+
"\"hbase.hregion.memstore.flush.size\" (" + flushSize +
100+
") is too small, which might cause" + " very frequent flushing.";
101+
warnOrThrowExceptionForFailure(logWarn, message, null);
102+
}
103+
104+
// check that coprocessors and other specified plugin classes can be loaded
105+
try {
106+
checkClassLoading(conf, td);
107+
} catch (Exception ex) {
108+
warnOrThrowExceptionForFailure(logWarn, ex.getMessage(), null);
109+
}
110+
111+
if (checkCompression) {
112+
// check compression can be loaded
113+
try {
114+
checkCompression(td);
115+
} catch (IOException e) {
116+
warnOrThrowExceptionForFailure(logWarn, e.getMessage(), e);
117+
}
118+
}
119+
120+
if (checkEncryption) {
121+
// check encryption can be loaded
122+
try {
123+
checkEncryption(conf, td);
124+
} catch (IOException e) {
125+
warnOrThrowExceptionForFailure(logWarn, e.getMessage(), e);
126+
}
127+
}
128+
129+
// Verify compaction policy
130+
try {
131+
checkCompactionPolicy(conf, td);
132+
} catch (IOException e) {
133+
warnOrThrowExceptionForFailure(false, e.getMessage(), e);
134+
}
135+
// check that we have at least 1 CF
136+
if (td.getColumnFamilyCount() == 0) {
137+
String message = "Table should have at least one column family.";
138+
warnOrThrowExceptionForFailure(logWarn, message, null);
139+
}
140+
141+
// check that we have minimum 1 region replicas
142+
int regionReplicas = td.getRegionReplication();
143+
if (regionReplicas < 1) {
144+
String message = "Table region replication should be at least one.";
145+
warnOrThrowExceptionForFailure(logWarn, message, null);
146+
}
147+
148+
for (ColumnFamilyDescriptor hcd : td.getColumnFamilies()) {
149+
if (hcd.getTimeToLive() <= 0) {
150+
String message = "TTL for column family " + hcd.getNameAsString() + " must be positive.";
151+
warnOrThrowExceptionForFailure(logWarn, message, null);
152+
}
153+
154+
// check blockSize
155+
if (hcd.getBlocksize() < 1024 || hcd.getBlocksize() > 16 * 1024 * 1024) {
156+
String message = "Block size for column family " + hcd.getNameAsString() +
157+
" must be between 1K and 16MB.";
158+
warnOrThrowExceptionForFailure(logWarn, message, null);
159+
}
160+
161+
// check versions
162+
if (hcd.getMinVersions() < 0) {
163+
String message =
164+
"Min versions for column family " + hcd.getNameAsString() + " must be positive.";
165+
warnOrThrowExceptionForFailure(logWarn, message, null);
166+
}
167+
// max versions already being checked
168+
169+
// HBASE-13776 Setting illegal versions for ColumnFamilyDescriptor
170+
// does not throw IllegalArgumentException
171+
// check minVersions <= maxVerions
172+
if (hcd.getMinVersions() > hcd.getMaxVersions()) {
173+
String message = "Min versions for column family " + hcd.getNameAsString() +
174+
" must be less than the Max versions.";
175+
warnOrThrowExceptionForFailure(logWarn, message, null);
176+
}
177+
178+
// check replication scope
179+
checkReplicationScope(hcd);
180+
// check bloom filter type
181+
checkBloomFilterType(hcd);
182+
183+
// check data replication factor, it can be 0(default value) when user has not explicitly
184+
// set the value, in this case we use default replication factor set in the file system.
185+
if (hcd.getDFSReplication() < 0) {
186+
String message = "HFile Replication for column family " + hcd.getNameAsString() +
187+
" must be greater than zero.";
188+
warnOrThrowExceptionForFailure(logWarn, message, null);
189+
}
190+
}
191+
}
192+
193+
private static void checkReplicationScope(final ColumnFamilyDescriptor cfd) throws IOException {
194+
// check replication scope
195+
WALProtos.ScopeType scop = WALProtos.ScopeType.valueOf(cfd.getScope());
196+
if (scop == null) {
197+
String message =
198+
"Replication scope for column family " + cfd.getNameAsString() + " is " + cfd.getScope() +
199+
" which is invalid.";
200+
201+
LOG.error(message);
202+
throw new DoNotRetryIOException(message);
203+
}
204+
}
205+
206+
private static void checkCompactionPolicy(Configuration conf, TableDescriptor td)
207+
throws IOException {
208+
// FIFO compaction has some requirements
209+
// Actually FCP ignores periodic major compactions
210+
String className = td.getValue(DefaultStoreEngine.DEFAULT_COMPACTION_POLICY_CLASS_KEY);
211+
if (className == null) {
212+
className = conf.get(DefaultStoreEngine.DEFAULT_COMPACTION_POLICY_CLASS_KEY,
213+
ExploringCompactionPolicy.class.getName());
214+
}
215+
216+
int blockingFileCount = HStore.DEFAULT_BLOCKING_STOREFILE_COUNT;
217+
String sv = td.getValue(HStore.BLOCKING_STOREFILES_KEY);
218+
if (sv != null) {
219+
blockingFileCount = Integer.parseInt(sv);
220+
} else {
221+
blockingFileCount = conf.getInt(HStore.BLOCKING_STOREFILES_KEY, blockingFileCount);
222+
}
223+
224+
for (ColumnFamilyDescriptor hcd : td.getColumnFamilies()) {
225+
String compactionPolicy =
226+
hcd.getConfigurationValue(DefaultStoreEngine.DEFAULT_COMPACTION_POLICY_CLASS_KEY);
227+
if (compactionPolicy == null) {
228+
compactionPolicy = className;
229+
}
230+
if (!compactionPolicy.equals(FIFOCompactionPolicy.class.getName())) {
231+
continue;
232+
}
233+
// FIFOCompaction
234+
String message = null;
235+
236+
// 1. Check TTL
237+
if (hcd.getTimeToLive() == ColumnFamilyDescriptorBuilder.DEFAULT_TTL) {
238+
message = "Default TTL is not supported for FIFO compaction";
239+
throw new IOException(message);
240+
}
241+
242+
// 2. Check min versions
243+
if (hcd.getMinVersions() > 0) {
244+
message = "MIN_VERSION > 0 is not supported for FIFO compaction";
245+
throw new IOException(message);
246+
}
247+
248+
// 3. blocking file count
249+
sv = hcd.getConfigurationValue(HStore.BLOCKING_STOREFILES_KEY);
250+
if (sv != null) {
251+
blockingFileCount = Integer.parseInt(sv);
252+
}
253+
if (blockingFileCount < 1000) {
254+
message =
255+
"Blocking file count '" + HStore.BLOCKING_STOREFILES_KEY + "' " + blockingFileCount +
256+
" is below recommended minimum of 1000 for column family " + hcd.getNameAsString();
257+
throw new IOException(message);
258+
}
259+
}
260+
}
261+
262+
private static void checkBloomFilterType(ColumnFamilyDescriptor cfd) throws IOException {
263+
Configuration conf = new CompoundConfiguration().addStringMap(cfd.getConfiguration());
264+
try {
265+
BloomFilterUtil.getBloomFilterParam(cfd.getBloomFilterType(), conf);
266+
} catch (IllegalArgumentException e) {
267+
throw new DoNotRetryIOException("Failed to get bloom filter param", e);
268+
}
269+
}
270+
271+
private static void checkCompression(final TableDescriptor td) throws IOException {
272+
for (ColumnFamilyDescriptor cfd : td.getColumnFamilies()) {
273+
CompressionTest.testCompression(cfd.getCompressionType());
274+
CompressionTest.testCompression(cfd.getCompactionCompressionType());
275+
}
276+
}
277+
278+
private static void checkEncryption(final Configuration conf, final TableDescriptor td)
279+
throws IOException {
280+
for (ColumnFamilyDescriptor cfd : td.getColumnFamilies()) {
281+
EncryptionTest.testEncryption(conf, cfd.getEncryptionType(), cfd.getEncryptionKey());
282+
}
283+
}
284+
285+
private static void checkClassLoading(final Configuration conf, final TableDescriptor td)
286+
throws IOException {
287+
RegionSplitPolicy.getSplitPolicyClass(td, conf);
288+
RegionCoprocessorHost.testTableCoprocessorAttrs(conf, td);
289+
}
290+
291+
// HBASE-13350 - Helper method to log warning on sanity check failures if checks disabled.
292+
private static void warnOrThrowExceptionForFailure(boolean logWarn, String message,
293+
Exception cause) throws IOException {
294+
if (!logWarn) {
295+
throw new DoNotRetryIOException(message + " Set " + TABLE_SANITY_CHECKS +
296+
" to false at conf or table descriptor if you want to bypass sanity checks", cause);
297+
}
298+
LOG.warn(message);
299+
}
300+
}

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,6 @@ public static void setUpBeforeClass() throws Exception {
103103
Configuration conf = TEST_UTIL.getConfiguration();
104104
conf.setStrings(CoprocessorHost.REGION_COPROCESSOR_CONF_KEY,
105105
MultiRowMutationEndpoint.class.getName());
106-
conf.setBoolean("hbase.table.sanity.checks", true); // enable for below
107-
// tests
108106
conf.setInt("hbase.regionserver.handler.count", 20);
109107
conf.setInt("hbase.bucketcache.size", 400);
110108
conf.setStrings(HConstants.BUCKET_CACHE_IOENGINE_KEY, "offheap");

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,6 @@ public static void setUpBeforeClass() throws Exception {
113113
Configuration conf = TEST_UTIL.getConfiguration();
114114
conf.setStrings(CoprocessorHost.REGION_COPROCESSOR_CONF_KEY,
115115
MultiRowMutationEndpoint.class.getName());
116-
conf.setBoolean("hbase.table.sanity.checks", true); // enable for below
117-
// tests
118116
conf.setInt("hbase.regionserver.handler.count", 20);
119117
conf.setInt("hbase.bucketcache.size", 400);
120118
conf.setStrings(HConstants.BUCKET_CACHE_IOENGINE_KEY, "offheap");

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,6 @@ protected static final void initialize(Class<?>... cps) throws Exception {
150150
Configuration conf = TEST_UTIL.getConfiguration();
151151
conf.setStrings(CoprocessorHost.REGION_COPROCESSOR_CONF_KEY,
152152
Arrays.stream(cps).map(Class::getName).toArray(String[]::new));
153-
conf.setBoolean("hbase.table.sanity.checks", true); // enable for below tests
154153
// We need more than one region server in this test
155154
TEST_UTIL.startMiniCluster(SLAVES);
156155
}

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626

2727
import java.io.IOException;
2828
import java.lang.reflect.Field;
29-
import org.apache.hadoop.conf.Configuration;
3029
import org.apache.hadoop.hbase.HBaseClassTestRule;
3130
import org.apache.hadoop.hbase.HBaseTestingUtility;
3231
import org.apache.hadoop.hbase.HColumnDescriptor;
@@ -36,6 +35,7 @@
3635
import org.apache.hadoop.hbase.testclassification.ClientTests;
3736
import org.apache.hadoop.hbase.testclassification.LargeTests;
3837
import org.apache.hadoop.hbase.util.Bytes;
38+
import org.apache.hadoop.hbase.util.TableDescriptorChecker;
3939
import org.junit.AfterClass;
4040
import org.junit.BeforeClass;
4141
import org.junit.ClassRule;
@@ -72,10 +72,6 @@ public static void setUpBeforeClass() throws Exception {
7272
Field field = HMaster.class.getDeclaredField("LOG");
7373
field.setAccessible(true);
7474
field.set(null, masterLogger);
75-
76-
Configuration conf = TEST_UTIL.getConfiguration();
77-
conf.setBoolean("hbase.table.sanity.checks", true); // enable for below tests
78-
// We need more than one region server in this test
7975
TEST_UTIL.startMiniCluster(1);
8076
}
8177

@@ -172,7 +168,7 @@ public void testIllegalTableDescriptor() throws Exception {
172168
htd.setMemStoreFlushSize(0);
173169

174170
// Check that logs warn on invalid table but allow it.
175-
htd.setConfiguration("hbase.table.sanity.checks", Boolean.FALSE.toString());
171+
htd.setConfiguration(TableDescriptorChecker.TABLE_SANITY_CHECKS, Boolean.FALSE.toString());
176172
checkTableIsLegal(htd);
177173

178174
verify(masterLogger).warn(contains("MEMSTORE_FLUSHSIZE for table "

0 commit comments

Comments
 (0)