wuyunfeng commented on a change in pull request #3454: URL: https://github.com/apache/incubator-doris/pull/3454#discussion_r426999378
########## File path: fe/src/main/java/org/apache/doris/external/EsStateStore.java ########## @@ -74,37 +59,48 @@ public void registerTable(EsTable esTable) { return; } esTables.put(esTable.getId(), esTable); - LOG.info("register a new table [{}] to sync list", esTable.toString()); + LOG.info("register a new table [{}] to sync list", esTable); } public void deRegisterTable(long tableId) { esTables.remove(tableId); LOG.info("deregister table [{}] from sync list", tableId); } - + @Override protected void runAfterCatalogReady() { for (EsTable esTable : esTables.values()) { try { - EsRestClient client = new EsRestClient(esTable.getSeeds(), - esTable.getUserName(), esTable.getPasswd()); - // if user not specify the es version, try to get the remote cluster versoin - // in the future, we maybe need this version - String indexMetaData = client.getIndexMetaData(esTable.getIndexName()); - if (indexMetaData == null) { + EsRestClient client = new EsRestClient(esTable.getSeeds(), esTable.getUserName(), Review comment: Whether can we introduce a cached `map` for client to avoid `new Client()` happens every loop? ########## File path: fe/src/main/java/org/apache/doris/external/EsRestClient.java ########## @@ -49,48 +54,103 @@ .readTimeout(10, TimeUnit.SECONDS) .build(); - private String basicAuth; - - private int nextClient = 0; + private Request.Builder builder; private String[] nodes; private String currentNode; + private int currentNodeIndex = 0; public EsRestClient(String[] nodes, String authUser, String authPassword) { this.nodes = nodes; + this.builder = new Request.Builder(); if (!Strings.isEmpty(authUser) && !Strings.isEmpty(authPassword)) { - basicAuth = Credentials.basic(authUser, authPassword); + this.builder.addHeader(HttpHeaders.AUTHORIZATION, Credentials.basic(authUser, authPassword)); } - selectNextNode(); + this.currentNode = nodes[currentNodeIndex]; } - private boolean selectNextNode() { - if (nextClient >= nodes.length) { - return false; + private void selectNextNode() { + currentNodeIndex++; + // reroute, because the previously failed node may have already been restored + if (currentNodeIndex >= nodes.length) { + currentNodeIndex = 0; } - currentNode = nodes[nextClient++]; - return true; + currentNode = nodes[currentNodeIndex]; } public Map<String, EsNodeInfo> getHttpNodes() throws Exception { Map<String, Map<String, Object>> nodesData = get("_nodes/http", "nodes"); if (nodesData == null) { return Collections.emptyMap(); } - Map<String, EsNodeInfo> nodes = new HashMap<>(); + Map<String, EsNodeInfo> nodesMap = new HashMap<>(); for (Map.Entry<String, Map<String, Object>> entry : nodesData.entrySet()) { EsNodeInfo node = new EsNodeInfo(entry.getKey(), entry.getValue()); if (node.hasHttp()) { - nodes.put(node.getId(), node); + nodesMap.put(node.getId(), node); } } - return nodes; + return nodesMap; + } + + public JSONObject getIndexProperties(String indexName, String mappingType) { Review comment: why this method modified by 'public' directly return JSONObject ########## File path: fe/src/main/java/org/apache/doris/external/EsStateStore.java ########## @@ -192,77 +141,75 @@ public EsTableState parseClusterState55(String responseStr, EsTable esTable) // } // then the docvalue context provided the mapping between the select field and real request field : // {"city": "city.raw"} - JSONObject indicesMetaMap = jsonObject.getJSONObject("metadata").getJSONObject("indices"); - JSONObject indexMetaMap = indicesMetaMap.optJSONObject(esTable.getIndexName()); - if (indexMetaMap != null && (esTable.isKeywordSniffEnable() || esTable.isDocValueScanEnable())) { - JSONObject mappings = indexMetaMap.optJSONObject("mappings"); - JSONObject rootSchema = mappings.optJSONObject(esTable.getMappingType()); - JSONObject schema = rootSchema.optJSONObject("properties"); - List<Column> colList = esTable.getFullSchema(); - for (Column col : colList) { - String colName = col.getName(); - if (!schema.has(colName)) { - continue; - } - JSONObject fieldObject = schema.optJSONObject(colName); - String fieldType = fieldObject.optString("type"); - // string-type field used keyword type to generate predicate - if (esTable.isKeywordSniffEnable()) { - // if text field type seen, we should use the `field` keyword type? - if ("text".equals(fieldType)) { - JSONObject fieldsObject = fieldObject.optJSONObject("fields"); - if (fieldsObject != null) { - for (String key : fieldsObject.keySet()) { - JSONObject innerTypeObject = fieldsObject.optJSONObject(key); - // just for text type - if ("keyword".equals(innerTypeObject.optString("type"))) { - esTable.addFetchField(colName, colName + "." + key); - } + List<Column> colList = esTable.getFullSchema(); + for (Column col : colList) { + String colName = col.getName(); + if (!properties.has(colName)) { + continue; + } + JSONObject fieldObject = properties.optJSONObject(colName); Review comment: Can we abstract this `processing json` to other place such as RestClient? In this way, ESStateStore would be more clearly. ---------------------------------------------------------------- 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: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org