github-actions[bot] commented on code in PR #67913:
URL: https://github.com/apache/doris/pull/67913#discussion_r4004322865
##########
regression-test/data/cloud_p0/node_mgr/test_compute_group_name_reconciliation.out:
##########
@@ -0,0 +1,21 @@
+-- This file is automatically generated. You should know what you did if you
want to edit this
Review Comment:
[P2] Generate this result by running the Docker suite
The commit that introduced this expected-result file says the new cloud
Docker case was not executed and no `.out` was generated or handwritten; the
follow-up commit also reports that the case was not run. Repository policy
requires regression outputs to be produced by the test script, and static
inspection cannot prove the debug-point orchestration reached all phases.
Please run this exact suite in record mode, replace this file with the
generated output, then run it normally and report the command/result.
##########
fe/fe-core/src/main/java/org/apache/doris/cloud/catalog/CloudInstanceStatusChecker.java:
##########
@@ -109,6 +127,7 @@ private void processVirtualClusters(List<Cloud.ClusterPB>
clusters) {
handleComputeClusters(computeClusters);
handleVirtualClusters(virtualClusters, computeClusters);
removeObsoleteVirtualGroups(virtualClusters);
+ cloudSystemInfoService.refreshComputeGroupNames(virtualClusters);
Review Comment:
[P1] Refresh only virtual groups that passed validation
This passes every raw virtual PB to the repair step even when
`handleExistingVirtualComputeGroup` returned without applying it. For example,
with installed valid `V:X`, a response `V:Y` whose policy or subgroup shape is
invalid leaves the local metadata named X, but `refreshComputeGroupNames` still
adds `Y -> V`. If V disappears in the next snapshot, removal only clears X, so
Y remains a permanent alias to a missing ID; before removal, Y routes through
the stale policy that validation explicitly rejected. Please refresh only
successfully reconciled virtual records (and clear all aliases owned by a
removed ID), with a rejected-rename test.
##########
fe/fe-core/src/main/java/org/apache/doris/cloud/catalog/CloudClusterChecker.java:
##########
@@ -146,19 +147,22 @@ private void checkToDelCluster(Map<String, ClusterPB>
remoteClusterIdToPB, Set<S
if (LOG.isDebugEnabled()) {
LOG.debug("begin to drop clusterId: {}", delId);
}
- String delClusterName =
cloudSystemInfoService.getClusterNameByClusterId(delId);
- if (delClusterName.isEmpty()) {
- return;
- }
- ((CloudEnv)
Env.getCurrentEnv()).getCacheHotspotMgr().cancelTableFilterJobsForClusterChange(
- delClusterName, "system cancel: compute group " +
delClusterName + " dropped");
List<Backend> toDel =
new
ArrayList<>(finalClusterIdToBackend.getOrDefault(delId, new ArrayList<>()));
+ // The name index may already belong to a same-name
replacement. Use the
+ // obsolete group's own BE tags, and never skip ID cleanup for
a missing name.
+ String delClusterName =
toDel.stream().map(Backend::getCloudClusterName).findFirst()
+ .orElseGet(() ->
cloudSystemInfoService.getClusterNameByClusterId(delId));
+ // Name-scoped jobs may already belong to the replacement
group.
+ if
(delId.equals(cloudSystemInfoService.getCloudClusterIdByName(delClusterName))) {
Review Comment:
[P1] Make warm-up cleanup independent of the transient name owner
This guard avoids cancelling a replacement's jobs, but it also skips the
only cancellation trigger when the mapping is temporarily missing. That state
is reachable after stale virtual cleanup; if Meta Service drops physical P:X
before the next repair, P is removed by ID while its persisted table-filter
jobs remain runnable across cycles and can later attach to a new X. The
parallel rename path has the opposite race: adding replacement B:X publishes `X
-> B`, a user can create B's job, then renaming A still unconditionally cancels
every X-scoped job. Please carry/check ID-qualified job ownership or serialize
publication and cleanup, and test both interleavings.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
