Skip to content

feat(server): OURIOS_COMPACTION_ENABLED to disable a pod's compaction sweep - #303

Merged
jensholdgaard merged 3 commits into
mainfrom
rfc0019-compaction-enable-flag
Jun 28, 2026
Merged

feat(server): OURIOS_COMPACTION_ENABLED to disable a pod's compaction sweep#303
jensholdgaard merged 3 commits into
mainfrom
rfc0019-compaction-enable-flag

Conversation

@jensholdgaard

@jensholdgaard jensholdgaard commented Jun 28, 2026

Copy link
Copy Markdown
Owner

What

Adds OURIOS_COMPACTION_ENABLED (default on) to ourios-server. When set to a falsey value (0/false/no/off), the process skips building and running the compaction sweep entirely. This is opt-out (unlike the opt-in OURIOS_RECEIVER_ENABLED / OURIOS_QUERIER_ENABLED roles).

Why

The binary unconditionally ran the background compaction sweep, so a multi-pod deployment (receiver StatefulSet + querier Deployment + compactor) had every pod sweeping on its interval. publish_cas makes concurrent sweeps correct (losers no-op), but it's redundant load — N pods each listing the bucket every interval. This flag lets the Helm chart disable compaction on the receiver + querier pods so a single dedicated compactor sweeps (the clean topology for the S3-native chart, workstream C).

Behavior / invariants

  • Default-on → no behavior change for existing single-process or default deployments.
  • When disabled, the process builds no Compactor and no audit sink; the run loop runs the receiver/querier (if enabled) + waits for SIGINT/SIGTERM (the compaction branch is a pending future).
  • Hazard docs: apply RFC maturity-model amendments #4 (small-file problem): unchanged for any deployment that keeps at least one sweeper running. Disabling compaction on every pod (no dedicated compactor) is an operator misconfiguration — the chart wires exactly one dedicated compactor. publish_cas (RFC 0009 §3.2 / RFC0013.3-4) keeps any concurrent sweeps safe regardless.
  • WAL / data-store wiring unchanged (RFC 0019).

Tests

build_config_compaction_is_opt_out (default + non-falsey → on; 0/false/no/off/trimmed → off); rfc0019_7 asserts default-on. cargo fmt, clippy -D warnings, cargo test -p ourios-server (20 passed), cargo doc -D warnings — all green.

Follow-on

The S3-native Helm chart (workstream C) sets OURIOS_COMPACTION_ENABLED=0 on the receiver + querier workloads and leaves it default-on for the dedicated compactor Deployment.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added an opt-out setting for background compaction, enabled by default and configurable through an environment variable.
    • Improved shutdown behavior so the server still stops cleanly whether compaction is enabled or disabled.
  • Bug Fixes

    • Clarified and tightened configuration handling for compaction timing and enablement values.
    • Updated defaults and checks so compaction behavior matches the documented environment settings.

@jensholdgaard
jensholdgaard requested a review from Copilot June 28, 2026 14:47
@coderabbitai

coderabbitai Bot commented Jun 28, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@jensholdgaard, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 42 minutes and 21 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

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 credits.

🚦 How do rate 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 see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8c3b9f5a-dd9a-47a6-a02a-8ca25869b34b

📥 Commits

Reviewing files that changed from the base of the PR and between b93b292 and 846a948.

📒 Files selected for processing (1)
  • crates/ourios-server/src/main.rs
📝 Walkthrough

Walkthrough

Adds compaction_enabled: bool to ServerConfig, controlled by a new OURIOS_COMPACTION_ENABLED env var that defaults to on. build_config gains a compaction_enabled_raw parameter with opt-out parsing. In main, the Compactor and ParquetAuditSink are constructed conditionally; the shutdown select! uses pending() for the disabled branch.

Changes

Compaction Opt-Out Control

Layer / File(s) Summary
ServerConfig field and build_config parsing
crates/ourios-server/src/main.rs
Adds compaction_enabled: bool to ServerConfig, extends build_config to accept compaction_enabled_raw with opt-out semantics (enabled unless explicitly falsey), and updates config_from_env to pass OURIOS_COMPACTION_ENABLED.
Conditional Compactor wiring in main
crates/ourios-server/src/main.rs
Constructs Compactor and ParquetAuditSink only when config.compaction_enabled is true; uses pending() for the disabled branch in the shutdown select!.
Test updates and new opt-out assertions
crates/ourios-server/src/main.rs
Updates all build_config call sites to the new signature and adds assertions verifying default-enabled behavior and explicit falsey token disabling.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 A flag in the env, a toggle so neat,
Compaction runs on, or steps back from its beat.
Default is true, unless told to rest,
"0", "false", or "no" — the rabbit knows best.
Pending forever when sleeping, no fuss,
The burrow stays tidy — leave shuffling to us!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the PR’s main change: adding an opt-out flag to disable compaction sweeps.
Description check ✅ Passed The description is substantive and covers summary, rationale, behavior, and tests, though it doesn’t follow the exact template headings.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch rfc0019-compaction-enable-flag

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.

…TION_ENABLED

The binary always ran the compaction sweep, so a multi-pod deployment had every
receiver/querier/compactor pod sweeping (publish_cas-safe but redundant object
listing each interval). Add OURIOS_COMPACTION_ENABLED (default on; opt-out via a
falsey value, unlike the opt-in receiver/querier roles): when off, the process
skips building/running the compactor entirely, so a deployment can disable it on
receiver+querier pods and run a single dedicated compactor. Default-on preserves
existing behavior; the small-file hazard (#4) is unchanged for any deployment
that keeps a sweeper running.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jensholdgaard
jensholdgaard force-pushed the rfc0019-compaction-enable-flag branch from b93b292 to 05444fe Compare June 28, 2026 14:49

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

Adds an opt-out switch for the server’s background compaction role so multi-pod deployments can run compaction on a single dedicated pod while other pods focus on receiver/querier roles.

Changes:

  • Introduces OURIOS_COMPACTION_ENABLED (default on) and threads it through config assembly.
  • Makes compactor/audit-sink construction conditional and keeps the main loop alive via a pending future when compaction is disabled.
  • Adds unit tests validating the opt-out behavior.

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

Comment thread crates/ourios-server/src/main.rs Outdated
Comment thread crates/ourios-server/src/main.rs Outdated

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/ourios-server/src/main.rs`:
- Around line 411-441: The compaction branch in main should emit subsystem
telemetry for both states and stop using stderr for hot-path failures. Update
the `compactor`/`compaction` flow to publish a Prometheus metric or gauge
indicating whether compaction is enabled or disabled for this pod, and make sure
the disabled `None` path records that state explicitly. Replace the `eprintln!`
calls inside `Compactor::run`’s callback with structured Ourios logging (using
the server’s logging path) so sweep errors and `compaction sweep failed` are
visible in telemetry with context.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 1b398417-a2b0-4307-a67a-43d4c88275cf

📥 Commits

Reviewing files that changed from the base of the PR and between 75f0139 and b93b292.

📒 Files selected for processing (1)
  • crates/ourios-server/src/main.rs

Comment thread crates/ourios-server/src/main.rs Outdated
Per review: build the compactor via an explicit if (clone audit_store only when
enabled, don't move store into a never-run closure); skip interval parsing when
compaction is disabled so a disabled pod won't fail on an unused knob; and log
the disabled state at startup so it's visible in a multi-pod rollout. Tests:
disabled config tolerates a bad interval.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread crates/ourios-server/src/main.rs Outdated
The slice-2d 'audit sink wired unconditionally' comment was left truncated and
contradicts the new compaction-enabled control flow; replace it with one comment
describing the conditional build.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@jensholdgaard
jensholdgaard merged commit e15d264 into main Jun 28, 2026
21 checks passed
@jensholdgaard
jensholdgaard deleted the rfc0019-compaction-enable-flag branch June 28, 2026 15:09
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.

2 participants