692383 - smarter host-based mysql connection pooling

Status: VERIFIED FIXED

(Cloud Services :: Server: Core, defect)

Description

[Pete Fritchman [:petef]]

With our node setup, we have ten different sqluris for one single mysql instance (one for each weaveN database). It would be great if the connection pooling was smart enough to keep connections per database *host*, and just set the proper database each time.  This way, we can run more gunicorn workers without exhausting connections on the MySQL side.

Comment 1

[Tarek Ziadé (:tarek)]

I am starting to experiment on this in SA, by setting up a shared pool that would be able to switch to a database via 'use' on the fly.

notes to myself

- share the same pool object "one host - N similar DBs"
- hook the 'checkout' event to run 'use' if needeed
- on first connect event, call the dialect init on all DBS.

Comment 2

[Tarek Ziadé (:tarek)]

Attachment: per-server pool

Here's a first draft of a per-server pool system. in case a query is made on a server, a connection to this server will be reused, and the Database changed, without reconnections.

Let's hook it in Storage and let's try it on a stress test whenever you have a chance Pete

Comment 3

[Toby Elliott [:telliott]]

Bumping this one, since I think it is something we're going to want to continue with going forward. Pete - any thoughts on the proposal?

Comment 4

[Tarek Ziadé (:tarek)]

the code is there, if people have interest again one day (next fire)

Comment 5

[Bob Micheletto [:bobm]]

Please re-consider this bug.

Comment 6

[Bob Micheletto [:bobm]]

Also, please incorporate a version of SQL Alchemy that addresses the connection pool bug shown here: http://hg.sqlalchemy.org/sqlalchemy/rev/aff95843c12f#l2.17

Comment 7

[Ryan Kelly [:rfkelly]]

Attachment: add option to share connection pools for the same db host

Here's my attempt at the-simplest-thing-that-could-possibly-work, taking the ideas from Tarek's patch but trying to avoid changes to any of the surrounding code.

Tarek, can you comment on why you did not end up using events in your original patch?  It seems to work OK here but I may be missing something.

I'm a little concerned about the overhead of executing "USE dbname" ahead of every query, since it means an additional round-trip to the db server.  It's the simplest option, so we should measure the overhead and see, but I wonder if it would be better to use fully-qualified dbname.tablename references and avoid this constant db switching.

Comment 8

[Toby Elliott [:telliott]]

Comment on attachment 723308 [details] [diff] [review]
add option to share connection pools for the same db host

A few things about this patch make me nervous - mostly what damage could be caused if the switch failed, and some scaling concerns over the extra query 
(even though it's very fast), but it's good to move forward to testing and staging load.

Comment 9

[Ryan Kelly [:rfkelly]]

Per Bug 850057, this is now in production.  Closing, will re-open if it doesn't seem to be doing that it's supposed to do.

Comment 10

[James Bonacci [:jbonacci]]

Verified via bug 850057