jerryshao commented on code in PR #12509:
URL: https://github.com/apache/gravitino/pull/12509#discussion_r3812027806
##########
core/src/main/java/org/apache/gravitino/job/JobManager.java:
##########
@@ -592,11 +595,30 @@ void pullAndUpdateJobStatus() {
}
if (newStatus != job.status()) {
+ boolean isStarted = newStatus == JobHandle.Status.STARTED;
boolean isFinished =
newStatus == JobHandle.Status.SUCCEEDED
|| newStatus == JobHandle.Status.FAILED
|| newStatus == JobHandle.Status.CANCELLED;
+ // SUCCEEDED/FAILED prove the job actually ran, even when no
poll ever observed
+ // it as STARTED (e.g. it transitioned QUEUED -> terminal
between two polls). In
+ // that case, fall back to the job's queued time as the
best-known lower bound for
+ // startedAt - leaving it unset would incorrectly imply the job
never started.
+ // CANCELLED is deliberately excluded: a cancelled job may have
been killed while
+ // still QUEUED and never started at all, so leaving startedAt
unset stays
+ // accurate there.
+ boolean provenToHaveStarted =
Review Comment:
Fixed — removed the SUCCEEDED/FAILED backfill fallback entirely. Confirmed
both points in code: `NoSuchJobException` from the executor does get converted
to `FAILED` above, and `LocalJobExecutor.runJob`'s outer `catch (Exception e)`
can reach `FAILED` directly from `QUEUED` if `processBuilder.start()` itself
throws, skipping the `STARTED` transition entirely. `startedAt` now stays unset
unless a `STARTED` transition was actually observed.
--
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]
