dybyte commented on code in PR #9749:
URL: https://github.com/apache/seatunnel/pull/9749#discussion_r2291674423
##########
seatunnel-engine/seatunnel-engine-server/src/main/java/org/apache/seatunnel/engine/server/CoordinatorService.java:
##########
@@ -681,6 +681,9 @@ && getJobHistoryService().getJobMetrics(jobId)
} else {
runningJobInfoIMap.remove(jobId);
runningJobMasterMap.remove(jobId);
+ // remove jobId from pendingJobMasterMap when
jobSubmitFuture is
+ // exceptionally.
+ pendingJobMasterMap.remove(jobId);
Review Comment:
> Both `pendingJob` and `pendingJobMasterMap` reference the `JobMaster`. The
current problem is that one object is often cleared without clearing another.
The `pendingJobMasterMap` retrieves the `JobMaster` by jobid and marks which
pending tasks have been canceled (removed from the `pendingJobMasterMap`).
Wouldn't it be better if the pendingJob data structure `PeekBlockingQueue`
supported these two functions?
I agree with your suggestion. Alternatively, we could consider using
composition by introducing a `PendingJobManager` that holds `PeekBlockingQueue`
and the `map`, to keep queue logic separate and improve maintainability. Just
an idea for consideration.
--
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]
