Skip to content

Reset doctrine object manager at the end of the request if closed #121

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
Aug 3, 2018
Merged

Reset doctrine object manager at the end of the request if closed #121

merged 2 commits into from
Aug 3, 2018

Conversation

dnna
Copy link
Contributor

@dnna dnna commented Aug 3, 2018

Putting this up for discussion. This prevents the "Entity manager is closed" error from propagating to future requests. Using this together with https://github.com/facile-it/doctrine-mysql-come-back on my project I have not had any database related issues in the past few weeks.

@andig
Copy link
Contributor

andig commented Aug 3, 2018

As per php-pm/php-pm#391 it might also make sense to check if the entity manager‘s db connection is still open? Unfortunately that also brings the burden of error handling into our turf? I‘m a little unsure if this isn‘t really application responsibility?

@dnna
Copy link
Contributor Author

dnna commented Aug 3, 2018

Calling resetManager already forces a reconnect AFAIK, so it should also prevent cases where the connection was closed from propagating to future requests.

That said, this PR does not aim to solve the connection timeout problem, because connection timeouts will still happen at random times and throw exceptions that are hard for the user to debug during the request. It just aims to improve general stability by preventing any database errors from rendering the worker unusable until restarted.

@dnna
Copy link
Contributor Author

dnna commented Aug 3, 2018

The best practice for handling connection timeouts in my opinion is to use a connection wrapper that auto-reconnects on errors (like the one I mentioned above). That's probably an application responsibility (unless we hijack the container and inject our own connection wrapper) but maybe we should mention the issue as part of the documentation so users don't have to dig through several issues and PRs to find the solution?

@andig
Copy link
Contributor

andig commented Aug 3, 2018

Calling resetManager already forces a reconnect

Yeah- the question is if isOpen checks for closed connection. In my non-symfony code I'm checking for connection, too. However I've got hold of the credentials which isn't the case here.

@andig andig merged commit 23ab519 into php-pm:master Aug 3, 2018
@dnna
Copy link
Contributor Author

dnna commented Aug 3, 2018

I'm not sure if isOpen checks for a closed connection but it shouldn't matter - if the connection is closed it will crash in the next request and then the entity manager will be closed :)

@dnna dnna deleted the patch-1 branch August 3, 2018 14:46
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.

2 participants