jerryshao commented on code in PR #13982:
URL: https://github.com/apache/gravitino/pull/13982#discussion_r4322505431
##########
maintenance/jobs/src/main/java/org/apache/gravitino/maintenance/jobs/iceberg/IcebergJobUtils.java:
##########
@@ -34,13 +41,30 @@
*/
public final class IcebergJobUtils {
+ private static final Logger LOG =
LoggerFactory.getLogger(IcebergJobUtils.class);
+
private static final String ICEBERG_SPARK_CATALOG =
"org.apache.iceberg.spark.SparkCatalog";
/**
* Matches a job template placeholder that no job configuration value
replaced, e.g. {@code
* {{use_caching}}}.
*/
private static final Pattern UNRESOLVED_PLACEHOLDER_PATTERN =
Pattern.compile("^\\{\\{[^{}]*}}$");
+ /**
+ * The Spark configurations that spark-submit decides. Set in the custom
Spark configurations of a
+ * job, which the driver applies, they would override the submitted value,
e.g. run a job in local
+ * mode inside the driver pod of a cluster.
+ */
+ private static final Set<String> SUBMIT_DECIDED_CONFIGS =
+ ImmutableSet.of("spark.master", "spark.submit.deployMode");
+
+ /**
+ * The prefix of the Kubernetes configurations, which a job run may not set
in its custom Spark
+ * configurations: the driver creates the executor pods from them, with
their image, service
+ * account, Secrets, and volumes. Only the job executor configurations and
job templates set them.
+ */
+ private static final String SPARK_KUBERNETES_PREFIX = "spark.kubernetes.";
Review Comment:
Added comments in both IcebergJobUtils and SparkApplicationUtils that point
at each other and say the rules must stay aligned; they can't share code since
the jobs jar runs in the driver without the executor plugin.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
plugins/k8s-job-executor/src/main/java/org/apache/gravitino/job/k8s/spark/SparkApplicationUtils.java:
##########
@@ -147,14 +174,25 @@ public static GenericKubernetesResource
buildSparkApplication(
+ " the k8s job executor only runs Spark jobs in %s mode",
SPARK_DEPLOY_MODE, deployMode, template.name(),
CLUSTER_DEPLOY_MODE));
}
+ // Likewise, a job that asks for another master, such as local[*], is
rejected rather than
+ // silently run on Kubernetes.
+ String master = StringUtils.trimToNull(sparkConf.get(SPARK_MASTER));
+ if (master != null && !master.startsWith(K8S_MASTER_PREFIX)) {
+ throw new IllegalArgumentException(
+ String.format(
+ "%s is set to %s by job template %s or by the k8s job executor
configurations, but"
+ + " the k8s job executor only runs Spark jobs on Kubernetes.
Leave %s unset,"
+ + " for a built-in job by leaving the spark_master parameter
empty",
+ SPARK_MASTER, master, template.name(), SPARK_MASTER));
+ }
Review Comment:
The message now says to leave spark.master unset in the job template and the
executor configurations, and mentions the spark_master parameter only for a
builtin- template (tested both cases).
_🤖 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]
