elixir-ecto / ecto_sql

SQL-based adapters for Ecto and database migrations
https://hexdocs.pm/ecto_sql
Apache License 2.0
578 stars 312 forks source link

Add support for pool_count #636

Closed josevalim closed 1 month ago

josevalim commented 1 month ago

Benchmarks coming soon.

josevalim commented 1 month ago

The following benchmark:

Benchee.run(
  %{"checkout" => fn -> Ecto.Bench.PgRepo.checkout(fn -> :ok end) end},
  time: 5,
  parallel: 8
)

Goes from 20k (per process) with pool_count of 1, 30k with pool count of 4, to 48k with pool count of 8 on a machine with 10 cores.

When doing a Repo.get with parallel: 32, going from pool count of 1 to 16 increased throughout in 40% (from 1.4k to 2.0k per process).

greg-rychlewski commented 1 month ago

Sorry I might be mistaken because I haven't looked at partition supervisor. Are you basically starting multiple pools to reduce contention at checkout time?

If this is the case, I was wondering if your tests were run by splitting up the total number of connections. For example if you would start 100 connections on 1 pool did you start 100 / num_partitions connections on each partitioned pool?

The reason I ask is because normally it's better to watch the number of connections you are making to your db to avoid issues.

josevalim commented 1 month ago

Good call. I double checked and here are the numbers:

pool_count pool_size ips
1 32 1.53 K
2 16 1.87 K
4 8 2.13 K
8 4 2.39 K
16 2 2.54 K
32 2 2.54 K
64 2 2.54 K

One may ask: should we instead aim for a high pool_count instead of pool_size? A pool is useful because, in case of high traffic, it gives you the first connection available. Having multiple pools with low size means you have to wait even if other pool is available.

So in case of benchmarks, were all queries take roughly the same time, pool_count will definitely be better. In case of heterogeneous workflows, a pool will most likely give you better overall latency.

But the benchmarks also show the pool itself has some contention over high concurrency count. I am unsure how much it matters in practice, but it will definitely help us squeeze additional performance in benchmarks such as TechEmpower.

josevalim commented 1 month ago

:green_heart: :blue_heart: :purple_heart: :yellow_heart: :heart:

I will investigate CI if it continues to not run the new build post-merging.