HyukjinKwon opened a new pull request, #50855:
URL: https://github.com/apache/spark/pull/50855
### What changes were proposed in this pull request?
This PR proposes to explicitly close
`ExecutePlanResponseReattachableIterator` after usage.
### Why are the changes needed?
There might be a corner case deadlock as below. The main reason is that the
point of calling `__del__` can be arbitrary and it could depend on each other.
```
Dumping Threads....
File "/.../versions/3.9.21/lib/python3.9/threading.py", line 937, in
_bootstrap
self._bootstrap_inner()
File "/.../versions/3.9.21/lib/python3.9/threading.py", line 980, in
_bootstrap_inner
self.run()
File "/.../versions/3.9.21/lib/python3.9/threading.py", line 917, in run
self._target(*self._args, **self._kwargs)
File "/.../versions/3.9.21/lib/python3.9/concurrent/futures/thread.py",
line 85, in _worker
del work_item
File "/.../python/pyspark/sql/connect/client/reattach.py", line 347, in
__del__
return self.close()
File "/.../python/pyspark/sql/connect/client/reattach.py", line 343, in
close
self._release_all()
File "/.../python/pyspark/sql/connect/client/reattach.py", line 241, in
_release_all
with self._lock:
---------------
File "/.../versions/3.9.21/lib/python3.9/threading.py", line 937, in
_bootstrap
self._bootstrap_inner()
File "/.../versions/3.9.21/lib/python3.9/threading.py", line 980, in
_bootstrap_inner
self.run()
File "/.../versions/3.9.21/lib/python3.9/threading.py", line 917, in run
self._target(*self._args, **self._kwargs)
File
"/.../versions/pyspark-dev-3.9/lib/python3.9/site-packages/grpc/_channel.py",
line 1751, in channel_spin
event = state.channel.next_call_event()
---------------
File "<string>", line 44, in <module>
File "/.../python/pyspark/sql/connect/session.py", line 890, in stop
self.client.close()
File "/.../python/pyspark/sql/connect/client/core.py", line 1234, in close
ExecutePlanResponseReattachableIterator.shutdown()
File "/.../python/pyspark/sql/connect/client/reattach.py", line 82, in
shutdown
cls._get_or_create_release_thread_pool().shutdown()
File "/.../versions/3.9.21/lib/python3.9/concurrent/futures/thread.py",
line 235, in shutdown
t.join()
File "/.../versions/3.9.21/lib/python3.9/threading.py", line 1060, in join
self._wait_for_tstate_lock()
File "/.../versions/3.9.21/lib/python3.9/threading.py", line 1080, in
_wait_for_tstate_lock
if lock.acquire(block, timeout):
File "<string>", line 1, in <module>
File "<string>", line 1, in <module>
```
### Does this PR introduce _any_ user-facing change?
For corner cases, yes. There might be a deadlock, and this PR fixes it.
### How was this patch tested?
Difficult to test. It was more to just remove the cause itself.
### Was this patch authored or co-authored using generative AI tooling?
No.
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
