Skip to content

RUBY-3909 Close monitoring connection when server marked Unknown from network error#3076

Draft
comandeo-mongo wants to merge 1 commit into
mongodb:masterfrom
comandeo-mongo:3909-close-monitor-connection
Draft

RUBY-3909 Close monitoring connection when server marked Unknown from network error#3076
comandeo-mongo wants to merge 1 commit into
mongodb:masterfrom
comandeo-mongo:3909-close-monitor-connection

Conversation

@comandeo-mongo

Copy link
Copy Markdown
Contributor

Description

The Server Monitoring spec ("hello or legacy hello Cancellation") requires:

When a client marks a server Unknown from Network error when reading or writing, clients MUST cancel the hello or legacy hello check on that server and close the current monitoring connection.

The Ruby driver only implemented this partially. Server#unknown! called monitor.stop_push_monitor!, which closes the streaming (PushMonitor) connection, but never touched the polling Monitor'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 :auto on FaaS) there is no PushMonitor, so nothing was closed at all.

Change

  • Add 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_lock and disconnected outside the lock, following the spec's cancelCheck pseudocode; check was 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_error option (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.rbcancel_check!, connection-lock-guarded check, store_connection/clear_connection.
  • lib/mongo/server.rbunknown! gates on :network_error vs :stop_push_monitor.
  • lib/mongo/server/connection.rb, lib/mongo/server/connection_pool.rb — pass network_error: only for genuine network errors.

Test plan

  • New unit specs in spec/mongo/server/monitor_spec.rb (#cancel_check! closes and clears the connection; a live polling check then establishes a fresh connection) and spec/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.
  • Regression: the cancel-server-check unified SDAM test still passes.
  • Ran against a local replica set:
    • spec/mongo/server/monitor_spec.rb, spec/mongo/server_spec.rb, spec/mongo/server/connection_spec.rb, spec/mongo/server/push_monitor_spec.rb
    • spec/spec_tests/sdam_unified_spec.rb, spec/integration/server_monitor_spec.rb
    • 189 examples, 0 failures. RuboCop clean.

Jira: https://jira.mongodb.org/browse/RUBY-3909

… 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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant