Skip to content

Commit

Permalink
feat(shell-api): add maxTimeMS config options MONGOSH-537 (#1068)
Browse files Browse the repository at this point in the history
  • Loading branch information
addaleax authored Aug 11, 2021
1 parent 24bf52b commit 94ca4d9
Show file tree
Hide file tree
Showing 10 changed files with 132 additions and 76 deletions.
1 change: 1 addition & 0 deletions packages/cli-repl/src/cli-repl.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ describe('CliRepl', () => {
it('returns the list of available config options when asked to', () => {
expect(cliRepl.listConfigOptions()).to.deep.equal([
'displayBatchSize',
'maxTimeMS',
'enableTelemetry',
'snippetIndexSourceURLs',
'snippetRegistryURL',
Expand Down
99 changes: 49 additions & 50 deletions packages/shell-api/src/collection.ts

Large diffs are not rendered by default.

43 changes: 25 additions & 18 deletions packages/shell-api/src/database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ export default class Database extends ShellApiWithMongoClass {
_mongo: Mongo;
_name: string;
_collections: Record<string, Collection>;
_baseOptions: CommandOperationOptions;
_session: Session | undefined;
_cachedCollectionNames: string[] = [];

Expand All @@ -68,11 +67,7 @@ export default class Database extends ShellApiWithMongoClass {
this._name = name;
const collections: Record<string, Collection> = {};
this._collections = collections;
this._baseOptions = {};
if (session !== undefined) {
this._session = session;
this._baseOptions.session = session._session;
}
this._session = session;
const proxy = new Proxy(this, {
get: (target, prop): any => {
if (prop in target) {
Expand All @@ -97,6 +92,18 @@ export default class Database extends ShellApiWithMongoClass {
return proxy;
}

async _baseOptions(): Promise<CommandOperationOptions> {
const options: CommandOperationOptions = {};
if (this._session) {
options.session = this._session._session;
}
const maxTimeMS = await this._internalState.shellApi.config.get('maxTimeMS');
if (typeof maxTimeMS === 'number') {
options.maxTimeMS = maxTimeMS;
}
return options;
}

/**
* Internal method to determine what is printed for this class.
*/
Expand Down Expand Up @@ -126,23 +133,23 @@ export default class Database extends ShellApiWithMongoClass {
return this._mongo._serviceProvider.runCommandWithCheck(
this._name,
cmd,
{ ...this._mongo._getExplicitlyRequestedReadPref(), ...this._baseOptions, ...options }
{ ...this._mongo._getExplicitlyRequestedReadPref(), ...await this._baseOptions(), ...options }
);
}

public async _runAdminCommand(cmd: Document, options: CommandOperationOptions = {}): Promise<Document> {
return this._mongo._serviceProvider.runCommandWithCheck(
ADMIN_DB,
cmd,
{ ...this._mongo._getExplicitlyRequestedReadPref(), ...this._baseOptions, ...options }
{ ...this._mongo._getExplicitlyRequestedReadPref(), ...await this._baseOptions(), ...options }
);
}

private async _listCollections(filter: Document, options: ListCollectionsOptions): Promise<Document[]> {
return await this._mongo._serviceProvider.listCollections(
this._name,
filter,
{ ...this._mongo._getExplicitlyRequestedReadPref(), ...this._baseOptions, ...options }
{ ...this._mongo._getExplicitlyRequestedReadPref(), ...await this._baseOptions(), ...options }
) || [];
}

Expand Down Expand Up @@ -202,7 +209,7 @@ export default class Database extends ShellApiWithMongoClass {
return await this._mongo._serviceProvider.runCommand(
this._name,
cmd,
this._baseOptions
await this._baseOptions()
);
} catch (e) {
return e;
Expand Down Expand Up @@ -317,7 +324,7 @@ export default class Database extends ShellApiWithMongoClass {
const providerCursor = this._mongo._serviceProvider.aggregateDb(
this._name,
pipeline,
{ ...this._baseOptions, ...aggOptions },
{ ...await this._baseOptions(), ...aggOptions },
dbOptions
);
const cursor = new AggregationCursor(this._mongo, providerCursor);
Expand Down Expand Up @@ -362,7 +369,7 @@ export default class Database extends ShellApiWithMongoClass {
async dropDatabase(writeConcern?: WriteConcern): Promise<Document> {
return await this._mongo._serviceProvider.dropDatabase(
this._name,
{ ...this._baseOptions, writeConcern }
{ ...await this._baseOptions(), writeConcern }
);
}

Expand Down Expand Up @@ -591,7 +598,7 @@ export default class Database extends ShellApiWithMongoClass {
return await this._mongo._serviceProvider.createCollection(
this._name,
name,
{ ...this._baseOptions, ...options }
{ ...await this._baseOptions(), ...options }
);
}

Expand All @@ -601,7 +608,7 @@ export default class Database extends ShellApiWithMongoClass {
assertArgsDefinedType([name, source, pipeline], ['string', 'string', true], 'Database.createView');
this._emitDatabaseApiCall('createView', { name, source, pipeline, options });
const ccOpts = {
...this._baseOptions,
...await this._baseOptions(),
viewOn: source,
pipeline: pipeline
} as Document;
Expand Down Expand Up @@ -1032,15 +1039,15 @@ export default class Database extends ShellApiWithMongoClass {
setFreeMonitoring: 1,
action: 'enable'
},
this._baseOptions
await this._baseOptions()
);
let result: any;
let error: any;
try {
result = await this._mongo._serviceProvider.runCommand(
ADMIN_DB,
{ getFreeMonitoringStatus: 1 },
this._baseOptions
await this._baseOptions()
);
} catch (err) {
error = err;
Expand All @@ -1060,7 +1067,7 @@ export default class Database extends ShellApiWithMongoClass {
getParameter: 1,
cloudFreeMonitoringEndpointURL: 1
},
this._baseOptions
await this._baseOptions()
);
return `Unable to get immediate response from the Cloud Monitoring service. Please check your firewall settings to ensure that mongod can communicate with '${urlResult.cloudFreeMonitoringEndpointURL || '<unknown>'}'`;
}
Expand Down Expand Up @@ -1419,7 +1426,7 @@ export default class Database extends ShellApiWithMongoClass {
this._emitDatabaseApiCall('watch', { pipeline, options });
const cursor = new ChangeStreamCursor(
this._mongo._serviceProvider.watch(pipeline, {
...this._baseOptions,
...await this._baseOptions(),
...options
}, {}, this._name),
this._name,
Expand Down
2 changes: 1 addition & 1 deletion packages/shell-api/src/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,7 @@ export async function setHideIndex(coll: Collection, index: string | Document, h
collMod: coll._name,
index: cmd
},
coll._database._baseOptions
await coll._database._baseOptions()
);
}

Expand Down
36 changes: 36 additions & 0 deletions packages/shell-api/src/integration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2204,6 +2204,42 @@ describe('Shell API (integration)', function() {
});
});

describe('maxTimeMS support', () => {
skipIfServerVersion(testServer, '< 4.2');

beforeEach(async() => {
await collection.insertMany([...Array(10).keys()].map(i => ({ i })));
const cfg = new ShellUserConfig();
internalState.setEvaluationListener({
getConfig(key: string) { return cfg[key]; },
setConfig(key: string, value: any) { cfg[key] = value; return 'success'; },
listConfigOptions() { return Object.keys(cfg); }
});
});

it('config changes affect maxTimeMS', async() => {
await shellApi.config.set('maxTimeMS', 100);
try {
// eslint-disable-next-line no-constant-condition
await (await collection.find({ $where: function() { while (true); } })).next();
expect.fail('missed exception');
} catch (err) {
expect(err.codeName).to.equal('MaxTimeMSExpired');
}
});

it('maxTimeMS can be passed explicitly', async() => {
await shellApi.config.set('maxTimeMS', null);
try {
// eslint-disable-next-line no-constant-condition
await (await collection.find({ $where: function() { while (true); } }, {}, { maxTimeMS: 100 })).next();
expect.fail('missed exception');
} catch (err) {
expect(err.codeName).to.equal('MaxTimeMSExpired');
}
});
});

describe('cursor map/forEach', () => {
beforeEach(async() => {
await collection.insertMany([...Array(10).keys()].map(i => ({ i })));
Expand Down
10 changes: 5 additions & 5 deletions packages/shell-api/src/interruptor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,13 +80,13 @@ describe('interruptor', () => {

it('causes an interrupt error to be thrown on exit', async() => {
let resolveCall: (result: any) => void;
serviceProvider.runCommandWithCheck.callsFake(() => {
return new Promise(resolve => {
resolveCall = resolve;
});
});
serviceProvider.runCommandWithCheck.resolves(new Promise(resolve => {
resolveCall = resolve;
}));

const runCommand = database.runCommand({ some: 1 });
await new Promise(setImmediate);
await new Promise(setImmediate); // ticks due to db._baseOptions() being async
internalState.interrupted.set();
resolveCall({ ok: 1 });

Expand Down
2 changes: 1 addition & 1 deletion packages/shell-api/src/shell-api.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -731,7 +731,7 @@ describe('ShellApi', () => {
it('will work with defaults', async() => {
expect(await config.get('displayBatchSize')).to.equal(20);
expect((await toShellResult(config)).printable).to.deep.equal(
new Map([['displayBatchSize', 20], ['enableTelemetry', false]] as any));
new Map([['displayBatchSize', 20], ['maxTimeMS', null], ['enableTelemetry', false]] as any));
});

it('rejects setting all config keys', async() => {
Expand Down
4 changes: 3 additions & 1 deletion packages/shell-api/src/shell-internal-state.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,12 @@ describe('ShellInternalState', () => {
expect(() => run('db = 42')).to.throw("[COMMON-10002] Cannot reassign 'db' to non-Database type");
});

it('allows setting db to a db and causes prefetching', () => {
it('allows setting db to a db and causes prefetching', async() => {
serviceProvider.listCollections
.resolves([ { name: 'coll1' }, { name: 'coll2' } ]);
expect(run('db = db.getSiblingDB("moo"); db.getName()')).to.equal('moo');
await new Promise(setImmediate);
await new Promise(setImmediate); // ticks due to db._baseOptions() being async
expect(serviceProvider.listCollections.calledWith('moo', {}, {
readPreference: 'primaryPreferred',
nameOnly: true
Expand Down
5 changes: 5 additions & 0 deletions packages/types/src/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ describe('config validation', () => {
expect(await validate('displayBatchSize', 0)).to.equal('displayBatchSize must be a positive integer');
expect(await validate('displayBatchSize', 1)).to.equal(null);
expect(await validate('displayBatchSize', Infinity)).to.equal(null);
expect(await validate('maxTimeMS', 'foo')).to.equal('maxTimeMS must be null or a positive integer');
expect(await validate('maxTimeMS', -1)).to.equal('maxTimeMS must be null or a positive integer');
expect(await validate('maxTimeMS', 0)).to.equal('maxTimeMS must be null or a positive integer');
expect(await validate('maxTimeMS', 1)).to.equal(null);
expect(await validate('maxTimeMS', null)).to.equal(null);
expect(await validate('enableTelemetry', 'foo')).to.equal('enableTelemetry must be a boolean');
expect(await validate('enableTelemetry', -1)).to.equal('enableTelemetry must be a boolean');
expect(await validate('enableTelemetry', false)).to.equal(null);
Expand Down
6 changes: 6 additions & 0 deletions packages/types/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,7 @@ export interface MongoshBus {

export class ShellUserConfig {
displayBatchSize = 20;
maxTimeMS: number | null = null;
enableTelemetry = false;
}

Expand All @@ -355,6 +356,11 @@ export class ShellUserConfigValidator {
return `${key} must be a positive integer`;
}
return null;
case 'maxTimeMS':
if (value !== null && (typeof value !== 'number' || value <= 0)) {
return `${key} must be null or a positive integer`;
}
return null;
case 'enableTelemetry':
if (typeof value !== 'boolean') {
return `${key} must be a boolean`;
Expand Down

0 comments on commit 94ca4d9

Please sign in to comment.