perf(server): skip usage scans when access keys have no data limits - #1712
Open
OskarEichler wants to merge 2 commits into
Open
OskarEichler wants to merge 2 commits into
OskarEichler wants to merge 2 commits into
Conversation
Greptile SummaryThis 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.
Confidence Score: 5/5The 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.
|
| 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Data-limit enforcement scans thirty days of Prometheus usage even when no key has an effective limit.
Changes
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 --checkpassed. 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
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.