GODRIVER-1901 Add details to wait queue timeout errors #612
Conversation
| c.mu.Lock() | ||
| defer c.mu.Unlock() | ||
| if c.connection == nil { | ||
| return fmt.Errorf("attempted to pin a connection for a %s, but the connection has already been returned to the pool", reason) | ||
| } | ||
|
|
||
| // Only use the provided callbacks for the first reference to avoid double-counting pinned connection statistics |
There was a problem hiding this comment.
I may be misunderstanding something basic. It wasn't clear to me from the design's description of pinning, but is there a case when a cursor or transaction would pin the same connection multiple times? I don't understand why we would ever need to consider the case a connection is pinned multiple times.
There was a problem hiding this comment.
Creating a cursor inside a transaction would create two references to the same connection.
| assert.Nil(t, err, "PinToTransaction error: %v", err) | ||
| assertPoolPinnedStats(t, pool, 0, 1) | ||
|
|
||
| err = conn.PinToCursor() |
There was a problem hiding this comment.
Oh, maybe this answers my previous question. You may need to pin the same connection to a transaction and a cursor?
There was a problem hiding this comment.
Ah right, that's the double counting protection.
| errorMsg = fmt.Sprintf("%s; maxPoolSize: %d, connections in use by cursors: %d, connections in use by transactions: %d", | ||
| errorMsg, w.maxPoolSize, w.PinnedCursorConnections, w.PinnedTransactionConnections) | ||
| return fmt.Sprintf("%s, connections in use by other operations: %d", errorMsg, | ||
| w.maxPoolSize-(w.PinnedCursorConnections+w.PinnedTransactionConnections)) |
There was a problem hiding this comment.
If I am correct about connections being pinned by both transactions and cursors, is it possible that this could result in a negative number? E.g. max pool size is 1, and the only connection is pinned by both.
There was a problem hiding this comment.
The Connection#pin function always increments the connection's reference count but only calls the provided updatePoolFn for the first reference (see the if c.refCount == 0 conditional). So if a cursor is created in a transaction, the connection reference count should be 2, pool.pinnedCursorConnections should be 0, and pool.pinnedTransactionConnections should be 1. The math shouldn't yield a negative number in this case.
divjotarora
force-pushed
the
godriver1901-lb-timeout-errors
branch
from
761c97d to
a401307
Compare
This PR modifies the
pooltype to track the number of connections that are checked out for cursors and transactions. TheConnectiontype increments these statistics when aPin*function is called and decrements when the connection is expired or returned to the pool. Per the spec, I've also updated theWaitQueueTimeoutErrortype to include these statistics. They're accessible in code via exported fields on the error type and in the error message generated by the type'sErrorfunction.