Is it correct to cache this per-fiber?

[ioquatix]

connection_pool/lib/connection_pool.rb

Line 117 in 428c06f

if ::Thread.current[@key]

Looking at this code, I noticed it caches a connection per fiber.

I'm a little concerned that someone might expect:

pool.with |connection1|
  connection1 ... # start a pipeline
  pool.with |connection2|
    connection2.get(...) # get some value
  end
  connection1 ... # end/commit pipeline
end

I don't understand why connection1 and connection2 should be the same connection. I would probably expect them to be distinct, otherwise the state from one might leak into another. It's probably even more odd if this operation is inside a library, that might be expecting a clean connection, but is instead gets spooky action at a distance with a user connection. This could result in some pretty hard to debug issues.

[mperham]

Yes this is intentional: while a connection is checked out, further checkouts will return the same connection. It's had this semantic for many years now.

[ioquatix]

Based on your response, there are two things I think we can do:

• Change the behaviour so that the same connection is not returned. Because a connection pool is a cache, I don't think anyone should be depending on this behaviour, i.e. a connection pool could be validly implemented by just returning a completely new connection every time (but would be slow), OR
• Add documentation regarding the situations in which with do |connection| can actually return a connection which is already in use and may have state (e.g. transactions, pipeline, etc) attached to it.