refactor: deprecate DAG-level `maxActiveRuns` and enhance queue stability by yohamta0 · Pull Request #1598 · dagucloud/dagu

yohamta0 · 2026-01-22T12:07:19Z

Summary by CodeRabbit

Deprecations
- Deprecated maxActiveRuns for local DAG-based queues; configure concurrency via a global queue (queue field).
- Local queues now enforce FIFO (sequential) processing with fixed concurrency of 1.
Bug Fixes
- Improved queue processing with in-flight tracking to avoid duplicate task dispatches.
Tests & Docs
- Added tests and API schema/docs updates to validate deprecation warnings and FIFO local-queue behavior.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2026-01-22T12:07:46Z

📝 Walkthrough

Walkthrough

Deprecates DAG.MaxActiveRuns and updates OpenAPI/schema docs. Local (DAG-based) queues are treated as FIFO with maxConcurrency = 1. Queue processor removed DAG-driven concurrency, added atomic in-flight tracking and goroutine dispatch; builder emits deprecation warnings. Tests updated to assert FIFO/local-queue behavior.

Changes

Cohort / File(s)	Summary
API & Schema `api/v2/api.gen.go`, `api/v2/api.yaml`, `schemas/dag.schema.json`	Marked `DAG.maxActiveRuns` deprecated (added `deprecated: true` and deprecation text); clarified `Queue.maxConcurrency` description: dag-based queues are effectively `1` (FIFO).
Core DAG code & tests `internal/core/dag.go`, `internal/core/dag_test.go`	Added deprecation comments for `MaxActiveRuns`; adjusted test name/comments to reflect deprecation while preserving behavior.
Spec builder & warnings `internal/core/spec/dag.go`, `internal/core/spec/builder_test.go`	Emit build warning when local queue DAGs set non-default `maxActiveRuns`; added tests covering warning scenarios.
Queue processor implementation & tests `internal/service/scheduler/queue_processor.go`, `internal/service/scheduler/queue_processor_test.go`	Removed DAG-driven dynamic max-concurrency logic and setters; added `inflight atomic.Int32`, `incInflight`/`decInflight`; dispatch now uses goroutines and inflight-aware capacity (`free = maxConc - alive - inflight`); tests updated to assert local FIFO invariant.
Frontend queue API `internal/service/frontend/api/v2/queues.go`	Stop inspecting DAGs when creating queues; `getOrCreateQueue` now uses queue name/config only; local queues not initialized from DAG metadata.
Integration tests `internal/intg/queue/queue_test.go`	Test renamed and reworked to validate sequential (FIFO) processing for local queues; timing/assertions adjusted accordingly.
Execution command & spec loading `internal/cmd/exec_spec.go`	Simplified `loadOpts` composite literal; removed explicit `dag.MaxActiveRuns = -1` assignment.
Config tests `internal/cmn/config/queue_config_test.go`	Added unit test validating queue config loading from YAML via Viper.

Sequence Diagram(s)

sequenceDiagram
Client->>Scheduler: Enqueue item (QueuedItemData)
Scheduler->>QueueProcessor: ProcessQueueItems()
Note right of QueueProcessor: compute free = maxConc - alive - inflight
QueueProcessor->>Queue: dequeue item
Queue->>Queue: incInflight()
Queue->>WorkerPool: dispatch DAG run (goroutine)
WorkerPool->>DAGExecutor: run DAG
DAGExecutor-->>WorkerPool: run complete
WorkerPool->>Queue: decInflight()
Queue->>QueueProcessor: notify completion (monitorStartup/backoff logic)

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

fix(scheduler): queue processor should respect maxConcurrency config in global config #1482: Overlaps queue_processor concurrency and queue configuration changes; likely conflicting edits to concurrency handling.
fix: queue processing bug #1595: Directly relates to alternate approaches for setting queue concurrency from DAG metadata; code-level overlap in scheduler logic.
feat(api): add singleton flag to enqueue API #1483: Touches enqueue/queue logic and DAG maxActiveRuns handling; overlaps deprecation/removal of DAG-driven concurrency.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 15.38% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main changes: deprecating DAG-level maxActiveRuns and improving queue stability. It directly corresponds to the substantial refactoring across multiple files.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@internal/service/scheduler/queue_processor.go`:
- Around line 312-315: The in-flight key uses only queueName + runID which can
collide across different DAGs that reuse run IDs; update the key construction to
incorporate the DAG identifier from the run object (e.g., use runRef.DAGID or
runRef.Spec.DAGName / other canonical DAG identifier available on runRef) so the
key becomes queueName + "/" + dagIdentifier + "/" + runID (or similar), and
replace usages of inFlightKey accordingly to ensure uniqueness across DAGs.

🧹 Nitpick comments (4)

internal/service/frontend/api/v2/queues.go (1)

65-68: Consider removing the unused DAG preload now that queue creation ignores it.

The DAG is still read once per group, but its value isn’t used after getOrCreateQueue stopped taking a DAG. If there’s no side-effect needed, dropping that read will save store I/O.

internal/core/spec/builder_test.go (1)

3309-3390: Consider table‑driving the deprecation warning cases to reduce duplication.
The subtests share the same structure (YAML + expected warning). A small table would make additions simpler. As per coding guidelines, prefer table‑driven cases.

api/v2/api.gen.go (1)

485-487: Add an explicit deprecation reason in the OpenAPI schema.
The generated “Deprecated: … no x-deprecated-reason was set” isn’t very user-facing. Consider adding x-deprecated-reason in the schema so these comments become meaningful for both DAG and DAGDetails.

Also applies to: 534-536

internal/intg/queue/queue_test.go (1)

27-41: Clarify: Does TestGlobalConcurrency still use the deprecated maxActiveRuns intentionally?

The maxActiveRuns: 1 on line 31 is set but the test uses a global queue with maxConcurrency=3. Since maxActiveRuns is deprecated for local queues, consider whether this field should be removed from this test spec to avoid confusion, or add a comment clarifying it's intentionally ignored here.

yohamta0 · 2026-01-22T14:46:00Z

@coderabbitai review

coderabbitai · 2026-01-22T14:46:12Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

codecov · 2026-01-22T15:21:53Z

Codecov Report

❌ Patch coverage is 94.44444% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 60.05%. Comparing base (bb3c1a6) to head (b50b403).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/service/scheduler/queue_processor.go	90.90%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1598      +/-   ##
==========================================
+ Coverage   59.99%   60.05%   +0.06%     
==========================================
  Files         288      288              
  Lines       32660    32636      -24     
==========================================
+ Hits        19594    19601       +7     
+ Misses      11312    11281      -31     
  Partials     1754     1754

Files with missing lines	Coverage Δ
internal/cmd/exec_spec.go	`61.01% <100.00%> (-1.28%)`	⬇️
internal/core/dag.go	`69.71% <ø> (ø)`
internal/core/spec/dag.go	`88.29% <100.00%> (+0.08%)`	⬆️
internal/service/scheduler/queue_processor.go	`48.58% <90.90%> (+4.06%)`	⬆️

... and 3 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bb3c1a6...b50b403. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

yohamta0 added 3 commits January 22, 2026 19:42

initial implementation

439a6e8

refactor

4a01aeb

refactor

0463fb7

yohamta0 changed the title ~~refactor: deprecate DAG-leve maxActiveRuns field and enhance queue stability~~ refactor: deprecate DAG-level maxActiveRuns and enhance queue stability Jan 22, 2026

coderabbitai Bot reviewed Jan 22, 2026

View reviewed changes

Comment thread internal/service/scheduler/queue_processor.go Outdated

yohamta0 added 6 commits January 22, 2026 23:04

apply feedback

ea801fa

revert chagnes for queue processor

4675370

remove dynamic config change

99234b0

fix bug of ExecuteDAG in queue processor

333d2b6

fix concurrency issue

1f2fc0a

Fix

b50b403

yohamta0 merged commit 2fbe261 into main Jan 22, 2026
5 checks passed

yohamta0 deleted the deprecate-local-queue-active-run branch January 22, 2026 15:20

coderabbitai Bot mentioned this pull request Jan 25, 2026

refactor: polling to SSE for realtime update #1608

Merged

coderabbitai Bot mentioned this pull request Feb 14, 2026

feat: adopt DAG schema to snake_case #1670

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor: deprecate DAG-level `maxActiveRuns` and enhance queue stability#1598

refactor: deprecate DAG-level `maxActiveRuns` and enhance queue stability#1598
yohamta0 merged 9 commits into
mainfrom
deprecate-local-queue-active-run

yohamta0 commented Jan 22, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jan 22, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

yohamta0 commented Jan 22, 2026

Uh oh!

coderabbitai Bot commented Jan 22, 2026

Uh oh!

Uh oh!

codecov Bot commented Jan 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

yohamta0 commented Jan 22, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yohamta0 commented Jan 22, 2026

Uh oh!

coderabbitai Bot commented Jan 22, 2026

Uh oh!

Uh oh!

codecov Bot commented Jan 22, 2026

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

yohamta0 commented Jan 22, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jan 22, 2026 •

edited

Loading