Skip to content

Commit

Permalink
feat: expire unused assets in [site] uploads
Browse files Browse the repository at this point in the history
This expires any previously uploaded assets when using a Sites / `[site]` configuration. Because we currently do a full iteration of a namespace's keys when publishing, for rapidly changing sites this means that uploads get slower and slower. We can't just delete unused assets because it leads to occasional 404s on older publishes while we're publishing. So we expire previous assets while uploading new ones. The implementation/constraints of the kv api means that uploads may become slower, but should hopefully be faster overall. These optimisations also only matter for rapidly changing sites, so common usecases still have the same perf characteristics.
  • Loading branch information
threepointone committed Mar 17, 2022
1 parent e95bea2 commit 6b88c08
Show file tree
Hide file tree
Showing 6 changed files with 262 additions and 91 deletions.
7 changes: 7 additions & 0 deletions .changeset/thick-keys-worry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"wrangler": patch
---

feat: expire unused assets in `[site]` uploads

This expires any previously uploaded assets when using a Sites / `[site]` configuration. Because we currently do a full iteration of a namespace's keys when publishing, for rapidly changing sites this means that uploads get slower and slower. We can't just delete unused assets because it leads to occasional 404s on older publishes while we're publishing. So we expire previous assets while uploading new ones. The implementation/constraints of the kv api means that uploads may become slower, but should hopefully be faster overall. These optimisations also only matter for rapidly changing sites, so common usecases still have the same perf characteristics.
25 changes: 9 additions & 16 deletions packages/wrangler/src/__tests__/helpers/mock-kv.ts
Original file line number Diff line number Diff line change
@@ -1,38 +1,31 @@
import { createFetchResult, setMockRawResponse } from "./mock-cfetch";
import type { NamespaceKeyInfo } from "../../kv";

export function mockKeyListRequest(
expectedNamespaceId: string,
expectedKeys: string[],
expectedKeys: NamespaceKeyInfo[],
keysPerRequest = 1000,
blankCursorValue: "" | undefined | null = undefined
) {
const requests = { count: 0 };
// See https://api.cloudflare.com/#workers-kv-namespace-list-a-namespace-s-keys
const expectedKeyObjects = expectedKeys.map((name) => ({
name,
expiration: 123456789,
metadata: {},
}));

setMockRawResponse(
"/accounts/:accountId/storage/kv/namespaces/:namespaceId/keys",
"GET",
([_url, accountId, namespaceId], _init, query) => {
requests.count++;
expect(accountId).toEqual("some-account-id");
expect(namespaceId).toEqual(expectedNamespaceId);
if (expectedKeyObjects.length <= keysPerRequest) {
return createFetchResult(expectedKeyObjects);
if (expectedKeys.length <= keysPerRequest) {
return createFetchResult(expectedKeys);
} else {
const start = parseInt(query.get("cursor") ?? "0") || 0;
const end = start + keysPerRequest;
const cursor = end < expectedKeyObjects.length ? end : blankCursorValue;
return createFetchResult(
expectedKeyObjects.slice(start, end),
true,
[],
[],
{ cursor }
);
const cursor = end < expectedKeys.length ? end : blankCursorValue;
return createFetchResult(expectedKeys.slice(start, end), true, [], [], {
cursor,
});
}
}
);
Expand Down
81 changes: 27 additions & 54 deletions packages/wrangler/src/__tests__/kv.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -673,89 +673,76 @@ describe("wrangler", () => {

describe("list", () => {
it("should list the keys of a namespace specified by namespace-id", async () => {
const keys = ["key-1", "key-2", "key-3"];
const keys = [
{ name: "key-1" },
{ name: "key-2", expiration: 123456789 },
{ name: "key-3" },
];
mockKeyListRequest("some-namespace-id", keys);
await runWrangler("kv:key list --namespace-id some-namespace-id");
expect(std.err).toMatchInlineSnapshot(`""`);
expect(std.out).toMatchInlineSnapshot(`
"[
{
\\"name\\": \\"key-1\\",
\\"expiration\\": 123456789,
\\"metadata\\": {}
\\"name\\": \\"key-1\\"
},
{
\\"name\\": \\"key-2\\",
\\"expiration\\": 123456789,
\\"metadata\\": {}
\\"expiration\\": 123456789
},
{
\\"name\\": \\"key-3\\",
\\"expiration\\": 123456789,
\\"metadata\\": {}
\\"name\\": \\"key-3\\"
}
]"
`);
});

it("should list the keys of a namespace specified by binding", async () => {
writeWranglerConfig();
const keys = ["key-1", "key-2", "key-3"];
const keys = [{ name: "key-1" }, { name: "key-2" }, { name: "key-3" }];
mockKeyListRequest("bound-id", keys);

await runWrangler("kv:key list --binding someBinding");
expect(std.err).toMatchInlineSnapshot(`""`);
expect(std.out).toMatchInlineSnapshot(`
"[
{
\\"name\\": \\"key-1\\",
\\"expiration\\": 123456789,
\\"metadata\\": {}
\\"name\\": \\"key-1\\"
},
{
\\"name\\": \\"key-2\\",
\\"expiration\\": 123456789,
\\"metadata\\": {}
\\"name\\": \\"key-2\\"
},
{
\\"name\\": \\"key-3\\",
\\"expiration\\": 123456789,
\\"metadata\\": {}
\\"name\\": \\"key-3\\"
}
]"
`);
});

it("should list the keys of a preview namespace specified by binding", async () => {
writeWranglerConfig();
const keys = ["key-1", "key-2", "key-3"];
const keys = [{ name: "key-1" }, { name: "key-2" }, { name: "key-3" }];
mockKeyListRequest("preview-bound-id", keys);
await runWrangler("kv:key list --binding someBinding --preview");
expect(std.err).toMatchInlineSnapshot(`""`);
expect(std.out).toMatchInlineSnapshot(`
"[
{
\\"name\\": \\"key-1\\",
\\"expiration\\": 123456789,
\\"metadata\\": {}
\\"name\\": \\"key-1\\"
},
{
\\"name\\": \\"key-2\\",
\\"expiration\\": 123456789,
\\"metadata\\": {}
\\"name\\": \\"key-2\\"
},
{
\\"name\\": \\"key-3\\",
\\"expiration\\": 123456789,
\\"metadata\\": {}
\\"name\\": \\"key-3\\"
}
]"
`);
});

it("should list the keys of a namespace specified by binding, in a given environment", async () => {
writeWranglerConfig();
const keys = ["key-1", "key-2", "key-3"];
const keys = [{ name: "key-1" }, { name: "key-2" }, { name: "key-3" }];
mockKeyListRequest("env-bound-id", keys);
await runWrangler(
"kv:key list --binding someBinding --env some-environment"
Expand All @@ -764,27 +751,21 @@ describe("wrangler", () => {
expect(std.out).toMatchInlineSnapshot(`
"[
{
\\"name\\": \\"key-1\\",
\\"expiration\\": 123456789,
\\"metadata\\": {}
\\"name\\": \\"key-1\\"
},
{
\\"name\\": \\"key-2\\",
\\"expiration\\": 123456789,
\\"metadata\\": {}
\\"name\\": \\"key-2\\"
},
{
\\"name\\": \\"key-3\\",
\\"expiration\\": 123456789,
\\"metadata\\": {}
\\"name\\": \\"key-3\\"
}
]"
`);
});

it("should list the keys of a preview namespace specified by binding, in a given environment", async () => {
writeWranglerConfig();
const keys = ["key-1", "key-2", "key-3"];
const keys = [{ name: "key-1" }, { name: "key-2" }, { name: "key-3" }];
mockKeyListRequest("preview-env-bound-id", keys);
await runWrangler(
"kv:key list --binding someBinding --preview --env some-environment"
Expand All @@ -793,19 +774,13 @@ describe("wrangler", () => {
expect(std.out).toMatchInlineSnapshot(`
"[
{
\\"name\\": \\"key-1\\",
\\"expiration\\": 123456789,
\\"metadata\\": {}
\\"name\\": \\"key-1\\"
},
{
\\"name\\": \\"key-2\\",
\\"expiration\\": 123456789,
\\"metadata\\": {}
\\"name\\": \\"key-2\\"
},
{
\\"name\\": \\"key-3\\",
\\"expiration\\": 123456789,
\\"metadata\\": {}
\\"name\\": \\"key-3\\"
}
]"
`);
Expand All @@ -822,9 +797,9 @@ describe("wrangler", () => {
describe(`cursor - ${blankCursorValue}`, () => {
it("should make multiple requests for paginated results", async () => {
// Create a lot of mock keys, so that the fetch requests will be paginated
const keys: string[] = [];
const keys: NamespaceKeyInfo[] = [];
for (let i = 0; i < 550; i++) {
keys.push("key-" + i);
keys.push({ name: "key-" + i });
}
// Ask for the keys in pages of size 100.
const requests = mockKeyListRequest(
Expand All @@ -837,9 +812,7 @@ describe("wrangler", () => {
"kv:key list --namespace-id some-namespace-id --limit 100"
);
expect(std.err).toMatchInlineSnapshot(`""`);
expect(
JSON.parse(std.out).map((k: NamespaceKeyInfo) => k.name)
).toEqual(keys);
expect(JSON.parse(std.out)).toEqual(keys);
expect(requests.count).toEqual(6);
});
});
Expand Down
Loading

0 comments on commit 6b88c08

Please sign in to comment.