RUBY-3909 Close monitoring connection when server marked Unknown from network error#3076
Draft
comandeo-mongo wants to merge 1 commit into
Draft
RUBY-3909 Close monitoring connection when server marked Unknown from network error#3076comandeo-mongo wants to merge 1 commit into
comandeo-mongo wants to merge 1 commit into
Conversation
… network error The Server Monitoring spec requires that when a server is marked Unknown from a network error while reading or writing, the driver cancel the in-progress hello check and close the current monitoring connection, so the next check runs over a freshly established connection. Previously Server#unknown! only stopped the streaming PushMonitor. The polling Monitor's connection (which also serves RTT in streaming mode) was reused on recovery, letting a server be re-validated over a stale socket instead of proving a fresh connection can be established. Add Monitor#cancel_check!, which stops the PushMonitor and closes the polling connection under a dedicated lock (following the spec's cancelCheck pseudocode). Server#unknown! calls it via a new :network_error option, distinguishing network errors (connection suspect, cancel the check) from non-network connection errors such as auth failures (only the PushMonitor is torn down, as before).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The Server Monitoring spec ("hello or legacy hello Cancellation") requires:
The Ruby driver only implemented this partially.
Server#unknown!calledmonitor.stop_push_monitor!, which closes the streaming (PushMonitor) connection, but never touched the pollingMonitor's own connection. That connection is reused on recovery (Monitor#check), and in streaming mode it also serves RTT measurement, so it survived the failure. As a result a degraded node that resets application sockets and refuses new connections while its old monitor socket stays healthy (the spec's Azure idle-close example; a failed-disk node is another) could be re-validated over the pre-failure socket and marked available again, so operations kept being routed to a node that could not serve them. This is one of the suspected contributing factors behind RUBY-3890.In polling mode (
server_monitoring_mode: :poll, or:autoon FaaS) there is no PushMonitor, so nothing was closed at all.Change
Monitor#cancel_check!: stops the PushMonitor (interrupting its awaited hello read) and closes the polling connection, so the next check must establish a fresh one. The connection is copied under a dedicated@connection_lockand disconnected outside the lock, following the spec'scancelCheckpseudocode;checkwas restructured to snapshot the connection under the lock and write it back with an identity guard, so a concurrent cancel cannot leave a stale socket installed or raise.Server#unknown!distinguishes a new:network_erroroption (cancel the check and close the connection) from:stop_push_monitor(non-network connection error, e.g. an auth failure: only tear down the PushMonitor, as before). This matches the spec's cancel trigger (isNetworkError(error) or not error.completedHandshake) and the pymongo reference, which does not cancel the monitor check on authentication failures.Files
lib/mongo/server/monitor.rb—cancel_check!, connection-lock-guardedcheck,store_connection/clear_connection.lib/mongo/server.rb—unknown!gates on:network_errorvs:stop_push_monitor.lib/mongo/server/connection.rb,lib/mongo/server/connection_pool.rb— passnetwork_error:only for genuine network errors.Test plan
spec/mongo/server/monitor_spec.rb(#cancel_check!closes and clears the connection; a live polling check then establishes a fresh connection) andspec/mongo/server_spec.rb(#unknown!cancels the check on a network error, only stops the PushMonitor otherwise). These cover connection identity, which the unified format cannot express.cancel-server-checkunified SDAM test still passes.spec/mongo/server/monitor_spec.rb,spec/mongo/server_spec.rb,spec/mongo/server/connection_spec.rb,spec/mongo/server/push_monitor_spec.rbspec/spec_tests/sdam_unified_spec.rb,spec/integration/server_monitor_spec.rbJira: https://jira.mongodb.org/browse/RUBY-3909