Skip to content
New issue

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

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

Already on GitHub? # to your account

fix: revalidation with file-system-cache #58508

Merged
merged 4 commits into from
Nov 16, 2023
Merged

Conversation

gtjamesa
Copy link
Contributor

@gtjamesa gtjamesa commented Nov 15, 2023

What?

When using the file system cache with isrMemoryCacheSize: 0, time-based revalidation is not working, and the file is constantly updated. I have also added some debug logging to mirror that in the fetch-cache handler

Detailed explanation in #58507

Why?

The cached object's tags are incorrectly accessed, causing the cache to be rewritten every hit. This is catastrophic for a caching system that relies on file modification timestamps. The tags are one level up in the object from where they are currently being accessed.

Below shows a cached fetch representation on disk. When written, the tags reside at obj.tags instead of obj.data.tags

{
  "kind": "FETCH",
  "data": {
    "headers": {
      "connection": "keep-alive",
      "content-encoding": "br",
      "content-type": "application/json; charset=utf-8",
      "date": "Wed, 15 Nov 2023 21:17:42 GMT",
      "server": "nginx/1.18.0 (Ubuntu)",
      "transfer-encoding": "chunked",
      "vary": "Accept-Encoding"
    },
    "body": "[SNIP]",
    "status": 200,
    "url": "https://timeapi.io/api/Time/current/zone?timeZone=UTC"
    # this is where the current code is trying to pull the tags
    # "tags": [ "time-with-fetch" ]
  },
  "revalidate": 20,
  # tags actually live here
  "tags": [
    "time-with-fetch"
  ]
}

Fixes #58507

@ztanner ztanner added the CI approved Approve running CI for fork label Nov 16, 2023
@ijjk
Copy link
Member

ijjk commented Nov 16, 2023

Stats from current PR

Default Build
General Overall increase ⚠️
vercel/next.js canary gtjamesa/next.js fs-cache-fix Change
buildDuration 10.1s 10.3s ⚠️ +231ms
buildDurationCached 5.9s 6.5s ⚠️ +579ms
nodeModulesSize 199 MB 199 MB ⚠️ +9.15 kB
nextStartRea..uration (ms) 415ms 403ms N/A
Client Bundles (main, webpack)
vercel/next.js canary gtjamesa/next.js fs-cache-fix Change
199-HASH.js gzip 28.8 kB 28.8 kB N/A
3f784ff6-HASH.js gzip 53.3 kB 53.3 kB
494.HASH.js gzip 180 B 181 B N/A
framework-HASH.js gzip 45.2 kB 45.2 kB
main-app-HASH.js gzip 241 B 240 B N/A
main-HASH.js gzip 31.7 kB 31.8 kB N/A
webpack-HASH.js gzip 1.7 kB 1.7 kB
Overall change 100 kB 100 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary gtjamesa/next.js fs-cache-fix Change
polyfills-HASH.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary gtjamesa/next.js fs-cache-fix Change
_app-HASH.js gzip 194 B 195 B N/A
_error-HASH.js gzip 182 B 181 B N/A
amp-HASH.js gzip 504 B 506 B N/A
css-HASH.js gzip 322 B 323 B N/A
dynamic-HASH.js gzip 2.5 kB 2.5 kB
edge-ssr-HASH.js gzip 253 B 255 B N/A
head-HASH.js gzip 348 B 347 B N/A
hooks-HASH.js gzip 369 B 368 B N/A
image-HASH.js gzip 4.3 kB 4.3 kB N/A
index-HASH.js gzip 256 B 256 B
link-HASH.js gzip 2.63 kB 2.63 kB N/A
routerDirect..HASH.js gzip 311 B 311 B
script-HASH.js gzip 384 B 383 B N/A
withRouter-HASH.js gzip 307 B 308 B N/A
1afbb74e6ecf..834.css gzip 106 B 106 B
Overall change 3.17 kB 3.17 kB
Client Build Manifests
vercel/next.js canary gtjamesa/next.js fs-cache-fix Change
_buildManifest.js gzip 484 B 483 B N/A
Overall change 0 B 0 B
Rendered Page Sizes
vercel/next.js canary gtjamesa/next.js fs-cache-fix Change
index.html gzip 528 B 527 B N/A
link.html gzip 539 B 540 B N/A
withRouter.html gzip 525 B 522 B N/A
Overall change 0 B 0 B
Edge SSR bundle Size
vercel/next.js canary gtjamesa/next.js fs-cache-fix Change
edge-ssr.js gzip 92.6 kB 92.6 kB N/A
page.js gzip 145 kB 145 kB N/A
Overall change 0 B 0 B
Middleware size
vercel/next.js canary gtjamesa/next.js fs-cache-fix Change
middleware-b..fest.js gzip 627 B 624 B N/A
middleware-r..fest.js gzip 150 B 151 B N/A
middleware.js gzip 24.8 kB 24.8 kB N/A
edge-runtime..pack.js gzip 1.92 kB 1.92 kB
Overall change 1.92 kB 1.92 kB
Next Runtimes
vercel/next.js canary gtjamesa/next.js fs-cache-fix Change
app-page-exp...dev.js gzip 167 kB 167 kB
app-page-exp..prod.js gzip 93.4 kB 93.4 kB
app-page-tur..prod.js gzip 94.1 kB 94.1 kB
app-page-tur..prod.js gzip 88.7 kB 88.7 kB
app-page.run...dev.js gzip 137 kB 137 kB
app-page.run..prod.js gzip 88 kB 88 kB
app-route-ex...dev.js gzip 23.8 kB 23.8 kB
app-route-ex..prod.js gzip 16.4 kB 16.4 kB
app-route-tu..prod.js gzip 16.4 kB 16.4 kB
app-route-tu..prod.js gzip 16 kB 16 kB
app-route.ru...dev.js gzip 23.2 kB 23.2 kB
app-route.ru..prod.js gzip 16 kB 16 kB
pages-api-tu..prod.js gzip 9.37 kB 9.37 kB
pages-api.ru...dev.js gzip 9.64 kB 9.64 kB
pages-api.ru..prod.js gzip 9.37 kB 9.37 kB
pages-turbo...prod.js gzip 21.8 kB 21.8 kB
pages.runtim...dev.js gzip 22.5 kB 22.5 kB
pages.runtim..prod.js gzip 21.8 kB 21.8 kB
server.runti..prod.js gzip 49 kB 49.1 kB N/A
Overall change 875 kB 875 kB
Diff details
Diff for page.js

Diff too large to display

Diff for edge-ssr.js

Diff too large to display

Diff for server.runtime.prod.js

Diff too large to display

Commit: f159a28

@ijjk
Copy link
Member

ijjk commented Nov 16, 2023

Tests Passed

Copy link
Member

@ztanner ztanner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the PR & for deep diving into this!

@kodiakhq kodiakhq bot merged commit df4c2aa into vercel:canary Nov 16, 2023
@gtjamesa gtjamesa deleted the fs-cache-fix branch November 17, 2023 15:20
@github-actions github-actions bot added the locked label Dec 2, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 2, 2023
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
CI approved Approve running CI for fork locked type: next
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[File system cache] Time-based revalidation not working when ISR memory cache disabled
3 participants