perf(storage): avoid extension discovery for internal JSON resources - #2212
Draft
jeswr wants to merge 8 commits into
Draft
perf(storage): avoid extension discovery for internal JSON resources#2212jeswr wants to merge 8 commits into
jeswr wants to merge 8 commits into
Conversation
…ions ExtensionBasedMapper.mapUrlToDocumentPath did a full readdir(folder) on every document read without a known content-type, to discover the file's extension. For large directories - e.g. the internal account-index storage with tens of thousands of flat entries - this is O(folder size) per read, dominating CPU under auth/OIDC load: every login findByEmail / client lookup scanned the whole index directory (positive AND negative lookups). - Probe the exact file and the common `$.<ext>` variants via `stat` (O(1)) before falling back to readdir, so positive lookups avoid the scan. - Skip the readdir fallback entirely for the reserved `/.internal/` storage, whose resources are always JSON, so negative index lookups (e.g. a login for a non-existent email) are O(1) too. Pod resources keep the readdir fallback since they may use arbitrary extensions. Behaviour-preserving (verified by algorithm-equivalence tests across exact match, common/uncommon extension, empty name, directory entries; internal reads confirmed JSON-only). Measured on a production instance: node CPU 232% -> ~48% and stable, index directory scans ~90/3s -> 0, pod root read 3.08s -> ~80ms. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR improves performance of file-based account storage lookups by ensuring internal /.internal/ resources are mapped with known document/metadata content types (JSON + Turtle), avoiding directory scans for extension discovery when resolving internal resources.
Changes:
- Introduces
ContainerContentTypeMapperto inject configured content types for resources under a specific container. - Updates suffix and subdomain identifier configurations to wrap the existing extension-based mappers with
ContainerContentTypeMapperfor/.internal/. - Adds unit coverage to ensure internal lookups (including encoded
%2Einternal) do not trigger directory reads.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/storage/mapping/ContainerContentTypeMapper.test.ts | Adds unit tests validating correct content-type injection, rejection of mismatched content types, and no readdir calls for internal resources. |
| src/storage/mapping/ContainerContentTypeMapper.ts | Adds a wrapper mapper that enforces configured document/metadata content types within a target container before delegating to the underlying mapper. |
| src/index.ts | Exports the new mapper from the public entrypoint. |
| config/util/identifiers/suffix.json | Wraps ExtensionBasedMapper with ContainerContentTypeMapper to treat /.internal/ as JSON with Turtle metadata. |
| config/util/identifiers/subdomain.json | Wraps SubdomainExtensionBasedMapper with ContainerContentTypeMapper to apply the same /.internal/ mapping in subdomain setups. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
jeswr
marked this pull request as draft
August 19, 2026 11:49
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.
Problem
Internal account storage uses JSON resources, but the file identifier mapper was not given that content type. Resolving an internal resource therefore scanned its parent directory to discover the extension, making account-index lookups O(n).
On a production instance with roughly 20,000 accounts this caused sustained 232% Node.js CPU and about 2,000
getdentscalls every two seconds.Change
ContainerContentTypeMapperto supply configured document and metadata types within a container./.internal/resources as JSON with Turtle metadata for suffix and subdomain setups.Production result
getdents/ 2sPerformance check
For 500 lookups in a simulated 20,000-entry account directory, five runs gave:
mainThe unit test also asserts that internal lookups, including an encoded
%2Einternalpath, never callreaddir.Tests