jerryshao commented on code in PR #14230:
URL: https://github.com/apache/gravitino/pull/14230#discussion_r4386091224
##########
plugins/k8s-job-executor/src/main/java/org/apache/gravitino/job/k8s/K8sJobExecutor.java:
##########
@@ -170,9 +176,10 @@ public String submitJob(JobContext context, JobTemplate
jobTemplate) {
try {
getSparkApplicationClient(namespace).resource(app).create();
} catch (KubernetesClientException e) {
- if (isUnreachable(e)) {
- // The SparkApplication may have been created although no response
came back. Gravitino
- // records no job for a failed submission, so delete it, otherwise it
would run unknown.
+ if (isUnreachable(e) || e.getCode() >=
HttpURLConnection.HTTP_INTERNAL_ERROR) {
Review Comment:
The check is `e.getCode() >= HTTP_INTERNAL_ERROR`, a lower bound only, so
6xx codes (and any other code above 500) already trigger the cleanup. No change
needed.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
plugins/k8s-job-executor/src/main/java/org/apache/gravitino/job/k8s/spark/SparkApplicationStatusUtils.java:
##########
@@ -251,21 +251,22 @@ public static JobExecutionInfo executionInfoOf(
}
/**
- * Returns whether the driver of a SparkApplication is still pending or
running. It is decided by
- * the driver container, as a sidecar can keep the pod running after the
driver exited, and by the
- * pod only if the driver container can't be told.
+ * Returns whether the driver of a SparkApplication is still pending or
running. A pod that has
+ * terminated runs nothing, even if its container status still says running,
for example after its
+ * node was lost. Otherwise it is decided by the driver container, as a
sidecar can keep the pod
+ * running after the driver exited, and the driver runs if its container
can't be told.
*
* @param app the SparkApplication
* @param driver the driver pod
* @return true if the driver hasn't terminated
*/
public static boolean isDriverRunning(GenericKubernetesResource app, Pod
driver) {
- ContainerState state = driverContainerState(driver,
getDriverContainerName(app));
- if (state != null) {
- return state.getTerminated() == null;
- }
String phase = driver.getStatus() == null ? null :
driver.getStatus().getPhase();
- return !"Succeeded".equals(phase) && !"Failed".equals(phase);
+ if ("Succeeded".equals(phase) || "Failed".equals(phase)) {
Review Comment:
fabric8 7.2.0 has no public constants for the pod phases (only
`Readiness.isPodSucceeded`, with no counterpart for Failed), so I moved the two
phases into a `POD_TERMINATED_PHASES` constant in 337ed88d9d.
_🤖 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]
