feat(insights): pyth metadata caching #2921
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
6 Skipped Deployments
|
alexcambose
changed the title
cprussin
left a comment
cprussin
left a comment
There was a problem hiding this comment.
Mostly looking really good. My main feedback is to get rid of typecasting wherever possible. Other than that I recommend adding some comments to explain why using internal fetches is necessary here and the steps you've tried to avoid doing that since it's not really intuitive why we landed on this architecture as a necessity, and it would be easy for someone to not understand it and try to refactor it away without realizing the issues
apps/insights/src/app/api/pyth/get-feeds-for-publisher/[publisher]/route.ts
Outdated
Show resolved
Hide resolved
apps/insights/src/app/api/pyth/get-feeds-for-publisher/[publisher]/route.ts
Outdated
Show resolved
Hide resolved
apps/insights/src/app/api/pyth/get-feeds-for-publisher/[publisher]/route.ts
Show resolved
Hide resolved
| return NextResponse.json({ error: "Invalid cluster" }, { status: 400 }); | ||
| } | ||
|
|
||
| let feeds = await getFeedsCached(cluster) as Omit<z.infer<typeof priceFeedsSchema>[number], 'price'>[]; |
There was a problem hiding this comment.
We should not typecast here, the typecast will be unnecessary if you just do this like this:
const feeds = await getFeedsCached(cluster);
return NextResponse.json(stringify(
excludePriceComponents ? feeds.map(({ price, ...feed }) => feed) : feeds
));
apps/insights/src/config/server.ts
Outdated
| } else { | ||
| return `http://localhost:3003`; | ||
| } | ||
| })(); |
There was a problem hiding this comment.
@alexcambose I think this comment still stands, I would get rid of this config and just use the request origin when building urls
|
@cprussi added a helper function to use the headers, lmk if it makes sense! I'll go ahead and merge this |
Summary
The issue is around having a lot of our UI data coming from a call to get pyth metadata.
Now we use a redis cache for storing function call results (functions that would require the pyth metadata or clickhouse data), and also API endpoints for native nextjs caching.
The pyth metadata function call is stored in an in memory cache and deduped if there are multiple calls at the same time.
Cache TTL is 24h with a stale of another 24h.
Rationale
Caching data that we don't need to be up to date to make everything faster.
How has this been tested?