-
-
Notifications
You must be signed in to change notification settings - Fork 5
Description
Problem
In getFromCache, the implementation returns the first readFile Promise without awaiting it, so the function exits on the first root and never tries subsequent roots. When the first root (e.g., local tmp) misses but a later root (e.g., s3://...) has the object, the code won’t attempt it, leading to unnecessary rebuilds/writes and higher RTT.
Affected Code
File: src/service/registry/cache-registry/cache-registry-service.ts
Function: getFromCache
Current:
private async getFromCache(path: MaybeArray<string>): Promise<Buffer | undefined> {
for (const currentPath of ensureArray(path)) {
const match = this.fileSystem.readFile(currentPath);
if (match != null) return match;
}
return undefined;
}Proposed Fix (minimal change)
Await readFile sequentially and return on the first hit:
for (const currentPath of ensureArray(path)) {
const match = await this.fileSystem.readFile(currentPath);
if (match != null) return match;
}Behavior & Compatibility
- Single-root setups: no change.
- Multi-root setups (e.g., local + S3): fixes skipping later roots, aligning with intended semantics.
Performance Impact
- Local hits: unchanged.
- Local miss but remote hit: now correctly hits remote and avoids rebuild/write, reducing cold-path RTT.
- Network requests: only performed when needed (true local-first behavior).
How to Verify
- Configure constant.path.cacheRoots to include both a local directory and an S3 path (e.g., s3://bucket/prefix...).
- Ensure the object exists only in S3.
- Call the cache read path.
- Before fix: will miss and trigger rebuild/write.
- After fix: will hit S3 and return quickly.
Optional note (S3 optimization)
S3 readFile can use GetObject and return undefined on NoSuchKey/404 to avoid an extra HeadObject RTT. Enabling keep-alive for the Node HTTP agent can further reduce connection overhead.