Skip to content

perf: fix CPU spike — add early $match before $unwind in CNRepos pipeline#203

Merged
snomiao merged 4 commits into
mainfrom
perf/fix-cnrepos-cpu-spike
Apr 24, 2026
Merged

perf: fix CPU spike — add early $match before $unwind in CNRepos pipeline#203
snomiao merged 4 commits into
mainfrom
perf/fix-cnrepos-cpu-spike

Conversation

@snomiao

@snomiao snomiao commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Investigated 2026-04-23T22:12Z Atlas CPU spike (98.5% CPU on comfyorg-prod-efe3368)
  • Query Profiler confirmed: comfy-pr.CNRepos reads → 737 ops × avg 5.49 min = 67.44 CPU-hours in 10.5h window; Uses Index: No
  • Root cause: baseCRPullStatusPipeline() scanned all CNRepos docs, $unwinded every crPulls.data entry, then filtered — processing everything before discarding irrelevant documents

Fixes:

  • Add early $match { crPulls.data: { $elemMatch: { "comments.state": "ok" } } } before $unwind to skip repos with no fetched PR comments
  • Add supporting multikey index on crPulls.data.comments.state (scalar Task field — using comments.data would fail with "Cannot index parallel arrays")
  • Add sparse indexes on GithubActionUpdateTask.error and .pullRequestMessage (Performance Advisor flagged 4310-doc regex scans at 0.42/hr)

Test plan

  • Monitor Atlas Query Profiler after deploy — CNRepos read avg execution time should drop from 5.49 min toward <1s
  • Verify idx_crpulls_comments_state index builds successfully in Atlas (no "parallel arrays" error)
  • Dashboard /details page still renders correctly with the filtered pipeline
  • analyzePullsStatus() still returns same data (the post-$unwind .match({ "crPulls.data.comments.data": { $exists: true } }) is kept as safety net)

Investigation Evidence

Atlas Query Profiler (2026-04-23 22:00 JST → 2026-04-24 08:30 JST):

Collection Op Count Mean Sum
CNRepos read 737 5.49 min 67.44 hr
CNRepos write 108 319 ms 34.50 s

Actual query logged: aggregate CNRepos → $set → $unwind "$crPulls.data" → $match → ... (Num Yields: 5,043, Uses Index: No)

Reviewed by Gemini 2.5 Flash ✅, Gemini 2.5 Pro ✅ (flagged parallel-array index issue → fixed), Codex gpt-5.4 ✅

🤖 Generated with Claude Code

…line

Investigated 2026-04-23T22:12Z Atlas CPU spike (98.5% CPU). Query Profiler
showed comfy-pr.CNRepos reads caused 67.44 CPU-hours in 10.5h window:
737 ops × avg 5.49 min, with Uses Index: No.

Root cause: baseCRPullStatusPipeline() in analyzePullsStatus.ts ran a
full-collection scan, then $unwind on crPulls.data array, then $match —
processing every document and every crPull entry before any filtering.

Fix:
- Add early $match { crPulls.data: { $elemMatch: { comments.state: "ok" } } }
  before $unwind to skip repos with no fetched PR comments
- Add supporting multikey index on crPulls.data.comments.state (scalar Task
  field — avoids "Cannot index parallel arrays" from using comments.data)
- Add sparse indexes on GithubActionUpdateTask.error and .pullRequestMessage
  (Performance Advisor flagged 4310-doc scans for regex queries)

Reviewed by: Gemini 2.5 Flash (LGTM), Gemini 2.5 Pro (flagged parallel-array
index issue → fixed), Codex gpt-5.4 (no issues with specific changes)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 24, 2026 15:40
@vercel

vercel Bot commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
comfy-pr Ready Ready Preview, Comment Apr 24, 2026 3:59pm

Request Review

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses a production MongoDB Atlas CPU spike by reducing CNRepos aggregation work (filtering earlier in the pipeline) and adding supporting indexes for the affected query patterns.

Changes:

  • Add an early $match (via $elemMatch on crPulls.data.comments.state) before $unwind in baseCRPullStatusPipeline() to avoid scanning/unwinding repos that have no fetched PR comments.
  • Add a multikey index on CNRepos for crPulls.data.comments.state to support the early filter without hitting the “parallel arrays” limitation.
  • Add sparse indexes on GithubActionUpdateTask.error and .pullRequestMessage intended to reduce scans flagged by Performance Advisor.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/analyzePullsStatus.ts Adds early $match stage before $unwind to reduce work in the CNRepos aggregation pipeline.
src/CNRepos.ts Adds a multikey index on crPulls.data.comments.state to support the new early filter.
src/GithubActionUpdateTask/GithubActionUpdateTask.ts Adds sparse indexes on error and pullRequestMessage for query performance.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/GithubActionUpdateTask/GithubActionUpdateTask.ts Outdated
Comment thread src/GithubActionUpdateTask/GithubActionUpdateTask.ts Outdated
Comment thread src/CNRepos.ts Outdated
…limit

Codex review flagged that pullRequestMessage values are ~1.6KB (generated
from the PR template), which can exceed MongoDB's B-tree key size limit and
cause write failures with "key too large to index" error.

Keep only the error field index (short strings, safe to index).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot review flagged that .catch(() => {}) silently hides index creation
failures, masking schema/parallel-array issues in production. Change to
.catch(console.error) so startup logs show problems while still being
non-blocking (performance-only indexes, not correctness constraints).

Also clarify that the error index primarily helps { error: { $exists: false } }
(the main task scanner query), not the unanchored regex queries.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@snomiao

snomiao commented Apr 24, 2026

Copy link
Copy Markdown
Contributor Author

Addressed Copilot review comments (1f7c609):

Comment 1 & 3 (.catch silencing): Changed .catch(() => {}) to .catch(console.error) on both the GithubActionUpdateTask.error index and the CNRepos.idx_crpulls_comments_state index. Failures now surface in startup logs while remaining non-blocking. These are performance-only indexes, permitted as non-awaited per CLAUDE.md.

Comment 2 (regex + B-tree): Acknowledged — unanchored regex like /\bRETRYABLE\b/ won't use the index. Added clarifying comment that the error index primarily benefits the { error: { $exists: false } } query (the main task scanner at line 92 of updateGithubActionTaskList.ts), not the regex queries. Refactoring those queries to use structured error codes is a separate improvement.

Gemini 2.5 Pro review flagged that a sparse index cannot support
{ error: { $exists: false } } queries — sparse indexes exclude docs
where the field is missing, so $exists:false still requires a full scan.

Use a regular (non-sparse) index so all documents are included,
enabling the main task scanner query to use the index.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@snomiao
snomiao enabled auto-merge (squash) April 24, 2026 16:01
@snomiao
snomiao merged commit 30e7d47 into main Apr 24, 2026
5 checks passed
@snomiao
snomiao deleted the perf/fix-cnrepos-cpu-spike branch April 24, 2026 16:02
@github-actions

Copy link
Copy Markdown

🎉 This PR is included in version 1.4.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants