Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvement/bb 632 bump mongo db driver 6 #2599

Merged
merged 11 commits into from
Jan 10, 2025
11 changes: 11 additions & 0 deletions .github/workflows/tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -192,3 +192,14 @@ jobs:
# will crash if they end up in memory all at the same time (circuit breaking
# ineffective) while waiting to be committed to the kafka topic.
NODE_OPTIONS: '--max-old-space-size=150'

- name: Debug wait
uses: scality/actions/[email protected]
with:
tmate-server-host: ${{ secrets.TMATE_SERVER_HOST }}
tmate-server-port: ${{ secrets.TMATE_SERVER_PORT }}
tmate-server-rsa-fingerprint: ${{ secrets.TMATE_SERVER_RSA_FINGERPRINT }}
tmate-server-ed25519-fingerprint: ${{ secrets.TMATE_SERVER_ED25519_FINGERPRINT }}
continue-on-error: true
timeout-minutes: 60
if: failure() && runner.debug == '1'
70 changes: 34 additions & 36 deletions extensions/notification/configManager/MongoConfigManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,50 +90,48 @@
}

/**
* Connects to MongoDB using the MongoClientInterface
* and retreives the metastore collection
* Connect to MongoDB using the MongoClientInterface
* and retrieve the metastore collection
* @param {Function} cb callback
* @returns {undefined}
*/
_setupMongoClient(cb) {
async _setupMongoClient(cb) {
const mongoUrl = constructConnectionString(this._mongoConfig);
MongoClient.connect(mongoUrl, {
replicaSet: this._mongoConfig.replicaSet,
useNewUrlParser: true,
},
(err, client) => {
if (err) {
this._logger.error('Could not connect to MongoDB', {
method: 'MongoConfigManager._setupMongoClient',
error: err.message,
});
return cb(err);
}

try {
const client = await new MongoClient(mongoUrl, {
replicaSet: this._mongoConfig.replicaSet,
useNewUrlParser: true,
readPreference: this._mongoConfig.readPreference,
}).connect();

this._logger.debug('Connected to MongoDB', {
method: 'MongoConfigManager._setupMongoClient',
});
try {
this._mongoClient = client.db(this._mongoConfig.database, {
ignoreUndefined: true,
});
this._metastore = this._mongoClient.collection(this._bucketMetastore);

this._mongoClient = client.db(this._mongoConfig.database, {
ignoreUndefined: true,
});
this._metastore = this._mongoClient.collection(this._bucketMetastore);
// get mongodb version
getMongoVersion(this._mongoClient, (err, version) => {
if (err) {
this._logger.error('Could not get MongoDB version', {
method: 'MongoConfigManager._setupMongoClient',
error: err.message,
});
return cb(err);
}
this._mongoVersion = version;
return cb();
});
return undefined;
} catch (error) {
return cb(error);
}
});
return getMongoVersion(this._mongoClient, (err, version) => {
if (err) {
this._logger.error('Could not get MongoDB version', {

Check warning on line 119 in extensions/notification/configManager/MongoConfigManager.js

View check run for this annotation

Codecov / codecov/patch/Backbeat

extensions/notification/configManager/MongoConfigManager.js#L119

Added line #L119 was not covered by tests
method: 'MongoConfigManager._setupMongoClient',
error: err.message,
});
return cb(err);

Check warning on line 123 in extensions/notification/configManager/MongoConfigManager.js

View check run for this annotation

Codecov / codecov/patch/Backbeat

extensions/notification/configManager/MongoConfigManager.js#L123

Added line #L123 was not covered by tests
}
this._mongoVersion = version;
return cb();
});
} catch (err) {
this._logger.error('Could not connect to MongoDB', {
method: 'MongoConfigManager._setupMongoClient',
error: err.message,
});
return cb(err);
}
}

/**
Expand Down
6 changes: 3 additions & 3 deletions extensions/oplogPopulator/OplogPopulator.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,11 @@ class OplogPopulator {
*/
async _setupMongoClient() {
try {
const client = await MongoClient.connect(this._mongoUrl, {
const client = await new MongoClient(this._mongoUrl, {
replicaSet: this._replicaSet,
useNewUrlParser: true,
useUnifiedTopology: true,
});
readPreference: this._mongoConfig.readPreference,
}).connect();
// connect to metadata DB
this._mongoClient = client.db(this._database, {
ignoreUndefined: true,
Expand Down
2 changes: 1 addition & 1 deletion extensions/replication/tasks/ReplicateObject.js
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ class ReplicateObject extends BackbeatTask {
};
return this.backbeatSource.getMetadata(params, (err, blob) => {
if (err) {
err.origin = 'source';
err.origin = 'source'; // eslint-disable-line no-param-reassign
log.error('error getting metadata blob from S3', {
method: 'ReplicateObject._refreshSourceEntry',
error: err,
Expand Down
48 changes: 24 additions & 24 deletions extensions/utils/LocationStatusStream.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,18 +63,11 @@
*/
_setupMongoClient(cb) {
const mongoUrl = constructConnectionString(this._mongoConfig);
MongoClient.connect(mongoUrl, {
const client = new MongoClient(mongoUrl, {
replicaSet: this._mongoConfig.replicaSet,
useNewUrlParser: true,
useUnifiedTopology: true,
}, (err, client) => {
if (err) {
this._log.error('Could not connect to MongoDB', {
method: 'ServiceStatusManager._setupMongoClient',
error: err.message,
});
return cb(err);
}
});
return client.connect().then(client => {
// connect to metadata DB
this._mongoClient = client.db(this._mongoConfig.database, {
ignoreUndefined: true,
Expand All @@ -96,6 +89,12 @@
return cb();
});
return undefined;
}).catch(err => {
this._log.error('Could not connect to MongoDB', {

Check warning on line 93 in extensions/utils/LocationStatusStream.js

View check run for this annotation

Codecov / codecov/patch/Backbeat

extensions/utils/LocationStatusStream.js#L93

Added line #L93 was not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should be able to test this case, just need to pass an invalid url

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

method: 'ServiceStatusManager._setupMongoClient',
error: err.message,
});
return cb(err);

Check warning on line 97 in extensions/utils/LocationStatusStream.js

View check run for this annotation

Codecov / codecov/patch/Backbeat

extensions/utils/LocationStatusStream.js#L97

Added line #L97 was not covered by tests
});
}

Expand All @@ -106,22 +105,23 @@
*/
_initializeLocationStatuses(cb) {
this._locationStatusColl.find({})
.toArray((err, locations) => {
if (err) {
this._log.error('Could not fetch location statuses from mongo', {
method: 'ServiceStatusManager._initializeLocationStatuses',
error: err.message,
});
return cb(err);
.toArray()
.then(locations => {
locations.forEach(location => {
const isPaused = location.value[this._serviceName].paused;
if (isPaused) {
this._pauseServiceForLocation(location._id);
}
locations.forEach(location => {
const isPaused = location.value[this._serviceName].paused;
if (isPaused) {
this._pauseServiceForLocation(location._id);
}
});
return cb();
});
return cb();
})
.catch(err => {
this._log.error('Could not fetch location statuses from mongo', {
method: 'ServiceStatusManager._initializeLocationStatuses',
error: err.message,
});
return cb(err);
});
}

/**
Expand Down
22 changes: 10 additions & 12 deletions lib/api/BackbeatAPI.js
Original file line number Diff line number Diff line change
Expand Up @@ -1335,21 +1335,13 @@ class BackbeatAPI {
});
return cb();
} else {
// To be removed once the mongodb drivers are bumped
// BB-585
const mongoUrl = constructConnectionString(mongoConfig);
return MongoClient.connect(mongoUrl, {
const client = new MongoClient(mongoUrl, {
replicaSet: mongoConfig.replicaSet,
useNewUrlParser: true,
useUnifiedTopology: true,
}, (err, client) => {
if (err) {
this._logger.error('Could not connect to MongoDB', {
method: 'BackbeatAPI._setupMongoClient',
error: err.message,
});
return cb(err);
}
});

return client.connect().then(client => {
// connect to metadata DB
this._mongoClient = client.db(mongoConfig.database, {
ignoreUndefined: true,
Expand All @@ -1358,6 +1350,12 @@ class BackbeatAPI {
method: 'BackbeatAPI._setupMongoClient',
});
return cb();
}).catch(err => {
this._logger.error('Could not connect to MongoDB', {
method: 'BackbeatAPI._setupMongoClient',
error: err.message,
});
return cb(err);
});
}
}
Expand Down
62 changes: 33 additions & 29 deletions lib/util/LocationStatusManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,19 +68,23 @@
* @return {undefined}
*/
_initCollection(cb) {
this._mongoClient.createCollection(locationStatusCollection, err => {
// in case the collection already exists, we ignore the error
if (err && err.codeName !== 'NamespaceExists') {
this._logger.error('Could not create mongo collection', {
method: 'LocationStatusManager._initCollection',
collection: locationStatusCollection,
error: err.message,
});
return cb(err);
}
this._locationStatusColl = this._mongoClient.collection(locationStatusCollection);
return cb();
});
return this._mongoClient.createCollection(locationStatusCollection)
.then(() => {
this._locationStatusColl = this._mongoClient.collection(locationStatusCollection);
return cb();
})
.catch(err => {
if (err.codeName !== 'NamespaceExists') {
this._logger.error('Could not create mongo collection', {

Check warning on line 78 in lib/util/LocationStatusManager.js

View check run for this annotation

Codecov / codecov/patch/Backbeat

lib/util/LocationStatusManager.js#L78

Added line #L78 was not covered by tests
method: 'LocationStatusManager._initCollection',
collection: locationStatusCollection,
error: err.message,
});
return cb(err);

Check warning on line 83 in lib/util/LocationStatusManager.js

View check run for this annotation

Codecov / codecov/patch/Backbeat

lib/util/LocationStatusManager.js#L83

Added line #L83 was not covered by tests
}
this._locationStatusColl = this._mongoClient.collection(locationStatusCollection);
return cb();
});
}

/**
Expand All @@ -89,16 +93,17 @@
* @returns {undefined}}
*/
_listCollectionDocuments(cb) {
this._locationStatusColl.find({}, (err, cursor) => {
if (err) {
this._locationStatusColl.find({})
.toArray()
.then(docs => cb(null, docs))
.catch(err => {
this._logger.error('Could not list documents', {
method: 'LocationStatusManager._listCollectionDocuments',
collection: locationStatusCollection,
error: err.message,
});
}
return cursor.toArray(cb);
});
cb(err);

Check warning on line 105 in lib/util/LocationStatusManager.js

View check run for this annotation

Codecov / codecov/patch/Backbeat

lib/util/LocationStatusManager.js#L105

Added line #L105 was not covered by tests
});
}

/**
Expand Down Expand Up @@ -139,15 +144,14 @@
_id: {
$in: invalidLocations
}
}, err => {
if (err) {
this._logger.error('Could not delete invalid locations', {
method: 'LocationStatusManager._deleteInvalidLocations',
error: err.message,
});
return cb(err);
}
return cb(null, locations);
})
.then(() => cb(null, locations))
.catch(err => {
this._logger.error('Could not delete invalid locations', {

Check warning on line 150 in lib/util/LocationStatusManager.js

View check run for this annotation

Codecov / codecov/patch/Backbeat

lib/util/LocationStatusManager.js#L150

Added line #L150 was not covered by tests
method: 'LocationStatusManager._deleteInvalidLocations',
error: err.message,
});
cb(err);

Check warning on line 154 in lib/util/LocationStatusManager.js

View check run for this annotation

Codecov / codecov/patch/Backbeat

lib/util/LocationStatusManager.js#L154

Added line #L154 was not covered by tests
});
}

Expand All @@ -166,10 +170,10 @@
async.eachLimit(newLocations, 10, (location, next) => {
const locationConfig = new LocationStatus(this._supportedServices);
this._locationStatusStore[location] = locationConfig;
this._locationStatusColl.insert({
this._locationStatusColl.insertOne({
_id: location,
value: locationConfig.getValue(),
}, next);
}).then(() => next()).catch(next);
}, err => {
if (err) {
this._logger.error('Could not add new locations', {
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"oplog_populator": "node extensions/oplogPopulator/OplogPopulatorTask.js",
"mongo_queue_processor": "node extensions/mongoProcessor/mongoProcessorTask.js",
"garbage_collector": "node extensions/gc/service.js",
"test": "mocha --recursive tests/unit/oplogPopulator --timeout 30000",
"test": "mocha --recursive tests/unit --timeout 30000",
"cover:test": "nyc --clean --silent yarn run test && nyc report --report-dir ./coverage/test --reporter=lcov",
"ft_test": "mocha --recursive $(find tests/functional -name '*.js') --timeout 30000",
"ft_test:notification": "mocha --recursive $(find tests/functional/notification -name '*.js') --timeout 30000",
Expand Down Expand Up @@ -59,7 +59,7 @@
"ioredis": "^4.9.5",
"joi": "^17.6.0",
"minimatch": "^3.0.4",
"mongodb": "^3.1.13",
"mongodb": "^6.11.0",
"node-forge": "^0.7.6",
"node-rdkafka": "^2.12.0",
"node-rdkafka-prometheus": "^1.0.0",
Expand Down
3 changes: 2 additions & 1 deletion tests/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
},
"mongo": {
"logName": "s3-recordlog",
"replicaSetHosts": "localhost:27018"
"replicaSetHosts": "localhost:27018",
"readPreference": "primary"
},
"probeServer": {
"bindAddress": "localhost",
Expand Down
Loading
Loading