You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by mo...@apache.org on 2023/03/02 16:45:20 UTC
[doris] 04/09: [improvement][fix](catalog) check required properties when creating catalog and fix jdbc catalog issue (#17209)
This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch branch-1.2-lts
in repository https://gitbox.apache.org/repos/asf/doris.git
commit f9a04b3732c40ce7ad3ec5b7460eff7dfaa84ea8
Author: Mingyu Chen <mo...@163.com>
AuthorDate: Wed Mar 1 17:08:36 2023 +0800
[improvement][fix](catalog) check required properties when creating catalog and fix jdbc catalog issue (#17209)
Check required properties when creating catalog.
To avoid some strange error when missing required properties
This PR add checks for:
hms catalog: check the validation of dfs.ha properties
jdbc catalog: check jdbc_url, driver_url, driver_class is set.
Fix NPE when init MasterCatalogExecutor
The MasterCatalogExecutor may be called by FrontendServiceImpl from BE, which does not have ConnectionContext.
Add more jdbc url param to resolve Chinese issue
add useUnicode=true&characterEncoding=utf-8 by default in jdbc catalog when connecting to MySQL
Update FAQ doc of catalog
---
.../org/apache/doris/catalog/HdfsResource.java | 1 +
.../org/apache/doris/catalog/JdbcResource.java | 53 +++++++++++++++++---
.../org/apache/doris/datasource/CatalogMgr.java | 8 ++-
.../apache/doris/datasource/ExternalCatalog.java | 5 ++
.../doris/datasource/HMSExternalCatalog.java | 36 ++++++++++++++
.../doris/datasource/JdbcExternalCatalog.java | 16 ++++++
.../org/apache/doris/qe/MasterCatalogExecutor.java | 7 ++-
.../apache/doris/datasource/CatalogMgrTest.java | 58 ++++++++++++++++++++++
8 files changed, 175 insertions(+), 9 deletions(-)
diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/HdfsResource.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/HdfsResource.java
index 0421949ff1..d812d8ec53 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/catalog/HdfsResource.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/HdfsResource.java
@@ -51,6 +51,7 @@ public class HdfsResource extends Resource {
public static String HADOOP_KERBEROS_KEYTAB = "hadoop.kerberos.keytab";
public static String HADOOP_SHORT_CIRCUIT = "dfs.client.read.shortcircuit";
public static String HADOOP_SOCKET_PATH = "dfs.domain.socket.path";
+ public static String DSF_NAMESERVICES = "dfs.nameservices";
@SerializedName(value = "properties")
private Map<String, String> properties;
diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/JdbcResource.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/JdbcResource.java
index c02666d928..d43098b7db 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/catalog/JdbcResource.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/JdbcResource.java
@@ -247,21 +247,36 @@ public class JdbcResource extends Resource {
String newJdbcUrl = jdbcUrl.replaceAll(" ", "");
String dbType = parseDbType(newJdbcUrl);
if (dbType.equals(MYSQL)) {
- // 1. `yearIsDateType` is a parameter of JDBC, and the default is true.
+ // `yearIsDateType` is a parameter of JDBC, and the default is true.
// We force the use of `yearIsDateType=false`
- // 2. `tinyInt1isBit` is a parameter of JDBC, and the default is true.
+ newJdbcUrl = checkAndSetJdbcBoolParam(newJdbcUrl, "yearIsDateType", "true", "false");
+ // `tinyInt1isBit` is a parameter of JDBC, and the default is true.
// We force the use of `tinyInt1isBit=false`, so that for mysql type tinyint,
// it will convert to Doris tinyint, not bit.
- newJdbcUrl = checkJdbcUrlParam(newJdbcUrl, "yearIsDateType", "true", "false");
- newJdbcUrl = checkJdbcUrlParam(newJdbcUrl, "tinyInt1isBit", "true", "false");
+ newJdbcUrl = checkAndSetJdbcBoolParam(newJdbcUrl, "tinyInt1isBit", "true", "false");
+ // set useUnicode and characterEncoding to false and utf-8
+ newJdbcUrl = checkAndSetJdbcBoolParam(newJdbcUrl, "useUnicode", "false", "true");
+ newJdbcUrl = checkAndSetJdbcParam(newJdbcUrl, "characterEncoding", "utf-8");
}
if (dbType.equals(MYSQL) || dbType.equals(POSTGRESQL)) {
- newJdbcUrl = checkJdbcUrlParam(newJdbcUrl, "useCursorFetch", "false", "true");
+ newJdbcUrl = checkAndSetJdbcBoolParam(newJdbcUrl, "useCursorFetch", "false", "true");
}
return newJdbcUrl;
}
- private static String checkJdbcUrlParam(String jdbcUrl, String params, String unexpectedVal, String expectedVal) {
+ /**
+ * Check jdbcUrl param, if the param is not set, set it to the expected value.
+ * If the param is set to an unexpected value, replace it with the expected value.
+ * If the param is set to the expected value, do nothing.
+ *
+ * @param jdbcUrl
+ * @param params
+ * @param unexpectedVal
+ * @param expectedVal
+ * @return
+ */
+ private static String checkAndSetJdbcBoolParam(String jdbcUrl, String params, String unexpectedVal,
+ String expectedVal) {
String unexpectedParams = params + "=" + unexpectedVal;
String expectedParams = params + "=" + expectedVal;
if (jdbcUrl.contains(expectedParams)) {
@@ -280,4 +295,30 @@ public class JdbcResource extends Resource {
}
return jdbcUrl;
}
+
+ /**
+ * Check jdbcUrl param, if the param is set, do thing.
+ * If the param is not set, set it to expected value.
+ *
+ * @param jdbcUrl
+ * @param params
+ * @param value
+ * @return
+ */
+ private static String checkAndSetJdbcParam(String jdbcUrl, String params, String expectedVal) {
+ String expectedParams = params + "=" + expectedVal;
+ if (jdbcUrl.contains(expectedParams)) {
+ return jdbcUrl;
+ } else {
+ if (jdbcUrl.contains("?")) {
+ if (jdbcUrl.charAt(jdbcUrl.length() - 1) != '?') {
+ jdbcUrl += "&";
+ }
+ } else {
+ jdbcUrl += "?";
+ }
+ jdbcUrl += expectedParams;
+ }
+ return jdbcUrl;
+ }
}
diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogMgr.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogMgr.java
index 8373c7b8d9..65f88d428f 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogMgr.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogMgr.java
@@ -246,7 +246,10 @@ public class CatalogMgr implements Writable, GsonPostProcessable {
}
long id = Env.getCurrentEnv().getNextId();
CatalogLog log = CatalogFactory.constructorCatalogLog(id, stmt);
- replayCreateCatalog(log);
+ CatalogIf catalog = replayCreateCatalog(log);
+ if (catalog instanceof ExternalCatalog) {
+ ((ExternalCatalog) catalog).checkProperties();
+ }
Env.getCurrentEnv().getEditLog().logCatalogLog(OperationType.OP_CREATE_CATALOG, log);
} finally {
writeUnlock();
@@ -450,11 +453,12 @@ public class CatalogMgr implements Writable, GsonPostProcessable {
/**
* Reply for create catalog event.
*/
- public void replayCreateCatalog(CatalogLog log) throws DdlException {
+ public CatalogIf replayCreateCatalog(CatalogLog log) throws DdlException {
writeLock();
try {
CatalogIf catalog = CatalogFactory.constructorFromLog(log);
addCatalog(catalog);
+ return catalog;
} finally {
writeUnlock();
}
diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java
index def5756f15..68a3a013fa 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java
@@ -26,6 +26,7 @@ import org.apache.doris.catalog.external.HMSExternalDatabase;
import org.apache.doris.catalog.external.IcebergExternalDatabase;
import org.apache.doris.catalog.external.JdbcExternalDatabase;
import org.apache.doris.cluster.ClusterNamespace;
+import org.apache.doris.common.DdlException;
import org.apache.doris.common.io.Text;
import org.apache.doris.common.io.Writable;
import org.apache.doris.common.util.Util;
@@ -147,6 +148,10 @@ public abstract class ExternalCatalog implements CatalogIf<ExternalDatabase>, Wr
// hms client, read properties from hive-site.xml, es client
protected abstract void initLocalObjectsImpl();
+ // check if all required properties are set when creating catalog
+ public void checkProperties() throws DdlException {
+ }
+
// init schema related objects
protected abstract void init();
diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/HMSExternalCatalog.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/HMSExternalCatalog.java
index a4d1bab514..0cf7afdc53 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/datasource/HMSExternalCatalog.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/HMSExternalCatalog.java
@@ -24,9 +24,11 @@ import org.apache.doris.catalog.HdfsResource;
import org.apache.doris.catalog.external.ExternalDatabase;
import org.apache.doris.catalog.external.HMSExternalDatabase;
import org.apache.doris.common.Config;
+import org.apache.doris.common.DdlException;
import org.apache.doris.datasource.hive.PooledHiveMetaStoreClient;
import org.apache.doris.datasource.hive.event.MetastoreNotificationFetchException;
+import com.google.common.base.Strings;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import org.apache.hadoop.conf.Configuration;
@@ -63,6 +65,40 @@ public class HMSExternalCatalog extends ExternalCatalog {
catalogProperty = new CatalogProperty(resource, props);
}
+ @Override
+ public void checkProperties() throws DdlException {
+ super.checkProperties();
+ // check the dfs.ha properties
+ // 'dfs.nameservices'='your-nameservice',
+ // 'dfs.ha.namenodes.your-nameservice'='nn1,nn2',
+ // 'dfs.namenode.rpc-address.your-nameservice.nn1'='172.21.0.2:4007',
+ // 'dfs.namenode.rpc-address.your-nameservice.nn2'='172.21.0.3:4007',
+ // 'dfs.client.failover.proxy.provider.your-nameservice'='xxx'
+ String dfsNameservices = catalogProperty.getOrDefault(HdfsResource.DSF_NAMESERVICES, "");
+ if (Strings.isNullOrEmpty(dfsNameservices)) {
+ return;
+ }
+ String namenodes = catalogProperty.getOrDefault("dfs.ha.namenodes." + dfsNameservices, "");
+ if (Strings.isNullOrEmpty(namenodes)) {
+ throw new DdlException("Missing dfs.ha.namenodes." + dfsNameservices + " property");
+ }
+ String[] names = namenodes.split(",");
+ for (String name : names) {
+ String address = catalogProperty.getOrDefault("dfs.namenode.rpc-address." + dfsNameservices + "." + name,
+ "");
+ if (Strings.isNullOrEmpty(address)) {
+ throw new DdlException(
+ "Missing dfs.namenode.rpc-address." + dfsNameservices + "." + name + " property");
+ }
+ }
+ String failoverProvider = catalogProperty.getOrDefault("dfs.client.failover.proxy.provider." + dfsNameservices,
+ "");
+ if (Strings.isNullOrEmpty(failoverProvider)) {
+ throw new DdlException(
+ "Missing dfs.client.failover.proxy.provider." + dfsNameservices + " property");
+ }
+ }
+
public String getHiveMetastoreUris() {
return catalogProperty.getOrDefault(HMSResource.HIVE_METASTORE_URIS, "");
}
diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/JdbcExternalCatalog.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/JdbcExternalCatalog.java
index f196e07154..0fc5e22515 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/datasource/JdbcExternalCatalog.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/JdbcExternalCatalog.java
@@ -39,6 +39,12 @@ import java.util.Map;
public class JdbcExternalCatalog extends ExternalCatalog {
private static final Logger LOG = LogManager.getLogger(JdbcExternalCatalog.class);
+ private static final List<String> REQUIRED_PROPERTIES = Lists.newArrayList(
+ JdbcResource.JDBC_URL,
+ JdbcResource.DRIVER_URL,
+ JdbcResource.DRIVER_CLASS
+ );
+
// Must add "transient" for Gson to ignore this field,
// or Gson will throw exception with HikariCP
private transient JdbcClient jdbcClient;
@@ -50,6 +56,16 @@ public class JdbcExternalCatalog extends ExternalCatalog {
this.catalogProperty = new CatalogProperty(resource, processCompatibleProperties(props));
}
+ @Override
+ public void checkProperties() throws DdlException {
+ super.checkProperties();
+ for (String requiredProperty : REQUIRED_PROPERTIES) {
+ if (!catalogProperty.getProperties().containsKey(requiredProperty)) {
+ throw new DdlException("Required property '" + requiredProperty + "' is missing");
+ }
+ }
+ }
+
@Override
public void onClose() {
if (jdbcClient != null) {
diff --git a/fe/fe-core/src/main/java/org/apache/doris/qe/MasterCatalogExecutor.java b/fe/fe-core/src/main/java/org/apache/doris/qe/MasterCatalogExecutor.java
index a26c687e10..e13d2daefd 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/qe/MasterCatalogExecutor.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/qe/MasterCatalogExecutor.java
@@ -40,7 +40,12 @@ public class MasterCatalogExecutor {
public MasterCatalogExecutor() {
ctx = ConnectContext.get();
- waitTimeoutMs = ctx.getSessionVariable().getQueryTimeoutS() * 1000;
+ if (ctx == null) {
+ // The method may be called by FrontendServiceImpl from BE, which does not have ConnectContext.
+ waitTimeoutMs = 300 * 1000;
+ } else {
+ waitTimeoutMs = ctx.getSessionVariable().getQueryTimeoutS() * 1000;
+ }
}
public void forward(long catalogId, long dbId) throws Exception {
diff --git a/fe/fe-core/src/test/java/org/apache/doris/datasource/CatalogMgrTest.java b/fe/fe-core/src/test/java/org/apache/doris/datasource/CatalogMgrTest.java
index f74bf032e5..78ab98ef1e 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/datasource/CatalogMgrTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/datasource/CatalogMgrTest.java
@@ -44,6 +44,7 @@ import org.apache.doris.catalog.external.HMSExternalDatabase;
import org.apache.doris.catalog.external.HMSExternalTable;
import org.apache.doris.common.AnalysisException;
import org.apache.doris.common.DdlException;
+import org.apache.doris.common.ExceptionChecker;
import org.apache.doris.common.FeConstants;
import org.apache.doris.datasource.hive.HiveMetaStoreCache;
import org.apache.doris.datasource.hive.HiveMetaStoreCache.HivePartitionValues;
@@ -547,4 +548,61 @@ public class CatalogMgrTest extends TestWithFeService {
partitionNameToIdMap, idToUniqueIdsMap, singleUidToColumnRangeMap, partitionValuesMap);
}
+ @Test
+ public void testInvalidCreateCatalogProperties() throws Exception {
+ String createCatalogSql = "CREATE CATALOG bad_hive1 PROPERTIES (\n"
+ + " 'type'='hms',\n"
+ + " 'hive.metastore.uris' = 'thrift://172.21.0.1:7004',\n"
+ + " 'hadoop.username' = 'hive',\n"
+ + " 'dfs.nameservices'='your-nameservice',\n"
+ + " 'dfs.namenode.rpc-address.your-nameservice.nn1'='172.21.0.2:4007',\n"
+ + " 'dfs.namenode.rpc-address.your-nameservice.nn2'='172.21.0.3:4007',\n"
+ + " 'dfs.client.failover.proxy.provider.your-nameservice'"
+ + "='org.apache.hadoop.hdfs.server.namenode.ha.ConfiguredFailoverProxyProvider'\n"
+ + ");";
+ CreateCatalogStmt createStmt1 = (CreateCatalogStmt) parseAndAnalyzeStmt(createCatalogSql);
+ ExceptionChecker.expectThrowsWithMsg(DdlException.class, "Missing dfs.ha.namenodes.your-nameservice property",
+ () -> mgr.createCatalog(createStmt1));
+
+ createCatalogSql = "CREATE CATALOG bad_hive2 PROPERTIES (\n"
+ + " 'type'='hms',\n"
+ + " 'hive.metastore.uris' = 'thrift://172.21.0.1:7004',\n"
+ + " 'hadoop.username' = 'hive',\n"
+ + " 'dfs.nameservices'='your-nameservice',\n"
+ + " 'dfs.ha.namenodes.your-nameservice'='nn1,nn2',\n"
+ + " 'dfs.namenode.rpc-address.your-nameservice.nn2'='172.21.0.3:4007',\n"
+ + " 'dfs.client.failover.proxy.provider.your-nameservice'"
+ + "='org.apache.hadoop.hdfs.server.namenode.ha.ConfiguredFailoverProxyProvider'\n"
+ + ");";
+ CreateCatalogStmt createStmt2 = (CreateCatalogStmt) parseAndAnalyzeStmt(createCatalogSql);
+ ExceptionChecker.expectThrowsWithMsg(DdlException.class,
+ "Missing dfs.namenode.rpc-address.your-nameservice.nn1 property",
+ () -> mgr.createCatalog(createStmt2));
+
+ createCatalogSql = "CREATE CATALOG good_hive PROPERTIES (\n"
+ + " 'type'='hms',\n"
+ + " 'hive.metastore.uris' = 'thrift://172.21.0.1:7004',\n"
+ + " 'hadoop.username' = 'hive',\n"
+ + " 'dfs.nameservices'='your-nameservice',\n"
+ + " 'dfs.ha.namenodes.your-nameservice'='nn1,nn2',\n"
+ + " 'dfs.namenode.rpc-address.your-nameservice.nn1'='172.21.0.2:4007',\n"
+ + " 'dfs.namenode.rpc-address.your-nameservice.nn2'='172.21.0.3:4007'\n"
+ + ");";
+ CreateCatalogStmt createStmt3 = (CreateCatalogStmt) parseAndAnalyzeStmt(createCatalogSql);
+ ExceptionChecker.expectThrowsWithMsg(DdlException.class,
+ "Missing dfs.client.failover.proxy.provider.your-nameservice property",
+ () -> mgr.createCatalog(createStmt3));
+
+ createCatalogSql = "CREATE CATALOG bad_jdbc PROPERTIES (\n"
+ + " \"type\"=\"jdbc\",\n"
+ + " \"user\"=\"root\",\n"
+ + " \"password\"=\"123456\",\n"
+ + " \"jdbc_url\" = \"jdbc:mysql://127.0.0.1:3306/demo\",\n"
+ + " \"driver_class\" = \"com.mysql.jdbc.Driver\"\n"
+ + ")";
+ CreateCatalogStmt createStmt4 = (CreateCatalogStmt) parseAndAnalyzeStmt(createCatalogSql);
+ ExceptionChecker.expectThrowsWithMsg(DdlException.class,
+ "Required property 'driver_url' is missing",
+ () -> mgr.createCatalog(createStmt4));
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org