Skip to content

Commit

Permalink
Add option for concurrent cache downloads with timeout (#1484)
Browse files Browse the repository at this point in the history
* Add option for concurrent cache downloads with timeout

* Add release notes

* Fix lint
  • Loading branch information
chkimes committed Aug 7, 2023
1 parent 19e0016 commit f74ff15
Show file tree
Hide file tree
Showing 9 changed files with 242 additions and 32 deletions.
1 change: 1 addition & 0 deletions .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
}
],
"eslint-comments/no-use": "off",
"no-constant-condition": ["error", { "checkLoops": false }],
"github/no-then": "off",
"import/no-namespace": "off",
"no-shadow": "off",
Expand Down
4 changes: 4 additions & 0 deletions packages/cache/RELEASES.md
Original file line number Diff line number Diff line change
Expand Up @@ -160,3 +160,7 @@
### 3.2.1

- Updated @azure/storage-blob to `v12.13.0`

### 3.2.2

- Add new default cache download method to improve performance and reduce hangs [#1484](https://github.com/actions/toolkit/pull/1484)
27 changes: 21 additions & 6 deletions packages/cache/__tests__/cacheHttpClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,18 +84,24 @@ test('downloadCache uses storage SDK for Azure storage URLs', async () => {
'downloadCacheStorageSDK'
)

const downloadCacheHttpClientConcurrentMock = jest.spyOn(
downloadUtils,
'downloadCacheHttpClientConcurrent'
)

const archiveLocation = 'http://foo.blob.core.windows.net/bar/baz'
const archivePath = '/foo/bar'

await downloadCache(archiveLocation, archivePath)

expect(downloadCacheStorageSDKMock).toHaveBeenCalledTimes(1)
expect(downloadCacheStorageSDKMock).toHaveBeenCalledWith(
expect(downloadCacheHttpClientConcurrentMock).toHaveBeenCalledTimes(1)
expect(downloadCacheHttpClientConcurrentMock).toHaveBeenCalledWith(
archiveLocation,
archivePath,
getDownloadOptions()
)

expect(downloadCacheStorageSDKMock).toHaveBeenCalledTimes(0)
expect(downloadCacheHttpClientMock).toHaveBeenCalledTimes(0)
})

Expand All @@ -109,20 +115,26 @@ test('downloadCache passes options to download methods', async () => {
'downloadCacheStorageSDK'
)

const downloadCacheHttpClientConcurrentMock = jest.spyOn(
downloadUtils,
'downloadCacheHttpClientConcurrent'
)

const archiveLocation = 'http://foo.blob.core.windows.net/bar/baz'
const archivePath = '/foo/bar'
const options: DownloadOptions = {downloadConcurrency: 4}

await downloadCache(archiveLocation, archivePath, options)

expect(downloadCacheStorageSDKMock).toHaveBeenCalledTimes(1)
expect(downloadCacheStorageSDKMock).toHaveBeenCalled()
expect(downloadCacheStorageSDKMock).toHaveBeenCalledWith(
expect(downloadCacheHttpClientConcurrentMock).toHaveBeenCalledTimes(1)
expect(downloadCacheHttpClientConcurrentMock).toHaveBeenCalled()
expect(downloadCacheHttpClientConcurrentMock).toHaveBeenCalledWith(
archiveLocation,
archivePath,
getDownloadOptions(options)
)

expect(downloadCacheStorageSDKMock).toHaveBeenCalledTimes(0)
expect(downloadCacheHttpClientMock).toHaveBeenCalledTimes(0)
})

Expand All @@ -138,7 +150,10 @@ test('downloadCache uses http-client when overridden', async () => {

const archiveLocation = 'http://foo.blob.core.windows.net/bar/baz'
const archivePath = '/foo/bar'
const options: DownloadOptions = {useAzureSdk: false}
const options: DownloadOptions = {
useAzureSdk: false,
concurrentBlobDownloads: false
}

await downloadCache(archiveLocation, archivePath, options)

Expand Down
7 changes: 5 additions & 2 deletions packages/cache/__tests__/options.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ import {
getUploadOptions
} from '../src/options'

const useAzureSdk = true
const useAzureSdk = false
const concurrentBlobDownloads = true
const downloadConcurrency = 8
const timeoutInMs = 30000
const segmentTimeoutInMs = 600000
Expand All @@ -18,6 +19,7 @@ test('getDownloadOptions sets defaults', async () => {

expect(actualOptions).toEqual({
useAzureSdk,
concurrentBlobDownloads,
downloadConcurrency,
timeoutInMs,
segmentTimeoutInMs,
Expand All @@ -27,7 +29,8 @@ test('getDownloadOptions sets defaults', async () => {

test('getDownloadOptions overrides all settings', async () => {
const expectedOptions: DownloadOptions = {
useAzureSdk: false,
useAzureSdk: true,
concurrentBlobDownloads: false,
downloadConcurrency: 14,
timeoutInMs: 20000,
segmentTimeoutInMs: 3600000,
Expand Down
18 changes: 9 additions & 9 deletions packages/cache/package-lock.json

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

4 changes: 2 additions & 2 deletions packages/cache/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@actions/cache",
"version": "3.2.1",
"version": "3.2.2",
"preview": true,
"description": "Actions cache lib",
"keywords": [
Expand Down Expand Up @@ -40,7 +40,7 @@
"@actions/core": "^1.10.0",
"@actions/exec": "^1.0.1",
"@actions/glob": "^0.1.0",
"@actions/http-client": "^2.0.1",
"@actions/http-client": "^2.1.1",
"@actions/io": "^1.0.1",
"@azure/abort-controller": "^1.1.0",
"@azure/ms-rest-js": "^2.6.0",
Expand Down
32 changes: 24 additions & 8 deletions packages/cache/src/internal/cacheHttpClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@ import {
ITypedResponseWithError,
ArtifactCacheList
} from './contracts'
import {downloadCacheHttpClient, downloadCacheStorageSDK} from './downloadUtils'
import {
downloadCacheHttpClient,
downloadCacheHttpClientConcurrent,
downloadCacheStorageSDK
} from './downloadUtils'
import {
DownloadOptions,
UploadOptions,
Expand Down Expand Up @@ -171,14 +175,26 @@ export async function downloadCache(
const archiveUrl = new URL(archiveLocation)
const downloadOptions = getDownloadOptions(options)

if (
downloadOptions.useAzureSdk &&
archiveUrl.hostname.endsWith('.blob.core.windows.net')
) {
// Use Azure storage SDK to download caches hosted on Azure to improve speed and reliability.
await downloadCacheStorageSDK(archiveLocation, archivePath, downloadOptions)
if (archiveUrl.hostname.endsWith('.blob.core.windows.net')) {
if (downloadOptions.useAzureSdk) {
// Use Azure storage SDK to download caches hosted on Azure to improve speed and reliability.
await downloadCacheStorageSDK(
archiveLocation,
archivePath,
downloadOptions
)
} else if (downloadOptions.concurrentBlobDownloads) {
// Use concurrent implementation with HttpClient to work around blob SDK issue
await downloadCacheHttpClientConcurrent(
archiveLocation,
archivePath,
downloadOptions
)
} else {
// Otherwise, download using the Actions http-client.
await downloadCacheHttpClient(archiveLocation, archivePath)
}
} else {
// Otherwise, download using the Actions http-client.
await downloadCacheHttpClient(archiveLocation, archivePath)
}
}
Expand Down

0 comments on commit f74ff15

Please sign in to comment.