Skip to content

Commit b9bc28b

Browse files
committedSep 28, 2022
Merge branch 'bugfix/S3UTILS-105-fixBucketStreamHang' into q/1.13
2 parents 58a9e1c + 96bc4f3 commit b9bc28b

File tree

3 files changed

+75
-12
lines changed

3 files changed

+75
-12
lines changed
 

‎CompareRaftMembers/BucketStream.js

+5
Original file line numberDiff line numberDiff line change
@@ -149,17 +149,22 @@ class BucketStream extends stream.Readable {
149149
// retries: destroy the stream
150150
return this.destroy(err);
151151
}
152+
let hasPushed = false;
152153
for (const item of listing) {
153154
if (item) {
154155
const fullKey = `${this.bucketName}/${item.key}`;
155156
this.push({
156157
key: fullKey,
157158
value: item.value,
158159
});
160+
hasPushed = true;
159161
}
160162
}
161163
if (IsTruncated && !ended) {
162164
this.marker = Contents[Contents.length - 1].key;
165+
if (!hasPushed) {
166+
this._listMore();
167+
}
163168
} else {
164169
this.push(null);
165170
}

‎package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "s3utils",
3-
"version": "1.13.5",
3+
"version": "1.13.6",
44
"engines": {
55
"node": ">= 16"
66
},

‎tests/unit/CompareRaftMembers/BucketStream.js

+69-11
Original file line numberDiff line numberDiff line change
@@ -74,22 +74,50 @@ function buildTestBucketContents() {
7474
// introduce a replay key to check those are filtered out
7575
contents.push({
7676
key: `${versioning.VersioningConstants.DbPrefixes.Replay}foobar`,
77-
value: '{}',
77+
value: {},
78+
});
79+
return contents;
80+
}
81+
82+
function buildBucketWithReplayKeysContents() {
83+
const contents = [{
84+
key: 'foo',
85+
value: { location: null },
86+
}];
87+
for (let i = 0; i < 3000; ++i) {
88+
contents.push({
89+
key: `${versioning.VersioningConstants.DbPrefixes.Replay}replay-key-${`0000${i}`.slice(-4)}`,
90+
value: {},
91+
});
92+
}
93+
contents.push({
94+
key: 'éléphant',
95+
value: { location: null },
7896
});
7997
return contents;
8098
}
8199

82100
const TEST_BUCKET_CONTENTS = buildTestBucketContents();
101+
const BUCKET_WITH_REPLAY_KEYS_CONTENTS = buildBucketWithReplayKeysContents();
83102

84103
describe('BucketStream', () => {
85104
let httpServer;
86105
let reqCount = 0;
87106
beforeAll(() => {
88107
const handleListBucketRequest = (req, res, url) => {
89108
const marker = url.searchParams.get('marker');
109+
let rawContents;
110+
const bucketName = url.pathname.slice('/default/bucket/'.length);
111+
if (bucketName === 'test-bucket') {
112+
rawContents = TEST_BUCKET_CONTENTS;
113+
} else if (bucketName === 'bucket-with-replay-keys') {
114+
rawContents = BUCKET_WITH_REPLAY_KEYS_CONTENTS;
115+
} else {
116+
throw new Error(`unexpected bucket name ${bucketName}`);
117+
}
90118
let contents;
91119
let isTruncated;
92-
const startIndex = TEST_BUCKET_CONTENTS.findIndex(
120+
const startIndex = rawContents.findIndex(
93121
item => (!marker || item.key > marker),
94122
);
95123
if (startIndex === -1) {
@@ -98,7 +126,7 @@ describe('BucketStream', () => {
98126
} else {
99127
// limit to 1000 entries returned per page, like bucketd does
100128
const endIndex = startIndex + 1000;
101-
contents = TEST_BUCKET_CONTENTS
129+
contents = rawContents
102130
.slice(startIndex, endIndex)
103131
.map(item => {
104132
const { key, value } = item;
@@ -109,15 +137,15 @@ describe('BucketStream', () => {
109137
}
110138
return { key, value: JSON.stringify(listedValue) };
111139
});
112-
isTruncated = (endIndex < TEST_BUCKET_CONTENTS.length);
140+
isTruncated = (endIndex < rawContents.length);
113141
}
114-
const responseBody = JSON.stringify({
142+
const responseBody = Buffer.from(JSON.stringify({
115143
Contents: contents,
116144
IsTruncated: isTruncated,
117-
});
145+
}), 'utf8');
118146
res.writeHead(200, {
119147
'Content-Type': 'application/json',
120-
'Content-Length': responseBody.length,
148+
'Content-Length': responseBody.byteLength,
121149
});
122150
return res.end(responseBody);
123151
};
@@ -146,10 +174,11 @@ describe('BucketStream', () => {
146174
return res.end('OOPS');
147175
}
148176
const url = new URL(req.url, `http://${req.headers.host}`);
149-
if (url.pathname === '/default/bucket/test-bucket') {
150-
return handleListBucketRequest(req, res, url);
151-
}
152-
if (url.pathname.startsWith('/default/bucket/test-bucket/')) {
177+
if (url.pathname.startsWith('/default/bucket/')) {
178+
const path = url.pathname.slice('/default/bucket/'.length);
179+
if (path.indexOf('/') === -1) {
180+
return handleListBucketRequest(req, res, url);
181+
}
153182
return handleGetObjectRequest(req, res, url);
154183
}
155184
throw new Error(`unexpected request path ${url.pathname}`);
@@ -241,4 +270,33 @@ describe('BucketStream', () => {
241270
.on('error', done);
242271
});
243272
});
273+
274+
test('listing should continue when all keys in a page are ignored', done => {
275+
const bucketStream = new BucketStream({
276+
bucketdHost: 'localhost',
277+
bucketdPort: HTTP_TEST_PORT,
278+
bucketName: 'bucket-with-replay-keys',
279+
retryDelayMs: 50,
280+
maxRetryDelayMs: 1000,
281+
});
282+
const listedContents = [];
283+
bucketStream
284+
.on('data', item => {
285+
listedContents.push(item);
286+
})
287+
.on('end', () => {
288+
expect(listedContents).toEqual([
289+
{
290+
key: 'bucket-with-replay-keys/foo',
291+
value: '{"location":null}',
292+
},
293+
{
294+
key: 'bucket-with-replay-keys/éléphant',
295+
value: '{"location":null}',
296+
},
297+
]);
298+
done();
299+
})
300+
.on('error', done);
301+
});
244302
});

0 commit comments

Comments
 (0)
Please sign in to comment.