feat(temporal): expose max_concurrent_workflow_tasks in create_worker [BLDX-1282]#1790
Draft
BichitraCodesAtlan wants to merge 1 commit into
Draft
feat(temporal): expose max_concurrent_workflow_tasks in create_worker [BLDX-1282]#1790BichitraCodesAtlan wants to merge 1 commit into
BichitraCodesAtlan wants to merge 1 commit into
Conversation
… [BLDX-1282] Forward the optional `max_concurrent_workflow_tasks` kwarg to Temporal's Worker(...) when set, leaving Temporal's default in place when omitted. Use case: cron-burst deployments where unbounded workflow-task concurrency spawns more sandboxes than the worker can drain, tripping TMPRL1101 and bloating memory. Tested with two new unit tests in tests/unit/execution/test_worker.py: forwarded-when-set, and omitted-by-default. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Contributor
📜 Docstring Coverage ReportRESULT: PASSED (minimum: 30.0%, actual: 77.9%) Detailed Coverage Report |
Contributor
📦 Trivy Vulnerability Scan Results
Report SummaryCould not generate summary table (data length mismatch: 9 vs 8). Scan Result Detailsrequirements.txtuv.lock |
Contributor
📦 Trivy Secret Scan Results
Report SummaryCould not generate summary table (data length mismatch: 9 vs 8). Scan Result Detailsrequirements.txtuv.lock |
Collaborator
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified FilesNo covered modified files...
|
Contributor
|
🛠 Full Test Coverage Report: https://k.atlan.dev/coverage/application-sdk/pr/1790 |
1 similar comment
Contributor
|
🛠 Full Test Coverage Report: https://k.atlan.dev/coverage/application-sdk/pr/1790 |
Collaborator
🧪 SDR Integration Tests (testcontainer) — mysqlStatus: Passed ✅ Temporal workflows: ✅ workflow_include_main_db · ✅ workflow_mixed_filters Extracted + transformed assetsWorkflow run 1 — mysql-0822058c0bd5691a-503a2056
Workflow run 2 — mysql-3b377dc2ca6be340-8c576da8
What this validates
Re-runPush a new commit, or re-run from the Actions tab. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
Discussion-first PR
This PR is opened as a draft to anchor the team discussion on the problem statement before locking in the implementation shape. The patch is intentionally minimal (2 LOC in
create_worker) and is only meant to make the conversation concrete.Problem
Temporal's
Worker(...)constructor acceptsmax_concurrent_workflow_tasks, which caps the number of in-flight workflow tasks the worker will poll for at once. Each in-flight workflow task initializes a workflow sandbox, so this also caps concurrent sandbox initializations. SDK'screate_workerdoes not forward this parameter, leaving callers stuck with Temporal's default.Concrete consumer: Automation Engine (AUT-904) hit TMPRL1101 sandbox-deadlock-detection issues in production. When many cron workflows fire simultaneously, Temporal's default workflow-task concurrency initializes far more sandboxes than the worker has activity capacity to drain. Excess sandboxes sit idle, trip the deadlock detector after the configured timeout, and bloat resident memory via sandbox churn.
AE's empirically-determined fix: pin
max_concurrent_workflow_tasksto matchmax_concurrent_activities(default 1 in production). This keeps the active sandbox count bounded by what the worker can actually drain. Today AE maintains a copy of SDK'screate_worker(inautomation_engine/clients/temporal.py) just for this knob and one other (extra_workflows— see BLDX-1281).Open questions for the team
max_concurrent_activitiesautomatically?Worker(...)kwargs the team thinks we should expose at the same time, while we're touching the signature?Summary of the proposed change
max_concurrent_workflow_tasks: int | None = Nonekeyword-only param tocreate_worker.Worker(...); whenNone(default), the kwarg is not passed and Temporal's default applies. Behavior unchanged for existing callers.Test plan
tests/unit/execution/test_worker.py:test_max_concurrent_workflow_tasks_forwarded_when_set— verifies the kwarg lands onWorker(...).test_max_concurrent_workflow_tasks_default_is_omitted— verifies omission keeps Temporal's default.tests/unit/execution/test_worker.pystill pass (21 total).uv run pre-commit run --files application_sdk/execution/_temporal/worker.py tests/unit/execution/test_worker.py— ruff, ruff-format, isort, pyright all pass.🤖 Generated with Claude Code