Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/23055#discussion_r236737983
--- Diff:
core/src/main/scala/org/apache/spark/api/python/PythonRunner.scala ---
@@ -74,8 +74,13 @@ private[spark] abstract class BasePythonRunner[IN, OUT](
private val reuseWorker = conf.getBoolean("spark.python.worker.reuse",
true)
// each python worker gets an equal part of the allocation. the worker
pool will grow to the
// number of concurrent tasks, which is determined by the number of
cores in this executor.
- private val memoryMb = conf.get(PYSPARK_EXECUTOR_MEMORY)
+ private val memoryMb = if (Utils.isWindows) {
--- End diff --
It's a minor point, but you seem to be the one making a big fuss about it.
Why is this code needed? It's not doing anything useful if you keep the
python check around. The JVM doesn't understand exactly what Python supports of
not, it's better to let the python code decide that.
You say we should disable the feature on Windows. The python-side changes
already do that. We should not remove the extra memory requested from the
resource manager just because you're running on Windows - you'll still need
that memory, you'll just get a different error message if you end up using more
than you requested.
> If we can find a workaround on Windows, we can fix the Python worker and
enable it.
Exactly, and that should not require making any changes on the JVM side.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
