1892: feat: allow an external semaphore for executor task concurrency

[martin-augment]

1892: To review by AI

[coderabbitai]

Warning

Review limit reached

@martin-augment, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 1 minute

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6eb36608-bb0d-4d75-a56c-74a41c31544a

📥 Commits

Reviewing files that changed from the base of the PR and between 027a6e9 and 7170edf.

📒 Files selected for processing (7)

• .cursor/rules.md
• AGENTS.md
• CLAUDE.md
• ballista/executor/src/execution_loop.rs
• ballista/executor/src/executor_process.rs
• ballista/executor/src/standalone.rs
• examples/examples/mtls-cluster.rs

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch pr-1892-2026-06-29-11-59-22

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[gemini-code-assist]

Code Review

This pull request introduces an optional available_task_slots semaphore parameter to the poll_loop function in the Ballista executor, allowing callers to share or monitor task concurrency limits. Call sites in executor_process.rs, standalone.rs, and mtls-cluster.rs have been updated to pass None by default. Additionally, AI agent rules and configuration files (.cursor/rules.md, AGENTS.md, and CLAUDE.md) were added. There are no review comments, and I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[augmentcode]

🤖 Augment PR Summary

Summary: This PR extends the executor pull-based polling loop to optionally use a caller-provided semaphore for task concurrency.

Changes:

• Added an available_task_slots: Option<Arc<Semaphore>> parameter to execution_loop::poll_loop
• Defaulted to creating an internal semaphore sized to executor_specification.task_slots when None is passed
• Updated executor process startup, standalone executor, and the mTLS cluster example to pass None

Technical Notes: This enables sharing/observing task-slot permits externally (e.g., for cross-loop coordination or busy-state visibility).

_{🤖 Was this summary useful? React with 👍 or 👎}

[augmentcode]

Review completed. 2 suggestions posted.

Comment augment review to trigger a new review at any time.

[augmentcode]

ballista/executor/src/execution_loop.rs:69 — Now that available_task_slots can be provided externally, the later acquire().await.unwrap() / acquire_owned().await.unwrap() calls can panic if the semaphore is closed by the caller. Consider handling AcquireError and exiting (or retrying) gracefully to avoid crashing the executor.

Severity: medium

Other Locations

• ballista/executor/src/execution_loop.rs:97
• ballista/executor/src/execution_loop.rs:144

_{🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.}

[augmentcode]

ballista/executor/src/execution_loop.rs:62 — The docs mention sharing this semaphore "across poll loops"; if multiple loops run concurrently, each loop will independently report num_free_slots based on available_permits(), which can lead to the scheduler over-assigning tasks relative to real capacity. It may be worth clarifying in the doc comment what coordination is expected to avoid double-counting free slots.

Severity: low

_{🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.}

[claude]

Code Review

This PR adds an available_task_slots: Option<Arc<Semaphore>> parameter to poll_loop, allowing callers to supply an external semaphore instead of always creating one internally. All existing call sites pass None, preserving backward behavior. The feature enables future use cases such as sharing a single concurrency gate across multiple poll loops or observing executor busy-state externally.

---

Findings (most severe first)

ballista/executor/src/execution_loop.rs:65 — Breaking public API change

poll_loop is pub async fn in the ballista-executor crate (a published library). Adding a required 4th argument to a public function is a semver-breaking change: any downstream caller compiled against the prior 3-argument signature will fail to compile. Consider wrapping the new parameter in a configuration struct (e.g. PollLoopOptions) so future parameters can be added without further breakage, or add a deprecation shim for the old 3-argument form.

---

ballista/executor/src/execution_loop.rs:84 — External semaphore capacity not validated against thread pool size

The DedicatedExecutor is always created with executor_specification.task_slots threads (line 93), regardless of the external semaphore's permit count. If a caller passes a semaphore with N > task_slots permits, the loop reports N free slots to the scheduler (line 112) and accepts N tasks, spawning all of them to a thread pool that can only run task_slots concurrently. The extra N - task_slots tasks queue inside DedicatedExecutor while their OwnedSemaphorePermits are already consumed — so available_permits() drops to 0 and the scheduler sees zero free slots even though those queued tasks haven't started. This breaks the permit/capacity accounting.

At minimum, add a debug assertion or return an error if the supplied semaphore's initial permits differ from task_slots. The doc-comment should also state this constraint explicitly.

---

ballista/executor/src/execution_loop.rs:97 and :144 — .unwrap() panics if external semaphore is closed

Both acquire().await.unwrap() (line 97) and acquire_owned().await.unwrap() (line 144) panic when the semaphore has been closed — acquire returns Err(AcquireError) in that case. With the internal semaphore (the None path), this is impossible because no external code holds a reference. With an external Arc<Semaphore>, the caller can call semaphore.close() — e.g., as a graceful-shutdown signal — which causes poll_loop to panic rather than return a clean Err. Replace .unwrap() with ? after mapping the error, or return early with a log line:

// line 97
let permit = available_task_slots.acquire().await
    .map_err(|_| BallistaError::Internal("task slot semaphore closed".into()))?;

// line 144
let permit = available_task_slots.clone().acquire_owned().await
    .map_err(|_| BallistaError::Internal("task slot semaphore closed".into()))?;

---

ballista/executor/src/execution_loop.rs:97 — Zero-permit semaphore causes a silent permanent deadlock

If a caller passes Arc<Semaphore::new(0)>, the acquire().await on line 97 blocks forever on the very first loop iteration. No permits are ever released (no task can start, so no drop(permit) on line 350 ever fires). The loop hangs silently with no log output and no error. A guard at the start of the function would catch this:

assert!(available_task_slots.available_permits() > 0 || ..., "semaphore must have > 0 permits");

---

Minor cleanup

ballista/executor/src/executor_process.rs:508 — The inline comment // available_task_slots: use internal semaphore duplicates what the poll_loop doc-comment already says. The other two call sites (standalone.rs, mtls-cluster.rs) pass None without a comment; this asymmetry adds noise without adding information.

ballista/executor/src/execution_loop.rs:144 — available_task_slots.clone().acquire_owned() clones the Arc (atomic refcount increment + decrement) on every dispatched task. Hoist the clone before the for task in tasks loop:

let slots = Arc::clone(&available_task_slots);
// ... inside loop:
let permit = Arc::clone(&slots).acquire_owned().await ...;