Skip to content

Commit cdc93ca

Browse files
authored
fix: remove readPreferenceTags for database&collection lookups if the cluster is sharded and readPreferenceTags was set COMPASS-9111 (#6818)
* set readPreference=secondaryPreferred for database&collection lookups if readPreferenceTags was set * rather send a read preference with just the tags removed * send everything as options * Add a test plus a comment
1 parent e7633d7 commit cdc93ca

File tree

2 files changed

+98
-3
lines changed

2 files changed

+98
-3
lines changed

packages/data-service/src/connect.spec.ts

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,63 @@ describe('connect', function () {
483483
});
484484
});
485485

486+
it('connects to sharded with readPreferenceTags', async function () {
487+
const options = envs.getConnectionOptions('sharded');
488+
/*
489+
See ticket COMPASS-9111
490+
491+
This test is using readPreference=nearest because this cluster has node with the tag
492+
ANALYTICS. readPreference=secondary would more closely mirror the
493+
original ticket, but this cluster also has no secondaries so that would
494+
fail regardless of readPreferenceTags.
495+
496+
Ideally people would use readPreference=secondaryPreferred, but that works
497+
regardless so isn't a good test and if it was the case that people used
498+
that in the first place we'd never need this ticket.
499+
500+
readPreference=nearest tries to find one that matches the criteria and
501+
since the config server doesn't know about tags the following operations
502+
would hang unless we remove the tags. You can confirm this manually by
503+
hacking maybeOverrideReadPreference in data-service.ts.
504+
*/
505+
const connectionString =
506+
options.connectionString +
507+
'&readPreference=nearest&readPreferenceTags=nodeType:ANALYTICS';
508+
const connectionOptions = {
509+
...options,
510+
connectionString,
511+
};
512+
await testConnection(connectionOptions, {
513+
authenticatedUserRoles: [{ db: 'admin', role: 'root' }],
514+
authenticatedUsers: [{ db: 'admin', user: 'root' }],
515+
});
516+
517+
const dataService = await connect({
518+
connectionOptions,
519+
});
520+
521+
/*
522+
Without us removing the read preference tags these operations would fail.
523+
524+
Normal database operations like find or aggregate will still fail
525+
regardless because the cluster does not have a node with the ANALYTICS
526+
tag, but this test never executes any of those
527+
*/
528+
try {
529+
const databases = await dataService.listDatabases();
530+
const databaseNames = databases
531+
.map((d) => d.name)
532+
.filter((name) => !['local'].includes(name));
533+
for (const databaseName of databaseNames) {
534+
// don't really care what's in there, just that the calls succeed
535+
await dataService.listCollections(databaseName);
536+
await dataService.databaseStats(databaseName);
537+
}
538+
} finally {
539+
await dataService.disconnect();
540+
}
541+
});
542+
486543
describe('ssh', function () {
487544
it('connects with ssh (sshPassword)', async function () {
488545
await testConnection(envs.getConnectionOptions('sshPassword'), {

packages/data-service/src/data-service.ts

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ import type {
5353
SearchIndexDescription,
5454
ReadPreferenceMode,
5555
} from 'mongodb';
56+
import { ReadPreference } from 'mongodb';
5657
import ConnectionStringUrl from 'mongodb-connection-string-url';
5758
import parseNamespace from 'mongodb-ns';
5859
import type {
@@ -133,6 +134,25 @@ function isReadPreferenceSet(connectionString: string): boolean {
133134
);
134135
}
135136

137+
function readPreferenceWithoutTags(readPreference: ReadPreference) {
138+
return new ReadPreference(readPreference.mode, undefined, readPreference);
139+
}
140+
141+
function maybeOverrideReadPreference(
142+
isMongos: boolean,
143+
readPreference?: ReadPreference
144+
): {
145+
readPreference?: ReadPreference;
146+
} {
147+
// see ticket COMPASS-9111 for why this is necessary
148+
if (isMongos && readPreference?.tags) {
149+
const newReadPreference = readPreferenceWithoutTags(readPreference);
150+
return { readPreference: newReadPreference };
151+
}
152+
153+
return {};
154+
}
155+
136156
let id = 0;
137157

138158
type ClientType = 'CRUD' | 'META';
@@ -1264,7 +1284,13 @@ class DataServiceImpl extends WithLogContext implements DataService {
12641284
try {
12651285
const cursor = this._database(databaseName, 'CRUD').listCollections(
12661286
filter,
1267-
{ nameOnly }
1287+
{
1288+
nameOnly,
1289+
...maybeOverrideReadPreference(
1290+
this.isMongos(),
1291+
this._crudClient?.readPreference
1292+
),
1293+
}
12681294
);
12691295
// Iterate instead of using .toArray() so we can emit
12701296
// collection info update events as they come in.
@@ -1399,7 +1425,13 @@ class DataServiceImpl extends WithLogContext implements DataService {
13991425
} as {
14001426
listDatabases: 1;
14011427
},
1402-
{ enableUtf8Validation: false }
1428+
{
1429+
enableUtf8Validation: false,
1430+
...maybeOverrideReadPreference(
1431+
this.isMongos(),
1432+
this._crudClient?.readPreference
1433+
),
1434+
}
14031435
);
14041436
return databases.map((x) => ({
14051437
...x,
@@ -2542,7 +2574,13 @@ class DataServiceImpl extends WithLogContext implements DataService {
25422574
const stats = await runCommand(
25432575
db,
25442576
{ dbStats: 1 },
2545-
{ enableUtf8Validation: false }
2577+
{
2578+
enableUtf8Validation: false,
2579+
...maybeOverrideReadPreference(
2580+
this.isMongos(),
2581+
this._crudClient?.readPreference
2582+
),
2583+
}
25462584
);
25472585
const normalized = adaptDatabaseInfo(stats);
25482586
return { name, ...normalized };

0 commit comments

Comments
 (0)