-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HBASE-29082: Support for custom meta table name suffix #6632
base: HBASE-29081
Are you sure you want to change the base?
Conversation
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
* Added in HBASE-XXXXX to support having multiple hbase:meta tables (with distinct names )to | ||
* enable storage sharing by more than one clusters. | ||
*/ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: remove empty line
String suffix_val = String.valueOf(conf.getStrings( | ||
HConstants.HBASE_META_TABLE_SUFFIX, HConstants.HBASE_META_TABLE_SUFFIX_DEFAULT_VALUE)); | ||
|
||
if (suffix_val == null||suffix_val.isEmpty()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use proper formatting:
if (suffix_val == null || suffix_val.isEmpty()) {
LOG.debug("Default value for Hbase meta table is being chosen."); | ||
return TableName.META_TABLE_NAME; | ||
} | ||
LOG.debug("Suffix value for Hbase meta table is being chosen."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of emitting 2 separate log messages, add a single one at INFO level.
LOG.info("Meta table name: {}", META_TABLE_NAME);
String suffix_val = String.valueOf(conf.getStrings( | ||
HConstants.HBASE_META_TABLE_SUFFIX, HConstants.HBASE_META_TABLE_SUFFIX_DEFAULT_VALUE)); | ||
|
||
if (suffix_val == null||suffix_val.isEmpty()) { | ||
return TableName.META_TABLE_NAME; | ||
} | ||
return valueOf(NamespaceDescriptor.SYSTEM_NAMESPACE_NAME_STR, "meta_" | ||
+ suffix_val); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't duplicate the logic, put it at a common place: keep it here or in the other class, or put it in a common utility.
@@ -78,6 +94,7 @@ private static TableDescriptor writeFsLayout(Path rootDir, Configuration conf) | |||
Path tableDir = CommonFSUtils.getTableDir(rootDir, TableName.META_TABLE_NAME); | |||
if (fs.exists(tableDir) && !fs.delete(tableDir, true)) { | |||
LOG.warn("Can not delete partial created meta table, continue..."); | |||
throw new HBaseIOException("Specified meta table already exists."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might not need this I believe for 2 reasons:
- we should keep the original logic here for backward compatiblity,
- you throw this exception only in the case when HBase tried to delete
tableDir
, but it wasn't successful
No description provided.