Skip to content

Add PYTHON_THREADPOOL_THREAD_COUNT app setting #744

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

Merged
merged 4 commits into from
Sep 24, 2020
Merged

Conversation

Hazhzeng
Copy link
Contributor

Description

This is a dependency of enabling async thread pool. Getting this change into the codebase will allow us configure the threadpool more easily when benchmarking the async threadpool performance.

Fixes #675


Now we allow customer to configure number of thread in the synchronous threadpool by setting PYTHON_THREADPOOL_THREAD_COUNT application setting. If the application settings is not set, we will default it to 1.

I'm wondering whether we should call this PYTHON_SYNC_THREADPOOL_THREAD_COUNT.
If we introduce async thread pool later on, should we introduce a new PYTHON_ASYNC_THREADPOOL_THREAD_COUNT app settings? It is possible that if a function app mixes both sync and async functions, the actual number of threads will be 2x. Adding this new setting will has a better the transparency for customer to control their function app, but will make the app setting more complicate.

In my opinion, both async threadpool and sync threadpool should use the value in PYTHON_THREADPOOL_THREAD_COUNT.

PR information

  • The title of the PR is clear and informative.
  • There are a small number of commits, each of which has an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, see this page.
  • If applicable, the PR references the bug/issue that it fixes in the description.
  • New Unit tests were added for the changes made and CI is passing.

Quality of Code and Contribution Guidelines

@@ -15,6 +15,7 @@

# Feature Flags (app settings)
PYTHON_ROLLBACK_CWD_PATH = "PYTHON_ROLLBACK_CWD_PATH"
PYTHON_THREADPOOL_THREAD_COUNT = "PYTHON_THREADPOOL_THREAD_COUNT"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stefanushinardi - Could you please check if you want to change the name?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - putting my reasoning here for future reference: it makes sense to put the PYTHON_* prefix for this as without it users might get confused and use this config for other languages

@Hazhzeng Hazhzeng requested a review from vrdmr September 24, 2020 20:20
Copy link
Member

@vrdmr vrdmr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

some nits.

@Hazhzeng Hazhzeng merged commit 825f0b0 into dev Sep 24, 2020
@Hazhzeng Hazhzeng deleted the hazeng/async-threadpool branch September 24, 2020 21:28
vrdmr added a commit that referenced this pull request Oct 2, 2020
* Add PYTHON_THREADPOOL_THREAD_COUNT app setting (#744)
* Adding support for debug logs in executed functions. (#745)
* Bumping up the version to 1.1.6 (#748)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate making the number of threads in the Azure Functions Python Worker thread pool configurable
3 participants