framework icon indicating copy to clipboard operation
framework copied to clipboard

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

Open GrahamCampbell opened this issue 1 year ago • 10 comments
trafficstars

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 avatar Feb 10 '24 15:02 GrahamCampbell

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

deleugpn avatar Feb 12 '24 13:02 deleugpn

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 avatar Feb 14 '24 22:02 GrahamCampbell

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.

GrahamCampbell avatar Feb 14 '24 22:02 GrahamCampbell

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

huynt57 avatar Feb 15 '24 04:02 huynt57

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.

GrahamCampbell avatar Feb 15 '24 09:02 GrahamCampbell

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

jameshulse avatar Feb 19 '24 05:02 jameshulse

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

coppolafab avatar Feb 19 '24 08:02 coppolafab

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 😃

tpetry avatar Feb 19 '24 09:02 tpetry

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

taylorotwell avatar Feb 19 '24 15:02 taylorotwell

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 avatar Feb 19 '24 15:02 GrahamCampbell