Skip to content

Commit aba8cb1

Browse files
committed
CLDSRV-992: Bound and settle the 1000-object writes in Multi-Object Delete setup
The "Multi-Object Delete Success" beforeEach uploads 1000 objects behind what looks like a concurrency limiter of 20. It is not one. objects.map() invokes all 1000 async calls synchronously; the first 20 push and the other 980 all park on the same `await Promise.race(queued)`. When the first put settles they all wake, and each splices one entry off the head of the queue regardless of whether that entry had settled, so the queue empties, every waiter's `while` test goes false, and the remaining puts fire together. Running the limiter against an instrumented stub gives a peak of 999 concurrent puts against an intended 20. That is the ServiceUnavailable seen in run 33681270087: a thousand simultaneous puts, not twenty. It also explains why one failure becomes seven. Promise.all rejects on the first error while the other puts are still in flight, so the hook throws with writes outstanding, and the bucket cannot be deleted. All four suites in this file share the bucket name multi-object-delete-234-634, so once it is left behind with objects in it, every later suite's cleanup fails with BucketNotEmpty -- which is exactly the shape of the run: one ServiceUnavailable followed by six BucketNotEmpty. Two changes, both already present on development/9.3 and newer in one form or another: - Replace the limiter with a fixed pool of 20 workers draining a shared queue. Concurrency is genuinely bounded and every put is settled before the hook returns, on the failure path too, so teardown can no longer race them. - Empty the bucket before deleting it in all four cleanup hooks. The 9.3 line gained this in 9eb5777, but that commit is an aws-sdk v3 migration of the whole file and is not something to cherry-pick onto 9.2; this is the equivalent written for the v2 client, and nothing else from it. Clears CLDSRV-992 row F10 (all seven failures). Issue: CLDSRV-992
1 parent ce53804 commit aba8cb1

1 file changed

Lines changed: 42 additions & 19 deletions

File tree

tests/functional/aws-node-sdk/test/object/multiObjectDelete.js

Lines changed: 42 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -68,29 +68,43 @@ describe('Multi-Object Delete Success', function success() {
6868
objects.push(`${key}${i}`);
6969
}
7070
const parallel = 20;
71-
const queued = [];
72-
const putObjectWithLimit = async key => {
73-
while (queued.length >= parallel) {
74-
await Promise.race(queued);
75-
queued.splice(0, queued.findIndex(p => p === queued[0]) + 1);
71+
// A fixed pool of workers draining a shared queue. The previous
72+
// limiter admitted every put at once (measured peak in flight: 999
73+
// of 1000), which is what provoked the ServiceUnavailable; and
74+
// because Promise.all rejects on the first failure, it left the
75+
// rest running, so writes kept landing after the teardown had
76+
// emptied the bucket and deleteBucket then failed with
77+
// BucketNotEmpty. Here concurrency is really bounded, and every put
78+
// has settled before this hook returns, failure included.
79+
const pending = objects.slice();
80+
const putErrors = [];
81+
await Promise.all(Array.from({ length: parallel }, async () => {
82+
while (pending.length > 0) {
83+
const objectKey = pending.shift();
84+
try {
85+
await s3.putObject({
86+
Bucket: bucketName,
87+
Key: objectKey,
88+
Body: 'somebody',
89+
}).promise();
90+
} catch (err) {
91+
putErrors.push(err);
92+
}
7693
}
77-
const result = s3.putObject({
78-
Bucket: bucketName,
79-
Key: key,
80-
Body: 'somebody',
81-
}).promise();
82-
queued.push(result);
83-
return result;
84-
};
85-
const putPromises = objects.map(key => putObjectWithLimit(key));
86-
await Promise.all(putPromises);
94+
}));
95+
if (putErrors.length > 0) {
96+
throw putErrors[0];
97+
}
8798
} catch (err) {
8899
process.stdout.write(`Error creating objects: ${err}\n`);
89100
throw err;
90101
}
91102
});
92103

93-
afterEach(() => s3.deleteBucket({ Bucket: bucketName }).promise());
104+
afterEach(async () => {
105+
await bucketUtil.empty(bucketName);
106+
await s3.deleteBucket({ Bucket: bucketName }).promise();
107+
});
94108

95109
it('should batch delete 1000 objects', done => {
96110
const objects = createObjectsList(1000);
@@ -150,7 +164,10 @@ describe('Multi-Object Delete Error Responses', () => {
150164
});
151165
});
152166

153-
afterEach(() => s3.deleteBucket({ Bucket: bucketName }).promise());
167+
afterEach(async () => {
168+
await bucketUtil.empty(bucketName);
169+
await s3.deleteBucket({ Bucket: bucketName }).promise();
170+
});
154171

155172
it('should return error if request deletion of more than 1000 objects',
156173
() => {
@@ -240,7 +257,10 @@ describe('Multi-Object Delete Access', function access() {
240257
});
241258
});
242259

243-
after(() => s3.deleteBucket({ Bucket: bucketName }).promise());
260+
after(async () => {
261+
await bucketUtil.empty(bucketName);
262+
await s3.deleteBucket({ Bucket: bucketName }).promise();
263+
});
244264

245265
it('should return access denied error for each object where no acl ' +
246266
'permission', () => {
@@ -339,7 +359,10 @@ describeSkipIfCeph('Multi-Object Delete with Object Lock', () => {
339359
});
340360
});
341361

342-
after(() => s3.deleteBucket({ Bucket: bucketName }).promise());
362+
after(async () => {
363+
await bucketUtil.empty(bucketName);
364+
await s3.deleteBucket({ Bucket: bucketName }).promise();
365+
});
343366

344367
it('should not delete locked objects', () => {
345368
const objects = createObjectsList(5, versionIds);

0 commit comments

Comments
 (0)