Skip to content

Conversation

@gggeek
Copy link
Contributor

@gggeek gggeek commented Jan 26, 2017

No description provided.

@gggeek
Copy link
Contributor Author

gggeek commented Jan 26, 2017

Side note: it might be that this PR works for the 90% scenario, ie. eZP connected to a single DB, but not for the case where one eZP install uses different db connections for different siteaccesses.
I am looking into making that 2nd case work as well, if possible in a generic way.

@gggeek
Copy link
Contributor Author

gggeek commented Jan 27, 2017

Fixed the problem with the case where multiple doctrine db connections are declared. Ready for review.

@gggeek
Copy link
Contributor Author

gggeek commented Nov 26, 2017

ping

@gggeek
Copy link
Contributor Author

gggeek commented Mar 18, 2018

Anyone up for review ?

@pkamps
Copy link
Contributor

pkamps commented Mar 25, 2018

It's a common problem for long running scripts. I wonder if the kernel would be able to handle it in a better way (and not just through a 'Server has gone away' error).

@gggeek
Copy link
Contributor Author

gggeek commented Mar 26, 2018

@pkamps it is true that this is a problem as well for some long-running scripts, however it is a hard problem to fix for the general case, as it is pretty much related to mysql version, mysql configuration (which can sit in 3 places: mysql server, mysql client, php.ini) and the activity of the app (how frequently it executes queries).
And we can not just blindly re-execute failed queries, because they might have been part of transactions that got rolled back etc...

On the other hand this is a known error bound to happen almost all of the time, even though the indexation script does do frequent db queries, for the reason that the script does use the 'fork' system call, and it does not handle properly the passing around of the db-connection file descriptor. As far as I am aware, it is the only legacy script that does use fork.
So it makes sense to have a specific fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants