Skip to content

Fix: Timeout while listing CAS mappings#2903

Closed
sonarly[bot] wants to merge 1 commit into
mainfrom
sonarly-16915-n1-database-queries-in-cas-mapping-lookup-cause
Closed

Fix: Timeout while listing CAS mappings#2903
sonarly[bot] wants to merge 1 commit into
mainfrom
sonarly-16915-n1-database-queries-in-cas-mapping-lookup-cause

Conversation

@sonarly

@sonarly sonarly Bot commented Mar 20, 2026

Copy link
Copy Markdown

Automated fix for bug 16915

Severity: medium

Summary

The FindByDigest method in the CAS mapping data layer executes 2 sequential database queries per CAS mapping result (workflow_run + workflow) inside a loop, creating an N+1 query pattern that causes context deadline exceeded errors when a digest has many mappings or the database is under load.

User Impact

Users and API tokens attempting to download artifacts via CAS credentials fail intermittently with internal server errors when the database is slow or when a digest maps to many workflow runs. The 1117 goroutines indicate request pileup from cascading timeouts.

Root Cause

The CASMappingRepo.FindByDigest method in app/controlplane/pkg/data/casmapping.go has an N+1 query problem that causes context deadline exceeded under load.

Proximate cause:

At line 71-99, FindByDigest first queries all CAS mappings for a given digest, then for each mapping, calls IsPublic() which executes two additional sequential database queries:

// Line 82-83: For EACH mapping, 2 more DB queries
for _, m := range mappings {
    public, err := r.IsPublic(ctx, r.data.DB, m.WorkflowRunID)

IsPublic (line 125-143) runs:

  1. WorkflowRun.Query().Where(workflowrun.ID(runID)).First(ctx) — query workflow_run table
  2. Workflow.Query().Where(workflow.ID(wr.WorkflowID)).First(ctx) — query workflow table

For a digest with N mappings, this results in 1 + 2N database queries all executed sequentially. When the gRPC request context has a timeout, these cumulative queries can exhaust it, especially when the database is under load or responding slowly.

The error chain confirms this path exactly:

  • "failed to get workflow run: context deadline exceeded"IsPublic line 134
  • "failed to check if workflow is public: ..."FindByDigest line 89
  • "failed to list cas mappings: ..."FindCASMappingForDownloadByOrg line 137

Note on Sentry context mismatch: The Sentry "Request" context shows /controlplane.v1.StatusService/Statusz, but the stack trace clearly shows CASCredentialsService.Get. This is because sentry.ConfigureScope() (in sentrycontext/sentry_context.go line 54) modifies the global hub scope, not a per-request scope. A concurrent Statusz request overwrote the scope context before the CASCredentials error was captured.

Triggering cause:

The exact trigger is unknown without production database metrics and traffic data. The error occurred on the day v1.83.0 was deployed (2026-03-20). With 1117 goroutines running, the server was experiencing significant request pileup. The N+1 query pattern has existed since commit 2b136034 (2024-11-23), meaning it requires a specific condition to trigger — either a digest with many CAS mappings, database latency increase, or increased concurrent requests to the CASCredentialsService.Get endpoint. The deploy itself (which only changed Helm chart versions, not application code) could have caused a brief spike in readiness probe + client reconnection traffic that saturated the DB connection pool.

The fix should batch the workflow public status check — either by JOINing workflow_run and workflow tables in the initial query, or by collecting all workflow_run IDs and making a single batched query.

Introduced by: migmartri on 2024-11-23 in commit 2b13603

fix: improve cas-mapping performance (#1571)

Suggested Fix

Replaced the N+1 query pattern in FindByDigest with a new batchIsPublic method that resolves the public status of all CAS mappings in exactly 2 batched queries instead of 2*N sequential queries.

Before: For each CAS mapping returned by FindByDigest, IsPublic was called individually, executing 2 sequential DB queries per mapping (one for workflow_run, one for workflow). With N mappings, this resulted in 1 + 2N total queries, all sequential within the request context's deadline.

After: batchIsPublic collects all unique workflow run IDs, fetches them in a single WHERE id IN (...) query, then fetches all referenced workflows in another single query. Total: 3 queries regardless of mapping count.

The behavioral contract is preserved:

  • Mappings with nil workflow_run_idpublic = false
  • Mappings whose workflow run or workflow has been deleted → skipped (same as the previous ent.IsNotFound → continue behavior)
  • The single-lookup IsPublic method is retained for findByID which operates on a single mapping

No interface, type, or schema changes. No auto-generated files modified.


Generated by Sonarly

… N+1 queries

https://sonarly.com/issue/16915?type=bug

The `FindByDigest` method in the CAS mapping data layer executes 2 sequential database queries per CAS mapping result (workflow_run + workflow) inside a loop, creating an N+1 query pattern that causes context deadline exceeded errors when a digest has many mappings or the database is under load.

Fix: Replaced the N+1 query pattern in `FindByDigest` with a new `batchIsPublic` method that resolves the public status of all CAS mappings in exactly 2 batched queries instead of 2*N sequential queries.

**Before:** For each CAS mapping returned by `FindByDigest`, `IsPublic` was called individually, executing 2 sequential DB queries per mapping (one for `workflow_run`, one for `workflow`). With N mappings, this resulted in 1 + 2N total queries, all sequential within the request context's deadline.

**After:** `batchIsPublic` collects all unique workflow run IDs, fetches them in a single `WHERE id IN (...)` query, then fetches all referenced workflows in another single query. Total: 3 queries regardless of mapping count.

The behavioral contract is preserved:
- Mappings with nil `workflow_run_id` → `public = false`
- Mappings whose workflow run or workflow has been deleted → skipped (same as the previous `ent.IsNotFound → continue` behavior)
- The single-lookup `IsPublic` method is retained for `findByID` which operates on a single mapping

No interface, type, or schema changes. No auto-generated files modified.
@migmartri migmartri closed this Mar 20, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant