andygrove opened a new pull request, #6198:
URL: https://github.com/apache/datafusion-comet/pull/6198
## Which issue does this PR close?
Closes #6189.
## Rationale for this change
`CometDriverPlugin.warnIfExecutorMemoryOverheadUnset` warns whenever
`spark.executor.memoryOverhead` is unset. For large executors the tuning guide
recommends raising `spark.executor.memoryOverheadFactor` instead, so users who
follow that advice are still told to set `spark.executor.memoryOverhead`. The
warning also fires in local mode, where there is no executor container to size.
The native memory usage log already skips local mode.
## What changes are included in this PR?
- No warning when `spark.executor.memoryOverheadFactor` is set.
- No warning when the master is local: `local`, `local[N]` and
`local-cluster[...]`. This is the same `startsWith("local")` check the native
memory usage log uses.
- `spark.kubernetes.memoryOverheadFactor` also counts, but only when it
differs from the value spark-submit sets itself. In Kubernetes cluster mode,
`BasicDriverFeatureStep` passes that factor to the driver whether or not the
application set it: 0.4 for PySpark and SparkR applications
(`spark.kubernetes.resource.type` is `python` or `r`) and 0.1 otherwise. This
is the same in Spark 3.4 through 4.1. Checking only whether the key is present
would have turned the warning off for every cluster-mode application. An
application that sets nothing therefore still gets the warning. The one case
this cannot tell apart is an application that explicitly sets the same value
spark-submit would have passed on, and that application gets the warning as
before.
- The message now names both settings ("Neither
spark.executor.memoryOverhead nor spark.executor.memoryOverheadFactor is set
... Set one of them before creating the SparkContext"). It used to point only
at the absolute setting. For a large executor, the tuning guide warns that an
absolute value can end up smaller than the overhead Spark derives today.
Standalone masters (`spark://`) still get the warning, although Spark does
not size standalone executors from the overhead either. #6188 raises the same
point for the executor's memory log.
## How are these changes tested?
New cases in `CometPluginsMemoryOverheadWarningSuite`:
- no warning with `spark.executor.memoryOverheadFactor` set
- no warning with a Kubernetes factor other than the one spark-submit would
pass on (a client-mode configuration, a JVM application at 0.4, a PySpark
application at 0.5)
- a warning when the Kubernetes factor is the one spark-submit passes on for
Java (0.1), Python (0.4) and R (0.4) applications
- no failure when the Kubernetes factor does not parse
- no warning for `local`, `local[4]` and `local-cluster[2,1,1024]`
The existing cases now set a cluster master explicitly. Before the fix, the
new executor factor, Kubernetes factor and local mode cases failed. With the
fix, the suites in `CometPluginsSuite.scala` pass on the default Spark 4.1
profile and on Spark 3.5.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
