roryqi commented on code in PR #14230:
URL: https://github.com/apache/gravitino/pull/14230#discussion_r4386072185
##########
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:
nit: Does K8S define the constants `Succeeded` and `Failed`? It's better if
we can reuse them.
##########
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:
Some systems will treat HTTP code 6XX as the internal error 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]
