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

fix(NODE-6454): use timeoutcontext for state machine execute() cursor options #4291

Merged
merged 2 commits into from
Oct 31, 2024

Conversation

baileympearson
Copy link
Contributor

@baileympearson baileympearson commented Oct 24, 2024

Description

What is changing?

Instead of using the remainingTimeMS as the value for timeoutMS when running cursor operations in the state machine, we provide a CursorTimeoutContext that wraps the original timeout.

Is there new documentation needed for these changes?

What is the motivation for this change?

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

@aditi-khare-mongoDB aditi-khare-mongoDB added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Oct 24, 2024
@aditi-khare-mongoDB aditi-khare-mongoDB self-assigned this Oct 24, 2024
@aditi-khare-mongoDB
Copy link
Contributor

aditi-khare-mongoDB commented Oct 24, 2024

LGTM, I'll wait for CI to finish before approving

Edit: I'll wait for the end to end test case before approving

Copy link
Contributor

@aditi-khare-mongoDB aditi-khare-mongoDB left a comment

Choose a reason for hiding this comment

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

Lint is failing, I'll wait on re-reviewing until it's fixed

@baileympearson baileympearson force-pushed the NODE-6454 branch 2 times, most recently from f1f5d0d to 5660c45 Compare October 29, 2024 19:36
.catch(e => e);
it(
'the bulk write operation times out',
mergeTestMetadata(metadata, {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should fix some of the flakiness we see in CI.

src/client-side-encryption/state_machine.ts Outdated Show resolved Hide resolved
test/tools/utils.ts Show resolved Hide resolved
aditi-khare-mongoDB

This comment was marked as off-topic.

@aditi-khare-mongoDB aditi-khare-mongoDB added Team Review Needs review from team and removed Primary Review In Review with primary reviewer, not yet ready for team's eyes labels Oct 31, 2024
@aditi-khare-mongoDB aditi-khare-mongoDB merged commit 511e6c9 into NODE-6090 Oct 31, 2024
20 of 27 checks passed
@aditi-khare-mongoDB aditi-khare-mongoDB deleted the NODE-6454 branch October 31, 2024 15:08
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.

3 participants