Skip to content

Commit

Permalink
ARSN-370: fix memory leak
Browse files Browse the repository at this point in the history
The MongoDBReadStreams are not properly destroyed in both the
Bucket V1 and V0 cases. In the V1 case, only the pipe-ed stream,
the Transform one, is cleaned. In the V0 case, we directly call
the callback without properly cleaning the stream, leaving open,
in both cases, the mongodb cursors, that in turn affect the
mongos memory consumption.
  • Loading branch information
williamlardier committed Oct 5, 2023
1 parent 5fd675a commit 4879e92
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 0 deletions.
11 changes: 11 additions & 0 deletions lib/storage/metadata/mongoclient/MongoClientInterface.js
Original file line number Diff line number Diff line change
Expand Up @@ -1641,9 +1641,11 @@ class MongoClientInterface {
const c = this.getCollection(bucketName);
const getLatestVersion = this.getLatestVersion;
let stream;
let baseStream;
if (!params.secondaryStreamParams) {
// listing masters only (DelimiterMaster)
stream = new MongoReadStream(c, params.mainStreamParams, params.mongifiedSearch);
baseStream = stream;
if (vFormat === BUCKET_VERSIONS.v1) {
/**
* When listing masters only in v1 we can't just skip PHD
Expand Down Expand Up @@ -1685,6 +1687,11 @@ class MongoClientInterface {
},
});
stream = stream.pipe(resolvePhdKey);
// Propagate the 'end' event from resolvePhdKey to stream
// to properly cleanup resources.
resolvePhdKey.on('end', () => {
baseStream.emit('end');
});
}
} else {
// listing both master and version keys (delimiterVersion Algo)
Expand Down Expand Up @@ -1736,10 +1743,14 @@ class MongoClientInterface {
};
log.error(
'internalListObjectV1: error listing objects', logObj);
// call explicitely the destroy method to clean the mongodb cursor
stream.destroy();
cbOnce(err);
})
.on('end', () => {
const data = extension.result();
// call explicitely the destroy method to clean the mongodb cursor
stream.destroy();
cbOnce(null, data);
});
return undefined;
Expand Down
19 changes: 19 additions & 0 deletions tests/functional/metadata/mongodb/listObject.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ const MetadataWrapper =
require('../../../../lib/storage/metadata/MetadataWrapper');
const { versioning } = require('../../../../index');
const { BucketVersioningKeyFormat } = versioning.VersioningConstants;
const sinon = require('sinon');
const MongoReadStream = require('../../../../lib/storage/metadata/mongoclient/readStream');

const IMPL_NAME = 'mongodb';
const DB_NAME = 'metadata';
Expand Down Expand Up @@ -477,6 +479,23 @@ describe('MongoClientInterface::metadata.listObject', () => {
}),
], done);
});

it('Should properly destroy the MongoDBReadStream', done => {
const destroyStub = sinon.stub(MongoReadStream.prototype, 'destroy').callsFake((...args) => {
MongoReadStream.prototype.destroy.wrappedMethod.apply(this, ...args);
});
const params = {
listingType: 'DelimiterMaster',
maxKeys: 100,
};
return metadata.listObject(BUCKET_NAME, params, logger, err => {
assert.ifError(err);
assert(destroyStub.called, 'Destroy should have been called on MongoReadStream');
// Restore original destroy method
destroyStub.restore();
return done();
});
});
});
});
});

0 comments on commit 4879e92

Please sign in to comment.