From db2328c885c317d21c09443bc490eb20de0c1e8c Mon Sep 17 00:00:00 2001 From: Matei David Date: Fri, 22 Jan 2021 12:32:07 +0000 Subject: [PATCH 1/3] Introduce execution limiter for file uploads * TechDoc publishers for storage providers (i.e aws, google storage) should now rate limit concurrent executions allowing for file uploads to run in batches of 10. * Add PATCH changeset to techdocs common * Update yarn.lock --- .changeset/techdocs-dirty-rivers-hope.md | 5 +++++ packages/techdocs-common/package.json | 1 + .../techdocs-common/src/stages/publish/awsS3.ts | 9 +++++++-- .../src/stages/publish/googleStorage.ts | 14 +++++++++----- yarn.lock | 2 +- 5 files changed, 23 insertions(+), 8 deletions(-) create mode 100644 .changeset/techdocs-dirty-rivers-hope.md diff --git a/.changeset/techdocs-dirty-rivers-hope.md b/.changeset/techdocs-dirty-rivers-hope.md new file mode 100644 index 0000000000..3f49afa529 --- /dev/null +++ b/.changeset/techdocs-dirty-rivers-hope.md @@ -0,0 +1,5 @@ +--- +'@backstage/techdocs-common': patch +--- + +Add rate limiter for concurrent execution of file uploads in AWS and Google publishers diff --git a/packages/techdocs-common/package.json b/packages/techdocs-common/package.json index 0a1a673f3a..93825511cb 100644 --- a/packages/techdocs-common/package.json +++ b/packages/techdocs-common/package.json @@ -53,6 +53,7 @@ "json5": "^2.1.3", "mime-types": "^2.1.27", "mock-fs": "^4.13.0", + "p-limit": "^3.1.0", "recursive-readdir": "^2.2.2", "winston": "^3.2.1" }, diff --git a/packages/techdocs-common/src/stages/publish/awsS3.ts b/packages/techdocs-common/src/stages/publish/awsS3.ts index b9e9fe12e6..e7014df967 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.ts @@ -24,6 +24,7 @@ import { PublisherBase, PublishRequest, TechDocsMetadata } from './types'; import fs from 'fs-extra'; import { Readable } from 'stream'; import JSON5 from 'json5'; +import limiterFactory from 'p-limit'; const streamToBuffer = (stream: Readable): Promise => { return new Promise((resolve, reject) => { @@ -131,8 +132,8 @@ export class AwsS3Publish implements PublisherBase { // So collecting path of only the files is good enough. const allFilesToUpload = await getFileTreeRecursively(directory); + const limiter = limiterFactory(10); const uploadPromises: Array> = []; - for (const filePath of allFilesToUpload) { // Remove the absolute path prefix of the source directory // Path of all files to upload, relative to the root of the source directory @@ -149,7 +150,11 @@ export class AwsS3Publish implements PublisherBase { Body: fileContent, }; - uploadPromises.push(this.storageClient.putObject(params)); + // Rate limit the concurrent execution of file uploads to batches of 10 + const uploadFile = limiter(async () => + this.storageClient.putObject(params), + ); + uploadPromises.push(uploadFile); } await Promise.all(uploadPromises); this.logger.info( diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.ts b/packages/techdocs-common/src/stages/publish/googleStorage.ts index 82a9620e8f..d8fe0b7d26 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.ts @@ -26,6 +26,7 @@ import { Config } from '@backstage/config'; import { getHeadersForFileExtension, getFileTreeRecursively } from './helpers'; import { PublisherBase, PublishRequest, TechDocsMetadata } from './types'; import JSON5 from 'json5'; +import limitFactory from 'p-limit'; export class GoogleGCSPublish implements PublisherBase { static async fromConfig( @@ -102,6 +103,7 @@ export class GoogleGCSPublish implements PublisherBase { // So collecting path of only the files is good enough. const allFilesToUpload = await getFileTreeRecursively(directory); + const limiter = limitFactory(10); const uploadPromises: Array> = []; allFilesToUpload.forEach(filePath => { // Remove the absolute path prefix of the source directory @@ -110,12 +112,14 @@ export class GoogleGCSPublish implements PublisherBase { const relativeFilePath = filePath.replace(`${directory}/`, ''); const entityRootDir = `${entity.metadata.namespace}/${entity.kind}/${entity.metadata.name}`; const destination = `${entityRootDir}/${relativeFilePath}`; // GCS Bucket file relative path - // TODO: Upload in chunks of ~10 files instead of all files at once. - uploadPromises.push( - this.storageClient.bucket(this.bucketName).upload(filePath, { - destination, - }), + + // Rate limit the concurrent execution of file uploads to batches of 10 + const uploadFile = limiter(async () => + this.storageClient + .bucket(this.bucketName) + .upload(filePath, { destination }), ); + uploadPromises.push(uploadFile); }); Promise.all(uploadPromises) diff --git a/yarn.lock b/yarn.lock index f6762a63f0..cf807acf11 100644 --- a/yarn.lock +++ b/yarn.lock @@ -19780,7 +19780,7 @@ p-limit@^2.0.0, p-limit@^2.2.0: dependencies: p-try "^2.0.0" -p-limit@^3.0.1, p-limit@^3.0.2: +p-limit@^3.0.1, p-limit@^3.0.2, p-limit@^3.1.0: version "3.1.0" resolved "https://registry.npmjs.org/p-limit/-/p-limit-3.1.0.tgz#e1daccbe78d0d1388ca18c64fea38e3e57e3706b" integrity sha512-TYOanM3wGwNGsZN2cVTYPArw454xnXj5qmWF1bEoAc4+cU/ol7GVh7odevjp1FNHduHc3KZMcFduxU5Xc6uJRQ== From 36bd49fa5603fd31c7218c8ecef3a50402a236ce Mon Sep 17 00:00:00 2001 From: Matei David Date: Fri, 22 Jan 2021 13:38:49 +0000 Subject: [PATCH 2/3] Remove async keyword from anon limiter fn Signed-off-by: Matei David --- packages/techdocs-common/src/stages/publish/awsS3.ts | 6 ++---- .../techdocs-common/src/stages/publish/googleStorage.ts | 4 ++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/techdocs-common/src/stages/publish/awsS3.ts b/packages/techdocs-common/src/stages/publish/awsS3.ts index e7014df967..9a4852df03 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.ts @@ -150,10 +150,8 @@ export class AwsS3Publish implements PublisherBase { Body: fileContent, }; - // Rate limit the concurrent execution of file uploads to batches of 10 - const uploadFile = limiter(async () => - this.storageClient.putObject(params), - ); + // Rate limit the concurrent execution of file uploads to batches of 10 (per publish) + const uploadFile = limiter(() => this.storageClient.putObject(params)); uploadPromises.push(uploadFile); } await Promise.all(uploadPromises); diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.ts b/packages/techdocs-common/src/stages/publish/googleStorage.ts index d8fe0b7d26..8a06a07cb0 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.ts @@ -113,8 +113,8 @@ export class GoogleGCSPublish implements PublisherBase { const entityRootDir = `${entity.metadata.namespace}/${entity.kind}/${entity.metadata.name}`; const destination = `${entityRootDir}/${relativeFilePath}`; // GCS Bucket file relative path - // Rate limit the concurrent execution of file uploads to batches of 10 - const uploadFile = limiter(async () => + // Rate limit the concurrent execution of file uploads to batches of 10 (per publish) + const uploadFile = limiter(() => this.storageClient .bucket(this.bucketName) .upload(filePath, { destination }), From 27f529afb6e957f1b1e7f9444f626ab532182c5b Mon Sep 17 00:00:00 2001 From: Matei David Date: Mon, 25 Jan 2021 11:43:03 +0000 Subject: [PATCH 3/3] Rename limitFactory to createLimiter in p-limit import Signed-off-by: Matei David --- packages/techdocs-common/src/stages/publish/awsS3.ts | 4 ++-- packages/techdocs-common/src/stages/publish/googleStorage.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/techdocs-common/src/stages/publish/awsS3.ts b/packages/techdocs-common/src/stages/publish/awsS3.ts index 9a4852df03..20e6bc424d 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.ts @@ -24,7 +24,7 @@ import { PublisherBase, PublishRequest, TechDocsMetadata } from './types'; import fs from 'fs-extra'; import { Readable } from 'stream'; import JSON5 from 'json5'; -import limiterFactory from 'p-limit'; +import createLimiter from 'p-limit'; const streamToBuffer = (stream: Readable): Promise => { return new Promise((resolve, reject) => { @@ -132,7 +132,7 @@ export class AwsS3Publish implements PublisherBase { // So collecting path of only the files is good enough. const allFilesToUpload = await getFileTreeRecursively(directory); - const limiter = limiterFactory(10); + const limiter = createLimiter(10); const uploadPromises: Array> = []; for (const filePath of allFilesToUpload) { // Remove the absolute path prefix of the source directory diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.ts b/packages/techdocs-common/src/stages/publish/googleStorage.ts index 8a06a07cb0..8876007504 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.ts @@ -26,7 +26,7 @@ import { Config } from '@backstage/config'; import { getHeadersForFileExtension, getFileTreeRecursively } from './helpers'; import { PublisherBase, PublishRequest, TechDocsMetadata } from './types'; import JSON5 from 'json5'; -import limitFactory from 'p-limit'; +import createLimiter from 'p-limit'; export class GoogleGCSPublish implements PublisherBase { static async fromConfig( @@ -103,7 +103,7 @@ export class GoogleGCSPublish implements PublisherBase { // So collecting path of only the files is good enough. const allFilesToUpload = await getFileTreeRecursively(directory); - const limiter = limitFactory(10); + const limiter = createLimiter(10); const uploadPromises: Array> = []; allFilesToUpload.forEach(filePath => { // Remove the absolute path prefix of the source directory