Skip to content

Conversation

@indiejames
Copy link
Contributor

@indiejames indiejames commented Oct 23, 2025

Jira Issue ID

HARMONY-2186

Description

Updates memory limit for OPERA_RTC_S1_BROWSE to fix out of memory issues.

Local Test Steps

Add the service to LOCALLY_DEPLOYED_SERVICES and verify using kubectl describe that the pod has a 2Gi memory limit

PR Acceptance Checklist

  • Acceptance criteria met
  • Tests added/updated (if needed) and passing
  • Documentation updated (if needed)
  • Harmony in a Box tested (if changes made to microservices or new dependencies added)

Summary by CodeRabbit

  • Bug Fixes

    • Increased memory for browse operations from 512Mi to 2Gi to prevent out-of-memory failures and improve stability.
  • Chores

    • Added a new maintenance entry across multiple service configurations to track an active issue with notes and an expiry date.
  • Documentation

    • Added a changelog entry documenting the memory increase and the related maintenance note.

@coderabbitai
Copy link

coderabbitai bot commented Oct 23, 2025

Walkthrough

Increased OPERA_RTC_S1_BROWSE_LIMITS_MEMORY from 512Mi to 2Gi, added a 2025-10-23 changelog entry documenting that change, and inserted a new top-level exemption entry "1109241" into multiple .nsprc files across packages and services.

Changes

Cohort / File(s) Summary
Changelog
ENV_CHANGELOG.md
Added 2025-10-23 entry noting OPERA_RTC_S1_BROWSE_LIMITS_MEMORY increased to 2Gi.
Environment defaults
services/harmony/env-defaults
Updated OPERA_RTC_S1_BROWSE_LIMITS_MEMORY from 512Mi to 2Gi.
NSPRC configs
packages/util/.nsprc, services/cron-service/.nsprc, services/query-cmr/.nsprc, services/work-failer/.nsprc, services/work-updater/.nsprc, services/harmony/.nsprc, services/service-runner/.nsprc, services/work-scheduler/.nsprc
Added top-level key "1109241" with { "active": true, "notes": "Will fix in HARMONY-2207", "expiry": "2026-02-01" } to each file; adjusted surrounding punctuation (commas/braces) to maintain valid JSON.

Sequence Diagram(s)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Verify JSON syntax and trailing commas in all modified .nsprc files.
  • Confirm the environment default value change is in the expected format/location in services/harmony/env-defaults.
  • Check ENV_CHANGELOG.md date/format consistency.

Possibly related PRs

Suggested reviewers

  • ygliuvt
  • chris-durbin
  • joeyschultz

Poem

🐇 I hopped through JSON, a new key to sow,
Gave memory some room so the services grow.
A changelog whisper, a tiny config cheer,
Rabbit got coding — Harmony's near! 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title "HARMONY-2186: Update memory limit for OPERA_RTC_S1_BROWSE" accurately reflects the primary change in this pull request. The main modification is increasing the OPERA_RTC_S1_BROWSE_LIMITS_MEMORY environment variable from 512Mi to 2Gi to address out-of-memory issues, which is clearly captured in the title. While the PR also includes secondary changes to multiple .nsprc security policy files, the title appropriately focuses on the primary objective without unnecessary detail, and a teammate scanning the commit history would understand the core change at a glance.
Description Check ✅ Passed The pull request description follows the required template structure with all sections properly filled out. It includes the Jira Issue ID (HARMONY-2186), a concise description of the change, clear and actionable local test steps, and a completed acceptance checklist with appropriate items marked. The description provides sufficient context for reviewers to understand what changed and why, and the test steps are specific enough to verify the change locally using kubectl.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch harmony-2186

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b068cd6 and 288b9b1.

📒 Files selected for processing (3)
  • services/harmony/.nsprc (1 hunks)
  • services/service-runner/.nsprc (1 hunks)
  • services/work-scheduler/.nsprc (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: build (22.14.x)
  • GitHub Check: compare-services
  • GitHub Check: build (22.14.x)
🔇 Additional comments (1)
services/harmony/.nsprc (1)

11-16: Separate security exemptions from infrastructure changes in dedicated PR.

The security exemption entry 1109241 is properly formatted and consistent with existing entries. However, verification confirms this change appears unrelated to the PR's stated objective of updating the OPERA_RTC_S1_BROWSE memory limit:

  • Node Security Platform advisory 1109241 is not available in public records (NSP was deprecated)
  • No codebase references explain why this exemption is needed in this PR
  • The internal ticket reference (HARMONY-2207) provides no visibility into the issue or its urgency

To maintain clear change tracking and PR scope, bundle this security exemption with other related security updates in a dedicated PR rather than combining it with infrastructure/memory configuration changes.


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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
ENV_CHANGELOG.md (1)

5-8: LGTM with minor grammar suggestion.

The changelog entry properly documents the memory limit increase to address out-of-memory issues.

Consider this minor grammar improvement:

-- OPERA_RTC_S1_BROWSE_LIMITS_MEMORY - Increased to 2Gi to fix out of memory issues.
+- OPERA_RTC_S1_BROWSE_LIMITS_MEMORY - Increased to 2Gi to fix out-of-memory issues.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e19159d and df16b73.

📒 Files selected for processing (2)
  • ENV_CHANGELOG.md (1 hunks)
  • services/harmony/env-defaults (1 hunks)
🧰 Additional context used
🪛 LanguageTool
ENV_CHANGELOG.md

[grammar] ~7-~7: Use a hyphen to join words.
Context: ...ITS_MEMORY - Increased to 2Gi to fix out of memory issues. ## 2025-10-14 ### Cha...

(QB_NEW_EN_HYPHEN)


[grammar] ~7-~7: Use a hyphen to join words.
Context: ..._MEMORY - Increased to 2Gi to fix out of memory issues. ## 2025-10-14 ### Change...

(QB_NEW_EN_HYPHEN)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: build (22.14.x)
  • GitHub Check: compare-services
  • GitHub Check: build (22.14.x)
🔇 Additional comments (1)
services/harmony/env-defaults (1)

549-549: LGTM! Memory limit increase appropriately addresses OOM issues.

The increase from 512Mi to 2Gi provides necessary headroom to prevent out-of-memory errors. The gap between REQUESTS_MEMORY (512Mi) and LIMITS_MEMORY (2Gi) is consistent with patterns used by other services in this configuration file and allows Kubernetes to schedule pods efficiently while providing burst capacity.

@indiejames indiejames merged commit 0518806 into main Oct 30, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants