xerions / mariaex

Pure Elixir database driver for MariaDB / MySQL
Other
263 stars 89 forks source link

Prepared statements are never closed. #182

Open joeppeeters opened 7 years ago

joeppeeters commented 7 years ago

Mariaex is creating a lot of prepared statements when used in combination with Ecto. All queries are prepared before execution, but in most cases never closed. This mainly becomes a problem in a multi-tentant setup with a separate database for each tenant. Each query gets prepared for every tentant database.

In our case the with over 500 tenants and multiple connections to the database, each query gets prepared about 7000 times (worst case).

Due to this behaviour we're hitting the maximum number of prepared statements very quickly, resulting in this error: ** (Mariaex.Error) (1461): Can't create more than max_prepared_stmt_count statements (current value: 32768)

The main issue appears to be that mariaex doesn't cache every query in the LRU cache. When a query gets ejected from this cache the statement gets deallocated at the database server as well. As this never happens, prepared statements are never closed.

I think this really is a bug which needs to be fixed. Are the any other developers who came across this problem?

fishcakez commented 7 years ago

When Mariaex is used with Ecto many select queries using the DSL are prepared with a "name". This name is unique based on the DSL's AST. When a named query is prepared is remains "forever", only unnamed queries go in the LRU cache. Postgrex has a way to turn off named queries but Mariaex does not. Hopefully in Ecto 2.2 the SQL drivers will share a query cache (via DBConnection) and this will be configurable/more powerful than the current implementations.

fishcakez commented 7 years ago

There is no shared query cache in Ecto 2.2 but it would be good to add Postgrex's prepare: :unnamed feature to Ecto, and support disabling the LRU cache in Mariaex, see #188.

I will investigate disabling the Mariaex LRU cache by default in Ecto 3.0 because Ecto already gives names to queries it thinks are worth caching, so those don't go in the Mariaex LRU cache. Its also possible that https://github.com/elixir-ecto/db_connection/issues/99 will cause some GC of prepared queries, reducing the buildup.

atomkirk commented 6 years ago

Yeah we're seeing this a lot too. And when we look at the processes of our mysql server, we see tons of sleeping processes. Is that a result/related?

fishcakez commented 6 years ago

@atomkirk I don't think this issue would cause an increase in mysql processes. Rather I think that would rather be the result of using a high pool_size or pool_overflow.

atomkirk commented 6 years ago

Ok, but this is the error we're getting, so you think we're actually hitting the logical prepare statement limit because we're allowing too many connections? Any idea why there would gradually be a growing number of sleeping processes, all connected to our elixir instances using mariaex? (until we get the max prepared stms error?)

We don't specify max_pool. Whats the default? Whats a good max?

fishcakez commented 6 years ago

@atomkirk ah sorry, so yes it could be related.

There is pool_size (default 20) and pool_overflow (default 0). I think that is an ok pool size for most applications - check the queue_time in the logs, if it grows then there MIGHT be a benefit to increase pool size, but query time can become longer so may not be worthwhile. For postgreSQL I have found a smaller size than 20 often performs better for both of those measurements.

I don't know a reason for the number of processes to grow over time (unless using pool_overflow) as mariaex will connect eagerly - i.e. all workers in the pool connect immediately without waiting for a query.

michaelst commented 6 years ago

@fishcakez to help clarify the growing number of processes. We have an autoscaling instance group, most we ever have is 4 instances connecting to mysql. We have noticed that sometimes after the old scaled instances are killed, that the processes connecting to the db still exist.

archan937 commented 6 years ago

These PRs should solve this issue:

I already got v0.8 working with Ecto 2.2.10. Just add prepare: :unnamed to your repo config:

config :my_app, Repo,
  adapter: Ecto.Adapters.MySQL,
  database: "ecto_simple",
  username: "mysql",
  password: "mysql",
  hostname: "localhost",
  prepare: :unnamed

😄