-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dart connection pool too large? #1556
Comments
Also created redstone-dart/redstone_mapper_pg#1 |
For some background - postgresql performs best with ~2 connections per core. Having more than this number of connections will just create contention and significantly slow down the benchmark result. This problem is mitigated in some other databases which have connection multiplexing, allowing a single database worker thread/process to handle multiple connections. Postgresql doesn't do this, it starts a new process per connection, as it expects the user to use a correctly configured connection pool. Postgresql developers argue adding connection multiplexing just adds overhead which is unnecessary, and users should know what they are doing. In my opinion having a benchmark framework which purposely misconfigures postgresql doesn't provide useful results. All of the postgresql benchmarks should be allowed to run with an optimally sized connection pool. Otherwise you'll just make postgresql look slow compared to mysql, due to a poor postgresql configuration. So I would argue that the redstone benchmark is doing the correct thing, and the bug lies with the other postgresql benchmarks which use an incorrectly sized connection pool. A while back a had a look, and many of the other language postgresql benchmarks also ignore the connection pool size setting. |
Thanks! I think we're on the same page. The "bug" I referred to is in the redstone framework itself, not the benchmark. It's a funny situation where if they fix the bug, their performance here will probably decrease... until we fix the benchmarks. I probably made a mistake in how I phrased the issue title and comment. It sounds specific to Dart or the one framework named "dart" in this repository, but it is worth re-examining how we size the connection pools for any framework that uses PostgreSQL. (Wouldn't it be cool if the benchmarks could figure out the correct connection pool sizes for each framework automatically, based on performance? I bet that would take too long to run in practice though.) |
Actually, a few people have requested that this project dictate connection pool sizes, instead of the current approach to leaving it up to each framework. It's a moot point at the moment, because there is no easy way to implement the idea across all the different technology stacks in use. A victim of our own success in this particular respect.... |
Yup - on the same page now. Also forgot to mention, the reason the pool size is so small, is because there are actually separate per isolate connection pools. (isolate = thread but without shared mutable state). These Dart benchmarks typically start n isolates where n is the number of cores. I'm keen to contribute patches - but I am a little over committed right now. So I'll just leave my short rant here instead ;) |
Ok - I got curious and had another dig. I think the problem may be that the connection pool always has exactly one connection. db connections and isolates are both set to MAX_THREADS, and the pool size is set to "dbConnections ~/ isolates" which is always one. Oops! setup.sh $DART_HOME/bin/dart server.dart -a 0.0.0.0 -p 8080 -d ${MAX_THREADS} -i ${MAX_THREADS} server.dart var parser = new ArgParser()
..addOption('address', abbr: 'a', defaultsTo: '0.0.0.0')
..addOption('port', abbr: 'p', defaultsTo: '8080')
..addOption('dbconnections', abbr: 'd', defaultsTo: '256')
..addOption('isolates', abbr: 'i', defaultsTo: '1');
var arguments = parser.parse(args);
var isolates = int.parse(arguments['isolates']);
var dbConnections = int.parse(arguments['dbconnections']) ~/ isolates; |
@xxgreg Nice catch. Passing in We also plan to compare a few hard-coded connection pool sizes for the |
For whoever decides to patch (and test) this. I'd recommend trying 3 connections per isolate. One simple patch is to update setup.sh to do this: $DART_HOME/bin/dart server.dart -a 0.0.0.0 -p 8080 -d $((3 * MAX_THREADS)) -i $MAX_THREADS |
@michaelhixson Is this issue resolved? |
It's from too long ago. I don't remember. |
We should re-run the dart multiple queries test with a smaller database connection pool, like 3 connections. redstone-postgresql outperforms dart in round 10 for this test even though the technology involved is very similar. This might be due to a bug in redstone's connection pool where it ignores whatever pool size you specify and always uses a size of 1-3.
See this Reddit comment thread for context:
The text was updated successfully, but these errors were encountered: