mongobetween icon indicating copy to clipboard operation
mongobetween copied to clipboard

Revisit connection pinning logic for edge cases

Open kounat opened this issue 1 year ago • 1 comments

The new implementation with connection pinning introduces a couple of edge cases that this PR introduces a test reproduction for. Namely, the case where we're trying to concurrently execute more find queries than we have connections in the pool.

This creates a scenario where queries are stuck waiting for existing cursors to be exhausted before checking out a new connection; in the worst case, the existing cursors may time out or never close, causing the connections to stay in a checked-out state indefinitely.

We're proposing a solution below; however, this will require a change to the driver:

func (m *Mongo) RoundTrip(msg *Message, tags []string) (_ *Message, err error) {
	// ...

        if requestCursorID != 0 {
		var ok bool

		conn, ok = m.cursors.peek(requestCursorID, collection)
		if ok {
			m.log.Debug("Cached cursorID has been found", zap.Int64("cursor", requestCursorID), zap.String("collection", collection))
		}
		// Check the specific connection out of the pool -- if it is already checked out or in use, block until connection is ready.
		// We will need to implement the below function in the driver to support this.
		conn.Open()
	}

	// ...

	defer func() {
		// Return the connection back to the pool after each operation to allow for reuse by concurrent requests.
		if err := conn.Close(); err != nil {
			m.log.Error("Error closing Mongo connection", zap.Error(err), zap.String("address", addr.String()))
		}
	}()

	// ...
}

Right now there isn't an Open() function in driver.Connection interface that we can leverage to check out the same connection for a cursorID -- the existing checkoutConnection function in driver.Server only takes in a context and returns a random connection.

This solution will mimic the old implementation's logic, where connections are checked in and out after each operation and shared amongst queries. This way we won't have to exhaust the pool with checked out connections and have queries wait until entire find operations are finished.

kounat avatar Jul 17 '24 21:07 kounat

@kounat I'm wondering if we can reserve a set of connections specifically for cursors, ensuring that there are always open connections for non-cursor operations. Here is a POC: https://github.com/coinbase/mongobetween/pull/75

prestonvasquez avatar Jul 23 '24 02:07 prestonvasquez