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

feat(NODE-6342): support maxTimeMS for explain commands #4207

Merged
merged 13 commits into from
Sep 16, 2024
Merged

Conversation

baileympearson
Copy link
Contributor

@baileympearson baileympearson commented Aug 23, 2024

Description

What is changing?

Explain commands can now have a maxTimeMS specified for them.

Corresponds to mongodb/specifications#1640. The CSOT-related changes for explain helpers will be done in https://jira.mongodb.org/browse/NODE-6375.

Is there new documentation needed for these changes?

No.

What is the motivation for this change?

Release Highlight

Explain now supports maxTimeMS

Driver CRUD commands can be explained by providing the explain option:

collection.find({}).explain('queryPlanner'); // using the fluent cursor API
collection.deleteMany({}, { explain: 'queryPlanner' }); // as an option

However, if maxTimeMS was provided, the maxTimeMS value was applied to the command to explain, and consequently the server could take more than maxTimeMS to respond.

Now, maxTimeMS can be specified as a new option for explain commands:

collection.find({}).explain({ verbosity: 'queryPlanner', maxTimeMS: 2000 }); // using the fluent cursor API
collection.deleteMany({}, { 
	explain: {
		verbosity: 'queryPlanner',
		maxTimeMS: 2000
	}
); // as an option

If a top-level maxTimeMS option is provided in addition to the explain maxTimeMS, the explain-specific maxTimeMS is applied to the explain command, and the top-level maxTimeMS is applied to the explained command:

collection.deleteMany({}, { 
	maxTimeMS: 1000,
	explain: {
		verbosity: 'queryPlanner',
		maxTimeMS: 2000
	}
);

// the actual command that gets sent to the server looks like:
{
	explain: { delete: <collection name>, ..., maxTimeMS: 1000 },
    verbosity: 'queryPlanner',
	maxTimeMS: 2000 
}

Double check the following

  • Ran npm run check:lint script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: type(NODE-xxxx)[!]: description
    • Example: feat(NODE-1234)!: rewriting everything in coffeescript
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

@baileympearson baileympearson changed the title feat: support maxTimeMS for explain commands feat(NODE-6342): support maxTimeMS for explain commands Aug 23, 2024
@baileympearson baileympearson marked this pull request as ready for review August 23, 2024 21:24
@W-A-James W-A-James self-assigned this Aug 26, 2024
@W-A-James W-A-James self-requested a review August 26, 2024 15:37
@W-A-James W-A-James added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Aug 26, 2024
src/utils.ts Outdated Show resolved Hide resolved
test/integration/shared.js Show resolved Hide resolved
test/unit/explain.test.ts Outdated Show resolved Hide resolved
test/unit/utils.test.ts Outdated Show resolved Hide resolved
W-A-James
W-A-James previously approved these changes Sep 13, 2024
@W-A-James W-A-James added Team Review Needs review from team and removed Primary Review In Review with primary reviewer, not yet ready for team's eyes labels Sep 13, 2024
src/explain.ts Show resolved Hide resolved
src/explain.ts Outdated Show resolved Hide resolved
src/utils.ts Show resolved Hide resolved
src/utils.ts Outdated Show resolved Hide resolved
@W-A-James W-A-James merged commit 20396e1 into main Sep 16, 2024
28 of 30 checks passed
@W-A-James W-A-James deleted the explain-poc branch September 16, 2024 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team Review Needs review from team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants