-
Notifications
You must be signed in to change notification settings - Fork 530
RUBY-3616 Fix load balanced implementation #3013
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -87,6 +87,7 @@ def initialize(view, result, server, options = {}) | |||||
| @connection_global_id = result.connection_global_id | ||||||
| @context = @options[:context]&.with(connection_global_id: connection_global_id_for_context) || fresh_context | ||||||
| @explicitly_closed = false | ||||||
| @get_more_network_error = false | ||||||
| @lock = Mutex.new | ||||||
| if server.load_balancer? | ||||||
| # We need the connection in the cursor only in load balanced topology; | ||||||
|
|
@@ -297,19 +298,21 @@ def close(opts = {}) | |||||
| ctx = context ? context.refresh(timeout_ms: opts[:timeout_ms]) : fresh_context(opts) | ||||||
|
|
||||||
| unregister | ||||||
| read_with_one_retry do | ||||||
| spec = { | ||||||
| coll_name: collection_name, | ||||||
| db_name: database.name, | ||||||
| cursor_ids: [ id ], | ||||||
| } | ||||||
| op = Operation::KillCursors.new(spec) | ||||||
| execute_operation(op, context: ctx) | ||||||
| unless @get_more_network_error | ||||||
| read_with_one_retry do | ||||||
| spec = { | ||||||
| coll_name: collection_name, | ||||||
| db_name: database.name, | ||||||
| cursor_ids: [ id ], | ||||||
| } | ||||||
| op = Operation::KillCursors.new(spec) | ||||||
| execute_operation(op, context: ctx) | ||||||
| end | ||||||
| end | ||||||
|
|
||||||
| nil | ||||||
| rescue Error::OperationFailure::Family, Error::SocketError, Error::SocketTimeoutError, Error::ServerNotUsable | ||||||
| # Errors are swallowed since there is noting can be done by handling them. | ||||||
| rescue Error::OperationFailure::Family, Error::SocketError, Error::SocketTimeoutError, Error::ServerNotUsable, Error::ConnectionPerished | ||||||
| # Errors are swallowed since there is nothing can be done by handling them. | ||||||
comandeo-mongo marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| ensure | ||||||
| end_session | ||||||
| @cursor_id = 0 | ||||||
|
|
@@ -379,6 +382,9 @@ def get_more | |||||
| with_overload_retry(context: possibly_refreshed_context) do | ||||||
| process(execute_operation(get_more_operation)) | ||||||
| end | ||||||
| rescue Error::SocketError, Error::SocketTimeoutError | ||||||
|
||||||
| rescue Error::SocketError, Error::SocketTimeoutError | |
| rescue Error::SocketError, Error::SocketTimeoutError, Error::ConnectionPerished |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In load-balanced mode this code checks out a connection manually but does not guarantee it is checked back into the pool if
send_initial_query(or later logic) raises. That can leak checked-out connections and eventually exhaust the pool. Consider usingserver.with_connectionagain now thatConnectionPool#with_connectionskips check-in for pinned connections, or add anensurethat checks the connection in (when it is not pinned/owned by the created cursor).