anmolnar commented on code in PR #7558:
URL: https://github.com/apache/hbase/pull/7558#discussion_r2733850194
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/ZKConnectionRegistry.java:
##########
@@ -262,6 +264,14 @@ public CompletableFuture<ServerName> getActiveMaster() {
"ZKConnectionRegistry.getActiveMaster");
}
+ @Override
+ public CompletableFuture<TableName> getMetaTableName() {
+ return tracedFuture(
+ () -> CompletableFuture
+
.completedFuture(TableName.valueOf(NamespaceDescriptor.SYSTEM_NAMESPACE_NAME_STR,
"meta")),
Review Comment:
This is also hardcoded.
Is the RPC based connection registry going to be a requirement for custom
meta table names?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/HBaseServerBase.java:
##########
@@ -675,6 +675,14 @@ public String toString() {
return getServerName().toString();
}
+ @Override
+ public org.apache.hadoop.hbase.TableName getMetaTableName() {
+ // For now, always return the default meta table name.
+ // Future implementations may support custom meta table names from
configuration or storage.
+ return org.apache.hadoop.hbase.TableName
+
.valueOf(org.apache.hadoop.hbase.NamespaceDescriptor.SYSTEM_NAMESPACE_NAME_STR,
"meta");
Review Comment:
Why is it still hardcoded?
You're already initializing it in HMaster properly and just need to return
it.
##########
hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java:
##########
@@ -1209,8 +1209,7 @@ public enum OperationStatusCode {
@Deprecated
public static final List<String> HBASE_NON_USER_TABLE_DIRS =
Collections.unmodifiableList(Arrays.asList(
- (String[]) ArrayUtils.addAll(new String[] {
TableName.META_TABLE_NAME.getNameAsString() },
- HBASE_NON_TABLE_DIRS.toArray())));
+ (String[]) ArrayUtils.addAll(new String[] { "hbase:meta" },
HBASE_NON_TABLE_DIRS.toArray())));
Review Comment:
You don't need to do this refactoring, because later it will be harder to
find. Since you just deprecated the static meta table name, you can still use
it if there's no better option.
##########
hbase-common/src/main/java/org/apache/hadoop/hbase/TableName.java:
##########
@@ -66,7 +70,8 @@ public final class TableName implements Comparable<TableName>
{
+ NAMESPACE_DELIM + ")?)" + "(?:" + VALID_TABLE_QUALIFIER_REGEX + "))";
/** The hbase:meta table's name. */
- public static final TableName META_TABLE_NAME =
+ @Deprecated
+ public static TableName META_TABLE_NAME =
Review Comment:
Is it still being used somewhere? Shouldn't we just remove it?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/InitMetaProcedure.java:
##########
@@ -79,7 +83,7 @@ private static TableDescriptor writeFsLayout(Path rootDir,
MasterProcedureEnv en
throws IOException {
LOG.info("BOOTSTRAP: creating hbase:meta region");
FileSystem fs = rootDir.getFileSystem(env.getMasterConfiguration());
- Path tableDir = CommonFSUtils.getTableDir(rootDir,
TableName.META_TABLE_NAME);
+ Path tableDir = CommonFSUtils.getTableDir(rootDir,
TableName.valueOf("hbase", "meta"));
Review Comment:
Again, don't use hardcoded values.
##########
hbase-common/src/main/java/org/apache/hadoop/hbase/InnerStoreCellComparator.java:
##########
@@ -75,8 +75,13 @@ public static CellComparator
getInnerStoreCellComparator(TableName tableName) {
* @return CellComparator to use going off the {@code tableName} passed.
*/
public static CellComparator getInnerStoreCellComparator(byte[] tableName) {
- return Bytes.equals(tableName, TableName.META_TABLE_NAME.toBytes())
+ // Check if this is a meta table (hbase:meta or hbase:meta_*)
+ return isMetaTable(tableName)
? MetaCellComparator.META_COMPARATOR
: InnerStoreCellComparator.INNER_STORE_COMPARATOR;
}
+
+ private static boolean isMetaTable(byte[] tableName) {
+ return Bytes.startsWith(tableName, Bytes.toBytes("hbase:meta"));
Review Comment:
Same here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
