Skip to content
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

MySQL connect timeout exceeded should result in a re-connect, but it does not #48688

Open
GrahamCampbell opened this issue Oct 10, 2023 · 4 comments

Comments

@GrahamCampbell
Copy link
Member

GrahamCampbell commented Oct 10, 2023

Laravel Version

10.27.0

PHP Version

8.1.11

Database Driver & Version

MySQL

Description

The DB connector code is meant to retry failed connections, but failing to connect due to a connect timeout is not retried. I have observed this on MySQL, but possibly other drivers suffer the same issue.

Steps To Reproduce

Using the mysql driver, set the host to google.com. Only one attempt is made to connect to the DB because tryAgainIfCausedByLostConnection determines that message SQLSTATE[HY000] [2002] Operation timed out should not be re-tried. I think the tryAgainIfCausedByLostConnection logic is great for re-connecting post-connect, but is not so good for retrying the initial connect. I think we need to add additional message matching specifically for the connector code, only. A timeout should not otherwise be retried, since that could result in double inserts, for example.

@github-actions
Copy link

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

@kassah
Copy link

kassah commented Nov 28, 2023

This is interesting and simple in original concept, but comes with some caviats.

These issues may be ignorable in original draft when getting review on concept, but I think should be thought out (or accounted for) in the final PR.

I hope these don't prevent anyone from contributing, but instead just allows someone to be able to tackle it knowing some of the edge cases.

Wrinkles that add to the complexity:

  • Database transactions build up a state, if the connection goes away, that state is purged. A reconnection would not have this state.
  • Database variables if set will not persist on a reconnection. These are not currently handled within Laravel itself, so would be difficult to track currently.
  • Temp Tables are temporary tables that are selected into. These are connection dependent, so once a connection is dropped, they go away. They are not supported directly by Laravel, so are difficult to track currently.

Possible solutions:

Database Transactions

Currently Laravel tracks these, so it like the insert counter, we could rely on a counter for that.

Database Variables

We could either add first class support for these, and thus make them more trackable (like a counter). Or we can say that since they are not first class citizens, they should not be used at all.

Temp Tables

At least in my experience these are rarely used. Ignoring these could be somewhat safe, but may more mean this behavior needs to be more configurable?

Dealing with more than one wrinkle at once

Maybe mark the connection as not reconnnectable once any raw query is issued.

@driesvints
Copy link
Member

Closing this issue because it's inactive, already solved, old or not relevant anymore. Feel to open up a new issue if you're still experiencing this.

@GrahamCampbell
Copy link
Member Author

Is not solved and is still relevant.

@GrahamCampbell GrahamCampbell reopened this Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants