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

DBAL-275: Automatically attempt to reconnect a dropped persistent MySQL-connection (MySQL server has gone away) #1454

Closed
doctrinebot opened this issue May 14, 2012 · 14 comments
Assignees
Milestone

Comments

@doctrinebot
Copy link

Jira issue originally created by user peetersdiet:

For php-scripts that run for a long time (a.o. daemons) persistent connections will almost always be dropped by the MySQL-server after a set timeout (depending on wait_timeout). This will have Doctrine throw an exception and have the php-script terminate if not catched. It is not practical to catch the same Exception with a try-catch around every query.

I have fixed this for DBAL 2.1.6 by adding a custom layer of Statement-, Connection- and Driver classes.
Key functionalities:

  • The custom layer will transparently catch dropped connections and attempt a number of times (configurable) to reconnect.
  • The behaviour will not be triggered in a transaction (in that case it will revert to throwing an exception just like before).
  • The reconnect behaviour is not MySQL specific per se. It can be triggered by any Exception for any Driver-type if a Driver provides a method 'getReconnectExceptions'.
  • Minimal performance-impact. Only when an Exception is thrown will it be searched for a matching Exception to possibly trigger the behaviour. (also the reason a single stringmatch is used)

Why this functionality?

  • It is often not possible to change settings of a database-server.
  • In a production environment the MySQL wait_timeout is often set to mere seconds
  • Doctrine's use of persistent connections will become a little more persistent :)
  • More reliable and robust php-scripts built on top of the DBAL

See files in attached archive to get an idea of the code. Enabling the layer is currently done like this (Symfony2 yml):
doctrine:
dbal:
wrapper_class: DoP\DoPBundle\Doctrine\DBAL\Connection
driver_class: DoP\DoPBundle\Doctrine\DBAL\Driver\PDOMySql\Driver
options:
x_reconnect_attempts: 2

Maybe I overlook something, but I only see pro's, no cons, to this improvement. I have created this issue to poll if you think this is a welcome feature and are interested to have me rework the code into DBAL itself? Reworking it into DBAL itself would certainly greatly reduce my code.

If agreed I'll create a github pull request when finished with the code and take comments/improvements from there.

(Also, I have glanced over http://www.doctrine-project.org/contribute.html but did not find any coding/testing-guidelines. Can you point me in the right direction?)

Regards,

Dieter

@doctrinebot
Copy link
Author

Comment created by naderman:

Sounds like a rather useful addition to me, would be cool to see this as a default feature.

@doctrinebot
Copy link
Author

Comment created by jpauli:

We used this at work, it's simple, it could need more reflection

@doctrinebot
Copy link
Author

Comment created by @beberlei:

The problem with a generic solution here are the risks involved, we need to answer questions:

  1. did a transaction get aborted beccause of this
  2. is it safe to continue at this point with a second, new connection

I am not sure we can guarantee the 100% working.

@doctrinebot
Copy link
Author

Comment created by peetersdiet:

@julien:
I assume you are referring to the elaborated switch-statement? Well, since this is a DBAL I think data-integrity and performance are the more important things. In my experience when performance is important, you avoid reflection in PHP as much as possible. In the light of performance the switch-statement was a very deliberate substitution of a call to call_user_func_array. The number of arguments was finite and I am fairly sure all possible cases where covered.

@benjamin:
It seems I forgot to mention this in my statement above. Transactions are indeed an issue I deliberately avoided in this early version. As far as I know, transactions in MySQL are connection-based so the transaction will be rolled back when the server disconnects. A solution would be to incorporate a cache of all statements in a transaction. I was waiting on feedback of the Doctrine-developers on incorporating this code into the DBAL itself, before extending it with more functionality... and I now notice that it got assigned :)

I'll try to answer your two questions:

  1. If the connection is dropped when inside a transaction, the behaviour will revert to the normal Doctrine-behaviour, i.e. no reconnection-attempt will be made and the exception 'MySQL server has gone away' will be thrown.
  2. To be clear, I always try to write bug-free code, but I do not guarantee anything about this code. To answer your question: yes, it is safe to continue if you try-catch your transaction in your application-code. Upon the exception you reconnect to MySQL and repeat the lost transaction.

And as a last note, this code is just to have a workable solution. I tend to agree with anyone who thinks that the correct place to fix this problem is in the MySQL-client itself.

Dieter

@doctrinebot
Copy link
Author

Comment created by @beberlei:

My idea would be to throw an exception on reconnect like it is done atm, when transactionNestingLevel > 0, and otherwise proceed with doing the reconnect. I am not sure i am missing something here though.

@doctrinebot
Copy link
Author

Comment created by peetersdiet:

@benjamin: I noticed after commenting that you're the assignee ... and corrected my comment a bit.

Now, if you want I can extend the functionality to support transactions. But I prefer to do this directly into the DBAL, not as a layer on top. The resulting code should be a bit cleaner than this layer now.

What do you think?

_edit_
A bit of crossposting here :)

The way you suggest is the way this layer is implemented
_edit_

Dieter

@doctrinebot
Copy link
Author

Comment created by peetersdiet:

@benjamin
It's been a while since I wrote the code. Above answers are from memory. I will revise the code this evening and give you exact answers. Also, related to transactions, I think the way I handle the savepoints should be revised.

@doctrinebot
Copy link
Author

Comment created by peetersdiet:

@benjamin:
Sorry for the spam... :)

I couldn't help myself doing a quick verification. The answer to your question lies in the file Connection.php. The method DoP\DoPBundle\Doctrine\DBAL\Connection::validateReconnectAttempt also checks that the transactionNestingLevel < 1, so the method will always return false when in a transaction. I.o.w. when in a transaction no attempt to reconnect will be made and the exception is simply rethrown, as per the default Doctrine behaviour.

@doctrinebot
Copy link
Author

Comment created by pkruithof:

I could really use this for kong running cronjobs and daemonized scripts. Is this being worked on right now?

@doctrinebot
Copy link
Author

Comment created by peetersdiet:

Peter, you can use the above code, but it only works for statements outside of transactions. Also, the calls for savepoints in transactions need correction. Will do that when I find the time.

@doctrinebot
Copy link
Author

Comment created by saeven:

This was exactly what I needed...almost! Minor tweaks where it didn't work with current version of Doctrine on ZF2 (iterators req., and mismatched signatures). It also looked for 2006s at the head of the error message, when they can appear in the middle. Lastly, our darned modem was partly to blame, dropping on DNS lookups resulting in network errors. So I'll say big thanks Dieter, and I've posted a small set of tweaks to your classes at http://circlical.com/blog/2013/9/12/mysql-server-has-gone-away-atop-doctrine2-and-zend-framework-2. If this helps another soul, glad to give back; and, thanks again.

@doctrinebot
Copy link
Author

Comment created by @beberlei:

Merged in #414 through new $connection->ping() method.

@doctrinebot
Copy link
Author

Issue was closed with resolution "Fixed"

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants