Should reuse connection to mysql database across web transactions

RESOLVED FIXED

Status

Tree Management
Treeherder
RESOLVED FIXED
10 months ago
10 months ago

People

(Reporter: wlach, Assigned: emorley)

Tracking

Details

Attachments

(1 attachment)

MySQLdb:Connect takes an average of 35.6ms on the job view list, which easily makes it the long poll in the most time consuming of our transactions. According to heroku's docs, we should be able to virtually eliminate this by reusing mysql connections across requests:

https://devcenter.heroku.com/articles/python-concurrency-and-database-connections

Comment 1

10 months ago
Created attachment 8801154 [details] [review]
[treeherder] mozilla:db-config-tweaks > mozilla:master
(Assignee)

Updated

10 months ago
Attachment #8801154 - Flags: review?(wlachance)
(Assignee)

Comment 2

10 months ago
Good spot :-)

We should have been setting this from the start (even when we were in SCL3), but now we're using TLS for the DB connection (so more expensive connection handshake), the penalty for not re-using connections is even greater.
Comment on attachment 8801154 [details] [review]
[treeherder] mozilla:db-config-tweaks > mozilla:master

Thanks!
Attachment #8801154 - Flags: review?(wlachance) → review+

Comment 4

10 months ago
Commits pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/36d11bfe0b2f936285fda5cbe5c0e19aeaf188a6
Bug 1310016 - Remove read_only DB config

Since we no longer have a master-slave setup, and in all environments
DATABASE_URL is identical to DATABASE_RO_URL.

https://github.com/mozilla/treeherder/commit/12389d02c71d405977ad5c02730edcc2bf127c26
Bug 1310016 - Persist DB connections for 5 mins to improve performance

By default Django doesn't perform any connection pooling, so closes the
DB connection after each request:
https://docs.djangoproject.com/en/1.10/ref/settings/#conn-max-age

Reconnecting takes 20-40ms on production (due to use of TLS), which is
avoided with this change.
Looking on stage, it looks like we're still initializing a mysql transaction for most transactions with the exception of those that only go through the Django ORM. Good ol' datasource, can't wait until we get rid of it...
(Assignee)

Comment 6

10 months ago
Yeah unfortunately there's no way to make them use connection pooling easily for now, though soon hopefully we won't be using Datasource at all :-)
Status: NEW → RESOLVED
Last Resolved: 10 months ago
Resolution: --- → FIXED
(Assignee)

Comment 7

10 months ago
Remove the old environment variable on proto/stage/prod...

[~/src/treeherder]$ thh config:unset DATABASE_URL_RO
Unsetting DATABASE_URL_RO and restarting treeherder-prototype... done, v1216
 !    Release command executing: this config change will not be available until the command succeeds.
[~/src/treeherder]$ ths config:unset DATABASE_URL_RO
Unsetting DATABASE_URL_RO and restarting treeherder-stage... done, v443
 !    Release command executing: this config change will not be available until the command succeeds.
[~/src/treeherder]$ thp config:unset DATABASE_URL_RO
Unsetting DATABASE_URL_RO and restarting treeherder-prod... done, v88
 !    Release command executing: this config change will not be available until the command succeeds.
You need to log in before you can comment on or make changes to this bug.