Skip to content

[11.x] Improve MySQL connect init time by setting all our variables in a single shot #50044

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

Merged
merged 2 commits into from
Feb 21, 2024

Conversation

GrahamCampbell
Copy link
Member

@GrahamCampbell GrahamCampbell commented Feb 10, 2024

The current code does multiple round-trips to set all the variables we need for our config, both because there are multiple commands to run, but also because it's using prepare, for many of them - each use of prepare and execute causes 3 round trips - one to prepare, one to execute, and one to close statement (on garbage collection of the statement in PHP land). The MySQL SET command supports setting multiple things in a comma separated fashion. Refactoring to do this enables us to just run one SET statement against the server. This can make a real difference in a cloud situation such as AWS Lambda talking to an RDS database where we have to go cross-AZ with low single digit ms latency, instead of sub-ms latency. This also reduces load on the DB (fewer statements to execute), so spinning up a bunch of Lambdas in a burst will be less of a burden.


In principal this optimization could be applied to other drivers, too, but I've not included any of that in this PR.

@GrahamCampbell GrahamCampbell force-pushed the mysql-connection-setup-performance-improvement branch 4 times, most recently from bb926b9 to 629ecec Compare February 10, 2024 15:44
@deleugpn
Copy link
Contributor

Seems like a breaking change since my configureIsolationLevel method wouldn't be called after doing a patch update?

https://github.com/cgauge/laravel-aurora-connector/blob/main/src/AuroraConnector.php#L48

@GrahamCampbell
Copy link
Member Author

GrahamCampbell commented Feb 14, 2024

Yehhhh, it is technically a breaking change, but we do these kinds of breaking changes all the time in classes that people could be extending, such as adding new methods, which could in theory break code that extends and defines using the same name. That said, Laravel 11 is close around the corner, and if we have to re-target to that, it's not the end of the world.

@GrahamCampbell GrahamCampbell force-pushed the mysql-connection-setup-performance-improvement branch from 629ecec to 1c74086 Compare February 14, 2024 22:21
@GrahamCampbell
Copy link
Member Author

GrahamCampbell commented Feb 14, 2024

Just rebased this, without making any changes. Deploying into a real application... and consulting the general log to confirm this is doing what I expect.

EDIT: indeed, this is working and doing what was expected.

before:

2024-02-14T22:00:08.822527Z	82587 Connect	example@server on vapor using SSL/TLS
2024-02-14T22:00:08.825213Z	82587 Query	use `vapor`
2024-02-14T22:00:08.827073Z	82587 Prepare	set names 'utf8mb4' collate 'utf8mb4_unicode_ci'
2024-02-14T22:00:08.828216Z	82587 Execute	set names 'utf8mb4' collate 'utf8mb4_unicode_ci'
2024-02-14T22:00:08.829405Z	82587 Close stmt	
2024-02-14T22:00:08.829565Z	82587 Prepare	set session sql_mode='ONLY_FULL_GROUP_BY,NO_ZERO_IN_DATE,NO_ZERO_DATE,NO_ENGINE_SUBSTITUTION'
2024-02-14T22:00:08.830669Z	82587 Execute	set session sql_mode='ONLY_FULL_GROUP_BY,NO_ZERO_IN_DATE,NO_ZERO_DATE,NO_ENGINE_SUBSTITUTION'
2024-02-14T22:00:08.831859Z	82587 Close stmt	

after:

2024-02-14T22:34:09.399500Z	82873 Connect	example@server on vapor using SSL/TLS
2024-02-14T22:34:09.402313Z	82873 Query	USE `vapor`
2024-02-14T22:34:09.403671Z	82873 Query	SET NAMES 'utf8mb4' COLLATE 'utf8mb4_unicode_ci', SESSION sql_mode='ONLY_FULL_GROUP_BY,NO_ZERO_IN_DATE,NO_ZERO_DATE,NO_ENGINE_SUBSTITUTION'

We can see the huge speed increase there too, comparing time between first and last statement execution times between the two (9.3ms -> 4.1ms), and under high DB load due to a spike of connections, the difference will be even greater.

@huynt57
Copy link

huynt57 commented Feb 15, 2024

Very promising
May I use your idea for a package (for lower Laravel version) ? @GrahamCampbell

@GrahamCampbell
Copy link
Member Author

May I use your idea for a package (for lower Laravel version)

Sure, all code contributed here is under the terms of the MIT license, so as long as it's under the same license or a compatible license, there is no problem.

@jameshulse
Copy link
Contributor

The PostgresConnector has very similar functionality. It'd be great if someone could make the same improvement there.

@coppolafab
Copy link

Hi @GrahamCampbell , take a look at this old PR, it might give you some other interesting insights.

@tpetry
Copy link
Contributor

tpetry commented Feb 19, 2024

The PostgresConnector has very similar functionality. It'd be great if someone could make the same improvement there.

PG doesn't have syntax to set multiple configuration parameters in one statement. And its also not needed. You can set these parameters directly for a database user (ALTER ROLE myuser SET conf = val) so you don't have to execute any query for every database connection! So there is more latency reduced. It is now down to 0 😃

@taylorotwell
Copy link
Member

@GrahamCampbell would you mind sending this to master? Also note the MariaDbConnector.

@taylorotwell taylorotwell deleted the mysql-connection-setup-performance-improvement branch February 19, 2024 15:53
@GrahamCampbell GrahamCampbell restored the mysql-connection-setup-performance-improvement branch February 19, 2024 15:53
@GrahamCampbell GrahamCampbell marked this pull request as draft February 19, 2024 15:53
@GrahamCampbell GrahamCampbell changed the title [10.x] Improve MySQL connect init time by setting all our variables in a single shot [11.x] Improve MySQL connect init time by setting all our variables in a single shot Feb 19, 2024
@GrahamCampbell
Copy link
Member Author

Re-targetting at 11.x and updating to include the MariaDB driver. I won't do anything outside of MySQL and MairaDB within this PR.

@GrahamCampbell GrahamCampbell changed the base branch from 10.x to master February 19, 2024 21:13
@GrahamCampbell GrahamCampbell force-pushed the mysql-connection-setup-performance-improvement branch from 93d95ed to 9943279 Compare February 19, 2024 21:15
@GrahamCampbell GrahamCampbell force-pushed the mysql-connection-setup-performance-improvement branch from c23780b to 80d1f9a Compare February 19, 2024 21:17
@GrahamCampbell GrahamCampbell marked this pull request as ready for review February 19, 2024 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.