You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2021/01/30 09:18:04 UTC
[GitHub] [incubator-doris] stalary opened a new pull request #5325: WIP：Support doris connect https es server and support config esNodesDiscovery
stalary opened a new pull request #5325:
URL: https://github.com/apache/incubator-doris/pull/5325
## Proposed changes
Support doris connect https es server and support config esNodesDiscovery
## Types of changes
What types of changes does your code introduce to Doris?
_Put an `x` in the boxes that apply_
- [] Bugfix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [] Documentation Update (if none of the other choices apply)
- [] Code refactor (Modify the code structure, format the code, etc...)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] wuyunfeng commented on a change in pull request #5325: WIP：Support doris connect https es server and support config esNodesDiscovery
Posted by GitBox <gi...@apache.org>.
wuyunfeng commented on a change in pull request #5325:
URL: https://github.com/apache/incubator-doris/pull/5325#discussion_r580871634
##########
File path: fe/fe-core/src/main/java/org/apache/doris/catalog/EsTable.java
##########
@@ -242,10 +251,13 @@ private void validate(Map<String, String> properties) throws DdlException {
tableContext.put("enableDocValueScan", String.valueOf(enableDocValueScan));
tableContext.put("enableKeywordSniff", String.valueOf(enableKeywordSniff));
tableContext.put("maxDocValueFields", String.valueOf(maxDocValueFields));
+ tableContext.put(ES_NODES_DISCOVERY, String.valueOf(esNodesDiscovery));
+ tableContext.put(USE_SSL_CLIENT, String.valueOf(useSslClient));
}
public TTableDescriptor toThrift() {
TEsTable tEsTable = new TEsTable();
+ tEsTable.setUseSslClient(useSslClient);
Review comment:
I think you should put `useSslClient` into the `properties` of `TEsScanNode`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] stalary commented on a change in pull request #5325: [Doris On ES][WIP] Support external ES table with `SSL` secured and configurable node sniffing
Posted by GitBox <gi...@apache.org>.
stalary commented on a change in pull request #5325:
URL: https://github.com/apache/incubator-doris/pull/5325#discussion_r586547824
##########
File path: fe/fe-core/src/main/java/org/apache/doris/external/elasticsearch/PartitionPhase.java
##########
@@ -37,7 +38,16 @@ public PartitionPhase(EsRestClient client) {
@Override
public void execute(SearchContext context) throws DorisEsException {
shardPartitions = client.searchShards(context.sourceIndex());
- nodesInfo = client.getHttpNodes();
+ if (context.esNodesDiscovery()) {
+ nodesInfo = client.getHttpNodes();
+ } else {
+ nodesInfo = new HashMap<>();
+ String[] seeds = context.esTable().getSeeds();
+ boolean useSSL = context.esTable().isUseSslClient();
+ for (int i = 0; i < seeds.length; i++) {
+ nodesInfo.put(String.valueOf(i), new EsNodeInfo(String.valueOf(i), seeds[i], useSSL));
Review comment:
When creating a table without a protocol passed in, add it to the constructor
##########
File path: fe/fe-core/src/main/java/org/apache/doris/external/elasticsearch/EsUtil.java
##########
@@ -82,4 +85,13 @@ public static JSONObject getJsonObject(JSONObject jsonObject, String key, int fr
return null;
}
}
+
+ public static boolean getBooleanProperty(Map<String, String> properties, String name) throws DdlException {
Review comment:
I didn't quite understand it
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] wuyunfeng commented on a change in pull request #5325: [Doris On ES][WIP] Support external ES table with `SSL` secured and configurable node sniffing
Posted by GitBox <gi...@apache.org>.
wuyunfeng commented on a change in pull request #5325:
URL: https://github.com/apache/incubator-doris/pull/5325#discussion_r581721359
##########
File path: be/src/exec/es/es_scan_reader.h
##########
@@ -63,6 +64,8 @@ class ESScanReader {
std::string _query;
// Elasticsearch shards to fetch document
std::string _shards;
+ // whether use ssl client
+ bool _use_ssl_client;
Review comment:
init `_use_ssl_client`?
##########
File path: be/src/http/http_client.h
##########
@@ -66,6 +66,11 @@ class HttpClient {
curl_easy_setopt(_curl, CURLOPT_COPYPOSTFIELDS, post_body.c_str());
}
+ void use_ssl() {
+ curl_easy_setopt(_curl, CURLOPT_SSL_VERIFYPEER, false);
Review comment:
The third param should be a long type ??
##########
File path: be/src/exec/es/es_scan_reader.cpp
##########
@@ -134,6 +140,9 @@ Status ESScanReader::get_next(bool* scan_eos, std::unique_ptr<ScrollParser>& scr
_network_client.set_basic_auth(_user_name, _passwd);
_network_client.set_content_type("application/json");
_network_client.set_timeout_ms(_http_timeout_ms);
+ if (_use_ssl_client) {
Review comment:
put line 143 and 111 together?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] wuyunfeng merged pull request #5325: [Doris On ES][WIP] Support external ES table with `SSL` secured and configurable node sniffing
Posted by GitBox <gi...@apache.org>.
wuyunfeng merged pull request #5325:
URL: https://github.com/apache/incubator-doris/pull/5325
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] stalary commented on a change in pull request #5325: [Doris On ES][WIP] Support external ES table with `SSL` secured and configurable node sniffing
Posted by GitBox <gi...@apache.org>.
stalary commented on a change in pull request #5325:
URL: https://github.com/apache/incubator-doris/pull/5325#discussion_r581757278
##########
File path: fe/fe-core/src/main/java/org/apache/doris/catalog/EsTable.java
##########
@@ -186,36 +200,40 @@ private void validate(Map<String, String> properties) throws DdlException {
// enable doc value scan for Elasticsearch
if (properties.containsKey(DOC_VALUE_SCAN)) {
- try {
- enableDocValueScan = Boolean.parseBoolean(properties.get(DOC_VALUE_SCAN).trim());
- } catch (Exception e) {
- throw new DdlException("fail to parse enable_docvalue_scan, enable_docvalue_scan= "
- + properties.get(VERSION).trim() + " ,`enable_docvalue_scan`"
- + " should be like 'true' or 'false'， value should be double quotation marks");
- }
+ enableDocValueScan = EsUtil.getBooleanProperty(properties, DOC_VALUE_SCAN);
}
if (properties.containsKey(KEYWORD_SNIFF)) {
- try {
- enableKeywordSniff = Boolean.parseBoolean(properties.get(KEYWORD_SNIFF).trim());
- } catch (Exception e) {
- throw new DdlException("fail to parse enable_keyword_sniff, enable_keyword_sniff= "
- + properties.get(VERSION).trim() + " ,`enable_keyword_sniff`"
- + " should be like 'true' or 'false'， value should be double quotation marks");
+ enableKeywordSniff = EsUtil.getBooleanProperty(properties, KEYWORD_SNIFF);
+ }
+
+ if (properties.containsKey(ES_NODES_DISCOVERY)) {
+ esNodesDiscovery = EsUtil.getBooleanProperty(properties, ES_NODES_DISCOVERY);
+ }
+
+ if (properties.containsKey(USE_SSL_CLIENT)) {
Review comment:
in 96 line
##########
File path: fe/fe-core/src/main/java/org/apache/doris/catalog/EsTable.java
##########
@@ -186,36 +200,40 @@ private void validate(Map<String, String> properties) throws DdlException {
// enable doc value scan for Elasticsearch
if (properties.containsKey(DOC_VALUE_SCAN)) {
- try {
- enableDocValueScan = Boolean.parseBoolean(properties.get(DOC_VALUE_SCAN).trim());
- } catch (Exception e) {
- throw new DdlException("fail to parse enable_docvalue_scan, enable_docvalue_scan= "
- + properties.get(VERSION).trim() + " ,`enable_docvalue_scan`"
- + " should be like 'true' or 'false'， value should be double quotation marks");
- }
+ enableDocValueScan = EsUtil.getBooleanProperty(properties, DOC_VALUE_SCAN);
}
if (properties.containsKey(KEYWORD_SNIFF)) {
- try {
- enableKeywordSniff = Boolean.parseBoolean(properties.get(KEYWORD_SNIFF).trim());
- } catch (Exception e) {
- throw new DdlException("fail to parse enable_keyword_sniff, enable_keyword_sniff= "
- + properties.get(VERSION).trim() + " ,`enable_keyword_sniff`"
- + " should be like 'true' or 'false'， value should be double quotation marks");
+ enableKeywordSniff = EsUtil.getBooleanProperty(properties, KEYWORD_SNIFF);
+ }
+
+ if (properties.containsKey(ES_NODES_DISCOVERY)) {
+ esNodesDiscovery = EsUtil.getBooleanProperty(properties, ES_NODES_DISCOVERY);
+ }
+
+ if (properties.containsKey(USE_SSL_CLIENT)) {
Review comment:
in 96 line private boolean useSslClient = false;
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] stalary commented on a change in pull request #5325: [Doris On ES][WIP] Support external ES table with `SSL` secured and configurable node sniffing
Posted by GitBox <gi...@apache.org>.
stalary commented on a change in pull request #5325:
URL: https://github.com/apache/incubator-doris/pull/5325#discussion_r581757562
##########
File path: fe/fe-core/src/main/java/org/apache/doris/external/elasticsearch/PartitionPhase.java
##########
@@ -37,7 +38,15 @@ public PartitionPhase(EsRestClient client) {
@Override
public void execute(SearchContext context) throws DorisEsException {
shardPartitions = client.searchShards(context.sourceIndex());
- nodesInfo = client.getHttpNodes();
+ if (context.esNodesDiscovery()) {
+ nodesInfo = client.getHttpNodes();
+ } else {
+ nodesInfo = new HashMap<>();
+ String[] seeds = context.esTable().getSeeds();
+ for (int i = 0; i < seeds.length; i++) {
+ nodesInfo.put(String.valueOf(i), new EsNodeInfo(String.valueOf(i), seeds[i], context.esTable().isUseSslClient()));
Review comment:
ok
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] wuyunfeng commented on a change in pull request #5325: [Doris On ES][WIP] Support external ES table with `SSL` secured and configurable node sniffing
Posted by GitBox <gi...@apache.org>.
wuyunfeng commented on a change in pull request #5325:
URL: https://github.com/apache/incubator-doris/pull/5325#discussion_r581709959
##########
File path: be/src/exec/es/es_scan_reader.cpp
##########
@@ -134,6 +140,9 @@ Status ESScanReader::get_next(bool* scan_eos, std::unique_ptr<ScrollParser>& scr
_network_client.set_basic_auth(_user_name, _passwd);
_network_client.set_content_type("application/json");
_network_client.set_timeout_ms(_http_timeout_ms);
+ if (_use_ssl_client) {
Review comment:
put line 143 and 111 together?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] wuyunfeng commented on a change in pull request #5325: [Doris On ES][WIP] Support external ES table with `SSL` secured and configurable node sniffing
Posted by GitBox <gi...@apache.org>.
wuyunfeng commented on a change in pull request #5325:
URL: https://github.com/apache/incubator-doris/pull/5325#discussion_r598519935
##########
File path: fe/fe-core/src/main/java/org/apache/doris/external/elasticsearch/EsRestClient.java
##########
@@ -172,6 +166,13 @@ private String execute(String path) throws DorisEsException {
DorisEsException scratchExceptionForThrow = null;
OkHttpClient httpClient;
if (useSslClient) {
+ if (sslNetworkClient == null) {
Review comment:
This would leading to condition competition for `sslNetworkClient`, suggest use `synchronized ` method such as `getSSLNetworkClient`
##########
File path: fe/fe-core/src/main/java/org/apache/doris/external/elasticsearch/EsShardPartitions.java
##########
@@ -94,16 +94,22 @@ public static EsShardPartitions findShardPartitions(String indexName, String sea
return partitions;
}
- public void addHttpAddress(Map<String, EsNodeInfo> nodesInfo) {
+ public void addHttpAddress(Map<String, EsNodeInfo> nodesInfo, boolean httpSslEnabled) {
for (Map.Entry<Integer, List<EsShardRouting>> entry : shardRoutings.entrySet()) {
List<EsShardRouting> shardRoutings = entry.getValue();
for (EsShardRouting shardRouting : shardRoutings) {
String nodeId = shardRouting.getNodeId();
+ TNetworkAddress httpAddress;
if (nodesInfo.containsKey(nodeId)) {
- shardRouting.setHttpAddress(nodesInfo.get(nodeId).getPublishAddress());
+ httpAddress = nodesInfo.get(nodeId).getPublishAddress();
} else {
- shardRouting.setHttpAddress(randomAddress(nodesInfo));
+ httpAddress = randomAddress(nodesInfo);
}
+ // If ssl is enabled, determine if the https protocol is required
+ if (httpSslEnabled && !httpAddress.getHostname().startsWith("http")) {
+ httpAddress.setHostname("https://" + httpAddress.getHostname());
Review comment:
why `setHostname` ?? Is `setHttpAddress` does not work??
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] wuyunfeng commented on a change in pull request #5325: [Doris On ES][WIP] Support external ES table with `SSL` secured and configurable node sniffing
Posted by GitBox <gi...@apache.org>.
wuyunfeng commented on a change in pull request #5325:
URL: https://github.com/apache/incubator-doris/pull/5325#discussion_r608360325
##########
File path: fe/fe-core/src/main/java/org/apache/doris/external/elasticsearch/EsRestClient.java
##########
@@ -207,4 +240,33 @@ private String execute(String path) throws DorisEsException {
}
return (T) (key != null ? map.get(key) : map);
}
+
+ /**
+ * support https
+ **/
+ private static class TrustAllCerts implements X509TrustManager {
+ public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException {}
+
+ public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException {}
+
+ public X509Certificate[] getAcceptedIssuers() {return new X509Certificate[0];}
+ }
+
+ private static class TrustAllHostnameVerifier implements HostnameVerifier {
+ public boolean verify(String hostname, SSLSession session) {
+ return true;
+ }
+ }
+
+ private static SSLSocketFactory createSSLSocketFactory() {
+ SSLSocketFactory ssfFactory;
+ try {
+ SSLContext sc = SSLContext.getInstance("TLS");
+ sc.init(null, new TrustManager[]{new TrustAllCerts()}, new SecureRandom());
+ ssfFactory = sc.getSocketFactory();
+ } catch (Exception e) {
+ throw new DorisEsException("createSSLSocketFactory error");
Review comment:
```suggestion
throw new DorisEsException("Errors happens when create ssl socket", e);
```
##########
File path: fe/fe-core/src/main/java/org/apache/doris/external/elasticsearch/EsRestClient.java
##########
@@ -141,6 +154,19 @@ public EsShardPartitions searchShards(String indexName) throws DorisEsException
}
return EsShardPartitions.findShardPartitions(indexName, searchShards);
}
+
+ /**
+ * init ssl networkClient use lazy way
+ **/
+ private synchronized void initSslNetworkClient() {
Review comment:
```suggestion
private synchronized NetworkClient getOrCreateSslNetworkClient() {
```
##########
File path: docs/en/extending-doris/doris-on-es.md
##########
@@ -328,6 +328,63 @@ This term does not match any term in the dictionary，and will not return any re
The type of `k4.keyword` is `keyword`, and writing data into ES is a complete term, so it can be matched
+### Enable ES node discovery(es\_nodes\_discovery=true)
Review comment:
```suggestion
### Enable nodes discovery mechanism, default is true
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] wuyunfeng commented on a change in pull request #5325: [Doris On ES][WIP] Support external ES table with `SSL` secured and configurable node sniffing
Posted by GitBox <gi...@apache.org>.
wuyunfeng commented on a change in pull request #5325:
URL: https://github.com/apache/incubator-doris/pull/5325#discussion_r608366015
##########
File path: docs/en/extending-doris/doris-on-es.md
##########
@@ -328,6 +328,63 @@ This term does not match any term in the dictionary，and will not return any re
The type of `k4.keyword` is `keyword`, and writing data into ES is a complete term, so it can be matched
+### Enable node discovery mechanism, default is true(es\_nodes\_discovery=true)
+
+```
+CREATE EXTERNAL TABLE `test` (
+ `k1` bigint(20) COMMENT "",
+ `k2` datetime COMMENT "",
+ `k3` varchar(20) COMMENT "",
+ `k4` varchar(100) COMMENT "",
+ `k5` float COMMENT ""
+) ENGINE=ELASTICSEARCH
+PROPERTIES (
+"hosts" = "http://192.168.0.1:8200,http://192.168.0.2:8200",
+"index" = "test”,
+"type" = "doc",
+"user" = "root",
+"password" = "root",
+
+"nodes_discovery" = "true"
+);
+```
+
+Parameter Description：
+
+Parameter | Description
+---|---
+**es\_nodes\_discovery** | Whether or not to enable ES node discovery. the default is true
+
+When enabled, Doris will find all available nodes from ES. If you only want Doris to access some nodes, you can turn this configuration off
Review comment:
```suggestion
Doris would find all available related data nodes (shards allocated on)from ES when this is true. Just set false if address of ES data nodes are not accessed by Doris BE, eg. the ES cluster is deployed in the intranet which isolated from your public Internet, and users access through a proxy
```
##########
File path: docs/en/extending-doris/doris-on-es.md
##########
@@ -328,6 +328,63 @@ This term does not match any term in the dictionary，and will not return any re
The type of `k4.keyword` is `keyword`, and writing data into ES is a complete term, so it can be matched
+### Enable node discovery mechanism, default is true(es\_nodes\_discovery=true)
+
+```
+CREATE EXTERNAL TABLE `test` (
+ `k1` bigint(20) COMMENT "",
+ `k2` datetime COMMENT "",
+ `k3` varchar(20) COMMENT "",
+ `k4` varchar(100) COMMENT "",
+ `k5` float COMMENT ""
+) ENGINE=ELASTICSEARCH
+PROPERTIES (
+"hosts" = "http://192.168.0.1:8200,http://192.168.0.2:8200",
+"index" = "test”,
+"type" = "doc",
+"user" = "root",
+"password" = "root",
+
+"nodes_discovery" = "true"
+);
+```
+
+Parameter Description：
+
+Parameter | Description
+---|---
+**es\_nodes\_discovery** | Whether or not to enable ES node discovery. the default is true
+
+When enabled, Doris will find all available nodes from ES. If you only want Doris to access some nodes, you can turn this configuration off
+
+### Enable SSL protocol when making an HTTP request, default is false(http\_ssl\_enable=true)
Review comment:
```suggestion
### whether ES cluster enables https access mode, if enabled should set value with`true`, default is false
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] wuyunfeng commented on a change in pull request #5325: [Doris On ES][WIP] Support external ES table with `SSL` secured and configurable node sniffing
Posted by GitBox <gi...@apache.org>.
wuyunfeng commented on a change in pull request #5325:
URL: https://github.com/apache/incubator-doris/pull/5325#discussion_r586145816
##########
File path: fe/fe-core/src/main/java/org/apache/doris/external/elasticsearch/EsUtil.java
##########
@@ -82,4 +85,13 @@ public static JSONObject getJsonObject(JSONObject jsonObject, String key, int fr
return null;
}
}
+
+ public static boolean getBooleanProperty(Map<String, String> properties, String name) throws DdlException {
Review comment:
```suggestion
public static boolean get(Map<String, String> properties, String name) throws DdlException {
```
```suggestion
public static boolean getBooleanProperty(Map<String, String> properties, String name) throws DdlException {
```
##########
File path: fe/fe-core/src/main/java/org/apache/doris/external/elasticsearch/PartitionPhase.java
##########
@@ -37,7 +38,16 @@ public PartitionPhase(EsRestClient client) {
@Override
public void execute(SearchContext context) throws DorisEsException {
shardPartitions = client.searchShards(context.sourceIndex());
- nodesInfo = client.getHttpNodes();
+ if (context.esNodesDiscovery()) {
+ nodesInfo = client.getHttpNodes();
+ } else {
+ nodesInfo = new HashMap<>();
+ String[] seeds = context.esTable().getSeeds();
+ boolean useSSL = context.esTable().isUseSslClient();
+ for (int i = 0; i < seeds.length; i++) {
+ nodesInfo.put(String.valueOf(i), new EsNodeInfo(String.valueOf(i), seeds[i], useSSL));
Review comment:
it seems `useSSL` is useless for `nodesInfo `?
##########
File path: fe/fe-core/src/main/java/org/apache/doris/external/elasticsearch/EsUtil.java
##########
@@ -21,43 +21,46 @@
import org.apache.doris.analysis.PartitionDesc;
import org.apache.doris.analysis.RangePartitionDesc;
import org.apache.doris.common.AnalysisException;
+import org.apache.doris.common.DdlException;
import org.json.JSONObject;
+import java.util.Map;
+
public class EsUtil {
-
+
public static void analyzePartitionAndDistributionDesc(PartitionDesc partitionDesc,
- DistributionDesc distributionDesc) throws AnalysisException {
+ DistributionDesc distributionDesc) throws AnalysisException {
if (partitionDesc == null && distributionDesc == null) {
return;
}
-
+
Review comment:
why add blank line on those if statement?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] wuyunfeng commented on a change in pull request #5325: [Doris On ES][WIP] Support external ES table with `SSL` secured and configurable node sniffing
Posted by GitBox <gi...@apache.org>.
wuyunfeng commented on a change in pull request #5325:
URL: https://github.com/apache/incubator-doris/pull/5325#discussion_r581739987
##########
File path: fe/fe-core/src/main/java/org/apache/doris/catalog/EsTable.java
##########
@@ -186,36 +200,40 @@ private void validate(Map<String, String> properties) throws DdlException {
// enable doc value scan for Elasticsearch
if (properties.containsKey(DOC_VALUE_SCAN)) {
- try {
- enableDocValueScan = Boolean.parseBoolean(properties.get(DOC_VALUE_SCAN).trim());
- } catch (Exception e) {
- throw new DdlException("fail to parse enable_docvalue_scan, enable_docvalue_scan= "
- + properties.get(VERSION).trim() + " ,`enable_docvalue_scan`"
- + " should be like 'true' or 'false'， value should be double quotation marks");
- }
+ enableDocValueScan = EsUtil.getBooleanProperty(properties, DOC_VALUE_SCAN);
}
if (properties.containsKey(KEYWORD_SNIFF)) {
- try {
- enableKeywordSniff = Boolean.parseBoolean(properties.get(KEYWORD_SNIFF).trim());
- } catch (Exception e) {
- throw new DdlException("fail to parse enable_keyword_sniff, enable_keyword_sniff= "
- + properties.get(VERSION).trim() + " ,`enable_keyword_sniff`"
- + " should be like 'true' or 'false'， value should be double quotation marks");
+ enableKeywordSniff = EsUtil.getBooleanProperty(properties, KEYWORD_SNIFF);
+ }
+
+ if (properties.containsKey(ES_NODES_DISCOVERY)) {
+ esNodesDiscovery = EsUtil.getBooleanProperty(properties, ES_NODES_DISCOVERY);
+ }
+
+ if (properties.containsKey(USE_SSL_CLIENT)) {
Review comment:
how about give `useSslClient` a default value : `false`??
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] wuyunfeng commented on a change in pull request #5325: [Doris On ES][WIP] Support external ES table with `SSL` secured and configurable node sniffing
Posted by GitBox <gi...@apache.org>.
wuyunfeng commented on a change in pull request #5325:
URL: https://github.com/apache/incubator-doris/pull/5325#discussion_r592102526
##########
File path: docs/en/extending-doris/doris-on-es.md
##########
@@ -328,6 +328,63 @@ This term does not match any term in the dictionary，and will not return any re
The type of `k4.keyword` is `keyword`, and writing data into ES is a complete term, so it can be matched
+### Enable ES node discovery(es\_nodes\_discovery=true)
+
+```
+CREATE EXTERNAL TABLE `test` (
+ `k1` bigint(20) COMMENT "",
+ `k2` datetime COMMENT "",
+ `k3` varchar(20) COMMENT "",
+ `k4` varchar(100) COMMENT "",
+ `k5` float COMMENT ""
+) ENGINE=ELASTICSEARCH
+PROPERTIES (
+"hosts" = "http://192.168.0.1:8200,http://192.168.0.2:8200",
+"index" = "test”,
+"type" = "doc",
+"user" = "root",
+"password" = "root",
+
+"es_nodes_discovery" = "true"
+);
+```
+
+Parameter Description：
+
+Parameter | Description
+---|---
+**es\_nodes\_discovery** | Whether or not to enable ES node discovery. the default is true
+
+When enabled, Doris will find all available nodes from ES. If you only want Doris to access some nodes, you can turn this configuration off
+
+### Use SSL authentication(es\_net\_ssl=true)
+
+```
+CREATE EXTERNAL TABLE `test` (
+ `k1` bigint(20) COMMENT "",
+ `k2` datetime COMMENT "",
+ `k3` varchar(20) COMMENT "",
+ `k4` varchar(100) COMMENT "",
+ `k5` float COMMENT ""
+) ENGINE=ELASTICSEARCH
+PROPERTIES (
+"hosts" = "http://192.168.0.1:8200,http://192.168.0.2:8200",
+"index" = "test”,
+"type" = "doc",
+"user" = "root",
+"password" = "root",
+
+"es_net_ssl" = "true"
Review comment:
```suggestion
"http_ssl_enabled" = "true"
```
If we provide CA in future, we can use :
```
"http_ssl_key" = xxx
"http_ssl_certificate" = path/to/cert
```
##########
File path: docs/en/extending-doris/doris-on-es.md
##########
@@ -328,6 +328,63 @@ This term does not match any term in the dictionary，and will not return any re
The type of `k4.keyword` is `keyword`, and writing data into ES is a complete term, so it can be matched
+### Enable ES node discovery(es\_nodes\_discovery=true)
+
+```
+CREATE EXTERNAL TABLE `test` (
+ `k1` bigint(20) COMMENT "",
+ `k2` datetime COMMENT "",
+ `k3` varchar(20) COMMENT "",
+ `k4` varchar(100) COMMENT "",
+ `k5` float COMMENT ""
+) ENGINE=ELASTICSEARCH
+PROPERTIES (
+"hosts" = "http://192.168.0.1:8200,http://192.168.0.2:8200",
+"index" = "test”,
+"type" = "doc",
+"user" = "root",
+"password" = "root",
+
+"es_nodes_discovery" = "true"
Review comment:
```suggestion
"nodes_discovery" = "true"
```
`nodes_discovery ` is already ok
##########
File path: fe/fe-core/src/main/java/org/apache/doris/external/elasticsearch/EsNodeInfo.java
##########
@@ -66,7 +71,7 @@ public EsNodeInfo(String id, Map<String, Object> map) throws DorisEsException {
String address = (String) httpMap.get("publish_address");
if (address != null) {
String[] scratch = address.split(":");
- this.publishAddress = new TNetworkAddress(scratch[0], Integer.valueOf(scratch[1]));
+ this.publishAddress = new TNetworkAddress((useSslClient ? "https://" : "") + scratch[0], Integer.parseInt(scratch[1]));
Review comment:
Actually I don’t want to replace here, can you consider a unified replacement in the `subphrase`?
##########
File path: fe/fe-core/src/main/java/org/apache/doris/external/elasticsearch/EsRestClient.java
##########
@@ -48,23 +57,33 @@
mapper.configure(SerializationConfig.Feature.USE_ANNOTATIONS, false);
}
- private static OkHttpClient networkClient = new OkHttpClient.Builder()
- .readTimeout(10, TimeUnit.SECONDS)
- .build();
+ private static OkHttpClient networkClient;
+
+ private static OkHttpClient sslNetworkClient;
private Request.Builder builder;
private String[] nodes;
private String currentNode;
private int currentNodeIndex = 0;
+ private boolean useSslClient;
- public EsRestClient(String[] nodes, String authUser, String authPassword) {
+ public EsRestClient(String[] nodes, String authUser, String authPassword, boolean useSslClient) {
this.nodes = nodes;
this.builder = new Request.Builder();
if (!Strings.isEmpty(authUser) && !Strings.isEmpty(authPassword)) {
this.builder.addHeader(HttpHeaders.AUTHORIZATION,
Credentials.basic(authUser, authPassword));
}
this.currentNode = nodes[currentNodeIndex];
+ this.useSslClient = useSslClient;
+ sslNetworkClient = new OkHttpClient.Builder()
Review comment:
Maybe you can lazy init this ssl client.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] stalary commented on a change in pull request #5325: [Doris On ES][WIP] Support external ES table with `SSL` secured and configurable node sniffing
Posted by GitBox <gi...@apache.org>.
stalary commented on a change in pull request #5325:
URL: https://github.com/apache/incubator-doris/pull/5325#discussion_r581761863
##########
File path: be/src/http/http_client.h
##########
@@ -66,6 +66,11 @@ class HttpClient {
curl_easy_setopt(_curl, CURLOPT_COPYPOSTFIELDS, post_body.c_str());
}
+ void use_ssl() {
+ curl_easy_setopt(_curl, CURLOPT_SSL_VERIFYPEER, false);
Review comment:
ok, I was careless
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] stalary commented on a change in pull request #5325: [Doris On ES][WIP] Support external ES table with `SSL` secured and configurable node sniffing
Posted by GitBox <gi...@apache.org>.
stalary commented on a change in pull request #5325:
URL: https://github.com/apache/incubator-doris/pull/5325#discussion_r581750419
##########
File path: be/src/exec/es/es_scan_reader.h
##########
@@ -63,6 +64,8 @@ class ESScanReader {
std::string _query;
// Elasticsearch shards to fetch document
std::string _shards;
+ // whether use ssl client
+ bool _use_ssl_client;
Review comment:
I thought C ++ would have a default, so let me change that
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] stalary commented on a change in pull request #5325: [Doris On ES][WIP] Support external ES table with `SSL` secured and configurable node sniffing
Posted by GitBox <gi...@apache.org>.
stalary commented on a change in pull request #5325:
URL: https://github.com/apache/incubator-doris/pull/5325#discussion_r586548476
##########
File path: fe/fe-core/src/main/java/org/apache/doris/external/elasticsearch/EsUtil.java
##########
@@ -82,4 +85,13 @@ public static JSONObject getJsonObject(JSONObject jsonObject, String key, int fr
return null;
}
}
+
+ public static boolean getBooleanProperty(Map<String, String> properties, String name) throws DdlException {
Review comment:
I didn't quite understand it
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] wuyunfeng commented on a change in pull request #5325: WIP：Support doris connect https es server and support config esNodesDiscovery
Posted by GitBox <gi...@apache.org>.
wuyunfeng commented on a change in pull request #5325:
URL: https://github.com/apache/incubator-doris/pull/5325#discussion_r580863477
##########
File path: be/src/exec/es/es_scan_reader.cpp
##########
@@ -103,6 +106,9 @@ Status ESScanReader::open() {
}
_network_client.set_basic_auth(_user_name, _passwd);
_network_client.set_content_type("application/json");
+ if (_use_ssl_client == "true") {
+ _network_client.set_ssl();
Review comment:
```suggestion
_network_client.use_ssl()
```
##########
File path: fe/fe-core/src/main/java/org/apache/doris/external/elasticsearch/EsNodeInfo.java
##########
@@ -96,6 +101,31 @@ public EsNodeInfo(String id, Map<String, Object> map) throws DorisEsException {
}
}
+ public EsNodeInfo(String id, String seed, boolean useSslClient) throws DorisEsException {
+ this.id = id;
+ if (seed.startsWith("http://")) {
+ seed = seed.substring(7);
+ }
+ if (seed.startsWith("https://")) {
+ seed = seed.substring(8);
+ }
+ String[] scratch = seed.split(":");
+ int port = 80;
+ String remoteHost = (useSslClient ? "https://" : "http://") + scratch[0];
+ if (scratch.length == 2) {
+ port = Integer.parseInt(scratch[1]);
+ }
+ LOG.info("--------------- remoteHost={}, port={} -------------", remoteHost, port);
Review comment:
delete this info log
##########
File path: gensrc/thrift/Descriptors.thrift
##########
@@ -227,6 +227,7 @@ struct TOdbcTable {
}
struct TEsTable {
+ 1: required bool useSslClient
Review comment:
use `properties` to transfer this `flag` to be
##########
File path: be/src/exec/es/es_scan_reader.h
##########
@@ -63,6 +64,8 @@ class ESScanReader {
std::string _query;
// Elasticsearch shards to fetch document
std::string _shards;
+ // whether use ssl client
+ std::string _use_ssl_client;
Review comment:
```suggestion
bool _use_ssl_client;
```
convert string to bool?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] stalary commented on a change in pull request #5325: [Doris On ES][WIP] Support external ES table with `SSL` secured and configurable node sniffing
Posted by GitBox <gi...@apache.org>.
stalary commented on a change in pull request #5325:
URL: https://github.com/apache/incubator-doris/pull/5325#discussion_r608655665
##########
File path: fe/fe-core/src/main/java/org/apache/doris/external/elasticsearch/EsRestClient.java
##########
@@ -207,4 +240,33 @@ private String execute(String path) throws DorisEsException {
}
return (T) (key != null ? map.get(key) : map);
}
+
+ /**
+ * support https
+ **/
+ private static class TrustAllCerts implements X509TrustManager {
+ public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException {}
+
+ public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException {}
+
+ public X509Certificate[] getAcceptedIssuers() {return new X509Certificate[0];}
+ }
+
+ private static class TrustAllHostnameVerifier implements HostnameVerifier {
+ public boolean verify(String hostname, SSLSession session) {
+ return true;
+ }
+ }
+
+ private static SSLSocketFactory createSSLSocketFactory() {
+ SSLSocketFactory ssfFactory;
+ try {
+ SSLContext sc = SSLContext.getInstance("TLS");
+ sc.init(null, new TrustManager[]{new TrustAllCerts()}, new SecureRandom());
+ ssfFactory = sc.getSocketFactory();
+ } catch (Exception e) {
+ throw new DorisEsException("createSSLSocketFactory error");
Review comment:
DorisEsException not support input exception
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] wuyunfeng commented on a change in pull request #5325: [Doris On ES][WIP] Support external ES table with `SSL` secured and configurable node sniffing
Posted by GitBox <gi...@apache.org>.
wuyunfeng commented on a change in pull request #5325:
URL: https://github.com/apache/incubator-doris/pull/5325#discussion_r583331970
##########
File path: be/src/http/http_client.h
##########
@@ -66,6 +66,11 @@ class HttpClient {
curl_easy_setopt(_curl, CURLOPT_COPYPOSTFIELDS, post_body.c_str());
}
+ void use_ssl() {
Review comment:
```suggestion
void use_ untrusted_ssl() {
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] wuyunfeng commented on a change in pull request #5325: [Doris On ES][WIP] Support external ES table with `SSL` secured and configurable node sniffing
Posted by GitBox <gi...@apache.org>.
wuyunfeng commented on a change in pull request #5325:
URL: https://github.com/apache/incubator-doris/pull/5325#discussion_r581739176
##########
File path: fe/fe-core/src/main/java/org/apache/doris/external/elasticsearch/PartitionPhase.java
##########
@@ -37,7 +38,15 @@ public PartitionPhase(EsRestClient client) {
@Override
public void execute(SearchContext context) throws DorisEsException {
shardPartitions = client.searchShards(context.sourceIndex());
- nodesInfo = client.getHttpNodes();
+ if (context.esNodesDiscovery()) {
+ nodesInfo = client.getHttpNodes();
+ } else {
+ nodesInfo = new HashMap<>();
+ String[] seeds = context.esTable().getSeeds();
+ for (int i = 0; i < seeds.length; i++) {
+ nodesInfo.put(String.valueOf(i), new EsNodeInfo(String.valueOf(i), seeds[i], context.esTable().isUseSslClient()));
Review comment:
```suggestion
boolean useSSL = context.esTable().isUseSslClient();
nodesInfo.put(String.valueOf(i), new EsNodeInfo(String.valueOf(i), seeds[i], useSSL));
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org