8000 Improve performance and reliability when deploying multiple 2nd gen functions using single builds by blidd-google · Pull Request #6275 · firebase/firebase-tools · GitHub
[go: up one dir, main page]

Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Improve performance and reliability when deploying multiple 2nd gen functions using single builds. (#6275)
12 changes: 8 additions & 4 deletions src/deploy/functions/release/fabricator.ts
10000
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ export class Fabricator {
if (endpoint.platform === "gcfv1") {
await this.createV1Function(endpoint, scraper);
} else if (endpoint.platform === "gcfv2") {
await this.createV2Function(endpoint);
await this.createV2Function(endpoint, scraper);
} else {
assertExhaustive(endpoint.platform);
}
Expand All @@ -191,7 +191,7 @@ export class Fabricator {
if (update.endpoint.platform === "gcfv1") {
await this.updateV1Function(update.endpoint, scraper);
} else if (update.endpoint.platform === "gcfv2") {
await this.updateV2Function(update.endpoint);
await this.updateV2Function(update.endpoint, scraper);
} else {
assertExhaustive(update.endpoint.platform);
}
Expand Down Expand Up @@ -276,7 +276,7 @@ export class Fabricator {
}
}

async createV2Function(endpoint: backend.Endpoint): Promise<void> {
async createV2Function(endpoint: backend.Endpoint, scraper: SourceTokenScraper): Promise<void> {
const storageSource = this.sources[endpoint.codebase!]?.storage;
if (!storageSource) {
logger.debug("Precondition failed. Cannot create a GCFv2 function without storage");
Expand Down Expand Up @@ -351,11 +351,13 @@ export class Fabricator {
while (!resultFunction) {
resultFunction = await this.functionExecutor
.run(async () => {
apiFunction.buildConfig.sourceToken = await scraper.getToken();
const op: { name: string } = await gcfV2.createFunction(apiFunction);
return await poller.pollOperation<gcfV2.OutputCloudFunction>({
...gcfV2PollerOptions,
pollerName: `create-${endpoint.codebase}-${endpoint.region}-${endpoint.id}`,
operationResourceName: op.name,
onPoll: scraper.poller,
});
})
.catch(async (err: any) => {
Expand Down Expand Up @@ -463,7 +465,7 @@ export class Fabricator {
}
}

async updateV2Function(endpoint: backend.Endpoint): Promise<void> {
async updateV2Function(endpoint: backend.Endpoint, scraper: SourceTokenScraper): Promise<void> {
const storageSource = this.sources[endpoint.codebase!]?.storage;
if (!storageSource) {
logger.debug("Precondition failed. Cannot update a GCFv2 function without storage");
Expand All @@ -482,11 +484,13 @@ export class Fabricator {
const resultFunction = await this.functionExecutor
.run(
async () => {
apiFunction.buildConfig.sourceToken = await scraper.getToken();
const op: { name: string } = await gcfV2.updateFunction(apiFunction);
return await poller.pollOperation<gcfV2.OutputCloudFunction>({
...gcfV2PollerOptions,
pollerName: `update-${endpoint.codebase}-${endpoint.region}-${endpoint.id}`,
operationResourceName: op.name,
onPoll: scraper.poller,
});
},
{ retryCodes: [...DEFAULT_RETRY_CODES, CLOUD_RUN_RESOURCE_EXHAUSTED_CODE] }
Expand Down
14 changes: 12 additions & 2 deletions src/deploy/functions/release/sourceTokenScraper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,15 @@ type TokenFetchState = "NONE" | "FETCHING" | "VALID";
*/
export class SourceTokenScraper {
private tokenValidDurationMs;
private fetchTimeoutMs;
private resolve!: (token?: string) => void;
private promise: Promise<string | undefined>;
private expiry: number | undefined;
private fetchState: TokenFetchState;

constructor(validDurationMs = 1500000) {
constructor(validDurationMs = 1500000, fetchTimeoutMs = 180_000) {
this.tokenValidDurationMs = validDurationMs;
this.fetchTimeoutMs = fetchTimeoutMs;
this.promise = new Promise((resolve) => (this.resolve = resolve));
this.fetchState = "NONE";
}
Expand All @@ -27,7 +29,15 @@ export class SourceTokenScraper {
this.fetchState = "FETCHING";
return undefined;
} else if (this.fetchState === "FETCHING") {
return this.promise; // wait until we get a source token
const timeout = new Promise<undefined>((resolve) => {
setTimeout(() => {
this.fetchState = "NONE";
resolve(undefined);
}, this.fetchTimeoutMs);
});
// wait until we get a source token, or the timeout occurs
// and we reset the fetch state and return an undefined token.
return Promise.race([this.promise, timeout]);
} else if (this.fetchState === "VALID") {
if (this.isTokenExpired()) {
this.fetchState = "FETCHING";
Expand Down
1 change: 1 addition & 0 deletions src/gcp/cloudfunctionsv2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export interface BuildConfig {
runtime: runtimes.Runtime;
entryPoint: string;
source: Source;
sourceToken?: string;
environmentVariables: Record<string, string>;

// Output only
Expand Down
69 changes: 40 additions & 29 deletions src/test/deploy/functions/release/fabricator.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ describe("Fabricator", () => {
}
);

await fab.createV2Function(ep);
await fab.createV2Function(ep, new scraper.SourceTokenScraper());
expect(pubsub.createTopic).to.have.been.called;
expect(gcfv2.createFunction).to.have.been.called;
});
Expand All @@ -476,7 +476,7 @@ describe("Fabricator", () => {
}
);

await expect(fab.createV2Function(ep)).to.be.rejectedWith(
await expect(fab.createV2Function(ep, new scraper.SourceTokenScraper())).to.be.rejectedWith(
reporter.DeploymentError,
"create topic"
);
Expand All @@ -500,7 +500,7 @@ describe("Fabricator", () => {
}
);

await fab.createV2Function(ep);
await fab.createV2Function(ep, new scraper.SourceTokenScraper());
expect(eventarc.getChannel).to.have.been.called;
expect(eventarc.createChannel).to.not.have.been.called;
expect(gcfv2.createFunction).to.have.been.called;
Expand Down Expand Up @@ -530,7 +530,7 @@ describe("Fabricator", () => {
}
);

await fab.createV2Function(ep);
await fab.createV2Function(ep, new scraper.SourceTokenScraper());
expect(eventarc.createChannel).to.have.been.called;
expect(gcfv2.createFunction).to.have.been.called;
});
Expand Down Expand Up @@ -568,7 +568,7 @@ describe("Fabricator", () => {
}
);

await fab.createV2Function(ep);
await fab.createV2Function(ep, new scraper.SourceTokenScraper());
expect(eventarc.createChannel).to.have.been.calledOnceWith({ name: channelName });
expect(poller.pollOperation).to.have.been.called;
});
Expand All @@ -594,22 +594,27 @@ describe("Fabricator", () => {
}
);

await expect(fab.createV2Function(ep)).to.eventually.be.rejectedWith(
reporter.DeploymentError,
"upsert eventarc channel"
);
await expect(
fab.createV2Function(ep, new scraper.SourceTokenScraper())
).to.eventually.be.rejectedWith(reporter.DeploymentError, "upsert eventarc channel");
});

it("throws on create function failure", async () => {
gcfv2.createFunction.rejects(new Error("Server failure"));

const ep = endpoint({ httpsTrigger: {} }, { platform: "gcfv2" });
await expect(fab.createV2Function(ep)).to.be.rejectedWith(reporter.DeploymentError, "create");
await expect(fab.createV2Function(ep, new scraper.SourceTokenScraper())).to.be.rejectedWith(
reporter.DeploymentError,
"create"
);

gcfv2.createFunction.resolves({ name: "op", done: false });
poller.pollOperation.rejects(new Error("Fail whale"));

await expect(fab.createV2Function(ep)).to.be.rejectedWith(reporter.DeploymentError, "create");
await expect(fab.createV2Function(ep, new scraper.SourceTokenScraper())).to.be.rejectedWith(
reporter.DeploymentError,
"create"
);
});

it("deletes broken function and retries on cloud run quota exhaustion", async () => {
Expand All @@ -620,7 +625,7 @@ describe("Fabricator", () => {
poller.pollOperation.resolves({ name: "op" });

const ep = endpoint({ httpsTrigger: {} }, { platform: "gcfv2" });
await fab.createV2Function(ep);
await fab.createV2Function(ep, new scraper.SourceTokenScraper(1500000, 0));

expect(gcfv2.createFunction).to.have.been.calledTwice;
expect(gcfv2.deleteFunction).to.have.been.called;
Expand All @@ -632,7 +637,7 @@ describe("Fabricator", () => {
run.setInvokerCreate.rejects(new Error("Boom"));

const ep = endpoint({ httpsTrigger: {} }, { platform: "gcfv2" });
await expect(fab.createV2Function(ep)).to.be.rejectedWith(
await expect(fab.createV2Function(ep, new scraper.SourceTokenScraper())).to.be.rejectedWith(
reporter.DeploymentError,
"set invoker"
);
Expand All @@ -645,7 +650,7 @@ describe("Fabricator", () => {
run.setInvokerCreate.resolves();
const ep = endpoint({ httpsTrigger: {} }, { platform: "gcfv2" });

await fab.createV2Function(ep);
await fab.createV2Function(ep, new scraper.SourceTokenScraper());
expect(run.setInvokerCreate).to.have.been.calledWith(ep.project, "service", ["public"]);
});

Expand All @@ -662,7 +667,7 @@ describe("Fabricator", () => {
{ platform: "gcfv2" }
);

await fab.createV2Function(ep);
await fab.createV2Function(ep, new scraper.SourceTokenScraper());
expect(run.setInvokerCreate).to.have.been.calledWith(ep.project, "service", ["custom@"]);
});

Expand All @@ -672,7 +677,7 @@ describe("Fabricator", () => {
run.setInvokerCreate.resolves();
const ep = endpoint({ httpsTrigger: { invoker: ["private"] } }, { platform: "gcfv2" });

await fab.createV2Function(ep);
await fab.createV2Function(ep, new scraper.SourceTokenScraper());
expect(run.setInvokerCreate).to.not.have.been.called;
});
});
Expand All @@ -684,7 +689,7 @@ describe("Fabricator", () => {
run.setInvokerCreate.resolves();
const ep = endpoint({ callableTrigger: {} }, { platform: "gcfv2" });

await fab.createV2Function(ep);
await fab.createV2Function(ep, new scraper.SourceTokenScraper());
expect(run.setInvokerCreate).to.have.been.calledWith(ep.project, "service", ["public"]);
});
});
Expand All @@ -696,7 +701,7 @@ describe("Fabricator", () => {
run.setInvokerCreate.resolves();
const ep = endpoint({ taskQueueTrigger: {} }, { platform: "gcfv2" });

await fab.createV2Function(ep);
await fab.createV2Function(ep, new scraper.SourceTokenScraper());
expect(run.setInvokerCreate).to.not.have.been.called;
});

Expand All @@ -712,7 +717,7 @@ describe("Fabricator", () => {
},
{ platform: "gcfv2" }
);
await fab.createV2Function(ep);
await fab.createV2Function(ep, new scraper.SourceTokenScraper());
expect(run.setInvokerCreate).to.have.been.calledWith(ep.project, "service", ["custom@"]);
});
});
Expand All @@ -727,7 +732,7 @@ describe("Fabricator", () => {
{ platform: "gcfv2" }
);

await fab.createV2Function(ep);
await fab.createV2Function(ep, new scraper.SourceTokenScraper());
expect(run.setInvokerCreate).to.have.been.calledWith(ep.project, "service", ["public"]);
});
});
Expand All @@ -741,7 +746,7 @@ describe("Fabricator", () => {
{ platform: "gcfv2" }
);

await fab.createV2Function(ep);
await fab.createV2Function(ep, new scraper.SourceTokenScraper());
expect(run.setInvokerCreate).to.not.have.been.called;
});
});
Expand All @@ -751,11 +756,17 @@ describe("Fabricator", () => {
gcfv2.updateFunction.rejects(new Error("Server failure"));

const ep = endpoint({ httpsTrigger: {} }, { platform: "gcfv2" });
await expect(fab.updateV2Function(ep)).to.be.rejectedWith(reporter.DeploymentError, "update");
await expect(fab.updateV2Function(ep, new scraper.SourceTokenScraper())).to.be.rejectedWith(
reporter.DeploymentError,
"update"
);

gcfv2.updateFunction.resolves({ name: "op", done: false });
poller.pollOperation.rejects(new Error("Fail whale"));
await expect(fab.updateV2Function(ep)).to.be.rejectedWith(reporter.DeploymentError, "update");
await expect(fab.updateV2Function(ep, new scraper.SourceTokenScraper())).to.be.rejectedWith(
reporter.DeploymentError,
"update"
);
});

it("throws on set invoker failure", async () => {
Expand All @@ -764,7 +775,7 @@ describe("Fabricator", () => {
run.setInvokerUpdate.rejects(new Error("Boom"));

const ep = endpoint({ httpsTrigger: { invoker: ["private"] } }, { platform: "gcfv2" });
await expect(fab.updateV2Function(ep)).to.be.rejectedWith(
await expect(fab.updateV2Function(ep, new scraper.SourceTokenScraper())).to.be.rejectedWith(
reporter.DeploymentError,
"set invoker"
);
Expand All @@ -783,7 +794,7 @@ describe("Fabricator", () => {
{ platform: "gcfv2" }
);

await fab.updateV2Function(ep);
await fab.updateV2Function(ep, new scraper.SourceTokenScraper());
expect(run.setInvokerUpdate).to.have.been.calledWith(ep.project, "service", ["custom@"]);
});

Expand All @@ -800,7 +811,7 @@ describe("Fabricator", () => {
{ platform: "gcfv2" }
);

await fab.updateV2Function(ep);
await fab.updateV2Function(ep, new scraper.SourceTokenScraper());
expect(run.setInvokerUpdate).to.have.been.calledWith(ep.project, "service", ["custom@"]);
});

Expand All @@ -817,7 +828,7 @@ describe("Fabricator", () => {
{ platform: "gcfv2" }
);

await fab.updateV2Function(ep);
await fab.updateV2Function(ep, new scraper.SourceTokenScraper());
expect(run.setInvokerUpdate).to.have.been.calledWith(ep.project, "service", ["public"]);
});

Expand All @@ -827,7 +838,7 @@ describe("Fabricator", () => {
run.setInvokerUpdate.resolves();
const ep = endpoint({ httpsTrigger: {} }, { platform: "gcfv2" });

await fab.updateV2Function(ep);
await fab.updateV2Function(ep, new scraper.SourceTokenScraper());
expect(run.setInvokerUpdate).to.not.have.been.called;
});

Expand All @@ -840,7 +851,7 @@ describe("Fabricator", () => {
{ platform: "gcfv2" }
);

await fab.updateV2Function(ep);
await fab.updateV2Function(ep, new scraper.SourceTokenScraper());
expect(run.setInvokerUpdate).to.not.have.been.called;
});
});
Expand Down
6 changes: 6 additions & 0 deletions src/test/deploy/functions/release/sourceTokenScraper.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ describe("SourceTokenScraper", () => {
await expect(scraper.getToken()).to.eventually.equal("magic token #2");
});

it("resets fetch state after timeout and returns undefined token", async () => {
const scraper = new SourceTokenScraper(100000, 10);
await expect(scraper.getToken()).to.eventually.be.undefined;
await expect(scraper.getToken()).to.eventually.be.undefined;
});

it("concurrent requests for source token", async () => {
const scraper = new SourceTokenScraper();

Expand Down
0