feat(concurrency): add opt-in per-domain concurrency limit scope#1267
Open
izrail09 wants to merge 2 commits into
Open
feat(concurrency): add opt-in per-domain concurrency limit scope#1267izrail09 wants to merge 2 commits into
izrail09 wants to merge 2 commits into
Conversation
Adds TaskDef.concurrencyLimitScope (TASK_DEF default | DOMAIN). When set to DOMAIN, concurrentExecLimit is counted per (task_def_name, domain) so each task domain gets an independent concurrency budget; TASK_DEF preserves the existing cluster-wide, per-task-name behavior. Enforcement is implemented across the SQL and Redis persistence backends: - Postgres/MySQL/SQLite: a nullable `domain` column on task_in_progress (Flyway migrations), counted with each dialect's null-safe match (IS NOT DISTINCT FROM / <=> / IS). NULL is the legacy no-domain bucket and the default TASK_DEF path runs the original SQL unchanged. - Redis (redis-persistence + redis-concurrency-limit): domain-qualified in-progress/limit keys for DOMAIN scope only; TASK_DEF keeps legacy keys. - Cassandra falls back to per-task-definition counting for now (no automatic schema-migration path); documented as a follow-up. Backward compatible: the field defaults to TASK_DEF, the default code path is unchanged, and DOMAIN's Redis key change applies only to opt-in tasks. Tests: shared ExecutionDAOTest#testTaskExceedsLimitPerDomain runs for Postgres, MySQL, SQLite, and Redis. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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.
Pull Request type
NOTE: Please remember to run
./gradlew spotlessApplyto fix any format violations.Changes in this PR
Adds an opt-in per-domain scope for task concurrency limits.
Problem.
concurrentExecLimitis counted only by task-definition name — a single cluster-wide cap shared across every domain a task runs under. But the task queue is already partitioned by domain (domain:taskType), so when the same task is used by multiple workflows/tenants separated by domain, one busy domain can starve the others: they all contend on a single in-progress count keyed by name.What's new. A new
TaskDef.concurrencyLimitScopeenum:TASK_DEF(default) — unchanged: the limit is a single cap counted pertask_def_nameacross all domains.DOMAIN—concurrentExecLimitis counted per(task_def_name, domain), so each domain gets an independent concurrency budget. Tasks with no domain fall into the legacy no-domain bucket.Backward compatibility. The field defaults to
TASK_DEF; absent JSON values deserialize toTASK_DEF; the default code path is byte-for-byte the existing behavior, so existing task definitions are unaffected. BecauseDOMAINis opt-in, the Redis key-layout change applies only to tasks that explicitly opt in (new behavior) — there is no rolling-upgrade impact on existing tasks.Persistence coverage.
domaincolumn is added totask_in_progress(migrationsV17/V11/V5).DOMAINscope counts and orders by(task_def_name, domain)using each dialect's null-safe match (IS NOT DISTINCT FROM/<=>/IS).NULLis the legacy no-domain bucket; the defaultTASK_DEFpath runs the original SQL unchanged (no PK change).redis-persistenceand the optionalredis-concurrency-limitmodule) —DOMAINscope keys the in-progress set / limit bucket by(task_def_name, domain);TASK_DEFkeeps the legacy per-name key.DOMAINscope currently falls back to per-task-definition (global) counting. Enforcing it per domain requires atask_def_limitschema change and Cassandra has no automatic migration path; left as a follow-up (consistent withgetInProgressTaskCountalready being unsupported there).Tests.
ExecutionDAOTest#testTaskExceedsLimitPerDomain(shared across Postgres, MySQL, SQLite, and Redis) verifies per-domain isolation and the null-domain bucket. The existingtestTaskExceedsLimit(default scope) continues to pass on all backends.Deliberately out of scope (follow-ups): protobuf/gRPC mapping for the new field (consistent with
inputSchema/outputSchema, which are also non-proto today), and per-domain concurrency metrics inWorkflowMonitor.Issue #1266
Alternatives considered
(task_def_name, domain)unconditionally, treating a null domain as the global bucket. Simpler, but silently changes behavior for existing users who combine task domains withconcurrentExecLimit(each domain would suddenly get its own full limit). Rejected to avoid a backward-incompatible semantic change.TaskDef(chosen) — per-definition, backward compatible by default, travels with the definition on export/import, and matches the existing per-TaskDefconfig idiom (retryLogic,timeoutPolicy, ...).