ivoson commented on code in PR #56649:
URL: https://github.com/apache/spark/pull/56649#discussion_r3456849268
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -4777,6 +4777,25 @@ object SQLConf {
"must be positive.")
.createWithDefault(100)
+ val PYTHON_UDF_MAX_BYTES_PER_BATCH =
+ buildConf("spark.sql.execution.python.udf.maxBytesPerBatch")
+ .internal()
+ .doc("Byte-size cap for a batch of rows sent to a worker for regular
(pickle-serialized, " +
+ "non-Arrow) Python UDF evaluation. Without it, BatchEvalPythonExec
batches purely by " +
+ "row count (spark.sql.execution.python.udf.maxRecordsPerBatch), so one
oversized batch " +
+ "can OOM the executor; a finite value caps the batch at the min of the
row-count and " +
+ "byte limits. The size is a best-effort per-row estimate of the
pickled size of the " +
+ "converted row (its accuracy is observable via the
pythonEstimatedInputBytes vs " +
+ "pythonDataSent metrics); a row larger than the cap still yields a
one-row batch. " +
+ "-1 (the default) means no limit.")
+ .version("5.0.0")
+ .withBindingPolicy(ConfigBindingPolicy.NOT_APPLICABLE)
+ .bytesConf(ByteUnit.BYTE)
+ .checkValue(x => x == -1 || (x > 0 && x <= Int.MaxValue),
Review Comment:
Good question. I'd keep the upper bound at `Int.MaxValue` rather than shrink
it.
2Gb is the physical ceiling. Each batch is pickled into a single contiguous
`byte[]` (`ByteArrayOutputStream.toByteArray`), and a JVM array length is an
`int`, so a batch can't exceed `Int.MaxValue` bytes no matter what the config
is set to. That's also why the Arrow path uses the same `<= Int.MaxValue`
bound, as you noted.
And the ceiling here is just input validation, not a recommended value: the
default is `-1` (off), and anyone who turns this on picks their own (typically
much smaller) cap. Any smaller ceiling (such as 512MB or 1GB) would be
arbitrary and could block a legitimate large-heap-executor use case, whereas
`Int.MaxValue` is the one bound that's actually meaningful (the array limit).
Does this make sense? pls lmk if there are more suggestions on this! Thanks.
--
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]
