gavinchou commented on code in PR #31537: URL: https://github.com/apache/doris/pull/31537#discussion_r1512130323
########## fe/fe-core/src/main/java/org/apache/doris/catalog/HdfsStorageVault.java: ########## @@ -0,0 +1,148 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.catalog; + +import org.apache.doris.cloud.proto.Cloud; +import org.apache.doris.cloud.rpc.MetaServiceProxy; +import org.apache.doris.common.DdlException; +import org.apache.doris.common.proc.BaseProcResult; +import org.apache.doris.common.security.authentication.AuthenticationConfig; +import org.apache.doris.rpc.RpcException; + +import com.google.common.collect.Maps; +import com.google.gson.annotations.SerializedName; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +import java.util.Map; + +/** + * HDFS resource + * <p> + * Syntax: + * CREATE STORAGE VAULT "remote_hdfs" + * PROPERTIES + * ( + * "type" = "hdfs", + * "fs.defaultFS" = "hdfs://10.220.147.151:8020", + * "hadoop.username" = "root" + * ); + */ +public class HdfsStorageVault extends StorageVault { + private static final Logger LOG = LogManager.getLogger(HdfsStorageVault.class); + public static final String HADOOP_FS_PREFIX = "dfs."; + public static String HADOOP_FS_NAME = "fs.defaultFS"; + 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"; + public static final String HDFS_PREFIX = "hdfs:"; + public static final String HDFS_FILE_PREFIX = "hdfs://"; + + @SerializedName(value = "properties") + private Map<String, String> properties; + + public HdfsStorageVault(String name, boolean ifNotExists) { + super(name, StorageVault.StorageVaultType.HDFS, ifNotExists); + properties = Maps.newHashMap(); + } + + @Override + public void alterMetaService() throws DdlException { + Cloud.HdfsParams hdfsParams = generateHdfsParam(properties); + Cloud.AlterHdfsParams.Builder alterHdfsParamsBuilder = Cloud.AlterHdfsParams.newBuilder(); + alterHdfsParamsBuilder.setVaultName(name); + alterHdfsParamsBuilder.setHdfs(hdfsParams); + Cloud.AlterObjStoreInfoRequest.Builder requestBuilder + = Cloud.AlterObjStoreInfoRequest.newBuilder(); + requestBuilder.setOp(Cloud.AlterObjStoreInfoRequest.Operation.ADD_HDFS_INFO); + requestBuilder.setHdfs(alterHdfsParamsBuilder.build()); + try { + Cloud.AlterObjStoreInfoResponse response = + MetaServiceProxy.getInstance().alterObjStoreInfo(requestBuilder.build()); + if (response.getStatus().getCode() == Cloud.MetaServiceCode.ALREADY_EXISTED + && ifNotExists()) { + return; + } + if (response.getStatus().getCode() != Cloud.MetaServiceCode.OK) { + LOG.warn("failed to alter storage vault response: {} ", response); + throw new DdlException(response.getStatus().getMsg()); + } + } catch (RpcException e) { + LOG.warn("failed to alter storage vault due to RpcException: {}", e); + throw new DdlException(e.getMessage()); + } + } + + @Override + public void modifyProperties(Map<String, String> properties) throws DdlException { + for (Map.Entry<String, String> kv : properties.entrySet()) { + replaceIfEffectiveValue(this.properties, kv.getKey(), kv.getValue()); + } + super.modifyProperties(properties); + } + + @Override + protected void setProperties(Map<String, String> properties) throws DdlException { + // `dfs.client.read.shortcircuit` and `dfs.domain.socket.path` should be both set to enable short circuit read. + // We should disable short circuit read if they are not both set because it will cause performance down. + if (!(enableShortCircuitRead(properties))) { + properties.put(HADOOP_SHORT_CIRCUIT, "false"); + } + this.properties = properties; + } + + @Override + public Map<String, String> getCopiedProperties() { + return Maps.newHashMap(properties); + } + + @Override + protected void getProcNodeData(BaseProcResult result) { + // String lowerCaseType = type.name().toLowerCase(); Review Comment: remove if not needed ########## gensrc/proto/cloud.proto: ########## @@ -187,6 +190,25 @@ message ObjectStoreInfoPB { optional bool sse_enabled = 15; } +message StorageVaultPB { + optional string id = 1; + optional string name = 2; + optional HdfsParams hdfs_infos = 3; // HdfsResource +} + +message HdfsParams { Review Comment: naming HdfsParams -> HdfsInfo ########## fe/fe-core/src/main/java/org/apache/doris/catalog/HdfsStorageVault.java: ########## @@ -0,0 +1,148 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.catalog; + +import org.apache.doris.cloud.proto.Cloud; +import org.apache.doris.cloud.rpc.MetaServiceProxy; +import org.apache.doris.common.DdlException; +import org.apache.doris.common.proc.BaseProcResult; +import org.apache.doris.common.security.authentication.AuthenticationConfig; +import org.apache.doris.rpc.RpcException; + +import com.google.common.collect.Maps; +import com.google.gson.annotations.SerializedName; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +import java.util.Map; + +/** + * HDFS resource + * <p> + * Syntax: + * CREATE STORAGE VAULT "remote_hdfs" + * PROPERTIES + * ( + * "type" = "hdfs", + * "fs.defaultFS" = "hdfs://10.220.147.151:8020", + * "hadoop.username" = "root" + * ); + */ +public class HdfsStorageVault extends StorageVault { + private static final Logger LOG = LogManager.getLogger(HdfsStorageVault.class); + public static final String HADOOP_FS_PREFIX = "dfs."; + public static String HADOOP_FS_NAME = "fs.defaultFS"; + 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"; + public static final String HDFS_PREFIX = "hdfs:"; + public static final String HDFS_FILE_PREFIX = "hdfs://"; + + @SerializedName(value = "properties") + private Map<String, String> properties; + + public HdfsStorageVault(String name, boolean ifNotExists) { + super(name, StorageVault.StorageVaultType.HDFS, ifNotExists); + properties = Maps.newHashMap(); + } + + @Override + public void alterMetaService() throws DdlException { Review Comment: move it to storage vault manager ########## fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVault.java: ########## @@ -0,0 +1,226 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.catalog; + +import org.apache.doris.analysis.CreateStorageVaultStmt; +import org.apache.doris.common.AnalysisException; +import org.apache.doris.common.DdlException; +import org.apache.doris.common.FeConstants; +import org.apache.doris.common.io.DeepCopy; +import org.apache.doris.common.io.Text; +import org.apache.doris.common.io.Writable; +import org.apache.doris.common.proc.BaseProcResult; +import org.apache.doris.persist.gson.GsonPostProcessable; +import org.apache.doris.persist.gson.GsonUtils; + +import com.google.common.base.Strings; +import com.google.gson.annotations.SerializedName; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +import java.io.DataInput; +import java.io.DataOutput; +import java.io.IOException; +import java.util.Map; +import java.util.concurrent.locks.ReentrantReadWriteLock; + +public abstract class StorageVault implements Writable, GsonPostProcessable { + private static final Logger LOG = LogManager.getLogger(StorageVault.class); + public static final String REFERENCE_SPLIT = "@"; + public static final String INCLUDE_DATABASE_LIST = "include_database_list"; + public static final String EXCLUDE_DATABASE_LIST = "exclude_database_list"; + public static final String LOWER_CASE_META_NAMES = "lower_case_meta_names"; + public static final String META_NAMES_MAPPING = "meta_names_mapping"; + + public enum StorageVaultType { + UNKNOWN, + S3, + HDFS; + + public static StorageVaultType fromString(String storageVaultTypeType) { + for (StorageVaultType type : StorageVaultType.values()) { + if (type.name().equalsIgnoreCase(storageVaultTypeType)) { + return type; + } + } + return UNKNOWN; + } + } + + @SerializedName(value = "name") Review Comment: do we have to persist this class? ########## gensrc/proto/cloud.proto: ########## @@ -691,20 +713,29 @@ message GetObjStoreInfoRequest { optional string cloud_unique_id = 1; // For auth }; +message AlterHdfsParams { + optional string vault_name = 1; + optional HdfsParams hdfs = 2; + } Review Comment: bad indention -- 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. To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org 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
