feat: add cache to check endpoint #1391
Conversation
Deploying with
|
| Latest commit: |
29504d0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://464f825a.nft-storage-1at.pages.dev |
vasco-santos
force-pushed
the
feat/add-cache-to-check-endpoint
branch
from
6cd8272
to
e361d46
Compare
vasco-santos
requested review from
hugomrdias and
alanshaw
and removed request for
hugomrdias
vasco-santos
force-pushed
the
feat/add-cache-to-check-endpoint
branch
from
e361d46
to
29504d0
Compare
| // cache status response with max age | ||
| 'Cache-Control': `public, max-age=${CACHE_MAX_AGE_NO_DEAL_YET}`, | ||
| } | ||
| : undefined // cache default |
There was a problem hiding this comment.
What is the default though?
There was a problem hiding this comment.
We should really be specific about the cache period. In this case it's unknown when another deal for the CID may become available so I expect we'd only cache for a relatively short period of time...
Was there an observed perf issue here? This makes the route more complicated and it's used in our monitoring so could mask any underlying issue...
There was a problem hiding this comment.
Per https://developers.cloudflare.com/workers/runtime-apis/cache/#headers and https://developers.cloudflare.com/cache/how-to/configure-cache-status-code/#edge-ttl default would be 120 minutes.
We can probably use default if we have at least a deal instead?
Was there an observed perf issue here? This makes the route more complicated and it's used in our monitoring so could mask any underlying issue...
Yesterday we had some timeouts for instance, I don't have empirical answers for this at the moment. The main motivation for this is the work in #1386 which will increase significantly the calls to check API
There was a problem hiding this comment.
If a CID is pinned and in has deals, then some caching of the repsonse would be sensible. Otherwise, caching should be minimal or avoided here.
I'm against landing #1386 as I dont want us to hit the nft.storage api for every single CID fetched from the gateway.
This PR adds cache response for check endpoint. Specially when content is Pinned and Filecoin Deal is made the response will be exactly the same.
In this PR, response is cached for 10 minutes when content requested has no deals yet and the content is Pinned. When it has deals, we rely on default cache TTL of CF cache API
It will avoid two heavy DB queries, including a query to cargo schema.