lukecwik commented on a change in pull request #11590:
URL: https://github.com/apache/beam/pull/11590#discussion_r424755373
##########
File path: sdks/python/apache_beam/utils/thread_pool_executor.py
##########
@@ -137,35 +101,33 @@ def submit(self, fn, *args, **kwargs):
"""
future = _base.Future()
work_item = _WorkItem(future, fn, args, kwargs)
- try:
- # Keep trying to get an idle worker from the queue until we find one
- # that accepts the work.
- while not self._idle_worker_queue.get(
- block=False).accepted_work(work_item):
- pass
- return future
- except queue.Empty:
- with self._lock:
- if self._shutdown:
- raise RuntimeError(
- 'Cannot schedule new tasks after thread pool '
- 'has been shutdown.')
-
- worker = _Worker(
- self._idle_worker_queue,
- self._permitted_thread_age_in_seconds,
- work_item)
+ with self._lock:
+ if self._shutdown:
+ raise RuntimeError(
+ 'Cannot schedule new tasks after thread pool has been shutdown.')
+ try:
+ self._idle_worker_queue.get(block=False).assign_work(work_item)
+
+ # If we have more idle threads then the max allowed, shutdown a thread.
+ if self._idle_worker_queue.qsize() > self._max_idle_threads:
+ try:
+ self._idle_worker_queue.get(block=False).shutdown()
Review comment:
`self._workers` is a weakref set so when the thread dies it should
automatically be removed from the set. Calling `shutdown` multiple times should
be a non-issue as well for each worker as all it does is make the worker
eligible to wake up.
Do you have any more details as to why you think this is an issue?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
