Hisoka-X commented on code in PR #5007:
URL: https://github.com/apache/seatunnel/pull/5007#discussion_r1250392071
##########
seatunnel-engine/seatunnel-engine-client/src/main/java/org/apache/seatunnel/engine/client/job/ClientJobProxy.java:
##########
@@ -93,21 +89,12 @@ private void submitJob(JobImmutableInformation
jobImmutableInformation) {
@Override
public JobStatus waitForJobComplete() {
try {
- jobResult =
- RetryUtils.retryWithException(
Review Comment:
Why remove retry?
##########
seatunnel-engine/seatunnel-engine-server/src/main/java/org/apache/seatunnel/engine/server/CoordinatorService.java:
##########
@@ -495,8 +495,14 @@ public PassiveCompletableFuture<Void> savePoint(long
jobId) {
public PassiveCompletableFuture<JobResult> waitForJobComplete(long jobId) {
JobMaster runningJobMaster = runningJobMasterMap.get(jobId);
if (runningJobMaster == null) {
- JobStatus jobStatus =
jobHistoryService.getJobDetailState(jobId).getJobStatus();
CompletableFuture<JobResult> future = new CompletableFuture<>();
+ JobHistoryService.JobState jobDetailState =
jobHistoryService.getJobDetailState(jobId);
+ if (jobDetailState == null) {
+ future.complete(new JobResult(JobStatus.FAILED, null));
Review Comment:
I suggest add new JobStatus `UNKNOWN`, not just `FAILED`. If the jobId was
wrong or sucessed before cluster restarted. The `FAILED` not right.
##########
seatunnel-engine/seatunnel-engine-client/src/main/java/org/apache/seatunnel/engine/client/job/ClientJobProxy.java:
##########
@@ -93,21 +89,12 @@ private void submitJob(JobImmutableInformation
jobImmutableInformation) {
@Override
public JobStatus waitForJobComplete() {
try {
- jobResult =
- RetryUtils.retryWithException(
- () -> {
- PassiveCompletableFuture<JobResult> jobFuture =
- doWaitForJobComplete();
- return jobFuture.get();
- },
- new RetryUtils.RetryMaterial(
- 100000,
- true,
- exception ->
-
ExceptionUtil.isOperationNeedRetryException(exception),
- Constant.OPERATION_RETRY_SLEEP));
+ PassiveCompletableFuture<JobResult> jobFuture =
doWaitForJobComplete();
+ jobResult = jobFuture.get();
if (jobResult == null) {
- throw new SeaTunnelEngineException("failed to fetch job
result");
+ LOGGER.severe(
+ "Unable to obtain the status of the job, it may have
been running during the last cluster shutdown.");
Review Comment:
What's meaning? `it may have been running during the last cluster shutdown` ?
--
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]
