Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Time based revalidation does not work as expected #828

Open
jshbrntt opened this issue Oct 21, 2024 · 3 comments
Open

Time based revalidation does not work as expected #828

jshbrntt opened this issue Oct 21, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@jshbrntt
Copy link

jshbrntt commented Oct 21, 2024

This effects both Next.js 14 and 15.

When setting { next: { revalidate: 1 } }, currently the default behaviour of the redis-strings and redis-stack handlers is to set an expiry on the entry set in Redis.

However, the expected behaviour of Time-based Revalidation is that if you set the revalidate option like so on a fetch request.

// Revalidate at most every hour
fetch('https://...', { next: { revalidate: 1} })

After the initial request has fetched the data from the external data source and stored it in the Data Cache, it will continue you return this cached data until it expires.

Even when it expires, it should return stale data, while fetching updated data in the background and updating the Data Cache (so the subsequent request will be up to date).

If the expiry is set on the entry in Redis when it is created, then it will not be able to return stale data and will instead always be fetching from the external data source and no data will ever be in the cache.

@jshbrntt jshbrntt added the bug Something isn't working label Oct 21, 2024
@better-salmon
Copy link
Contributor

@jshbrntt Hey there!

@neshca/cache-handler supports time-based revalidation and lets you adjust the cache expiration time. By default, the stale time of the cache matches the expiration time to conserve space in the cache storage. You can customize the stale time by setting the estimateExpireAge callback in the options. For further details, check out the documentation.

Setting the expiration age to be the same as the stale age is quite stringent. I plan to update the default estimateExpireAge in the upcoming major release, including Next.js 15 and node-redis 5 support. Keep an eye out for it!

@jshbrntt
Copy link
Author

jshbrntt commented Oct 22, 2024

@better-salmon how do I get this exact behaviour in the diagram?
https://nextjs.org/docs/app/building-your-application/caching#time-based-revalidation
image

It should always serve from cache even if it's stale, and revalidate and update the cache with fresh data in the background.

Therefore not blocking the page load and not making it wait for a delete or set call on the cache.

@jshbrntt
Copy link
Author

jshbrntt commented Oct 22, 2024

I tried setting the options you linked to in the docs.
https://caching-tools.github.io/next-shared-cache/api-reference/ttl-parameters#estimateexpireage

Although they fix the issue of setting the TTLs of the Redis cache entries to be longer than that of the revalidate value so they're still retrievable to be served as stale cache values.

When the revalidation/refresh of the state cache is triggered the page response is delayed as if it was fetching directly from the external data source. I think this is because it is awaiting a get (stale data), delete (stale data), fetch (fresh data) and a set (fresh data) before returning the page response.

When it should be doing the last 3 parts in the background outside of the page response flow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants