Skip to content

Commit

Permalink
fix review
Browse files Browse the repository at this point in the history
  • Loading branch information
@deardeng
deardeng committed Jan 2, 2025
1 parent aeaa111 commit 9a7f69a
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 26 deletions.
9 changes: 6 additions & 3 deletions cloud/src/meta-service/meta_service_resource.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2205,8 +2205,10 @@ void MetaServiceImpl::alter_cluster(google::protobuf::RpcController* controller,
case AlterClusterRequest::RENAME_CLUSTER: {
// SQL mode, cluster cluster name eq empty cluster name, need drop empty cluster first.
// but in http api, cloud control will drop empty cluster
bool drop_empty_cluster =
request->has_drop_empty_cluster() ? request->drop_empty_cluster() : false;
bool replace_if_existing_empty_target_cluster =
request->has_replace_if_existing_empty_target_cluster()
? request->replace_if_existing_empty_target_cluster()
: false;

msg = resource_mgr_->update_cluster(
instance_id, cluster,
Expand Down Expand Up @@ -2237,7 +2239,8 @@ void MetaServiceImpl::alter_cluster(google::protobuf::RpcController* controller,
}
c.set_cluster_name(cluster.cluster.cluster_name());
return msg;
}, drop_empty_cluster);
},
replace_if_existing_empty_target_cluster);
} break;
case AlterClusterRequest::UPDATE_CLUSTER_ENDPOINT: {
msg = resource_mgr_->update_cluster(
Expand Down
10 changes: 6 additions & 4 deletions cloud/src/resource-manager/resource_manager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ std::string ResourceManager::update_cluster(
const std::string& instance_id, const ClusterInfo& cluster,
std::function<bool(const ClusterPB&)> filter,
std::function<std::string(ClusterPB&, std::set<std::string>& cluster_names)> action,
bool drop_empty_cluster) {
bool replace_if_existing_empty_target_cluster) {
std::stringstream ss;
std::string msg;

Expand Down Expand Up @@ -522,8 +522,8 @@ std::string ResourceManager::update_cluster(

auto& clusters = const_cast<std::decay_t<decltype(instance.clusters())>&>(instance.clusters());

// check cluster_name is empty cluster, if empty and drop_empty_cluster == true, drop it
if (drop_empty_cluster) {
// check cluster_name is empty cluster, if empty and replace_if_existing_empty_target_cluster == true, drop it
if (replace_if_existing_empty_target_cluster) {
auto it = cluster_names.find(cluster_name);
if (it != cluster_names.end()) {
// found it, if it's an empty cluster, drop it from instance
Expand All @@ -539,7 +539,9 @@ std::string ResourceManager::update_cluster(
clusters.DeleteSubrange(idx, 1);
// Remove cluster name from set
cluster_names.erase(cluster_name);
LOG(INFO) << "Removed empty cluster, cluster_name=" << cluster_name;
LOG(INFO) << "remove empty cluster due to it is the target of a "
"rename_cluster, cluster_name="
<< cluster_name;
}
break;
}
Expand Down
4 changes: 2 additions & 2 deletions cloud/src/resource-manager/resource_manager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,15 +88,15 @@ class ResourceManager {
*
* @param cluster cluster to update, only cluster name and cluster id are concered
* @param action update operation code snippet
* @param drop_empty_cluster, find cluster.cluster_name is a empty cluster(no node), drop it
* @param replace_if_existing_empty_target_cluster, find cluster.cluster_name is a empty cluster(no node), drop it
* @filter filter condition
* @return empty string for success, otherwise failure reason returned
*/
virtual std::string update_cluster(
const std::string& instance_id, const ClusterInfo& cluster,
std::function<bool(const ClusterPB&)> filter,
std::function<std::string(ClusterPB&, std::set<std::string>& cluster_names)> action,
bool drop_empty_cluster = false);
bool replace_if_existing_empty_target_cluster = false);

/**
* Get instance from underlying storage with given transaction.
Expand Down
39 changes: 23 additions & 16 deletions fe/fe-core/src/main/java/org/apache/doris/cloud/system/CloudSystemInfoService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -327,9 +327,9 @@ private void alterBackendCluster(List<HostInfo> hostInfos, String computeGroupId
throw new DdlException("unable to alter backends due to empty cloud_instance_id");
}
// Issue rpc to meta to alter node, then fe master would add this node to its frontends
Cloud.ClusterPB clusterPB = Cloud.ClusterPB.newBuilder()
ClusterPB clusterPB = ClusterPB.newBuilder()
.setClusterId(computeGroupId)
.setType(Cloud.ClusterPB.Type.COMPUTE)
.setType(ClusterPB.Type.COMPUTE)
.build();

for (HostInfo hostInfo : hostInfos) {
Expand Down Expand Up @@ -844,10 +844,10 @@ private void alterFrontendCluster(FrontendNodeType role, String host, int editLo
.setCtime(System.currentTimeMillis() / 1000)
.build();

Cloud.ClusterPB clusterPB = Cloud.ClusterPB.newBuilder()
ClusterPB clusterPB = ClusterPB.newBuilder()
.setClusterId(Config.cloud_sql_server_cluster_id)
.setClusterName(Config.cloud_sql_server_cluster_name)
.setType(Cloud.ClusterPB.Type.SQL)
.setType(ClusterPB.Type.SQL)
.addNodes(nodeInfoPB)
.build();

Expand Down Expand Up @@ -888,10 +888,10 @@ private String tryCreateComputeGroup(String clusterName, String computeGroupId)
throw new DdlException("unable to create compute group due to empty cloud_instance_id");
}

Cloud.ClusterPB clusterPB = Cloud.ClusterPB.newBuilder()
ClusterPB clusterPB = ClusterPB.newBuilder()
.setClusterId(computeGroupId)
.setClusterName(clusterName)
.setType(Cloud.ClusterPB.Type.COMPUTE)
.setType(ClusterPB.Type.COMPUTE)
.build();

Cloud.AlterClusterRequest request = Cloud.AlterClusterRequest.newBuilder()
Expand All @@ -917,7 +917,7 @@ private String tryCreateComputeGroup(String clusterName, String computeGroupId)
Cloud.GetClusterResponse clusterResponse = getCloudCluster(clusterName, "", "");
if (clusterResponse.getStatus().getCode() == Cloud.MetaServiceCode.OK) {
if (clusterResponse.getClusterCount() > 0) {
Cloud.ClusterPB cluster = clusterResponse.getCluster(0);
ClusterPB cluster = clusterResponse.getCluster(0);
return cluster.getClusterId();
} else {
throw new UserException("Cluster information not found in the response");
Expand Down Expand Up @@ -1054,7 +1054,7 @@ public String waitForAutoStart(String clusterName) throws DdlException {
builder.setCloudUniqueId(Config.cloud_unique_id);
builder.setOp(Cloud.AlterClusterRequest.Operation.SET_CLUSTER_STATUS);

Cloud.ClusterPB.Builder clusterBuilder = Cloud.ClusterPB.newBuilder();
ClusterPB.Builder clusterBuilder = ClusterPB.newBuilder();
clusterBuilder.setClusterId(getCloudClusterIdByName(clusterName));
clusterBuilder.setClusterStatus(Cloud.ClusterStatus.TO_RESUME);
builder.setCluster(clusterBuilder);
Expand Down Expand Up @@ -1167,35 +1167,42 @@ public void renameComputeGroup(String originalName, String newGroupName) throws
String originalComputeGroupId = ((CloudSystemInfoService) Env.getCurrentSystemInfo())
.getCloudClusterIdByName(originalName);
if (Strings.isNullOrEmpty(originalComputeGroupId)) {
throw new DdlException("unable to rename compute group, "
+ "cant get compute group id by compute name " + originalName);
LOG.info("rename original compute group {} not found, unable to rename", originalName);
throw new DdlException("compute group '" + originalName + "' not found, unable to rename");
}
// check newGroupName has existed
if (((CloudSystemInfoService) Env.getCurrentSystemInfo()).getCloudClusterNames().contains(newGroupName)) {
LOG.info("rename new compute group {} has existed in instance, unable to rename", newGroupName);
throw new DdlException("compute group '" + newGroupName + "' has existed in warehouse, unable to rename");
}

Cloud.ClusterPB clusterPB = Cloud.ClusterPB.newBuilder()
ClusterPB clusterPB = ClusterPB.newBuilder()
.setClusterId(originalComputeGroupId)
.setClusterName(newGroupName)
.setType(Cloud.ClusterPB.Type.COMPUTE)
.setType(ClusterPB.Type.COMPUTE)
.build();

Cloud.AlterClusterRequest request = Cloud.AlterClusterRequest.newBuilder()
.setInstanceId(((CloudEnv) Env.getCurrentEnv()).getCloudInstanceId())
.setOp(Cloud.AlterClusterRequest.Operation.RENAME_CLUSTER)
.setDropEmptyCluster(true)
.setReplaceIfExistingEmptyTargetCluster(true)
.setCluster(clusterPB)
.build();


Cloud.AlterClusterResponse response;
Cloud.AlterClusterResponse response = null;
try {
response = MetaServiceProxy.getInstance().alterCluster(request);
LOG.info("alter rename compute group, request: {}, response: {}", request, response);
if (response.getStatus().getCode() != Cloud.MetaServiceCode.OK) {
LOG.warn("alter rename compute group not ok, response: {}", response);
throw new UserException("failed to rename compute group errorCode: " + response.getStatus().getCode()
+ " msg: " + response.getStatus().getMsg());
+ " msg: " + response.getStatus().getMsg() + " may be you can try later");
}
} catch (RpcException e) {
LOG.warn("alter rename compute group rpc exception");
throw new UserException("failed to alter rename compute group", e);
} finally {
LOG.info("alter rename compute group, request: {}, response: {}", request, response);
}
}
}
2 changes: 1 addition & 1 deletion gensrc/proto/cloud.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1096,7 +1096,7 @@ message AlterClusterRequest {
optional ClusterPB cluster = 3;
optional Operation op = 4;
// for SQL mode rename cluster, rename to cluster name eq instance empty cluster name, need drop empty cluster
optional bool drop_empty_cluster = 5 [default = false];
optional bool replace_if_existing_empty_target_cluster = 5;
}

message AlterClusterResponse {
Expand Down

0 comments on commit 9a7f69a

Please sign in to comment.