jerryshao commented on code in PR #12509:
URL: https://github.com/apache/gravitino/pull/12509#discussion_r3812030465
##########
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 =
+ newStatus == JobHandle.Status.SUCCEEDED || newStatus ==
JobHandle.Status.FAILED;
+ long startedAt;
+ if (isStarted) {
+ startedAt = Instant.now().toEpochMilli();
Review Comment:
Fixed — now only stamp `startedAt` when `job.startedAt() <= 0` (no start
time recorded yet); otherwise the existing value is preserved. A
CANCELLING->STARTED observation during an in-flight async cancellation no
longer overwrites the real start time.
##########
core/src/main/java/org/apache/gravitino/job/JobManager.java:
##########
@@ -517,7 +518,9 @@ public JobEntity cancelJob(String metalake, String jobId)
throws NoSuchJobExcept
.withLastModifier(PrincipalUtils.getCurrentPrincipal().getName())
.withLastModifiedTime(Instant.now())
.build())
- // CANCELLING is not a terminal state, so the job is not finished
yet.
+ // CANCELLING is not a terminal state; carry forward whatever
startedAt/finishedAt
+ // the job already had.
+ .withStartedAt(jobEntity.startedAt())
Review Comment:
Fixed — cancelJob now re-fetches the job entity fresh inside the write lock,
right before building the update, instead of reusing the snapshot taken before
the external `jobExecutor.cancelJob(...)` call. A concurrent status poll's
recorded startedAt/finishedAt can no longer be clobbered back to the sentinel.
--
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]
