docs(NODE-6223): timeoutMS does not govern auto-connect #4280
Conversation
|
Happy to take the tests out or adjust them, they were left over from when I started to implement connect timeout. |
Co-authored-by: Aditi Khare <[email protected]>
nbbeeken
added
the
Primary Review
aditi-khare-mongoDB
added
Team Review
| * `timeoutMS` specified at the client-level will bound the time any operation can take before throwing a timeout error. | ||
| * However, when the operation being run is automatically connecting your `MongoClient` the timeoutMS will only be used for the operation portion of task. | ||
| * This means the time to setup the `MongoClient` does not count against `timeoutMS`. | ||
| * If you are using `timeoutMS` we recommend connecting your client explicitly in advance of any operation to avoid this inconsistent execution time. |
There was a problem hiding this comment.
Is there anything that the user can set that will bound the execution time of autoconnect in this case?
There was a problem hiding this comment.
Nothing specific to auto-connect, any client options: socketTimeoutMS, waitQueueTimeoutMS, serverSelectionTimeoutMS, connectTimeoutMS are all general options that control those components of the driver for all operations. Auto-connect or just connect itself has its own set of operations that cannot be timed out at all, reading TLS files, SRV/TXT DNS lookup, monitoring start-up, server selection, and a ping. Also, auto-encryption can have an internal client that repeats the aforementioned as well.
Generally, folks observe a MongoServerSelectionError when the ping forces a server selection that the monitors fail to service in time. That cannot be set for only connect and only covers a common case.
Co-authored-by: Daria Pardue <[email protected]>
| @@ -509,6 +509,13 @@ export class MongoClient extends TypedEventEmitter<MongoClientEvents> implements | |||
| /** | |||
| * Connect to MongoDB using a url | |||
| * | |||
| * @remarks | |||
| * Calling `connect` is optional since the first operation you perform will call `connect` if it's needed. | |||
| * `timeoutMS` specified at the client-level will bound the time any operation can take before throwing a timeout error. | |||
There was a problem hiding this comment.
| * `timeoutMS` specified at the client-level will bound the time any operation can take before throwing a timeout error. | |
| * `timeoutMS` will bound the time any operation can take before throwing a timeout error. |
This is true, regardless of where timoeutMS is is specified.
| * @remarks | ||
| * Calling `connect` is optional since the first operation you perform will call `connect` if it's needed. | ||
| * `timeoutMS` specified at the client-level will bound the time any operation can take before throwing a timeout error. | ||
| * However, when the operation being run is automatically connecting your `MongoClient` the `timeoutMS` will only be used for the operation portion of task. |
There was a problem hiding this comment.
| * However, when the operation being run is automatically connecting your `MongoClient` the `timeoutMS` will only be used for the operation portion of task. | |
| * However, when the operation being run is automatically connecting your `MongoClient` the `timeoutMS` will not apply to the time taken to connect the MongoClient. |
I'd surmise that many users don't have a conception of the difference between "operation portion of the task" and "connect" - I think it's easier to just say that timeoutMS won't apply to the connecting logic.
| @@ -706,6 +713,13 @@ export class MongoClient extends TypedEventEmitter<MongoClientEvents> implements | |||
| * Connect to MongoDB using a url | |||
| * | |||
| * @remarks | |||
| * Calling `connect` is optional since the first operation you perform will call `connect` if it's needed. | |||
| * `timeoutMS` specified at the client-level will bound the time any operation can take before throwing a timeout error. | |||
There was a problem hiding this comment.
(same comments as above to this doc comment too).
Description
What is changing?
Added docs
Is there new documentation needed for these changes?
New docs for docs? interesting idea
What is the motivation for this change?
Auto connect or plain connect performs file system access, DNS lookup, monitor creation, mongocryptd launching, and more! each one of these steps has its own mechanism for cancellation that is not considered by the CSOT spec. Alternatively we considered ignoring cancelling those operations and simply racing with a timeout, however, the impact could be squashing important errors.
Auto connect needs a future rework to better align with the drivers "always connected" perspective, for now, its best to explicitly connect if there's concern that the connecting operation won't complete in a globally set timeoutMS value.
Double check the following
npm run check:lintscripttype(NODE-xxxx)[!]: descriptionfeat(NODE-1234)!: rewriting everything in coffeescript