morningman commented on code in PR #70130:
URL: https://github.com/apache/doris/pull/70130#discussion_r4475607174
##########
fe/fe-core/src/main/java/org/apache/doris/load/routineload/RoutineLoadJob.java:
##########
@@ -2110,6 +2110,29 @@ public void gsonPostProcess() throws IOException {
public abstract void modifyProperties(AlterRoutineLoadCommand command)
throws UserException;
+ /**
+ * Checks the workload group an ALTER leaves the job with, in the compute
group the ALTER leaves the job in, as
+ * CREATE does and as every task resolves it ({@code
KafkaTaskInfo#rePlan}): as the owner, whom the tasks run
+ * as. One that group does not have, or the owner may not use, would
otherwise fail every task with the
+ * retryable CREATE_TASKS_ERR, which auto resume repeats for good. Only an
ALTER that sets a workload group or
+ * declares a compute group changes what the tasks look up; a workload
group it does not set is the one the
+ * job keeps. Called on the statement thread under the job's write lock,
so that of two ALTERs running at once
+ * the later one checks what the earlier one left; never when an ALTER is
replayed.
+ */
+ protected void checkAlteredWorkloadGroup(Map<String, String>
alteredJobProperties) throws UserException {
+ String declaredComputeGroup = alteredJobProperties.get(COMPUTE_GROUP);
+ String alteredWorkloadGroup = alteredJobProperties.get(WORKLOAD_GROUP);
+ if (StringUtils.isEmpty(declaredComputeGroup) &&
StringUtils.isEmpty(alteredWorkloadGroup)) {
+ return;
+ }
+ String workloadGroup = StringUtils.isEmpty(alteredWorkloadGroup) ?
getWorkloadGroup() : alteredWorkloadGroup;
+ if (StringUtils.isEmpty(workloadGroup)) {
+ return;
+ }
+ CreateRoutineLoadInfo.checkWorkloadGroup(getUserIdentity(),
workloadGroup,
Review Comment:
The null owner is real, but only a job created before 1.1 has one, and on
master such a job cannot run anyway, so this changes how an ALTER of it fails,
not whether the job works. I am keeping the helper as it is.
- The owner has been recorded since 1.1 (#9492; meta version 110 is "For
routine load user info"), and every CREATE records the creating session's
identity (`KafkaRoutineLoadJob.java:591`, `:605`; Kinesis jobs only exist since
2026). Master does not load an image below meta version 140
(`MetaContext.java:37-41`), so such a job reaches master only by staying
unfinished through every upgrade since 1.0, and ALTER finds only unfinished
jobs.
- On master that job already fails every task.
`KafkaTaskInfo#createRoutineLoadTask` calls
`routineLoadJob.getUserIdentity().getQualifiedUser()` unconditionally
(`KafkaTaskInfo.java:131`, since #53799), and for a single-table job `rePlan`
first passes the same null owner to the same
`WorkloadGroupMgr.getWorkloadGroup` (`KafkaTaskInfo.java:204-215`). The
scheduler pauses the job with CREATE_TASKS_ERR
(`RoutineLoadTaskScheduler.java:229-236`), and `SHOW ROUTINE LOAD` on its
database throws at `RoutineLoadJob.java:1763`. No ALTER can give the job an
owner, so the way out is STOP and CREATE, with or without this PR.
- The NPE does not escape the statement: `StmtExecutor` returns it to the
client as an error (`StmtExecutor.java:1084-1091`), the job's write lock is
released in `finally`, nothing is merged and no edit log is written. Master
already makes this call with the job's owner for an ALTER that moves a job with
a workload group to another compute group (#69749).
- The existing fallbacks give no identity to check USAGE against:
`RoutineLoadManager#getAvailableBackendIds` picks backends by replica tag, and
`ComputeGroupBindingUtil#checkComputeGroupBeforeTask` skips its check. Skipping
here would accept the workload group unchecked, which is what this PR stops.
And a regression test cannot create such a job, since CREATE always records the
owner.
So I would rate this a Minor about the error an ALTER returns on a job that
cannot run, not a P1, and leave it out of this PR. If jobs without an owner are
worth handling, it should be one change that covers SHOW, the tasks and ALTER
together.
--
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]
