Hash cache keys to limit their length #553

vicb · 2025-04-08T15:33:01Z

fixes #550

⧓ Review in Butler Review #4OeVF3V8a

vicb/hash-cache-key

7 commit series (version 1)

Series	Commit Title	Status
7/7	fixup! readability	⏳
6/7	fixup! readability	⏳
5/7	fixup! rm .env	⏳
4/7	fixup! apply default values in computeCacheKey	⏳
3/7	fixup!	⏳
2/7	fixup! simplify	⏳
1/7	Hash cache keys to limit their length	⏳

Please leave review feedback in the Butler Review

changeset-bot · 2025-04-08T15:33:04Z

⚠️ No Changeset found

Latest commit: 21651ac

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

pkg-pr-new · 2025-04-08T15:33:56Z

Open in StackBlitz

pnpm add https://pkg.pr.new/@opennextjs/cloudflare@553

commit: cd6eb58

vicb · 2025-04-08T16:27:44Z

packages/cloudflare/src/api/overrides/incremental-cache/r2-incremental-cache.ts

+export function computeCacheKey(key: string, options: KeyOptions) {
+  const { isFetch, directory, buildId } = options;
+  const hash = createHash("sha256");
+  return `${directory}/${buildId}/${hash.update(key).digest("hex")}.${isFetch ? "fetch" : "cache"}`.replace(


Question: we have a directory for R2 but not for KV.
Should we unify that? if yes by adding or removing the directory?

I think the only point in having a directory is if people want to use the same bucket for other things than just Next cache.
In this case it makes sense to add it to KV as well.

I think the directory is a nice feature 👍
When moving to R2 today, it was useful to easily navigate the cache from the R2 dashboard.

It doesn't look like it'll be as useful in KV, but to stay consistent, I think it makes sense to use /.

Ideally there would be a single function for computing the cache key

@SamyPesse Thanks for your feedback!

Can I ask how it is helpful? Do you store other objects than cache in the same bucket?

I think it makes sense to use /

What do you mean by "use /"?

R2 has a directory as people are likely to want to re-use the same object storage across multiple apps and so it can namespace each app.

Agreed it would be nice to have a single function providing a consistent way to create cache keys across adapters.

people are likely to want to re-use the same object storage across multiple apps

Why is that?

I was thinking about deleting R2/KV before populating to avoid increase the storage space for each new deployment - in which case we would not really need a directory

people are likely to want to re-use the same object storage across multiple apps

Why is that?

If an org has 10 microfrontends for their website (not unrealistic), i view it as less overhead/complexity to have a single object storage bucket managing the data/route cache across apps than having separate buckets for each app. Realistically, you would never have one object storage per app on a platform - the data is segmented within that store by each app, but across the platform you'd use the same underlying data store. I expect Vercel's cache to have an object storage instance with lots of different projects, and cache entries segmented by projects for example.

I was thinking about deleting R2/KV before populating to avoid increase the storage space for each new deployment - in which case we would not really need a directory

The deployment currently in production would still depend on having access to that data in R2/KV before the next deployment happens as population is pre-deployment, so I'm not sure I understand how that would work without unintended side effects. Ideally it would be a post-deployment cleanup action for old builds.

Even post-deployment is a sticky situation as applications may still point some traffic to old builds to avoid version skew.

i view it as less overhead/complexity

I don't really get the complexity difference of a bucket name vs a directory.

The deployment currently in production would still depend on having access to that data in R2/KV ...

That's true.

On the other hand cleanup is not really possible today.
It would means batch listing + deleting.
First it is not possible to list given a prefix.
Second deleting entries one by one incurs $$$.

Something that we need to solve.

conico974 · 2025-04-08T16:36:08Z

packages/cloudflare/src/api/overrides/incremental-cache/r2-incremental-cache.ts

+export function computeCacheKey(key: string, options: KeyOptions) {
+  const { isFetch, directory, buildId } = options;
+  const hash = createHash("sha256");
+  return `${directory}/${buildId}/${hash.update(key).digest("hex")}.${isFetch ? "fetch" : "cache"}`.replace(


I think the only point in having a directory is if people want to use the same bucket for other things than just Next cache.
In this case it makes sense to add it to KV as well.

conico974 · 2025-04-08T16:45:43Z

packages/cloudflare/src/api/overrides/incremental-cache/r2-incremental-cache.ts

+export function computeCacheKey(key: string, options: KeyOptions) {
+  const { isFetch, directory, buildId } = options;
+  const hash = createHash("sha256");
+  return `${directory}/${buildId}/${hash.update(key).digest("hex")}.${isFetch ? "fetch" : "cache"}`.replace(


Ideally there would be a single function for computing the cache key

conico974 · 2025-04-08T16:49:07Z

packages/cloudflare/src/cli/commands/populate-cache.ts

+    const relativePath = path.relative(path.join(opts.outputDir, "cache"), fullPath);
+
+    if (relativePath.startsWith("__fetch")) {
+      const [__fetch, buildId, ...keyParts] = relativePath.split("/");


Do we need keyParts to be an array for fetch ? (i haven't checked, i might be wrong)

The Next.js fetch key is a sha-256 - so the array is not stricly needed but it doesn't hurt and makes code consistent.

packages/cloudflare/src/api/overrides/incremental-cache/kv-incremental-cache.ts

packages/cloudflare/src/api/overrides/incremental-cache/r2-incremental-cache.ts

james-elicx · 2025-04-08T17:05:09Z

packages/cloudflare/src/api/overrides/incremental-cache/r2-incremental-cache.ts

+export function computeCacheKey(key: string, options: KeyOptions) {
+  const { isFetch, directory, buildId } = options;
+  const hash = createHash("sha256");
+  return `${directory}/${buildId}/${hash.update(key).digest("hex")}.${isFetch ? "fetch" : "cache"}`.replace(


R2 has a directory as people are likely to want to re-use the same object storage across multiple apps and so it can namespace each app.

Agreed it would be nice to have a single function providing a consistent way to create cache keys across adapters.

vicb · 2025-04-09T08:06:18Z

I have addressed the feedback and created #556 to add directory to KV

conico974

LGTM

vicb requested review from conico974 and james-elicx April 8, 2025 16:26

vicb commented Apr 8, 2025

View reviewed changes

conico974 reviewed Apr 8, 2025

View reviewed changes

james-elicx reviewed Apr 8, 2025

View reviewed changes

vicb added 4 commits April 9, 2025 09:41

Hash cache keys to limit their length

c62ce7b

fixup! simplify

07df4b2

fixup!

f934521

fixup! apply default values in computeCacheKey

f49f9d5

vicb force-pushed the vicb/hash-cache-key branch from 1035fd9 to f49f9d5 Compare April 9, 2025 07:44

vicb mentioned this pull request Apr 9, 2025

[Feature] Support directory for KV inc cache #556

Closed

vicb requested review from conico974 and james-elicx April 9, 2025 07:57

vicb added 3 commits April 9, 2025 09:58

fixup! rm .env

3b9ced9

fixup! readability

21651ac

fixup! readability

cd6eb58

conico974 approved these changes Apr 9, 2025

View reviewed changes

vicb merged commit 930ab83 into main Apr 9, 2025
7 checks passed

vicb deleted the vicb/hash-cache-key branch April 9, 2025 08:59

Hash cache keys to limit their length #553

Hash cache keys to limit their length #553

Uh oh!

Conversation

vicb commented Apr 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot bot commented Apr 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

pkg-pr-new bot commented Apr 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SamyPesse Apr 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

james-elicx Apr 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

james-elicx Apr 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

james-elicx Apr 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vicb commented Apr 9, 2025

Uh oh!

conico974 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

vicb commented Apr 8, 2025 •

edited

Loading

changeset-bot bot commented Apr 8, 2025 •

edited

Loading

pkg-pr-new bot commented Apr 8, 2025 •

edited

Loading

SamyPesse Apr 8, 2025 •

edited

Loading

james-elicx Apr 8, 2025 •

edited

Loading

james-elicx Apr 8, 2025 •

edited

Loading

james-elicx Apr 8, 2025 •

edited

Loading