Skip to content

Commit a10e209

Browse files
Link-Link-
authored
Merge pull request #1882 from actions/enhance-blob-client
Enhance blob client resilience & performance
2 parents 9cc30cb + c02c929 commit a10e209

11 files changed

+514
-256
lines changed

Diff for: ‎packages/cache/__tests__/options.test.ts

+36-7
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@ const downloadConcurrency = 8
1111
const timeoutInMs = 30000
1212
const segmentTimeoutInMs = 600000
1313
const lookupOnly = false
14-
const uploadConcurrency = 4
15-
const uploadChunkSize = 32 * 1024 * 1024
1614

1715
test('getDownloadOptions sets defaults', async () => {
1816
const actualOptions = getDownloadOptions()
@@ -43,25 +41,56 @@ test('getDownloadOptions overrides all settings', async () => {
4341
})
4442

4543
test('getUploadOptions sets defaults', async () => {
44+
const expectedOptions: UploadOptions = {
45+
uploadConcurrency: 4,
46+
uploadChunkSize: 32 * 1024 * 1024,
47+
useAzureSdk: false
48+
}
4649
const actualOptions = getUploadOptions()
4750

48-
expect(actualOptions).toEqual({
49-
uploadConcurrency,
50-
uploadChunkSize
51-
})
51+
expect(actualOptions).toEqual(expectedOptions)
5252
})
5353

5454
test('getUploadOptions overrides all settings', async () => {
5555
const expectedOptions: UploadOptions = {
5656
uploadConcurrency: 2,
57-
uploadChunkSize: 16 * 1024 * 1024
57+
uploadChunkSize: 16 * 1024 * 1024,
58+
useAzureSdk: true
5859
}
5960

6061
const actualOptions = getUploadOptions(expectedOptions)
6162

6263
expect(actualOptions).toEqual(expectedOptions)
6364
})
6465

66+
test('env variables override all getUploadOptions settings', async () => {
67+
const expectedOptions: UploadOptions = {
68+
uploadConcurrency: 16,
69+
uploadChunkSize: 64 * 1024 * 1024,
70+
useAzureSdk: true
71+
}
72+
73+
process.env.CACHE_UPLOAD_CONCURRENCY = '16'
74+
process.env.CACHE_UPLOAD_CHUNK_SIZE = '64'
75+
76+
const actualOptions = getUploadOptions(expectedOptions)
77+
expect(actualOptions).toEqual(expectedOptions)
78+
})
79+
80+
test('env variables override all getUploadOptions settings but do not exceed caps', async () => {
81+
const expectedOptions: UploadOptions = {
82+
uploadConcurrency: 32,
83+
uploadChunkSize: 128 * 1024 * 1024,
84+
useAzureSdk: true
85+
}
86+
87+
process.env.CACHE_UPLOAD_CONCURRENCY = '64'
88+
process.env.CACHE_UPLOAD_CHUNK_SIZE = '256'
89+
90+
const actualOptions = getUploadOptions(expectedOptions)
91+
expect(actualOptions).toEqual(expectedOptions)
92+
})
93+
6594
test('getDownloadOptions overrides download timeout minutes', async () => {
6695
const expectedOptions: DownloadOptions = {
6796
useAzureSdk: false,

Diff for: ‎packages/cache/__tests__/restoreCacheV2.test.ts

+24-51
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@ import * as path from 'path'
33
import * as tar from '../src/internal/tar'
44
import * as config from '../src/internal/config'
55
import * as cacheUtils from '../src/internal/cacheUtils'
6-
import * as downloadCacheModule from '../src/internal/blob/download-cache'
6+
import * as cacheHttpClient from '../src/internal/cacheHttpClient'
77
import {restoreCache} from '../src/cache'
88
import {CacheFilename, CompressionMethod} from '../src/internal/constants'
99
import {CacheServiceClientJSON} from '../src/generated/results/api/v1/cache.twirp'
10-
import {BlobDownloadResponseParsed} from '@azure/storage-blob'
10+
import {DownloadOptions} from '../src/options'
1111

1212
jest.mock('../src/internal/cacheHttpClient')
1313
jest.mock('../src/internal/cacheUtils')
@@ -142,6 +142,7 @@ test('restore with gzip compressed cache found', async () => {
142142
const signedDownloadUrl = 'https://blob-storage.local?signed=true'
143143
const cacheVersion =
144144
'd90f107aaeb22920dba0c637a23c37b5bc497b4dfa3b07fe3f79bf88a273c11b'
145+
const options = {useAzureSdk: true} as DownloadOptions
145146

146147
const getCacheVersionMock = jest.spyOn(cacheUtils, 'getCacheVersion')
147148
getCacheVersionMock.mockReturnValue(cacheVersion)
@@ -169,17 +170,7 @@ test('restore with gzip compressed cache found', async () => {
169170
})
170171

171172
const archivePath = path.join(tempPath, CacheFilename.Gzip)
172-
const downloadCacheFileMock = jest.spyOn(
173-
downloadCacheModule,
174-
'downloadCacheFile'
175-
)
176-
downloadCacheFileMock.mockReturnValue(
177-
Promise.resolve({
178-
_response: {
179-
status: 200
180-
}
181-
} as BlobDownloadResponseParsed)
182-
)
173+
const downloadCacheMock = jest.spyOn(cacheHttpClient, 'downloadCache')
183174

184175
const fileSize = 142
185176
const getArchiveFileSizeInBytesMock = jest
@@ -189,7 +180,7 @@ test('restore with gzip compressed cache found', async () => {
189180
const extractTarMock = jest.spyOn(tar, 'extractTar')
190181
const unlinkFileMock = jest.spyOn(cacheUtils, 'unlinkFile')
191182

192-
const cacheKey = await restoreCache(paths, key)
183+
const cacheKey = await restoreCache(paths, key, [], options)
193184

194185
expect(cacheKey).toBe(key)
195186
expect(getCacheVersionMock).toHaveBeenCalledWith(
@@ -203,9 +194,10 @@ test('restore with gzip compressed cache found', async () => {
203194
version: cacheVersion
204195
})
205196
expect(createTempDirectoryMock).toHaveBeenCalledTimes(1)
206-
expect(downloadCacheFileMock).toHaveBeenCalledWith(
197+
expect(downloadCacheMock).toHaveBeenCalledWith(
207198
signedDownloadUrl,
208-
archivePath
199+
archivePath,
200+
options
209201
)
210202
expect(getArchiveFileSizeInBytesMock).toHaveBeenCalledWith(archivePath)
211203
expect(logInfoMock).toHaveBeenCalledWith(`Cache Size: ~0 MB (142 B)`)
@@ -226,6 +218,7 @@ test('restore with zstd compressed cache found', async () => {
226218
const signedDownloadUrl = 'https://blob-storage.local?signed=true'
227219
const cacheVersion =
228220
'8e2e96a184cb0cd6b48285b176c06a418f3d7fce14c29d9886fd1bb4f05c513d'
221+
const options = {useAzureSdk: true} as DownloadOptions
229222

230223
const getCacheVersionMock = jest.spyOn(cacheUtils, 'getCacheVersion')
231224
getCacheVersionMock.mockReturnValue(cacheVersion)
@@ -253,17 +246,7 @@ test('restore with zstd compressed cache found', async () => {
253246
})
254247

255248
const archivePath = path.join(tempPath, CacheFilename.Zstd)
256-
const downloadCacheFileMock = jest.spyOn(
257-
downloadCacheModule,
258-
'downloadCacheFile'
259-
)
260-
downloadCacheFileMock.mockReturnValue(
261-
Promise.resolve({
262-
_response: {
263-
status: 200
264-
}
265-
} as BlobDownloadResponseParsed)
266-
)
249+
const downloadCacheMock = jest.spyOn(cacheHttpClient, 'downloadCache')
267250

268251
const fileSize = 62915000
269252
const getArchiveFileSizeInBytesMock = jest
@@ -273,7 +256,7 @@ test('restore with zstd compressed cache found', async () => {
273256
const extractTarMock = jest.spyOn(tar, 'extractTar')
274257
const unlinkFileMock = jest.spyOn(cacheUtils, 'unlinkFile')
275258

276-
const cacheKey = await restoreCache(paths, key)
259+
const cacheKey = await restoreCache(paths, key, [], options)
277260

278261
expect(cacheKey).toBe(key)
279262
expect(getCacheVersionMock).toHaveBeenCalledWith(
@@ -287,9 +270,10 @@ test('restore with zstd compressed cache found', async () => {
287270
version: cacheVersion
288271
})
289272
expect(createTempDirectoryMock).toHaveBeenCalledTimes(1)
290-
expect(downloadCacheFileMock).toHaveBeenCalledWith(
273+
expect(downloadCacheMock).toHaveBeenCalledWith(
291274
signedDownloadUrl,
292-
archivePath
275+
archivePath,
276+
options
293277
)
294278
expect(getArchiveFileSizeInBytesMock).toHaveBeenCalledWith(archivePath)
295279
expect(logInfoMock).toHaveBeenCalledWith(`Cache Size: ~60 MB (62915000 B)`)
@@ -311,6 +295,7 @@ test('restore with cache found for restore key', async () => {
311295
const signedDownloadUrl = 'https://blob-storage.local?signed=true'
312296
const cacheVersion =
313297
'b8b58e9bd7b1e8f83d9f05c7e06ea865ba44a0330e07a14db74ac74386677bed'
298+
const options = {useAzureSdk: true} as DownloadOptions
314299

315300
const getCacheVersionMock = jest.spyOn(cacheUtils, 'getCacheVersion')
316301
getCacheVersionMock.mockReturnValue(cacheVersion)
@@ -338,17 +323,7 @@ test('restore with cache found for restore key', async () => {
338323
})
339324

340325
const archivePath = path.join(tempPath, CacheFilename.Gzip)
341-
const downloadCacheFileMock = jest.spyOn(
342-
downloadCacheModule,
343-
'downloadCacheFile'
344-
)
345-
downloadCacheFileMock.mockReturnValue(
346-
Promise.resolve({
347-
_response: {
348-
status: 200
349-
}
350-
} as BlobDownloadResponseParsed)
351-
)
326+
const downloadCacheMock = jest.spyOn(cacheHttpClient, 'downloadCache')
352327

353328
const fileSize = 142
354329
const getArchiveFileSizeInBytesMock = jest
@@ -358,7 +333,7 @@ test('restore with cache found for restore key', async () => {
358333
const extractTarMock = jest.spyOn(tar, 'extractTar')
359334
const unlinkFileMock = jest.spyOn(cacheUtils, 'unlinkFile')
360335

361-
const cacheKey = await restoreCache(paths, key, restoreKeys)
336+
const cacheKey = await restoreCache(paths, key, restoreKeys, options)
362337

363338
expect(cacheKey).toBe(restoreKeys[0])
364339
expect(getCacheVersionMock).toHaveBeenCalledWith(
@@ -372,9 +347,10 @@ test('restore with cache found for restore key', async () => {
372347
version: cacheVersion
373348
})
374349
expect(createTempDirectoryMock).toHaveBeenCalledTimes(1)
375-
expect(downloadCacheFileMock).toHaveBeenCalledWith(
350+
expect(downloadCacheMock).toHaveBeenCalledWith(
376351
signedDownloadUrl,
377-
archivePath
352+
archivePath,
353+
options
378354
)
379355
expect(getArchiveFileSizeInBytesMock).toHaveBeenCalledWith(archivePath)
380356
expect(logInfoMock).toHaveBeenCalledWith(`Cache Size: ~0 MB (142 B)`)
@@ -388,14 +364,14 @@ test('restore with cache found for restore key', async () => {
388364
expect(compressionMethodMock).toHaveBeenCalledTimes(1)
389365
})
390366

391-
test('restore with dry run', async () => {
367+
test('restore with lookup only enabled', async () => {
392368
const paths = ['node_modules']
393369
const key = 'node-test'
394-
const options = {lookupOnly: true}
395370
const compressionMethod = CompressionMethod.Gzip
396371
const signedDownloadUrl = 'https://blob-storage.local?signed=true'
397372
const cacheVersion =
398373
'd90f107aaeb22920dba0c637a23c37b5bc497b4dfa3b07fe3f79bf88a273c11b'
374+
const options = {lookupOnly: true, useAzureSdk: true} as DownloadOptions
399375

400376
const getCacheVersionMock = jest.spyOn(cacheUtils, 'getCacheVersion')
401377
getCacheVersionMock.mockReturnValue(cacheVersion)
@@ -416,10 +392,7 @@ test('restore with dry run', async () => {
416392
)
417393

418394
const createTempDirectoryMock = jest.spyOn(cacheUtils, 'createTempDirectory')
419-
const downloadCacheFileMock = jest.spyOn(
420-
downloadCacheModule,
421-
'downloadCacheFile'
422-
)
395+
const downloadCacheMock = jest.spyOn(cacheHttpClient, 'downloadCache')
423396

424397
const cacheKey = await restoreCache(paths, key, undefined, options)
425398

@@ -438,5 +411,5 @@ test('restore with dry run', async () => {
438411

439412
// creating a tempDir and downloading the cache are skipped
440413
expect(createTempDirectoryMock).toHaveBeenCalledTimes(0)
441-
expect(downloadCacheFileMock).toHaveBeenCalledTimes(0)
414+
expect(downloadCacheMock).toHaveBeenCalledTimes(0)
442415
})

Diff for: ‎packages/cache/__tests__/saveCache.test.ts

+12-2
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,12 @@ test('save with server error should fail', async () => {
270270
compression
271271
)
272272
expect(saveCacheMock).toHaveBeenCalledTimes(1)
273-
expect(saveCacheMock).toHaveBeenCalledWith(cacheId, archiveFile, undefined)
273+
expect(saveCacheMock).toHaveBeenCalledWith(
274+
cacheId,
275+
archiveFile,
276+
'',
277+
undefined
278+
)
274279
expect(getCompressionMock).toHaveBeenCalledTimes(1)
275280
})
276281

@@ -315,7 +320,12 @@ test('save with valid inputs uploads a cache', async () => {
315320
compression
316321
)
317322
expect(saveCacheMock).toHaveBeenCalledTimes(1)
318-
expect(saveCacheMock).toHaveBeenCalledWith(cacheId, archiveFile, undefined)
323+
expect(saveCacheMock).toHaveBeenCalledWith(
324+
cacheId,
325+
archiveFile,
326+
'',
327+
undefined
328+
)
319329
expect(getCompressionMock).toHaveBeenCalledTimes(1)
320330
})
321331

0 commit comments

Comments
 (0)