Skip to content

Commit 3a95f1d

Browse files
authored
fix(cli-repl): do not wait for connectionInfo in quiet non-REPL mode MONGOSH-1765 (#1962)
Do not wait for `fetchConnectionInfo()` and similar methods when they are not needed. This hopefully improves startup performance in real-world scenarios a bit further. In order to achieve this: - Implement a cache/lazy-loading ability for the connection info in the shell-api `ShellInstanceState` class. We now only fetch connection info here if requested, and only refresh it if the service provider instance changed (and not just the database itself). - Adjust usage of the `fetchConnectionInfo()` method so that we only wait for its result if we need it immediately. This also adds type safety by removing the `any` typing for `connectionInfo` in `ShellInstanceState`. I admittedly thought that this would be a quick fix, but unfortunately (as seen in the commit diff), this spiraled a bit into different packages and tests as a larger change to align the typings for this object.
1 parent 49a0d3b commit 3a95f1d

File tree

19 files changed

+279
-102
lines changed

19 files changed

+279
-102
lines changed

packages/autocomplete/src/index.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ export interface AutocompleteParameters {
2121
connectionInfo: () =>
2222
| undefined
2323
| {
24-
is_atlas: boolean;
25-
is_data_federation: boolean;
26-
server_version: string;
27-
is_local_atlas: boolean;
24+
is_atlas?: boolean;
25+
is_data_federation?: boolean;
26+
server_version?: string;
27+
is_local_atlas?: boolean;
2828
};
2929
apiVersionInfo: () => { version: string; strict: boolean } | undefined;
3030
getCollectionCompletionsForCurrentDb: (

packages/browser-repl/src/sandbox.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
import { IframeRuntime } from './iframe-runtime';
1515
import { Shell } from './index';
1616
import type { ShellOutputEntry } from './components/shell-output-line';
17+
import type { ConnectionInfo } from '@mongosh/service-provider-core';
1718

1819
injectGlobal({
1920
body: {
@@ -94,12 +95,13 @@ class DemoServiceProvider {
9495
};
9596
}
9697

97-
async getConnectionInfo(): Promise<object> {
98+
async getConnectionInfo(): Promise<ConnectionInfo> {
9899
return {
99100
buildInfo: await this.buildInfo(),
100101
extraInfo: {
101102
uri: 'mongodb://localhost/',
102103
},
104+
topology: null,
103105
};
104106
}
105107

packages/browser-runtime-core/src/open-context-runtime.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ export class OpenContextRuntime implements Runtime {
3333
private shellEvaluator: ShellEvaluator;
3434
private instanceState: ShellInstanceState;
3535
private evaluationListener: RuntimeEvaluationListener | null = null;
36-
private updatedConnectionInfoPromise: Promise<void> | null = null;
36+
private updatedConnectionInfoPromise: Promise<unknown> | null = null;
3737

3838
constructor(
3939
serviceProvider: ServiceProvider,

packages/cli-repl/src/cli-repl.spec.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1519,6 +1519,19 @@ describe('CliRepl', function () {
15191519
cliRepl,
15201520
await testServer.connectionString()
15211521
);
1522+
expect(
1523+
requests
1524+
.flatMap((req) =>
1525+
JSON.parse(req.body).batch.map((entry) => entry.event)
1526+
)
1527+
.sort()
1528+
.filter(Boolean)
1529+
).to.deep.equal([
1530+
'API Call',
1531+
'New Connection',
1532+
'Script Evaluated',
1533+
'Startup Time',
1534+
]);
15221535
expect(totalEventsTracked).to.equal(5);
15231536
});
15241537

packages/cli-repl/src/mongosh-repl.spec.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ describe('MongoshNodeRepl', function () {
4545
let ioProvider: MongoshIOProvider;
4646
let sp: StubbedInstance<ServiceProvider>;
4747
let serviceProvider: ServiceProvider;
48+
let calledServiceProviderFunctions: () => Partial<
49+
Record<keyof ServiceProvider, number>
50+
>;
4851
let config: Record<string, any>;
4952
const tmpdir = useTmpdir();
5053

@@ -83,9 +86,16 @@ describe('MongoshNodeRepl', function () {
8386
buildInfo: {
8487
version: '4.4.1',
8588
},
89+
topology: null,
8690
});
8791
sp.runCommandWithCheck.resolves({ ok: 1 });
8892
serviceProvider = sp;
93+
calledServiceProviderFunctions = () =>
94+
Object.fromEntries(
95+
Object.keys(sp)
96+
.map((key) => [key, sp[key]?.callCount])
97+
.filter(([, count]) => !!count)
98+
);
8999

90100
mongoshReplOptions = {
91101
input: input,
@@ -128,6 +138,7 @@ describe('MongoshNodeRepl', function () {
128138
/You can opt-out by running the .*disableTelemetry\(\).* command/
129139
);
130140
expect(config.disableGreetingMessage).to.equal(true);
141+
expect(sp.getConnectionInfo).to.have.been.calledOnce;
131142
});
132143

133144
it('evaluates javascript', async function () {
@@ -1248,6 +1259,7 @@ describe('MongoshNodeRepl', function () {
12481259
version: '4.4.1',
12491260
modules: ['enterprise'],
12501261
},
1262+
topology: null,
12511263
});
12521264

12531265
const initialized = await mongoshRepl.initialize(serviceProvider);
@@ -1279,6 +1291,7 @@ describe('MongoshNodeRepl', function () {
12791291
version: '4.4.1',
12801292
modules: ['enterprise'],
12811293
},
1294+
topology: null,
12821295
};
12831296

12841297
sp.getConnectionInfo.resolves(connectionInfo);
@@ -1408,6 +1421,7 @@ describe('MongoshNodeRepl', function () {
14081421
buildInfo: {
14091422
version: '4.4.1',
14101423
},
1424+
topology: null,
14111425
});
14121426
mongoshReplOptions.shellCliOptions = {
14131427
nodb: false,
@@ -1438,4 +1452,31 @@ describe('MongoshNodeRepl', function () {
14381452
expect(warnings).to.have.lengthOf(0);
14391453
});
14401454
});
1455+
1456+
context('interactions with the server during startup', function () {
1457+
it('calls a number of service provider functions by default', async function () {
1458+
await mongoshRepl.initialize(serviceProvider);
1459+
const calledFunctions = calledServiceProviderFunctions();
1460+
expect(Object.keys(calledFunctions).sort()).to.deep.equal([
1461+
'getConnectionInfo',
1462+
'getFleOptions',
1463+
'getRawClient',
1464+
'getURI',
1465+
'runCommandWithCheck',
1466+
]);
1467+
});
1468+
1469+
it('does not wait for getConnectionInfo in quiet plain-vm mode', async function () {
1470+
mongoshRepl.shellCliOptions.quiet = true;
1471+
mongoshRepl.shellCliOptions.jsContext = 'plain-vm';
1472+
sp.getConnectionInfo.callsFake(
1473+
() =>
1474+
new Promise(() => {
1475+
/* never resolve */
1476+
})
1477+
);
1478+
await mongoshRepl.initialize(serviceProvider);
1479+
expect(serviceProvider.getConnectionInfo).to.have.been.calledOnce;
1480+
});
1481+
});
14411482
});

packages/cli-repl/src/mongosh-repl.ts

Lines changed: 35 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -179,32 +179,52 @@ class MongoshNodeRepl implements EvaluationListener {
179179
serviceProvider: ServiceProvider,
180180
moreRecentMongoshVersion?: string | null
181181
): Promise<InitializationToken> {
182+
const usePlainVMContext = this.shellCliOptions.jsContext === 'plain-vm';
183+
182184
const instanceState = new ShellInstanceState(
183185
serviceProvider,
184186
this.bus,
185187
this.shellCliOptions
186188
);
189+
187190
const shellEvaluator = new ShellEvaluator(
188191
instanceState,
189192
(value: any) => value,
190193
markTime,
191194
!!this.shellCliOptions.exposeAsyncRewriter
192195
);
193196
instanceState.setEvaluationListener(this);
194-
await instanceState.fetchConnectionInfo();
195-
markTime(TimingCategories.REPLInstantiation, 'fetched connection info');
196-
197-
const { buildInfo, extraInfo } = instanceState.connectionInfo;
198-
let mongodVersion = extraInfo?.is_stream
199-
? 'Atlas Stream Processing'
200-
: buildInfo?.version;
201-
const apiVersion = serviceProvider.getRawClient()?.serverApi?.version;
202-
if (apiVersion) {
203-
mongodVersion =
204-
(mongodVersion ? mongodVersion + ' ' : '') +
205-
`(API Version ${apiVersion})`;
197+
198+
// Fetch connection metadata if not in quiet mode or in REPL mode:
199+
// not-quiet mode -> We'll need it for the greeting message (and need it now)
200+
// REPL mode -> We'll want it for fast autocomplete (and need it soon-ish, but not now)
201+
instanceState.setPreFetchCollectionAndDatabaseNames(!usePlainVMContext);
202+
// `if` commented out because we currently still want the connection info for
203+
// logging/telemetry but we may want to revisit that in the future:
204+
// if (!this.shellCliOptions.quiet || !usePlainVMContext)
205+
{
206+
const connectionInfoPromise = instanceState.fetchConnectionInfo();
207+
connectionInfoPromise.catch(() => {
208+
// Ignore potential unhandled rejection warning
209+
});
210+
if (!this.shellCliOptions.quiet) {
211+
const connectionInfo = await connectionInfoPromise;
212+
markTime(TimingCategories.REPLInstantiation, 'fetched connection info');
213+
214+
const { buildInfo, extraInfo } = connectionInfo ?? {};
215+
let mongodVersion = extraInfo?.is_stream
216+
? 'Atlas Stream Processing'
217+
: buildInfo?.version;
218+
const apiVersion = serviceProvider.getRawClient()?.serverApi?.version;
219+
if (apiVersion) {
220+
mongodVersion =
221+
(mongodVersion ? mongodVersion + ' ' : '') +
222+
`(API Version ${apiVersion})`;
223+
}
224+
await this.greet(mongodVersion, moreRecentMongoshVersion);
225+
}
206226
}
207-
await this.greet(mongodVersion, moreRecentMongoshVersion);
227+
208228
await this.printBasicConnectivityWarning(instanceState);
209229
markTime(TimingCategories.REPLInstantiation, 'greeted');
210230

@@ -220,7 +240,7 @@ class MongoshNodeRepl implements EvaluationListener {
220240

221241
let repl: REPLServer | null = null;
222242
let context: Context;
223-
if (this.shellCliOptions.jsContext !== 'plain-vm') {
243+
if (!usePlainVMContext) {
224244
repl = asyncRepl.start({
225245
// 'repl' is not supported in startup snapshots yet.
226246
// eslint-disable-next-line @typescript-eslint/no-var-requires
@@ -791,7 +811,7 @@ class MongoshNodeRepl implements EvaluationListener {
791811
this.output.write('Stopping execution...');
792812

793813
const mongodVersion: string | undefined =
794-
instanceState.connectionInfo.buildInfo?.version;
814+
instanceState.cachedConnectionInfo()?.buildInfo?.version;
795815
if (mongodVersion?.match(/^(4\.0\.|3\.)\d+/)) {
796816
this.output.write(
797817
this.clr(

packages/logging/src/setup-logger-and-telemetry.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ export function setupLoggerAndTelemetry(
141141
);
142142

143143
bus.on('mongosh:connect', function (args: ConnectEvent) {
144-
const connectionUri = redactURICredentials(args.uri);
144+
const connectionUri = args.uri && redactURICredentials(args.uri);
145145
// eslint-disable-next-line @typescript-eslint/no-unused-vars
146146
const { uri: _uri, ...argsWithoutUri } = args;
147147
const params = {

packages/service-provider-core/src/admin.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import type {
1313
AutoEncryptionOptions,
1414
Collection,
1515
} from './all-transport-types';
16-
import type { bson as BSON } from './index';
16+
import type { bson as BSON, ConnectionExtraInfo } from './index';
1717
import type { ReplPlatform } from './platform';
1818
import type {
1919
AWSEncryptionKeyOptions,
@@ -43,6 +43,12 @@ export interface CheckMetadataConsistencyOptions {
4343
checkIndexes?: 1;
4444
}
4545

46+
export interface ConnectionInfo {
47+
buildInfo: Document | null;
48+
topology: any | null;
49+
extraInfo: (ConnectionExtraInfo & { fcv?: string }) | null;
50+
}
51+
4652
export default interface Admin {
4753
/**
4854
* What platform (Compass/CLI/Browser)
@@ -87,7 +93,7 @@ export default interface Admin {
8793
/**
8894
* Return connection info
8995
*/
90-
getConnectionInfo(): Promise<Document>;
96+
getConnectionInfo(): Promise<ConnectionInfo>;
9197

9298
/**
9399
* Authenticate

packages/service-provider-core/src/connect-info.ts

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,35 +2,35 @@
22

33
import getBuildInfo from 'mongodb-build-info';
44

5-
export interface ConnectInfo {
6-
is_atlas: boolean;
7-
is_localhost: boolean;
8-
is_do: boolean;
9-
server_version: string;
10-
mongosh_version: string;
5+
export interface ConnectionExtraInfo {
6+
is_atlas?: boolean;
7+
is_localhost?: boolean;
8+
is_do?: boolean;
9+
server_version?: string;
10+
mongosh_version?: string;
1111
server_os?: string;
1212
server_arch?: string;
13-
is_enterprise: boolean;
13+
is_enterprise?: boolean;
1414
auth_type?: string;
15-
is_data_federation: boolean;
16-
is_stream: boolean;
15+
is_data_federation?: boolean;
16+
is_stream?: boolean;
1717
dl_version?: string;
1818
atlas_version?: string;
19-
is_genuine: boolean;
20-
non_genuine_server_name: string;
21-
node_version: string;
19+
is_genuine?: boolean;
20+
non_genuine_server_name?: string;
21+
node_version?: string;
2222
uri: string;
23-
is_local_atlas: boolean;
23+
is_local_atlas?: boolean;
2424
}
2525

26-
export default function getConnectInfo(
26+
export default function getConnectExtraInfo(
2727
uri: string,
2828
mongoshVersion: string,
2929
buildInfo: any,
3030
atlasVersion: any,
3131
topology: any,
3232
isLocalAtlas: boolean
33-
): ConnectInfo {
33+
): ConnectionExtraInfo {
3434
buildInfo ??= {}; // We're currently not getting buildInfo with --apiStrict.
3535
const { isGenuine: is_genuine, serverName: non_genuine_server_name } =
3636
getBuildInfo.getGenuineMongoDB(uri);

packages/service-provider-core/src/index.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import './textencoder-polyfill'; // for mongodb-connection-string-url in the java-shell
22
import ServiceProvider, { ServiceProviderCore } from './service-provider';
3-
import getConnectInfo, { ConnectInfo } from './connect-info';
3+
import getConnectExtraInfo, { ConnectionExtraInfo } from './connect-info';
44
import type { ReplPlatform } from './platform';
55
const DEFAULT_DB = 'test';
66
import { bsonStringifiers } from './printable-bson';
@@ -13,17 +13,18 @@ export { MapReduceOptions, FinalizeFunction } from './map-reduce-options';
1313
export {
1414
CreateEncryptedCollectionOptions,
1515
CheckMetadataConsistencyOptions,
16+
ConnectionInfo,
1617
} from './admin';
1718

1819
export { bson } from './bson-export';
1920

2021
export {
2122
ServiceProvider,
2223
ShellAuthOptions,
23-
getConnectInfo,
24+
getConnectExtraInfo,
2425
ReplPlatform,
2526
DEFAULT_DB,
2627
ServiceProviderCore,
2728
bsonStringifiers,
28-
ConnectInfo,
29+
ConnectionExtraInfo,
2930
};

0 commit comments

Comments
 (0)