Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java
Original file line number Diff line number Diff line change
Expand Up @@ -1601,6 +1601,16 @@ public enum OperationStatusCode {
*/
public static final int BATCH_ROWS_THRESHOLD_DEFAULT = 5000;

/**
* List of column families that cannot be deleted from the hbase:meta table.
* They are critical to cluster operation. This is a bit of an odd place to
* keep this list but then this is the tooling that does add/remove. Keeping
* it local!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is wrong, right? We don't have any tooling in HConstants.

This should go where table descriptors are defined and managed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, will move it back to ModifyTableProcedure

*/
public static final List<byte[]> UNDELETABLE_META_COLUMNFAMILIES = Collections.unmodifiableList(
Arrays.asList(HConstants.CATALOG_FAMILY, HConstants.TABLE_FAMILY,
HConstants.REPLICATION_BARRIER_FAMILY));

private HConstants() {
// Can't be instantiated with this ctor.
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
package org.apache.hadoop.hbase.master.procedure;

import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Set;
import java.util.function.Supplier;
Expand Down Expand Up @@ -59,15 +57,6 @@ public class ModifyTableProcedure
private TableDescriptor modifiedTableDescriptor;
private boolean deleteColumnFamilyInModify;
private boolean shouldCheckDescriptor;
/**
* List of column families that cannot be deleted from the hbase:meta table.
* They are critical to cluster operation. This is a bit of an odd place to
* keep this list but then this is the tooling that does add/remove. Keeping
* it local!
*/
private static final List<byte []> UNDELETABLE_META_COLUMNFAMILIES =
Collections.unmodifiableList(Arrays.asList(
HConstants.CATALOG_FAMILY, HConstants.TABLE_FAMILY, HConstants.REPLICATION_BARRIER_FAMILY));

public ModifyTableProcedure() {
super();
Expand Down Expand Up @@ -102,7 +91,7 @@ protected void preflightChecks(MasterProcedureEnv env, Boolean enabled) throws H
// If we are modifying the hbase:meta table, make sure we are not deleting critical
// column families else we'll damage the cluster.
Set<byte []> cfs = this.modifiedTableDescriptor.getColumnFamilyNames();
for (byte[] family : UNDELETABLE_META_COLUMNFAMILIES) {
for (byte[] family : HConstants.UNDELETABLE_META_COLUMNFAMILIES) {
if (!cfs.contains(family)) {
throw new HBaseIOException("Delete of hbase:meta column family " +
Bytes.toString(family));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import org.apache.hadoop.hbase.coprocessor.MultiRowMutationEndpoint;
import org.apache.hadoop.hbase.exceptions.DeserializationException;
import org.apache.hadoop.hbase.regionserver.BloomType;
import org.apache.hadoop.hbase.regionserver.NoSuchColumnFamilyException;
import org.apache.yetus.audience.InterfaceAudience;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -94,6 +95,10 @@ public class FSTableDescriptors implements TableDescriptors {
// TODO.
private final Map<TableName, TableDescriptor> cache = new ConcurrentHashMap<>();

// Default meta table descriptor, will be used by RegionServer during rolling upgrade until
// HMaster write latest 2.x meta table descriptor
private TableDescriptor defaultMetaTableDesc = null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens when using this "default descriptor" the regionserver attempts to update meta? Can that happen? We are assuming the master will rewrite soon, but is that valid? Regionservers don't run masters, they rely on an operator to do that. Who knows what the operator is doing.

Would it be better to address specific fallbacks where something is missing or not expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RegionServer can't update the meta descriptor,
org.apache.hadoop.hbase.regionserver.HRegionServer#canUpdateTableDescriptor


/**
* Construct a FSTableDescriptors instance using the hbase root dir of the given conf and the
* filesystem where that root dir lives. This instance can do write operations (is not read only).
Expand All @@ -112,6 +117,14 @@ public FSTableDescriptors(final FileSystem fs, final Path rootdir, final boolean
this.rootdir = rootdir;
this.fsreadonly = fsreadonly;
this.usecache = usecache;
// Create default in-memory meta table descriptor for RegionServer
if (this.fsreadonly) {
try {
defaultMetaTableDesc = createMetaTableDescriptorBuilder(fs.getConf()).build();
} catch (IOException ioe) {
LOG.warn("Exception occurred while creating meta table descriptor", ioe);
}
}
}

public static void tryUpdateMetaTableDescriptor(Configuration conf) throws IOException {
Expand All @@ -123,8 +136,10 @@ public static TableDescriptor tryUpdateAndGetMetaTableDescriptor(Configuration c
FileSystem fs, Path rootdir) throws IOException {
// see if we already have meta descriptor on fs. Write one if not.
try {
return getTableDescriptorFromFs(fs, rootdir, TableName.META_TABLE_NAME);
} catch (TableInfoMissingException e) {
TableDescriptor td = getTableDescriptorFromFs(fs, rootdir, TableName.META_TABLE_NAME);
validateMetaTableDescriptor(td);
return td;
} catch (TableInfoMissingException | NoSuchColumnFamilyException e) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense. The table might be missing altogether (what current code handles), or might be missing a column family due to legacy (what this change handles)

TableDescriptorBuilder builder = createMetaTableDescriptorBuilder(conf);
TableDescriptor td = builder.build();
LOG.info("Creating new hbase:meta table descriptor {}", td);
Expand All @@ -139,6 +154,21 @@ public static TableDescriptor tryUpdateAndGetMetaTableDescriptor(Configuration c
}
}

/**
* Validate meta table descriptor whether default column families exist or not
*/
private static void validateMetaTableDescriptor(TableDescriptor td)
throws NoSuchColumnFamilyException, TableInfoMissingException {
if (td == null) {
throw new TableInfoMissingException("Meta .tableinfo not found");
}
for (byte[] cf : HConstants.UNDELETABLE_META_COLUMNFAMILIES) {
if (!td.hasColumnFamily(cf)) {
throw new NoSuchColumnFamilyException("Column family " + Bytes.toString(cf) + " not found");
}
}
}

private static TableDescriptorBuilder createMetaTableDescriptorBuilder(final Configuration conf)
throws IOException {
// TODO We used to set CacheDataInL1 for META table. When we have BucketCache in file mode, now
Expand Down Expand Up @@ -223,6 +253,21 @@ public TableDescriptor get(TableName tableName) {
} catch (NullPointerException | IOException ioe) {
LOG.debug("Exception during readTableDecriptor. Current table name = " + tableName, ioe);
}

// Validate whether meta table descriptor is in HBase 2.x format
if (TableName.isMetaTableName(tableName) && defaultMetaTableDesc != null) {
try {
validateMetaTableDescriptor(tdmt);
// FS have proper meta table descriptor, we don't need to validate it again.
// Reset defaultMetaTableDesc
defaultMetaTableDesc = null;
} catch (TableInfoMissingException | NoSuchColumnFamilyException e) {
// Meta is still in old format, return the default meta table descriptor util we have meta
// descriptor in HBase 2.x format
return defaultMetaTableDesc;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems wrong.

createMetaTableDescriptorBuilder should return a builder for what the current version of the code expects.

If we need fallbacks to ride over an upgrade case, those fallbacks should be implemented where the errors are happening.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems wrong.

createMetaTableDescriptorBuilder should return a builder for what the current version of the code expects.

If we need fallbacks to ride over an upgrade case, those fallbacks should be implemented where the errors are happening.

In the current version of code createMetaTableDescriptorBuilder is used only during HMaster startup via InitMetaProcedure when table info file is missing.
Agree it is a hack (defaultMetaTableDesc = null), where RegionServer will get the default meta descriptor (createMetaTableDescriptorBuilder) until HMaster rewrite the proper one.

}
}

// last HTD written wins
if (usecache && tdmt != null) {
this.cache.put(tableName, tdmt);
Expand Down