Skip to content
This repository was archived by the owner on May 17, 2021. It is now read-only.

Conversation

@Anemy
Copy link
Member

@Anemy Anemy commented Dec 15, 2020

In 3.6.3 the driver updated how operations are run so that operations which have the aspect WRITE_OPERATION default to primary instead of using the connection's readPreference:
mongodb/node-mongodb-native@ddcd03d

This PR updates our use of the db.command function which is one of those operations with the aspect WRITE_OPERATION to explicitly pass the readPreference which the user set up in Compass. This makes it so we have the same behavior as before and previous direct connections to secondaries don't suddenly stop working.

I think we could do a lot of cleanup here. I'm also not quite sure why we're using db.command for listing databases instead of db.listDatabases. This should fix the issue though. We also shouldn't be reaching into db.s.readPreference. I can create a tech debt ticket 🤔

I'm not sure if anywhere else in our codebase we rely on db.command follow the readPreference of the connection, so I'm looking into that.

Connecting and looking at data in a secondary with a direct connection:
Screen Shot 2020-12-15 at 12 33 00 PM


const debug = require('debug')('mongodb-data-service:instance-detail-helper');

function getReadPreferenceOptions(db) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't db.s.readPreference a private method in the driver?

Also .. I think here we can just hardcode ReadPreference.PRIMARY_PREFERRED, i see no harm, may be missing something

Copy link
Member Author

@Anemy Anemy Dec 15, 2020

Choose a reason for hiding this comment

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

Yes, wrote this quick using a bit of what we already had https://github.com/mongodb-js/data-service/blob/master/lib/instance-detail-helper.js#L500 , I'll see if there's a public method we can use. Looks like we don't pass our connection model through these instance detail helpers.

this.database.admin().command({
listDatabases: 1
}, {
readPreference: this.model.readPreference
Copy link
Contributor

Choose a reason for hiding this comment

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

this also depends on how is used, if is only used by compass for building the UI i'd go with primary preferred always, what would be the consequence of listing dbs from a secondary?

@Anemy Anemy merged commit f2bd36d into master Dec 15, 2020
@Anemy Anemy deleted the COMPASS-4539/pass-read-preference-with-db-command branch December 15, 2020 19:34
Anemy added a commit that referenced this pull request Dec 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants