Skip to content

Commit

Permalink
Fix ISR page re-rendering after revalidate expiry
Browse files Browse the repository at this point in the history
* As outlined in vercel#24806 I there's currently a bug with the incremental
  static regeneration (ISR) logic incorrectly prolonging the life of
  rendered page content when it's the `LRUCache` is refreshed with
  potentially stale content from the filesystem cache.
* This because if a page is not present in the `LRUCache`, but is
  present in the filesystem cache then it's re-added to the `LRUCache`
  with a fresh cache expiry calculated from the current time using the
  `revalidate` window.
* If that page continues to be pushed out of the LRUCache before the
  next request comes in after the revalidation window expires then this
  creates an endless loop of the same stale content from the filesystem
  being served, without it being regenerated.
* This fix addresses that issue by adding a `fromTime` argument to the
  revalidation period calculation, so that when we're falling back to
  the filesystem we can use the filesystem's `lastModfied` timestamp
  for that file to anchor the revalidation calculation, rather than
  restarting from the current time in every case.
* That means that if page content from the filesystem is stale it will
  be marked as so when returned from the incremental cache `get` method
  (`isStale: true`), which tells next server to regenerate the page in
  the background.
* If however the the page content from the filesystem is still within
  the revalidate window (i.e. is not yet stale) then it will be added
  back to the LRUCache with the correct remaining duration for the
  `revalidateAfter`.

Fixes vercel#24806
  • Loading branch information
tommarshall committed May 5, 2021
1 parent ef32e16 commit f73d685
Showing 1 changed file with 12 additions and 10 deletions.
22 changes: 12 additions & 10 deletions packages/next/next-server/server/incremental-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,10 @@ export class IncrementalCache {
return path.join(this.incrementalOptions.pagesDir!, `${pathname}.${ext}`)
}

private calculateRevalidate(pathname: string): number | false {
private calculateRevalidate(
pathname: string,
fromTime: number
): number | false {
pathname = toRoute(pathname)

if (!this.prerenderManifest.routes[pathname]) {
Expand All @@ -95,8 +98,7 @@ export class IncrementalCache {

// in development we don't have a prerender-manifest
// and default to always revalidating to allow easier debugging
const curTime = new Date().getTime()
if (this.incrementalOptions.dev) return curTime - 1000
if (this.incrementalOptions.dev) return new Date().getTime() - 1000

const { initialRevalidateSeconds } = this.prerenderManifest.routes[
pathname
Expand All @@ -105,7 +107,7 @@ export class IncrementalCache {
}
const revalidateAfter =
typeof initialRevalidateSeconds === 'number'
? initialRevalidateSeconds * 1000 + curTime
? initialRevalidateSeconds * 1000 + fromTime
: initialRevalidateSeconds

return revalidateAfter
Expand All @@ -130,18 +132,17 @@ export class IncrementalCache {
}

try {
const html = await promises.readFile(
this.getSeedPath(pathname, 'html'),
'utf8'
)
const htmlPath = this.getSeedPath(pathname, 'html')
const html = await promises.readFile(htmlPath, 'utf8')
const { mtime } = await promises.stat(htmlPath)
const pageData = JSON.parse(
await promises.readFile(this.getSeedPath(pathname, 'json'), 'utf8')
)

data = {
html,
pageData,
revalidateAfter: this.calculateRevalidate(pathname),
revalidateAfter: this.calculateRevalidate(pathname, mtime.getTime()),
}
this.cache.set(pathname, data)
} catch (_) {
Expand Down Expand Up @@ -190,9 +191,10 @@ export class IncrementalCache {
}

pathname = normalizePagePath(pathname)

this.cache.set(pathname, {
...data,
revalidateAfter: this.calculateRevalidate(pathname),
revalidateAfter: this.calculateRevalidate(pathname, new Date().getTime()),
})

// TODO: This option needs to cease to exist unless it stops mutating the
Expand Down

0 comments on commit f73d685

Please sign in to comment.