HyukjinKwon commented on PR #58:
URL:
https://github.com/apache/spark-connect-rust/pull/58#issuecomment-5434046573
Thanks @viirya â thorough review, all fair. Addressed in the latest commit
(866acf0):
- **keepalive timeout coercion.** Fixed the real bug: `.max(0)` is gone. The
logic now lives in a validated `ChannelBuilder::keepalive()` that returns
`None` (never a zero-duration) when the interval *or* timeout is non-positive â
so `grpc_keepalive_timeout_ms=-1` disables keepalive, consistent with how a
`-1` interval already behaves, instead of producing an immediately-expiring
PING. Added a unit test for exactly the (valid-interval, `-1`-timeout)
combination.
- **max message length (128 MiB parsed but never applied).** Agreed it's the
same bug-class and a real >4 MiB decode hazard. Filed as a follow-up:
**SPARK-59037**, so it isn't rediscovered as a mystery decode error.
- **diagnostic layer in the description.** Folded the faulthandler watchdog
+ `_dump_server_jvm` into the PR description explicitly. Also tightened all
three: `found[-1]` instead of `[0]`, raised the jstack cap 20kâ60k (the first
run truncated exactly the execute/PythonRunner threads), a comment that the JVM
dump is only unambiguous at `--jobs 1`, and a try/except around the dump-file
open so an unwritable `RUST_HANG_DUMP` can't fail plugin import.
- **two root causes / framing.** Fully agree. Reframed both the PR and
SPARK-59032: keepalive makes the symptom *recoverable* but does **not** explain
"server produced nothing while the reference got an error in <1s," so the
ticket stays open for the true cause (the added jstack targets exactly that).
Noted the `FLAKY_FILES` retry is a 360s-cap-per-attempt trade â coverage over
speed.
- **smaller things.** Removed the duplicated lazy-connect comment, reverted
the `skipped_file` `"env"` string back to a bool, switched to inline `{e}`
captures, and trimmed the keepalive comment to the *why*.
--
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]
