Memoized/synchronized alternative to DB.open

[matthewmcgarvey]

Problem

In the process of adding Lucky to TechEmpower benchmarks, we found an issue when trying to run multiple instances of our app. The benchmarks would start load testing and suddenly we would get the error sorry, too many clients already from postgres. We had code in Lucky/Avram to memoize the connection which is why the error was unexpected. With some puts debugging, though, we did see that the memoized connection opening was happening more than the one time we expected. Because the load test made so many requests immediately, tons of requests were getting past the memoization before the connection had been opened. We ended up utilizing a Mutex to stop this but I see other frameworks with the same issue:

• Granite
• Crecto
• Jennifer

Clear would not be affected by this because it maintains its own connection pool implementation and instructs users to call an init method at the start of the application.

Request

I think it would be best for all of these frameworks and for users of this library to provide an API that avoids this implementation issue. Rather than requiring DB.open be called once and the connection pooling hidden in the implementation, could an API be made that might be like:

connection_pool = DB::ConnectionPool.new(ENV["DATABASE_URL"])
connection_pool.with_connection do |conn|
  conn.execute "SELECT * FROM users"
end

With this implementation, the connection could be lazily made in the DB::ConnectionPool so we could create it eagerly without establishing a connection. Then the DB::ConnectionPool would handle memoizing/synchronizing access to the connection pool and ORM/SQL libraries wouldn't have to.

Let me know what you think!

[bcardiff]

I don't think the issue is in the crystal-db side, actually.

The underlying reason for this issue is that the creation of the pool can change the running fiber. So while the pool is created on one fiber, another fiber serving a http request will start creating another pool.

To quickly solve this, there are two tracks at least.

1. Create the pool before receiving request.
2. Change the initial_pool_size to 0. That will cause the pool to not change the running fiber since all the operations will be memory initialization.

I would suggest 1. It has the benefit that if the database is wrongly configured you will detect it soon, and not during the first request.

The option 2 will work as long as you are in single thread. When using multi-threading you will have the same problem: multiple pools will be created, only one will survive.

The snippet you propose will be equivalent to option 2. It will work in single-thread because no fiber yield will happen, but will not work on multi-thread.

Finally, if the lazy initialization is needed, then any other context should be synchronized by the framework. So doing something in the crystal-db will be half way solution.

Does this make sense to you?