github-actions[bot] commented on code in PR #69749:
URL: https://github.com/apache/doris/pull/69749#discussion_r4415694077
##########
fe/fe-core/src/main/java/org/apache/doris/load/routineload/RoutineLoadJob.java:
##########
@@ -1488,6 +1509,20 @@ public void updateState(JobState jobState, ErrorReason
reason, boolean isReplay)
.build());
}
+ // For failures found by refreshKafkaPartitions(), which update() runs
before it takes the job lock.
+ // The state is checked again under the lock: a STOP that completed in
between must not be turned
Review Comment:
[P2] Stop auto-resume when a paused job loses group USAGE
`pauseIfNeedSchedule()` only records the supplied reason when `state ==
NEED_SCHEDULE`. An already `PAUSED` Kafka job with a retryable reason and no
custom partitions enters `updateKafkaPartitions()` through `needAutoResume()`,
but a revoked owner USAGE throws `CANNOT_RESUME_ERR` while this helper does
nothing because the state is still `PAUSED`. The old retryable `pauseReason`
remains, so `ScheduleRule` keeps auto-resuming and repeating the failed check.
Preserve the lock/STOP guard while recording the non-retryable reason (or
otherwise disabling auto-resume) for this path.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AlterRoutineLoadCommand.java:
##########
@@ -287,6 +289,18 @@ private void checkJobProperties() throws UserException {
analyzedJobProperties.put(CreateRoutineLoadInfo.UNIQUE_KEY_UPDATE_MODE,
modeStr.toUpperCase());
}
+ if (jobProperties.containsKey(CreateRoutineLoadInfo.COMPUTE_GROUP)) {
+ String computeGroup =
jobProperties.get(CreateRoutineLoadInfo.COMPUTE_GROUP);
+ if (!StringUtil.isEmpty(computeGroup)) {
+ // Unlike workload group, the compute group can be fully
validated right here, so do
+ // it now instead of letting a bad name pause the whole job on
the next task.
Review Comment:
[P2] Validate the retained workload group in the target compute group
`ALTER ROUTINE LOAD ... PROPERTIES("compute_group" = "cg_b")` validates
`cg_b` but leaves an existing `workload_group` unchanged without checking its
namespace. A job created with `compute_group=cg_a, workload_group=wg_a` (where
`wg_a` exists only in `cg_a`) can therefore persist the pair after switching to
`cg_b`; on resume, `KafkaTaskInfo.rePlan()` binds `cg_b` and `WorkloadGroupMgr`
throws because `wg_a` is absent there, so every task is paused with
`CREATE_TASKS_ERR`. Validate the effective workload group against the requested
compute group before logging the ALTER, or reject the cross-namespace change.
##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/mtmv/MTMVTask.java:
##########
@@ -1680,6 +1684,34 @@ private void setupComputeGroup(ConnectContext ctx) {
}
}
Review Comment:
[P2] Use one compute-group snapshot for binding and validation
`setupComputeGroup()` calls `applyComputeGroup()`, which reads
`mtmv.getComputeGroup()` and installs that value in the context, then
`checkComputeGroupBeforeTask()` reads the MV property again. `ALTER
MATERIALIZED VIEW` can update the property between those two read locks, so a
task can validate `cg_b` while its context remains bound to `cg_a` and the
refresh executes there. Capture the declared/effective group once and use that
snapshot for both context setup and the pre-task check (or validate the
context's actual group).
--
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]
