perf: cache full HistoryQueryResult + add edge cache headers#99
Open
hubsmoke wants to merge 37 commits into
Open
perf: cache full HistoryQueryResult + add edge cache headers#99hubsmoke wants to merge 37 commits into
hubsmoke wants to merge 37 commits into
Conversation
fix hex pad issue
Access pattern data
Revert "Access pattern data"
promote prod
…rn-data upgrade main
promote main
fix resolution for data files, error when can't find data
no superfluous return data structure
fix dpid resolution for maritime assets
fix resolution bug for non external cids
fix resolution behavior for external datasets
promote main
promote main
correct config
Push last two patches to `main`
promote main dpid resolver
Promote to main
promote main
promote main
fallback flight url
print CERAMIC_FLIGHT_URL
uncomment .env.example
Promote to main
Promote optimised dPID routes
Push FAIR changes to prod
Two optimizations targeting warm-path latency on /api/v2/resolve/dpid/{n}.
Today's warm hit takes ~3s because ceramic.loadStream(streamID) runs
on every request to enumerate the commit log — even though individual
commits are already Redis-cached. Public HTTP responses also have no
Cache-Control header, so Cloudflare doesn't cache them.
Changes (perf):
- Wrap getCodexHistory in a Redis cache for the entire HistoryQueryResult
(key: resolver-${DPID_ENV}-history-full-${streamId}). On hit, skip
ceramic.loadStream entirely. Per-commit cache below remains as cold-path
fallback. Exported getKeyForHistoryFull so future push-invalidation can
delete it.
- Add CACHE_TTL_HISTORY_FULL config (default 60s, env-tunable). Short by
design — bounds publish→visibility staleness since the resolver has
no push-invalidation path today.
- Set Cache-Control: public, s-maxage=60, stale-while-revalidate=300 on
successful resolveDpidHandler responses. Errors stay uncached.
Drive-by (unblocks pre-commit on this branch):
- eslint-disable comments on 4 pre-existing lint errors in getIpfsFolder.ts
(1 unused import referenced in commented-out code, 3 deliberate `any`
types around dynamic IPFS DAG fetches).
- Prettier auto-format on dpids.ts (trailing comma in generic) and
pagination.spec.ts (string collapse).
Expected impact: warm TTFB drops from 2.8-3.2s to <100ms (Cloudflare HIT)
or <200ms (Redis HIT). Cold-cold unchanged (~3s, Ceramic stream load).
Publish→visibility window: ≤60s.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR enhances API caching capabilities by introducing full-history Redis caching for history queries, adding Cloudflare edge caching to DPID resolution, and providing supporting configuration and linting updates across multiple API and utility files. ChangesCaching Enhancement & Infrastructure
Sequence DiagramsequenceDiagram
participant Client
participant APIHandler as API Handler<br/>(history)
participant Redis
participant Ceramic
participant Flight
participant CloudflareEdge as Cloudflare Edge
Client->>APIHandler: GET /history/:streamId
APIHandler->>Redis: Check full-history cache<br/>(getKeyForHistoryFull)
Redis-->>APIHandler: Cache hit or miss
alt Cache Hit
APIHandler-->>Client: Return cached result
else Cache Miss
APIHandler->>Ceramic: Query ceramic history
Ceramic-->>APIHandler: History data
alt Ceramic Fallback
APIHandler->>Flight: Query flight client<br/>(backup source)
Flight-->>APIHandler: Full history result
end
APIHandler->>Redis: Store result with<br/>CACHE_TTL_HISTORY_FULL
Redis-->>APIHandler: Cache stored
APIHandler->>CloudflareEdge: Response with<br/>Cache-Control headers
CloudflareEdge->>Client: Edge-cached response
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Microsoft Presidio Analyzer (2.2.362)src/api/v2/data/getIpfsFolder.tsMicrosoft Presidio Analyzer failed to scan this file Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/api/v2/data/getIpfsFolder.ts (1)
9-10: 💤 Low valueConsider removing the unused
isRawCodecCidimport instead of suppressing the lint warning.The
isRawCodecCidfunction is imported but only referenced in commented-out code (lines 422-434). If that code path is intentionally disabled, removing the import would be cleaner than suppressing the lint rule.Proposed fix
-// eslint-disable-next-line `@typescript-eslint/no-unused-vars` -import { hackyTsizeIsDir, isRawCodecCid, magicIsUnixFsDir, type PbLink } from "../../../util/ipfs.js"; +import { hackyTsizeIsDir, magicIsUnixFsDir, type PbLink } from "../../../util/ipfs.js";🤖 Prompt for 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. In `@src/api/v2/data/getIpfsFolder.ts` around lines 9 - 10, Remove the unused isRawCodecCid import from the import list at the top of getIpfsFolder.ts (the line currently importing hackyTsizeIsDir, isRawCodecCid, magicIsUnixFsDir, PbLink); keep the needed imports (hackyTsizeIsDir, magicIsUnixFsDir, PbLink) and drop isRawCodecCid (and then remove the accompanying eslint-disable-next-line `@typescript-eslint/no-unused-vars` comment if it only existed to silence this import). Also verify any commented-out code that references isRawCodecCid (around the block currently commented on) is either restored or left commented intentionally; if left commented, ensure no remaining unused import warnings.src/api/v2/resolvers/dpid.ts (1)
96-99: 💤 Low valueConsider extracting the hardcoded TTL values to maintain consistency with Redis cache TTL.
The
s-maxage=60is hardcoded here whileCACHE_TTL_HISTORY_FULLis defined as a configurable constant (defaulting to 60s). IfCACHE_TTL_HISTORY_FULLis changed via environment variable, the edge cache would remain at 60s, potentially causing inconsistent staleness behavior between Redis and Cloudflare layers.Proposed fix to use config constant
+import { CACHE_TTL_HISTORY_FULL } from "../../../util/config.js"; + // Cloudflare edge caching — only on success path (errors above remain uncached). -// s-maxage=60 caps shared-cache freshness; SWR allows up to 5min stale-while-revalidate. -res.set("Cache-Control", "public, s-maxage=60, stale-while-revalidate=300"); +// s-maxage caps shared-cache freshness; SWR allows up to 5min stale-while-revalidate. +res.set("Cache-Control", `public, s-maxage=${CACHE_TTL_HISTORY_FULL}, stale-while-revalidate=300`);🤖 Prompt for 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. In `@src/api/v2/resolvers/dpid.ts` around lines 96 - 99, The Cache-Control header in the DPID resolver is using a hardcoded s-maxage=60 which can drift from the configurable CACHE_TTL_HISTORY_FULL constant; update the response header construction in src/api/v2/resolvers/dpid.ts (where res.set(...) and return res.json(result) live) to interpolate/format the numeric value of CACHE_TTL_HISTORY_FULL instead of the literal 60 (and keep stale-while-revalidate as-is or derive it consistently if needed) so the Cloudflare edge TTL matches the Redis cache TTL.
🤖 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.
Nitpick comments:
In `@src/api/v2/data/getIpfsFolder.ts`:
- Around line 9-10: Remove the unused isRawCodecCid import from the import list
at the top of getIpfsFolder.ts (the line currently importing hackyTsizeIsDir,
isRawCodecCid, magicIsUnixFsDir, PbLink); keep the needed imports
(hackyTsizeIsDir, magicIsUnixFsDir, PbLink) and drop isRawCodecCid (and then
remove the accompanying eslint-disable-next-line
`@typescript-eslint/no-unused-vars` comment if it only existed to silence this
import). Also verify any commented-out code that references isRawCodecCid
(around the block currently commented on) is either restored or left commented
intentionally; if left commented, ensure no remaining unused import warnings.
In `@src/api/v2/resolvers/dpid.ts`:
- Around line 96-99: The Cache-Control header in the DPID resolver is using a
hardcoded s-maxage=60 which can drift from the configurable
CACHE_TTL_HISTORY_FULL constant; update the response header construction in
src/api/v2/resolvers/dpid.ts (where res.set(...) and return res.json(result)
live) to interpolate/format the numeric value of CACHE_TTL_HISTORY_FULL instead
of the literal 60 (and keep stale-while-revalidate as-is or derive it
consistently if needed) so the Cloudflare edge TTL matches the Redis cache TTL.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b231525a-9faa-422f-bf2a-149a09b473e3
📒 Files selected for processing (6)
src/api/v2/data/getIpfsFolder.tssrc/api/v2/queries/dpids.tssrc/api/v2/queries/history.tssrc/api/v2/resolvers/dpid.tssrc/util/config.tstest/util/pagination.spec.ts
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
3 participants
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.
Summary
Two optimizations targeting warm-path latency on
GET /api/v2/resolve/dpid/{n}(currently ~3s warm, even with Redis enabled).Why warm hits are slow today:
ceramic.loadStream(streamID)runs on every request ingetCodexHistoryto enumerate the commit log. Per-commit metadata is already Redis-cached, but the loadStream + enumerate work happens regardless. Public HTTP responses also have noCache-Controlheader, so Cloudflare can't cache them at the edge.Changes
getCodexHistory(keyresolver-${DPID_ENV}-history-full-${streamId}). On hit, return immediately and skipceramic.loadStreamentirely. Per-commit cache below remains as the cold-path fallback. ExportedgetKeyForHistoryFullso future push-invalidation (e.g. from desci-server publish flow) can delete it.CACHE_TTL_HISTORY_FULLenv var, default 60 seconds. Short by design — bounds publish→visibility staleness since the resolver has no push-invalidation today. Tunable without redeploy.Cache-Control: public, s-maxage=60, stale-while-revalidate=300on successfulresolveDpidHandlerresponses. Errors (400/404/500) remain uncached.Drive-by (unblocks pre-commit on this branch)
The repo's pre-commit eslint hook fails on 4 pre-existing errors in
src/api/v2/data/getIpfsFolder.ts(1 unused import referenced in commented-out code, 3 deliberateanytypes around dynamic IPFS DAG fetches). Added inlineeslint-disablecomments to unblock. Should be cleaned up properly in a follow-up. Also includes 2 trivial prettier auto-formats ondpids.tsandpagination.spec.tsfrom the same hook run.Rollout plan
This is the last of 3 perf PRs and has the largest blast radius. Companion PRs (in order):
dpid-rewrite-worker#2— preconnect (smallest, lowest risk)nodes-web-v2#1538— SSR resolver-unblock (largest user impact)Steps
mainwhen companion PRs have been live ≥30 min without regressions..github/workflows/build-dpid-resolver.yaml.kubernetes/deployment.staging.yamlviapushVersion.sh. Soak ≥30 min.kubernetes/deployment.prod.yaml.Pre-deploy checklist
REDIS_HOST/REDIS_PORTare set on staging + prod resolver pods (else cache is silently bypassed — code is null-safe).CACHE_TTL_HISTORY_FULL— leave at default 60s for first deploy, tune after measurement.Cache-Controlonbeta.dpid.org/api/*(default behavior should be fine, but confirm no override).Acceptance criteria
Staging soak (≥30 min before prod)
cf-cache-status: HITafter request 1.source: "redis-history-full"for warm hits.Production verification
Freshness validation (critical)
staging-resolver.desci.com/api/v2/resolve/dpid/{that_dpid}every 10s.CACHE_TTL_HISTORY_FULLbound).Rollback / mitigation
Risk level: medium-high. Touches the cache layer for every resolver consumer (search engines, citation tools, the SSR fallback path, third-party integrations).
Risks and mitigations
CACHE_TTL_HISTORY_FULLenv var without redeploy. Set to1for instant reads if needed.HistoryQueryResultredisService.getFromCache<T>JSON path — same asgetKeyForCommitcache that's been in prod. If a key fails to parse,getFromCachealready callsdel(key)to self-heal.beta.dpid.org/api/*(instant) + revert k8s image.setToCacheis fire-and-forget; concurrent requests during a cold window all runloadStreamonce each. Acceptable for this volume. SWR on the CF layer absorbs most of this.Rollback procedure
kubectl set image deployment/dpid-resolver dpid-resolver=<previous-tag> -n <ns>— ~30s rollout.KEYS resolver-*-history-full-*thenDEL(only if the cache itself caused issues; otherwise let TTLs expire naturally over 60s).Watch metrics for 1h post-deploy
/api/v2/resolve/*— threshold: >2x baseline.cf-cache-statusdistribution — HIT % climbing toward steady state.Test plan
npm run buildclean.npm test— 5 failures match baseline (pre-existing network-dependent tests hitting live Ceramic/registry; verified by stashing changes and re-running).🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
Performance
Code Quality