Skip to content

[HUDI-5027] Replace hardcoded hbase config keys with constant variables #6946

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.

Sign up for GitHub

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

Merged
xushiyan merged 6 commits into from
Oct 24, 2022
Merged

[HUDI-5027] Replace hardcoded hbase config keys with constant variables #6946

Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1651d2e
HUDI-5027. Improve SparkHoodieHBaseIndex#getHBaseConnection Use Hbase…
Oct 14, 2022
b9d722e
Merge branch 'apache:master' into HUDI-5027
slfan1989 Oct 14, 2022
7ba738a
Merge branch 'apache:master' into HUDI-5027
slfan1989 Oct 19, 2022
f564ccd
HUDI-5027. Fix CheckStyle.
Oct 20, 2022
a685766
Merge branch 'apache:master' into HUDI-5027
slfan1989 Oct 21, 2022
8609918
Merge branch 'apache:master' into HUDI-5027
slfan1989 Oct 23, 2022
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
25 changes: 17 additions & 8 deletions ...nt/hudi-spark-client/src/main/java/org/apache/hudi/index/hbase/SparkHoodieHBaseIndex.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,15 @@

import scala.Tuple2;

import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_SECURITY_AUTHENTICATION;
import static org.apache.hadoop.hbase.HConstants.ZOOKEEPER_QUORUM;
import static org.apache.hadoop.hbase.HConstants.ZOOKEEPER_ZNODE_PARENT;
import static org.apache.hadoop.hbase.HConstants.CLIENT_ZOOKEEPER_CLIENT_PORT;
import static org.apache.hadoop.hbase.security.SecurityConstants.MASTER_KRB_PRINCIPAL;
import static org.apache.hadoop.hbase.security.SecurityConstants.REGIONSERVER_KRB_PRINCIPAL;
import static org.apache.hadoop.hbase.security.User.HBASE_SECURITY_AUTHORIZATION_CONF_KEY;
import static org.apache.hadoop.hbase.security.User.HBASE_SECURITY_CONF_KEY;

/**
* Hoodie Index implementation backed by HBase.
*/
Expand Down Expand Up @@ -145,22 +154,22 @@ public HBaseIndexQPSResourceAllocator createQPSResourceAllocator(HoodieWriteConf
private Connection getHBaseConnection() {
Configuration hbaseConfig = HBaseConfiguration.create();
String quorum = config.getHbaseZkQuorum();
hbaseConfig.set("hbase.zookeeper.quorum", quorum);
hbaseConfig.set(ZOOKEEPER_QUORUM, quorum);
String zkZnodeParent = config.getHBaseZkZnodeParent();
if (zkZnodeParent != null) {
hbaseConfig.set("zookeeper.znode.parent", zkZnodeParent);
hbaseConfig.set(ZOOKEEPER_ZNODE_PARENT, zkZnodeParent);
}
String port = String.valueOf(config.getHbaseZkPort());
hbaseConfig.set("hbase.zookeeper.property.clientPort", port);
hbaseConfig.set(CLIENT_ZOOKEEPER_CLIENT_PORT, port);
Copy link
Member

@xushiyan xushiyan Oct 17, 2022

Choose a reason for hiding this comment

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

this constant resolves to hbase.client.zookeeper.property.clientPort. can you confirm if it's fully compatible across different hbase versions? could you pls also global search (e.g. hbase.zookeeper.property.clientPort ) and ensure nothing breaks?

Copy link
Contributor Author

@slfan1989 slfan1989 Oct 17, 2022

Choose a reason for hiding this comment

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

Thank you very much for your help reviewing the code, I will double check.

Copy link
Contributor Author

@slfan1989 slfan1989 Oct 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@xushiyan Thank you very much for your reminder! I am looking for the hbase.zookeeper.property.clientPort parameter in the Hbase project, this parameter should be ZOOKEEPER_CLIENT_PORT

image

This config is in the current Master version, and the modification record is 2015. ZOOKEEPER_CLIENT_PORT should be compatible with Hbase versions 1.x and 2.x.

image

Copy link
Contributor Author

@slfan1989 slfan1989 Oct 20, 2022

Choose a reason for hiding this comment

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

The configuration of the Hudi project involving HBase has been checked. There is a hard-coded situation in TestSparkHoodieHBaseIndex.java, which has been modified.


try {
String authentication = config.getHBaseIndexSecurityAuthentication();
if (authentication.equals("kerberos")) {
hbaseConfig.set("hbase.security.authentication", "kerberos");
hbaseConfig.set("hadoop.security.authentication", "kerberos");
hbaseConfig.set("hbase.security.authorization", "true");
hbaseConfig.set("hbase.regionserver.kerberos.principal", config.getHBaseIndexRegionserverPrincipal());
hbaseConfig.set("hbase.master.kerberos.principal", config.getHBaseIndexMasterPrincipal());
hbaseConfig.set(HBASE_SECURITY_CONF_KEY, "kerberos");
hbaseConfig.set(HADOOP_SECURITY_AUTHENTICATION, "kerberos");
hbaseConfig.set(HBASE_SECURITY_AUTHORIZATION_CONF_KEY, "true");
hbaseConfig.set(REGIONSERVER_KRB_PRINCIPAL, config.getHBaseIndexRegionserverPrincipal());
hbaseConfig.set(MASTER_KRB_PRINCIPAL, config.getHBaseIndexMasterPrincipal());

String principal = config.getHBaseIndexKerberosUserPrincipal();
String keytab = SparkFiles.get(config.getHBaseIndexKerberosUserKeytab());
Expand Down