Skip to content

Commit

Permalink
Merge pull request #1140 from actions/segment-download-timeout
Browse files Browse the repository at this point in the history
Introducing timeout in segment download to avoid the download stuck problem
  • Loading branch information
kotewar authored Aug 9, 2022
2 parents 3099549 + 3fd7f66 commit d714ea0
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 70 deletions.
67 changes: 6 additions & 61 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion packages/cache/RELEASES.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,4 +74,7 @@

### 3.0.1
- Fix [#833](https://github.com/actions/cache/issues/833) - cache doesn't work with github workspace directory.
- Fix [#809](https://github.com/actions/cache/issues/809) `zstd -d: no such file or directory` error on AWS self-hosted runners.
- Fix [#809](https://github.com/actions/cache/issues/809) `zstd -d: no such file or directory` error on AWS self-hosted runners.

### 3.0.2
- Added 1 hour timeout for the download stuck issue [#810](https://github.com/actions/cache/issues/810).
7 changes: 5 additions & 2 deletions packages/cache/__tests__/options.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
const useAzureSdk = true
const downloadConcurrency = 8
const timeoutInMs = 30000
const segmentTimeoutInMs = 3600000
const uploadConcurrency = 4
const uploadChunkSize = 32 * 1024 * 1024

Expand All @@ -17,15 +18,17 @@ test('getDownloadOptions sets defaults', async () => {
expect(actualOptions).toEqual({
useAzureSdk,
downloadConcurrency,
timeoutInMs
timeoutInMs,
segmentTimeoutInMs
})
})

test('getDownloadOptions overrides all settings', async () => {
const expectedOptions: DownloadOptions = {
useAzureSdk: false,
downloadConcurrency: 14,
timeoutInMs: 20000
timeoutInMs: 20000,
segmentTimeoutInMs: 3600000
}

const actualOptions = getDownloadOptions(expectedOptions)
Expand Down
4 changes: 2 additions & 2 deletions packages/cache/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/cache/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@actions/cache",
"version": "3.0.1",
"version": "3.0.2",
"preview": true,
"description": "Actions cache lib",
"keywords": [
Expand Down
11 changes: 9 additions & 2 deletions packages/cache/src/internal/downloadUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import {SocketTimeout} from './constants'
import {DownloadOptions} from '../options'
import {retryHttpClientResponse} from './requestUtils'

import {AbortController} from '@azure/abort-controller'

/**
* Pipes the body of a HTTP response to a stream
*
Expand Down Expand Up @@ -247,7 +249,12 @@ export async function downloadCacheStorageSDK(

try {
downloadProgress.startDisplayTimer()

const abortSignal = AbortController.timeout(
options.segmentTimeoutInMs || 3600000
)
abortSignal.addEventListener('abort', () => {
core.warning('Aborting cache download as it exceeded the timeout.')
})
while (!downloadProgress.isDone()) {
const segmentStart =
downloadProgress.segmentOffset + downloadProgress.segmentSize
Expand All @@ -263,11 +270,11 @@ export async function downloadCacheStorageSDK(
segmentStart,
segmentSize,
{
abortSignal,
concurrency: options.downloadConcurrency,
onProgress: downloadProgress.onProgress()
}
)

fs.writeFileSync(fd, result)
}
} finally {
Expand Down
15 changes: 14 additions & 1 deletion packages/cache/src/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,13 @@ export interface DownloadOptions {
* @default 30000
*/
timeoutInMs?: number

/**
* Time after which a segment download should be aborted if stuck
*
* @default 3600000
*/
segmentTimeoutInMs?: number
}

/**
Expand Down Expand Up @@ -84,7 +91,8 @@ export function getDownloadOptions(copy?: DownloadOptions): DownloadOptions {
const result: DownloadOptions = {
useAzureSdk: true,
downloadConcurrency: 8,
timeoutInMs: 30000
timeoutInMs: 30000,
segmentTimeoutInMs: 3600000
}

if (copy) {
Expand All @@ -99,11 +107,16 @@ export function getDownloadOptions(copy?: DownloadOptions): DownloadOptions {
if (typeof copy.timeoutInMs === 'number') {
result.timeoutInMs = copy.timeoutInMs
}

if (typeof copy.segmentTimeoutInMs === 'number') {
result.segmentTimeoutInMs = copy.segmentTimeoutInMs
}
}

core.debug(`Use Azure SDK: ${result.useAzureSdk}`)
core.debug(`Download concurrency: ${result.downloadConcurrency}`)
core.debug(`Request timeout (ms): ${result.timeoutInMs}`)
core.debug(`Segment download timeout (ms): ${result.segmentTimeoutInMs}`)

return result
}

0 comments on commit d714ea0

Please sign in to comment.