[SPARK-44433][3.5][PYTHON][CONNECT][SS][FOLLOWUP] Terminate listener process with removeListener and improvements #42340
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WweiL
commented
Aug 4, 2023
| pythonWorkerFactory = Some(workerFactory) | ||
| } finally { | ||
| conf.set(PYTHON_USE_DAEMON, prevConf) | ||
| } |
Contributor
Author
There was a problem hiding this comment.
This and the stop() method are different from master branch since the createPythonWorker method doesn't support custom modules at that time:
cc @ueshin to double check this
ueshin
reviewed
Aug 4, 2023
core/src/main/scala/org/apache/spark/api/python/StreamingPythonRunner.scala
Show resolved
Hide resolved
github-actions
bot
removed
GRAPHX
MESOS
BUILD
SPARK SHELL
YARN
EXAMPLES
DOCS
INFRA
R
AVRO
PANDAS API ON SPARK
PROTOBUF
labels
ueshin
approved these changes
Aug 4, 2023
Member
ueshin
left a comment
ueshin
left a comment
There was a problem hiding this comment.
Otherwise, LGTM, pending tests.
core/src/main/scala/org/apache/spark/api/python/StreamingPythonRunner.scala
Outdated
Show resolved
Hide resolved
Member
|
Thanks! merging to 3.5. |
ueshin
pushed a commit
that referenced
this pull request
Aug 5, 2023
…process with removeListener and improvements ### Master Branch PR: #42283 ### What changes were proposed in this pull request? This is a followup to #42116. It addresses the following issues: 1. When `removeListener` is called upon one listener, before the python process is left running, now it also get stopped. 2. When multiple `removeListener` is called on the same listener, in non-connect mode, subsequent calls will be noop. But before this PR, in connect it actually throws an error, which doesn't align with existing behavior, this PR addresses it. 3. Set the socket timeout to be None (\infty) for `foreachBatch_worker` and `listener_worker`, because there could be a long time between each microbatch. If not setting this, the socket will timeout and won't be able to process new data. ``` scala> Streaming query listener worker is starting with url sc://localhost:15002/;user_id=wei.liu and sessionId 886191f0-2b64-4c44-b067-de511f04b42d. Traceback (most recent call last): File "/usr/lib/python3.9/runpy.py", line 197, in _run_module_as_main return _run_code(code, main_globals, None, File "/usr/lib/python3.9/runpy.py", line 87, in _run_code exec(code, run_globals) File "/home/wei.liu/oss-spark/python/lib/pyspark.zip/pyspark/sql/connect/streaming/worker/listener_worker.py", line 95, in <module> File "/home/wei.liu/oss-spark/python/lib/pyspark.zip/pyspark/sql/connect/streaming/worker/listener_worker.py", line 82, in main File "/home/wei.liu/oss-spark/python/lib/pyspark.zip/pyspark/serializers.py", line 557, in loads File "/home/wei.liu/oss-spark/python/lib/pyspark.zip/pyspark/serializers.py", line 594, in read_int File "/usr/lib/python3.9/socket.py", line 704, in readinto return self._sock.recv_into(b) socket.timeout: timed out ``` ### Why are the changes needed? Necessary improvements ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Manual test + unit test Closes #42340 from WweiL/SPARK-44433-listener-followup-3.5. Authored-by: Wei Liu <[email protected]> Signed-off-by: Takuya UESHIN <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Master Branch PR: #42283
What changes were proposed in this pull request?
This is a followup to #42116. It addresses the following issues:
removeListeneris called upon one listener, before the python process is left running, now it also get stopped.removeListeneris called on the same listener, in non-connect mode, subsequent calls will be noop. But before this PR, in connect it actually throws an error, which doesn't align with existing behavior, this PR addresses it.foreachBatch_workerandlistener_worker, because there could be a long time between each microbatch. If not setting this, the socket will timeout and won't be able to process new data.Why are the changes needed?
Necessary improvements
Does this PR introduce any user-facing change?
No
How was this patch tested?
Manual test + unit test