This is an automated email from the ASF dual-hosted git repository. dataroaring pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push: new 3f256804373 [enhance](Cloud) Add one create vault property to control if use this vault as default vault (#34195) 3f256804373 is described below commit 3f256804373ad18261f78d8f03b5672e301657b1 Author: AlexYue <yj976240...@gmail.com> AuthorDate: Mon Apr 29 12:09:53 2024 +0800 [enhance](Cloud) Add one create vault property to control if use this vault as default vault (#34195) --- cloud/src/meta-service/meta_service_resource.cpp | 44 ++++++++++++++++++++++ .../doris/analysis/CreateStorageVaultStmt.java | 7 ++++ .../org/apache/doris/catalog/HdfsStorageVault.java | 4 +- .../org/apache/doris/catalog/S3StorageVault.java | 5 ++- .../org/apache/doris/catalog/StorageVault.java | 13 +++++-- .../org/apache/doris/catalog/StorageVaultMgr.java | 12 +++++- .../doris/cloud/catalog/HdfsStorageVaultTest.java | 4 +- gensrc/proto/cloud.proto | 2 + regression-test/suites/vaults/create/create.groovy | 26 ++++++++++++- .../suites/vaults/default/default.groovy | 30 +++++++++++++++ 10 files changed, 135 insertions(+), 12 deletions(-) diff --git a/cloud/src/meta-service/meta_service_resource.cpp b/cloud/src/meta-service/meta_service_resource.cpp index eefd01eb254..d36b85332b8 100644 --- a/cloud/src/meta-service/meta_service_resource.cpp +++ b/cloud/src/meta-service/meta_service_resource.cpp @@ -526,6 +526,19 @@ static int remove_hdfs_storage_vault(InstanceInfoPB& instance, Transaction* txn, return 0; } +// Log vault message and origin default storage vault message for potential tracing +static void set_default_vault_log_helper(const InstanceInfoPB& instance, + std::string_view vault_name, std::string_view vault_id) { + auto vault_msg = fmt::format("instance {} tries to set default vault as {}, id {}", + instance.instance_id(), vault_id, vault_name); + if (instance.has_default_storage_vault_id()) { + vault_msg = fmt::format("{}, origin default vault name {}, vault id {}", vault_msg, + instance.default_storage_vault_name(), + instance.default_storage_vault_id()); + } + LOG(INFO) << vault_msg; +} + void MetaServiceImpl::alter_obj_store_info(google::protobuf::RpcController* controller, const AlterObjStoreInfoRequest* request, AlterObjStoreInfoResponse* response, @@ -757,7 +770,18 @@ void MetaServiceImpl::alter_obj_store_info(google::protobuf::RpcController* cont last_item.set_sse_enabled(instance.sse_enabled()); if (request->op() == AlterObjStoreInfoRequest::ADD_OBJ_INFO) { instance.add_obj_info()->CopyFrom(last_item); + LOG_INFO("Instance {} tries to put obj info", instance.instance_id()); } else if (request->op() == AlterObjStoreInfoRequest::ADD_S3_VAULT) { + if (instance.storage_vault_names().end() != + std::find_if(instance.storage_vault_names().begin(), + instance.storage_vault_names().end(), + [&](const std::string& candidate_name) { + return candidate_name == request->vault().name(); + })) { + code = MetaServiceCode::ALREADY_EXISTED; + msg = fmt::format("vault_name={} already created", request->vault().name()); + return; + } StorageVaultPB vault; vault.set_id(last_item.id()); vault.set_name(request->vault().name()); @@ -766,6 +790,16 @@ void MetaServiceImpl::alter_obj_store_info(google::protobuf::RpcController* cont vault.mutable_obj_info()->MergeFrom(last_item); auto vault_key = storage_vault_key({instance.instance_id(), last_item.id()}); txn->put(vault_key, vault.SerializeAsString()); + if (request->has_set_as_default_storage_vault() && + request->set_as_default_storage_vault()) { + response->set_default_storage_vault_replaced( + instance.has_default_storage_vault_id()); + set_default_vault_log_helper(instance, vault.name(), vault.id()); + instance.set_default_storage_vault_id(vault.id()); + instance.set_default_storage_vault_name(vault.name()); + } + LOG_INFO("try to put storage vault_id={}, vault_name={}, vault_key={}", vault.id(), + vault.name(), hex(vault_key)); } } break; case AlterObjStoreInfoRequest::ADD_HDFS_INFO: { @@ -774,6 +808,14 @@ void MetaServiceImpl::alter_obj_store_info(google::protobuf::RpcController* cont ret != 0) { return; } + if (request->has_set_as_default_storage_vault() && + request->set_as_default_storage_vault()) { + response->set_default_storage_vault_replaced(instance.has_default_storage_vault_id()); + set_default_vault_log_helper(instance, *instance.storage_vault_names().rbegin(), + *instance.resource_ids().rbegin()); + instance.set_default_storage_vault_id(*instance.resource_ids().rbegin()); + instance.set_default_storage_vault_name(*instance.storage_vault_names().rbegin()); + } break; } case AlterObjStoreInfoRequest::ADD_BUILT_IN_VAULT: { @@ -812,6 +854,8 @@ void MetaServiceImpl::alter_obj_store_info(google::protobuf::RpcController* cont } auto pos = name_itr - instance.storage_vault_names().begin(); auto id_itr = instance.resource_ids().begin() + pos; + response->set_default_storage_vault_replaced(instance.has_default_storage_vault_id()); + set_default_vault_log_helper(instance, name, *id_itr); instance.set_default_storage_vault_id(*id_itr); instance.set_default_storage_vault_name(name); response->set_storage_vault_id(*id_itr); diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateStorageVaultStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateStorageVaultStmt.java index 652c15d4b8e..c6861736f3d 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateStorageVaultStmt.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateStorageVaultStmt.java @@ -37,10 +37,12 @@ import java.util.Map; // PROPERTIES (key1 = value1, ...) public class CreateStorageVaultStmt extends DdlStmt { private static final String TYPE = "type"; + private static final String SET_AS_DEFAULT = "set_as_default"; private final boolean ifNotExists; private final String vaultName; private final Map<String, String> properties; + private boolean setAsDefault; private StorageVault.StorageVaultType vaultType; public CreateStorageVaultStmt(boolean ifNotExists, String vaultName, Map<String, String> properties) { @@ -54,6 +56,10 @@ public class CreateStorageVaultStmt extends DdlStmt { return ifNotExists; } + public boolean setAsDefault() { + return setAsDefault; + } + public String getStorageVaultName() { return vaultName; } @@ -102,6 +108,7 @@ public class CreateStorageVaultStmt extends DdlStmt { if (type == null) { throw new AnalysisException("Storage Vault type can't be null"); } + setAsDefault = Boolean.parseBoolean(properties.getOrDefault(SET_AS_DEFAULT, "false")); setStorageVaultType(StorageVault.StorageVaultType.fromString(type)); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/HdfsStorageVault.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/HdfsStorageVault.java index 6332acd04b7..4614beef828 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/HdfsStorageVault.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/HdfsStorageVault.java @@ -67,8 +67,8 @@ public class HdfsStorageVault extends StorageVault { @SerializedName(value = "properties") private Map<String, String> properties; - public HdfsStorageVault(String name, boolean ifNotExists) { - super(name, StorageVault.StorageVaultType.HDFS, ifNotExists); + public HdfsStorageVault(String name, boolean ifNotExists, boolean setAsDefault) { + super(name, StorageVault.StorageVaultType.HDFS, ifNotExists, setAsDefault); properties = Maps.newHashMap(); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/S3StorageVault.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/S3StorageVault.java index 25f7e60ce3d..3f06286f47d 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/S3StorageVault.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/S3StorageVault.java @@ -56,8 +56,9 @@ public class S3StorageVault extends StorageVault { @SerializedName(value = "properties") private Map<String, String> properties; - public S3StorageVault(String name, boolean ifNotExists, CreateResourceStmt stmt) throws DdlException { - super(name, StorageVault.StorageVaultType.S3, ifNotExists); + public S3StorageVault(String name, boolean ifNotExists, + boolean setAsDefault, CreateResourceStmt stmt) throws DdlException { + super(name, StorageVault.StorageVaultType.S3, ifNotExists, setAsDefault); resource = Resource.fromStmt(stmt); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVault.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVault.java index 8bfa4648a8e..fb68581a00e 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVault.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVault.java @@ -63,6 +63,7 @@ public abstract class StorageVault { protected StorageVaultType type; protected String id; private boolean ifNotExists; + private boolean setAsDefault; private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(true); @@ -85,10 +86,11 @@ public abstract class StorageVault { public StorageVault() { } - public StorageVault(String name, StorageVaultType type, boolean ifNotExists) { + public StorageVault(String name, StorageVaultType type, boolean ifNotExists, boolean setAsDefault) { this.name = name; this.type = type; this.ifNotExists = ifNotExists; + this.setAsDefault = setAsDefault; } public static StorageVault fromStmt(CreateStorageVaultStmt stmt) throws DdlException, UserException { @@ -99,6 +101,10 @@ public abstract class StorageVault { return this.ifNotExists; } + public boolean setAsDefault() { + return this.setAsDefault; + } + public String getId() { return this.id; @@ -120,17 +126,18 @@ public abstract class StorageVault { StorageVaultType type = stmt.getStorageVaultType(); String name = stmt.getStorageVaultName(); boolean ifNotExists = stmt.isIfNotExists(); + boolean setAsDefault = stmt.setAsDefault(); StorageVault vault; switch (type) { case HDFS: - vault = new HdfsStorageVault(name, ifNotExists); + vault = new HdfsStorageVault(name, ifNotExists, setAsDefault); vault.modifyProperties(stmt.getProperties()); break; case S3: CreateResourceStmt resourceStmt = new CreateResourceStmt(false, ifNotExists, name, stmt.getProperties()); resourceStmt.analyzeResourceType(); - vault = new S3StorageVault(name, ifNotExists, resourceStmt); + vault = new S3StorageVault(name, ifNotExists, setAsDefault, resourceStmt); break; default: throw new DdlException("Unknown StorageVault type: " + type); diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVaultMgr.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVaultMgr.java index 3cc92d22485..c9f254e8ed9 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVaultMgr.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVaultMgr.java @@ -152,17 +152,23 @@ public class StorageVaultMgr { = Cloud.AlterObjStoreInfoRequest.newBuilder(); requestBuilder.setOp(Cloud.AlterObjStoreInfoRequest.Operation.ADD_HDFS_INFO); requestBuilder.setVault(alterHdfsInfoBuilder.build()); + requestBuilder.setSetAsDefaultStorageVault(vault.setAsDefault()); try { Cloud.AlterObjStoreInfoResponse response = MetaServiceProxy.getInstance().alterObjStoreInfo(requestBuilder.build()); if (response.getStatus().getCode() == Cloud.MetaServiceCode.ALREADY_EXISTED && hdfsStorageVault.ifNotExists()) { + LOG.info("Hdfs vault {} already existed", hdfsStorageVault.getName()); return; } if (response.getStatus().getCode() != Cloud.MetaServiceCode.OK) { - LOG.warn("failed to alter storage vault response: {} ", response); + LOG.warn("failed to create hdfs storage vault, vault name {}, response: {} ", + hdfsStorageVault.getName(), response); throw new DdlException(response.getStatus().getMsg()); } + LOG.info("Succeed to create hdfs vault {}, id {}, origin default vault replaced {}", + hdfsStorageVault.getName(), response.getStorageVaultId(), + response.getDefaultStorageVaultReplaced()); } catch (RpcException e) { LOG.warn("failed to alter storage vault due to RpcException: {}", e); throw new DdlException(e.getMessage()); @@ -190,17 +196,21 @@ public class StorageVaultMgr { alterObjVaultBuilder.setName(s3StorageVault.getName()); alterObjVaultBuilder.setObjInfo(objBuilder.build()); requestBuilder.setVault(alterObjVaultBuilder.build()); + requestBuilder.setSetAsDefaultStorageVault(vault.setAsDefault()); try { Cloud.AlterObjStoreInfoResponse response = MetaServiceProxy.getInstance().alterObjStoreInfo(requestBuilder.build()); if (response.getStatus().getCode() == Cloud.MetaServiceCode.ALREADY_EXISTED && s3StorageVault.ifNotExists()) { + LOG.info("S3 vault {} already existed", s3StorageVault.getName()); return; } if (response.getStatus().getCode() != Cloud.MetaServiceCode.OK) { LOG.warn("failed to alter storage vault response: {} ", response); throw new DdlException(response.getStatus().getMsg()); } + LOG.info("Succeed to create s3 vault {}, id {}, origin default vault replaced {}", + s3StorageVault.getName(), response.getStorageVaultId(), response.getDefaultStorageVaultReplaced()); } catch (RpcException e) { LOG.warn("failed to alter storage vault due to RpcException: {}", e); throw new DdlException(e.getMessage()); diff --git a/fe/fe-core/src/test/java/org/apache/doris/cloud/catalog/HdfsStorageVaultTest.java b/fe/fe-core/src/test/java/org/apache/doris/cloud/catalog/HdfsStorageVaultTest.java index a78f7ad7e73..5f92b9665b5 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/cloud/catalog/HdfsStorageVaultTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/cloud/catalog/HdfsStorageVaultTest.java @@ -158,7 +158,7 @@ public class HdfsStorageVaultTest { return resp.build(); } }; - StorageVault vault = new HdfsStorageVault("name", true); + StorageVault vault = new HdfsStorageVault("name", true, false); vault.modifyProperties(ImmutableMap.of( "type", "hdfs", "path", "abs/")); @@ -201,7 +201,7 @@ public class HdfsStorageVaultTest { return resp.build(); } }; - StorageVault vault = new HdfsStorageVault("name", true); + StorageVault vault = new HdfsStorageVault("name", true, false); Assertions.assertThrows(DdlException.class, () -> { mgr.setDefaultStorageVault(new SetDefaultStorageVaultStmt("non_existent")); diff --git a/gensrc/proto/cloud.proto b/gensrc/proto/cloud.proto index ba9017a5ecc..70ad5f79ff3 100644 --- a/gensrc/proto/cloud.proto +++ b/gensrc/proto/cloud.proto @@ -784,11 +784,13 @@ message AlterObjStoreInfoRequest { optional ObjectStoreInfoPB obj = 2; optional Operation op = 3; optional StorageVaultPB vault = 4; + optional bool set_as_default_storage_vault = 5; } message AlterObjStoreInfoResponse { optional MetaServiceResponseStatus status = 1; optional string storage_vault_id = 2; + optional bool default_storage_vault_replaced = 3; } message UpdateAkSkRequest { diff --git a/regression-test/suites/vaults/create/create.groovy b/regression-test/suites/vaults/create/create.groovy index 9070d82083c..56ce5dc3557 100644 --- a/regression-test/suites/vaults/create/create.groovy +++ b/regression-test/suites/vaults/create/create.groovy @@ -78,12 +78,12 @@ suite("create_vault") { ) """ - def create_stmt = """ + String create_stmt = sql """ show create table create_table_use_vault """ logger.info("the create table stmt is ${create_stmt}") - assertTrue(create_stmt.contains("\"storage_vault_name\" = \"create_hdfs_vault\"")) + assertTrue(create_stmt.contains("create_hdfs_vault")) expectExceptionLike({ sql """ @@ -129,6 +129,28 @@ suite("create_vault") { """ }, "already created") + sql """ + CREATE TABLE IF NOT EXISTS create_table_use_s3_vault ( + C_CUSTKEY INTEGER NOT NULL, + C_NAME INTEGER NOT NULL + ) + DUPLICATE KEY(C_CUSTKEY, C_NAME) + DISTRIBUTED BY HASH(C_CUSTKEY) BUCKETS 1 + PROPERTIES ( + "replication_num" = "1", + "storage_vault_name" = "create_s3_vault" + ) + """ + + sql """ + insert into create_table_use_s3_vault values(1,1); + """ + + sql """ + select * from create_table_use_s3_vault; + """ + + def vaults_info = try_sql """ show storage vault """ diff --git a/regression-test/suites/vaults/default/default.groovy b/regression-test/suites/vaults/default/default.groovy index 46f55a865ed..a69880b6030 100644 --- a/regression-test/suites/vaults/default/default.groovy +++ b/regression-test/suites/vaults/default/default.groovy @@ -39,6 +39,36 @@ suite("default_vault") { """ }, "supply") + sql """ + CREATE STORAGE VAULT IF NOT EXISTS create_s3_vault_for_default + PROPERTIES ( + "type"="S3", + "s3.endpoint"="${getS3Endpoint()}", + "s3.region" = "${getS3Region()}", + "s3.access_key" = "${getS3AK()}", + "s3.secret_key" = "${getS3SK()}", + "s3.root.path" = "ssb_sf1_p2_s3", + "s3.bucket" = "${getS3BucketName()}", + "s3.external_endpoint" = "", + "provider" = "${getS3Provider()}", + "set_as_default" = "true" + ); + """ + + def vaults_info = sql """ + show storage vault + """ + + // check if create_s3_vault_for_default is set as default + for (int i = 0; i < vaults_info.size(); i++) { + def name = vaults_info[i][0] + if (name.equals("create_s3_vault_for_default")) { + // isDefault is true + assertEquals(vaults_info[i][3], "true") + } + } + + sql """ set built_in_storage_vault as default storage vault """ --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org