fix(engine): wait for trigger slots on idempotent runs - #4755
Conversation
Idempotent triggers cannot fail-fast into Internal when the in-process slot limit is saturated; wait for a slot or return ResourceExhausted.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThe PR makes idempotent workflow triggers wait for an in-process trigger slot and preserves gRPC status errors returned through the trigger path.
Confidence Score: 4/5The PR should not merge until canceled idempotent trigger requests retain cancellation semantics instead of being returned as retryable capacity failures. The previously reported cancellation issue remains: acquireSlot wraps context cancellation together with ErrNoTriggerSlots, and both ingest implementations classify that combined error as ResourceExhausted. Files Needing Attention: internal/services/controllers/task/trigger/trigger.go, internal/services/admin/server_v1.go, internal/services/admin/v1/server.go Reviews (2): Last reviewed commit: "Merge branch 'main' into fix/idempotent-..." | Re-trigger Greptile |
| return ErrNoTriggerSlots | ||
| return func() { <-tw.semaphore }, nil | ||
| case <-ctx.Done(): | ||
| return nil, fmt.Errorf("%w: %w", ErrNoTriggerSlots, ctx.Err()) |
There was a problem hiding this comment.
When a client cancels an idempotent trigger while it is waiting for a slot, this error wraps both context.Canceled and ErrNoTriggerSlots, so both ingest callers return ResourceExhausted instead of Canceled, causing clients to treat a deliberately canceled request as a retryable capacity failure.
mrkaye97
left a comment
There was a problem hiding this comment.
lgtm pending greptile comment, also discussed offline the memory leak risk here
Description
Idempotent workflow triggers now wait for an available in-process trigger slot instead of failing immediately when the slot limit is saturated. If the request deadline expires while waiting, the API returns
ResourceExhaustedso clients can retry.Type of change
What's Changed
ResourceExhaustedinstead ofInternalChecklist
Changes have been:
🤖 AI Disclosure
I acknowledge that an LLM was used in the creation of this Pull Request, in accordance with Hatchet's AI_POLICY.md.
Details: Cursor Grok 4.6 wrote the slot-wait implementation, unit tests, and this PR description.