refactor(NODE-5675): refactor server selection and connection checkout to use abort signals for timeout management

[baileympearson]

Description

What is changing?

• A custom abort controller, TimeoutController, has been added that aborts its signal after a specified timeout
• Server selection and connection checkout have been refactored to use the TimeoutController instead of managing timeouts

Is there new documentation needed for these changes?

No.

What is the motivation for this change?

Release Highlight

Fill in title or leave empty for no highlight

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]

You might ask - why are we using AbortControllers and then manually consuming their signal, instead of making ConnectionPool.checkOut accept an abort signal? Really, there are a few reasons but it's primarily with the eventual state of CSOT in mind (note: everything here applies to server selection as well):

• We need to clear any timeouts we set for connection checkout after we successfully obtain a connection. Abort signals do not provide a "cancellation" interface - and if they did, it would break the Controller/Signal abstraction. Controllers control signals and signals notify other components of abortion.
• Given the above - in later work we will be introducing a TimeoutControllerFactory that is a property on the OperationContext. So eventually it will look like:

const waitQueueMember: WaitQueueMember = {
      callback,
      timeoutController: operationContext.timeoutFactory.controllerForConnectionCheckout()
}

• To answer "why don't we make connection checkout accept a signal instead and pass it in?" - it's two fold. First - we need to cancel the controller like the first bullet mentions. Second - mostly preference, but in terms of encapsulation, I'd rather have connection checkout be responsible for managing its own timeout. If we pass a signal in as an option, we'd need to construct a timeout signal outside of every place we call ConnectionPool.checkout.

[aditi-khare-mongoDB]

I agree, considering the perspective how this piece of code relates to CSOT as a whole. What was the reasoning for not including this information in, say, the PR description so that team reviewers will be able to access it easier?

[baileympearson]

Just preference. I try to answer questions that reviewers might have as they're reviewing the code in advance of anyone reviewing it. I've found the most effective way to do that is by adding comments at the places I think reviewers might have questions

[baileympearson]

TimeoutController intentionally returns an "infinite" timeout when timeout is <=0 or undefined. This unifies code paths for existing timeouts. We now always create a timeout controller to manage server selection and connection checkout, but only if a non-zero positive timeout was provided do we actually create a Nodejs timeout that cancels the request.

[aditi-khare-mongoDB]

This makes sense. Do you feel it would be beneficial to also capture this behavior in the PR description or the ticket information?