Skip to content

perf(server): skip usage scans when access keys have no data limits - #1712

Open
OskarEichler wants to merge 2 commits into
OutlineFoundation:masterfrom
OskarEichler:codex/unlimited-key-metrics
Open

OskarEichler wants to merge 2 commits into
OutlineFoundation:masterfrom
OskarEichler:codex/unlimited-key-metrics

Conversation

@OskarEichler

@OskarEichler OskarEichler commented Aug 27, 2026

Copy link
Copy Markdown

Why

Data-limit enforcement scans thirty days of Prometheus usage even when no key has an effective limit.

Changes

  • Skip the usage query when neither per-key nor default limits apply.
  • Still run the enforcement loop so removing the last limit unblocks previously limited keys and updates the proxy.

Verification

  • After splitting, all 65 existing manager-metrics, shared-metrics, access-key, and file-operation specs passed again on this isolated branch under Node 22.23.2.

  • Existing access-key management specs passed.

  • Controlled source checks verified no usage query for unlimited keys while previously blocked keys are re-enabled.

  • Patch isolation and git diff --check passed. Recombining all focused patches reproduces the reviewed source changes exactly.

  • No test/spec files were added or modified; controlled probe drivers are kept outside the repository.

Compatibility and remaining validation

  • Queries remain unchanged when any effective limit exists; zero-byte limits are still limits.
  • No production query-duration benchmark was performed.

Full npm installation/build checks remain incomplete: npm 12 rejected existing lockfile Git dependencies (EALLOWGIT). Isolated registry-only tooling was used without disabling that safeguard.

Scope

This is one focused part of the reliability review, based directly on upstream master; it does not include the other review patches. No installed client, live VPN/DNS setting, or production service was changed by this patch.

Second review — 2026-08-27

Coalesce overlapping data-limit enforcement into one worker, with a follow-up pass for intervening changes. Handle background enforcement failures through the existing logging boundary. Keep failed proxy updates pending so unchanged limit status does not silently prevent retry. Preserve scan skipping and re-enabling keys when the final limit is removed.

Twenty concurrent requests completed as two serialized passes; a failed proxy update retried despite unchanged computed status; removing the last limit restored the key without a Prometheus scan. All 65 selected existing server specs and targeted TypeScript checks passed on the isolated branch.

All touched files and nearby callers were reviewed again. Each focused patch was also checked in combination with the other seven patches for this repository. External probe drivers remain outside the repositories; no test/spec files were added or modified. Existing full-build and platform-validation limitations above still apply.

@OskarEichler
OskarEichler requested a review from fortuna as a code owner August 27, 2026 18:49
@greptile-apps

greptile-apps Bot commented Aug 27, 2026

Copy link
Copy Markdown

Greptile Summary

This PR avoids an expensive 30-day Prometheus usage query when no access key has an effective data limit, while retaining the enforcement loop so previously blocked keys are re-enabled after the final limit is removed.

  • Detects whether any per-key or default data limit applies.
  • Queries usage only when at least one effective limit exists.
  • Continues reconciling access-key eligibility and proxy configuration in the unlimited case.

Confidence Score: 5/5

The PR appears safe to merge, with no concrete correctness or security regressions identified.

The usage query is skipped only when all keys lack both per-key and default limits, while the existing reconciliation loop still clears stale reached-limit state and updates the proxy when necessary.

Important Files Changed

Filename Overview
src/shadowbox/server/server_access_key.ts The new effective-limit predicate matches the existing limit-resolution semantics, preserves zero-byte limits, and safely skips usage collection only when every key is unlimited.

Reviews (1): Last reviewed commit: "perf(server): skip usage scans when acce..." | Re-trigger Greptile

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