From 06beaf91d4ba29e45c35a1472a353fbadf36b668 Mon Sep 17 00:00:00 2001 From: Konrad Pabjan Date: Wed, 4 Mar 2020 18:03:52 +0100 Subject: [PATCH] More detailed status information during large uploads --- .../artifact/src/internal-config-variables.ts | 5 +-- .../src/internal-upload-http-client.ts | 20 ++++++---- .../src/internal-upload-status-reporter.ts | 38 +++++++++++++++---- packages/artifact/src/internal-utils.ts | 3 +- 4 files changed, 45 insertions(+), 21 deletions(-) diff --git a/packages/artifact/src/internal-config-variables.ts b/packages/artifact/src/internal-config-variables.ts index 1be57e46..9c3ebc3c 100644 --- a/packages/artifact/src/internal-config-variables.ts +++ b/packages/artifact/src/internal-config-variables.ts @@ -11,10 +11,7 @@ export function getUploadRetryCount(): number { } export function getRetryWaitTime(): number { - /* - Timeout errors can happen when uploading large amounts of files due to the huge number of http calls being made - */ - return 30000 + return 10000 } export function getDownloadFileConcurrency(): number { diff --git a/packages/artifact/src/internal-upload-http-client.ts b/packages/artifact/src/internal-upload-http-client.ts index 7653e180..1d3a8af4 100644 --- a/packages/artifact/src/internal-upload-http-client.ts +++ b/packages/artifact/src/internal-upload-http-client.ts @@ -132,7 +132,6 @@ export class UploadHttpClient { parallelUploads.map(async index => { while (currentFile < filesToUpload.length) { const currentFileParameters = parameters[currentFile] - this.statusReporter.incrementProcessedCount() currentFile += 1 if (abortPendingFileUploads) { failedItemsToReport.push(currentFileParameters.file) @@ -161,6 +160,7 @@ export class UploadHttpClient { abortPendingFileUploads = true } } + this.statusReporter.incrementProcessedCount() } }) ) @@ -217,9 +217,7 @@ export class UploadHttpClient { // the entire file should be uploaded with a single call if (uploadFileSize > parameters.maxChunkSize) { - throw new Error( - 'Chunk size is too large to upload with a single call' - ) + throw new Error('Chunk size is too large to upload with a single call') } const result = await this.uploadChunk( @@ -279,6 +277,11 @@ export class UploadHttpClient { continue } + // if an individual file is greater than 100MB (1024*1024*100) in size, display extra information about the upload status + if (uploadFileSize > 104857600){ + this.statusReporter.updateLargeFileStatus(parameters.file, `Uploading ${parameters.file} (${((offset/uploadFileSize)*100).toFixed(1)}%)`) + } + const start = offset const end = offset + chunkSize - 1 offset += parameters.maxChunkSize @@ -374,10 +377,10 @@ export class UploadHttpClient { while (retryCount <= retryLimit) { try { const response = await uploadChunkRequest() - // read the body to properly drain the response before possibly reusing the connection - await response.readBody() if (isSuccessStatusCode(response.message.statusCode)) { + // read the body to properly drain the response before possibly reusing the connection + await response.readBody() return true } else if (isRetryableStatusCode(response.message.statusCode)) { // dispose the existing connection and create a new one @@ -390,7 +393,7 @@ export class UploadHttpClient { return false } else { info( - `HTTP ${response.message.statusCode} during chunk upload, will retry at offset ${start} after 10 seconds. Retry count #${retryCount}. URL ${resourceUrl}` + `HTTP ${response.message.statusCode} during chunk upload, will retry at offset ${start} after ${getRetryWaitTime} milliseconds. Retry count #${retryCount}. URL ${resourceUrl}` ) await new Promise(resolve => setTimeout(resolve, getRetryWaitTime()) @@ -407,6 +410,9 @@ export class UploadHttpClient { // if an error is thrown, it is most likely due to a timeout, dispose of the connection, wait and retry with a new connection this.uploadHttpManager.disposeClient(httpClientIndex) + // eslint-disable-next-line no-console + console.log(error) + retryCount++ if (retryCount > retryLimit) { info( diff --git a/packages/artifact/src/internal-upload-status-reporter.ts b/packages/artifact/src/internal-upload-status-reporter.ts index 88549697..afce0229 100644 --- a/packages/artifact/src/internal-upload-status-reporter.ts +++ b/packages/artifact/src/internal-upload-status-reporter.ts @@ -1,14 +1,16 @@ import {info} from '@actions/core' -// Displays information about the progress/status of an artifact being uploaded +// displays information about the progress/status of an artifact being uploaded export class UploadStatusReporter { - private displayDelay = 10000 private totalNumberOfFilesToUpload = 0 private processedCount = 0 - private uploadStatus: NodeJS.Timeout | undefined + private largeUploads = new Map() + private totalUploadStatus: NodeJS.Timeout | undefined + private largeFileUploadStatus: NodeJS.Timeout | undefined constructor() { - this.uploadStatus = undefined + this.totalUploadStatus = undefined + this.largeFileUploadStatus = undefined } setTotalNumberOfFilesToUpload(fileTotal: number): void { @@ -17,7 +19,9 @@ export class UploadStatusReporter { startDisplayingStatus(): void { const _this = this - this.uploadStatus = setInterval(function() { + + // displays information about the total upload status every 10 seconds + this.totalUploadStatus = setInterval(function() { info( `Total file(s): ${ _this.totalNumberOfFilesToUpload @@ -26,12 +30,30 @@ export class UploadStatusReporter { 100 ).toFixed(1)}%)` ) - }, this.displayDelay) + }, 10000) + + // displays extra information about any large files that take a significant amount of time to upload every 1 second + this.largeFileUploadStatus = setInterval(function() { + for (const value of Array.from(_this.largeUploads.values())) { + info(value) + } + // delete all entires in the map after displaying the information so it will not be displayed again unless explicitly added + _this.largeUploads = new Map() + }, 1000) + } + + updateLargeFileStatus(fileName: string, displayInformation: string): void { + // any previously added display information should be overwritten for the specific large file because a map is being used + this.largeUploads.set(fileName, displayInformation) } stopDisplayingStatus(): void { - if (this.uploadStatus) { - clearInterval(this.uploadStatus) + if (this.totalUploadStatus) { + clearInterval(this.totalUploadStatus) + } + + if (this.largeFileUploadStatus) { + clearInterval(this.largeFileUploadStatus) } } diff --git a/packages/artifact/src/internal-utils.ts b/packages/artifact/src/internal-utils.ts index 59a487ae..13c75be3 100644 --- a/packages/artifact/src/internal-utils.ts +++ b/packages/artifact/src/internal-utils.ts @@ -42,8 +42,7 @@ export function isRetryableStatusCode(statusCode?: number): boolean { const retryableStatusCodes = [ HttpCodes.BadGateway, HttpCodes.ServiceUnavailable, - HttpCodes.GatewayTimeout, - HttpCodes.InternalServerError + HttpCodes.GatewayTimeout ] return retryableStatusCodes.includes(statusCode) }