JeremyXin commented on code in PR #11841:
URL: https://github.com/apache/seatunnel/pull/11841#discussion_r3922901541
##########
seatunnel-engine/seatunnel-engine-server/src/main/java/org/apache/seatunnel/engine/server/CoordinatorService.java:
##########
@@ -1394,9 +1396,12 @@ public PassiveCompletableFuture<Void> submitJob(
try {
JobImmutableInformation
submittedJobImmutableInformation =
deserializeJobImmutableInformation(jobImmutableInformation);
+ boolean isSavepointRestore =
Review Comment:
@nzw921rx The reason for re-deriving it inside `CoordinatorService` is to
make the restore decision converge on `JobImmutableInformation`, instead of
trusting an external legacy boolean that could become inconsistent with the
actual restore mode.
I agree the current API shape is not ideal, and there are two possible
follow-up cleanups:
1. Require all callers to always derive `isStartWithSavePoint` from
`JobImmutableInformation`, then remove the extra re-derivation in
`submitJob(...)`.
2. Remove `isStartWithSavePoint` from `submitJob(...)` entirely, and clean
up the related operation / codec path together so that restore semantics are
carried only by `JobImmutableInformation`.
So, which of the above options do you think is better? I think perhaps
option 2 might be more suitable in terms of engineering and safety?
--
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]
