Skip to content

Commit 73252d4

Browse files
committed
feat(queue): Make batch size configurable for artifact removal
adds `EXPIRE_ARTIFACTS_BATCH_SIZE` env var support
1 parent f682281 commit 73252d4

File tree

10 files changed

+39
-9
lines changed

10 files changed

+39
-9
lines changed

changelog/issue-6405.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,4 @@ reference: issue 6405
44
---
55

66
Expire artifacts supports both bulk deletion and single deletion. This can be configured for the deployment using `AWS_USE_BULK_DELETE` environment variable (`false` by default). This is needed because not all S3 compatible storages support bulk delete, specifically [GCS](https://cloud.google.com/storage/docs/migrating#methods-comparison).
7+
`EXPIRE_ARTIFACTS_BATCH_SIZE` can be used to control how many records to process at once, i.e. how many parallel delete requests would be sent to storage service (`100` by default).

dev-docs/dev-config-example.yml

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

docker/env/.queue

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ SIGN_PUBLIC_ARTIFACT_URLS=
77
WORKER_INFO_UPDATE_FREQUENCY=
88
PUBLIC_ARTIFACT_BUCKET_CDN=
99
TASK_CACHE_MAX_SIZE=
10+
EXPIRE_ARTIFACTS_BATCH_SIZE=
1011
TASKCLUSTER_ROOT_URL=http://taskcluster
1112
TASKCLUSTER_CLIENT_ID=static/taskcluster/queue
1213
TASKCLUSTER_ACCESS_TOKEN=j2Z6zW2QSLehailBXlosdw9e2Ti8R_Qh2M4buAEQfsMA

generated/references.json

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

infrastructure/k8s/templates/taskcluster-queue-secret.yaml

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

infrastructure/k8s/values.schema.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1057,6 +1057,9 @@
10571057
"title": "node debug env var",
10581058
"type": "string"
10591059
},
1060+
"expire_artifacts_batch_size": {
1061+
"type": "number"
1062+
},
10601063
"level": {
10611064
"type": "string"
10621065
},

services/queue/config.yml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,10 @@ defaults:
7171
# CDN for public artifact bucket
7272
publicArtifactBucketCDN: !env:optional PUBLIC_ARTIFACT_BUCKET_CDN
7373

74-
taskCacheMaxSize: !env:number:optional TASK_CACHE_MAX_SIZE
74+
taskCacheMaxSize: !env:number:optional TASK_CACHE_MAX_SIZE
75+
76+
# How many artifacts to remove at once when bulk deletion is disabled
77+
expireArtifactsBatchSize: !env:number:optional EXPIRE_ARTIFACTS_BATCH_SIZE
7578

7679

7780
taskcluster:

services/queue/src/main.js

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@ const DEFAULT_CLAIM_TIMEOUT = 1200;
2929
// default `expires`, `last_date_active` update frequency
3030
const DEFAULT_UPDATE_FREQUENCY = '30 minutes';
3131

32+
// for supported bulk deletion S3 operations
33+
const MAX_BULK_DELETE_SIZE = 1000;
34+
3235
require('./monitor');
3336

3437
// Create component loader
@@ -297,15 +300,24 @@ let load = loader({
297300
setup: ({ cfg, db, publicArtifactBucket, privateArtifactBucket, monitor }, ownName) => {
298301
return monitor.oneShot(ownName, async () => {
299302
const now = taskcluster.fromNow(cfg.app.artifactExpirationDelay);
300-
debug('Expiring artifacts at: %s, from before %s', new Date(), now);
303+
const useBulkDelete = !!cfg.aws.useBulkDelete;
304+
// when using bulk delete, maximum number of objects in bulk request is 1000
305+
// if using single delete, we have to be cautious not to overload the API with too many parallel requests
306+
// so we limit the batch size to 100 by default, which proved to be a good value in production
307+
const expireArtifactsBatchSize = useBulkDelete ? MAX_BULK_DELETE_SIZE : (cfg.expireArtifactsBatchSize || 100);
308+
309+
debug('Expiring artifacts at: %s, from before %s, useBulkDelete: %s, batchSize: %d',
310+
new Date(), now, useBulkDelete, expireArtifactsBatchSize);
311+
301312
const count = await artifactUtils.expire({
302313
db,
303314
publicBucket: publicArtifactBucket,
304315
privateBucket: privateArtifactBucket,
305316
monitor,
306317
ignoreError: false,
307318
expires: now,
308-
useBulkDelete: !!cfg.aws.useBulkDelete,
319+
expireArtifactsBatchSize,
320+
useBulkDelete,
309321
});
310322
debug('Expired %s artifacts', count);
311323
monitor.log.expiredArtifactsRemoved({ count, expires: now });

services/queue/src/utils.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,17 +42,18 @@ const artifactUtils = {
4242
* need to handle that case.
4343
*/
4444
async expire({ db, publicBucket, privateBucket, ignoreError, monitor,
45-
expires, useBulkDelete = false }) {
45+
expires, useBulkDelete, expireArtifactsBatchSize }) {
4646
let count = 0;
47-
const maxPageSize = useBulkDelete ? 1000 : 100;
47+
48+
assert(!useBulkDelete || expireArtifactsBatchSize <= 1000, 'expireArtifactsBatchSize must be <= 1000 when useBulkDelete is true');
4849

4950
// Fetch all expired artifacts and batch delete the S3 ones
5051
// then remove the entity from the database
5152
// repeat until there are no more expired artifacts
5253
while (true) {
5354
const rows = await db.fns.get_expired_artifacts_for_deletion({
5455
expires_in: expires,
55-
page_size_in: maxPageSize,
56+
page_size_in: expireArtifactsBatchSize,
5657
});
5758
if (!rows.length) {
5859
break;

services/queue/test/expireartifacts_test.js

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,10 @@ helper.secrets.mockSuite(testing.suiteName(), ['aws'], function (mock, skipping)
1717
const MAX_ARTIFACTS = 5;
1818

1919
[
20-
['expire s3 artifacts using bulk delete', true],
21-
['expire s3 artifacts using single delete', false],
22-
].forEach(([name, useBulkDelete]) =>
20+
['expire s3 artifacts using bulk delete', true, undefined],
21+
['expire s3 artifacts using single delete', false, undefined],
22+
['expire s3 artifacts using single delete and batch size 1', false, 1],
23+
].forEach(([name, useBulkDelete, batchSize]) =>
2324
test(name, async () => {
2425
const yesterday = taskcluster.fromNow('-1 day');
2526
const today = new Date();
@@ -28,6 +29,9 @@ helper.secrets.mockSuite(testing.suiteName(), ['aws'], function (mock, skipping)
2829

2930
await helper.load('cfg');
3031
helper.load.cfg('aws.useBulkDelete', useBulkDelete);
32+
if (batchSize) {
33+
helper.load.cfg('expireArtifactsBatchSize', batchSize);
34+
}
3135

3236
for (let i = 0; i < MAX_ARTIFACTS; i++) {
3337
await helper.db.fns.create_queue_artifact(

0 commit comments

Comments
 (0)