jerryshao commented on code in PR #13381:
URL: https://github.com/apache/gravitino/pull/13381#discussion_r4090706848
##########
core/src/main/java/org/apache/gravitino/connector/job/JobExecutor.java:
##########
@@ -60,15 +60,45 @@ public interface JobExecutor extends Closeable {
String submitJob(JobTemplate jobTemplate);
/**
- * Get the status of a job by its unique identifier. The status should be
one of the values in
- * {@link JobHandle.Status}. The implementors should query the external job
runner to get the job
- * status, and map the status to the values in {@link JobHandle.Status}.
+ * Get a snapshot of the job's execution state by its unique identifier,
including its status and
+ * when the job actually started and finished. The implementors should query
the external job
+ * runner to get the job state, and map the status to the values in {@link
JobHandle.Status}.
+ *
+ * <p>Gravitino pulls the job state periodically, so a job may go through
several statuses between
+ * two pulls, for example, from {@link JobHandle.Status#QUEUED} straight to
{@link
+ * JobHandle.Status#SUCCEEDED}. The timestamps are attributes of the job,
not of a particular
+ * status: once the job has started, the started time must be reported in
every later snapshot,
+ * including the terminal ones. Once reported, a timestamp should not change.
+ *
+ * <ul>
+ * <li>The started time is null if the job hasn't started executing, or if
it is unknown to the
+ * job executor.
+ * <li>The finished time is only set for a terminal status, and is null if
it is unknown to the
+ * job executor.
+ * </ul>
+ *
+ * <p>If the job runner can't tell when a job started or finished, leave the
time null. Gravitino
+ * then falls back to the time it observes the job running or finished,
which can be off by up to
+ * the job status pull interval. A job that starts and finishes between two
pulls is never
+ * observed running, so it has no started time in that case.
+ *
+ * @param jobId The unique identifier of the job.
+ * @return The execution snapshot of the job.
+ * @throws NoSuchJobException If the job with the given identifier does not
exist.
+ */
+ JobExecutionInfo getJobExecutionInfo(String jobId) throws NoSuchJobException;
Review Comment:
Fixed in 14674f051: `JobExecutorFactory` now checks at startup that the
executor implements `getJobExecutionInfo`, and fails with an actionable error
otherwise, so an old plugin can no longer reach the status pull. The method
stays abstract, so a rebuild fails to compile instead.
`TestJobExecutorFactory#testRejectJobExecutorBuiltAgainstOldSpi` compiles an
executor against the old SPI, checks that it throws `AbstractMethodError`
against the current one, and that the factory check rejects it.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
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]
