Skip to content

bpo-22281 ENH add introspection API for concurrent.futures Executor #4243

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

tomMoral
Copy link
Contributor

@tomMoral tomMoral commented Nov 2, 2017

This PR is building from the patch coming from the orignal issue (https://bugs.python.org/issue22281) to add an introspection API for the concurrent.futures executors. The following methods are added:

  • worker_count() : Total number of workers currently in the pool
  • active_worker_count() : Number of workers currently processing a work item
  • idle_worker_count(): Number of workers not processing a work item

  • task_count(): Total number of tasks currently being handled by the pool
  • active_task_count(): Number of tasks currently being processed by workers
  • waiting_task_count(): Number of submitted tasks not yet being processed by a worker

  • active_tasks(): A set of _WorkItem objects currently being processed by a worker.
  • waiting_tasks(): A list of _WorkItem objects currently waiting to be processed by a worker.

The discussion in the orignal issue proposed to use properties instead of methods and to add a couple functionalities. I am up for inputs on that. I think a nice addition would be some indication of the work load in the Executor, by timing the idle time and active time of each tasks and collecting it.

https://bugs.python.org/issue22281

@pitrou
Copy link
Member

pitrou commented Nov 3, 2017

As I said in the original issue, I think this should be a single stats() method that returns a dictionary. There are two reasons:

  • With several methods, there's a race condition: if I call waiting_task_count() then active_task_count(), I'm not sure the two numbers are comparable since they reflect snapshots at different points in time; a single method can guarantee the snapshot is consistent (perhaps it needs to take a lock for that, that's an implementation issue)
  • It makes the API simpler: we can add a lot of content to the stats dictionary without clobbering the API and the API docs :-)

Of course, the returned dictionary would be something like:

{'worker_count': 8,
 'active_worker_count': 3,
 'idle_worker_count': 5,
 # etc.
}

I'd also see something like a "status" field reflecting the state of the executor ("running", "shutting_down", "shutdown", "broken" perhaps).

@pitrou
Copy link
Member

pitrou commented Nov 3, 2017

By the way, you should probably base your work on PR #4241 because I'm soon gonna merge it :-)

@tomMoral tomMoral force-pushed the PR_executor_stats branch 2 times, most recently from d9768bc to 3a56a69 Compare November 3, 2017 15:43
@tomMoral
Copy link
Contributor Author

tomMoral commented Nov 3, 2017

I just rebased the PR and I will implement the stat API asap.

@pitrou
Copy link
Member

pitrou commented Nov 4, 2017

Woops... It looks like squashing PR #4241 on push left this PR with conflicts. I'm sorry :-/

@tomMoral tomMoral force-pushed the PR_executor_stats branch 4 times, most recently from ce0687d to 4887735 Compare November 14, 2017 13:45
@pitrou
Copy link
Member

pitrou commented Jan 5, 2018

@tomMoral would you like to revive this PR? Also @ogrisel

@tomMoral
Copy link
Contributor Author

tomMoral commented Jan 5, 2018

Yes I will rebase it and see what needs to be improved.

@tomMoral tomMoral force-pushed the PR_executor_stats branch 2 times, most recently from 029af66 to b8a36d5 Compare January 5, 2018 10:30
@tomMoral
Copy link
Contributor Author

tomMoral commented Jan 5, 2018

@pitrou I think this PR implements a basic API for the introspection. Now it is a matter of what should we add in it.

You mentioned the addition of a 'status' for the pool ("running", "shutting_down", "shutdown", "broken"). I don't know if you want to add more?

We could easily add timing for the different states of the futures. This would allow to gain info on the average waiting time/running time in the executor but I am not sure how interesting it is.

@pitrou
Copy link
Member

pitrou commented Jan 24, 2018

@tomMoral I think we don't need to add more statistics for now. OTOH the implementation is more complicated than I expected. Is _ExecutorFlags really needed or useful? I'm not sure we should add too much complexity for a little-used feature.

@tomMoral
Copy link
Contributor Author

@pitrou My first take was to implement the 'status' without it and I got some failure due to concurrency issues (if I remember correctly). The _ExecutorFags mechanism is probably not needed in itself but IMO, it simplifies the management of the flags and it makes is more robust for future changes as all the flags can be managed in a conscistent way. But if you think it is unnecessary, I will try a simpler implementation. Let me know what you think is best.

And sorry for the long delay, I just got back from vacations.

@csabella
Copy link
Contributor

Is there still interest in this change? It looks like there have been other changes merged since this was created. Thanks!

@github-actions
Copy link

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Aug 13, 2022
@python-cla-bot
Copy link

python-cla-bot bot commented Apr 6, 2025

The following commit authors need to sign the Contributor License Agreement:

CLA signed

@github-actions github-actions bot removed the stale Stale PR or inactive for long period of time. label Apr 6, 2025
Copy link

github-actions bot commented May 6, 2025

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label May 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review stale Stale PR or inactive for long period of time.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants