[5.1] beginTransaction(): Reconnect if connection is lost

[nhowell]

I couldn't figure out how to get PHPUnit to do what I wanted in order to write a test for this. There doesn't seem to be a way to do

$pdo->expects($this->once())->method('beginTransaction')->will($this->throwException(new ErrorException('MySQL server has gone away')));

for the first run of $this->getPdo()->beginTransaction() but then expect a second run of $this->getPdo()->beginTransaction() to run successfully.

Let me know if you have any suggestions on how to test this.

This PR should, at least partly, fix issue #15179.

[GrahamCampbell]

We need another test please.

[nhowell]

We need another test please.

I couldn't figure out how to write a test for this. Any tips?

[taylorotwell]

DIdn't we just revert something like this?

[nhowell]

DIdn't we just revert something like this?

Are you possibly referring to 0b94001?

[taylorotwell]

Yeah, that might be what is jogging my memory.

[nhowell]

I've added a couple tests.

[nhowell]

Ping @taylorotwell. So what do you think? 😄

[taylorotwell]

I dont know it seems like there would be a better way to do this without changing so much code and logic.

[nhowell]

I dont know it seems like there would be a better way to do this without changing so much code and logic.

I don't follow. Could you elaborate? There is very little code and logic changed. The only real logic change is this bit here that checks if the exception was caused by a lost connection, if so it reconnects then tries to begin the transaction again.

if ($this->causedByLostConnection($e)) {
    $this->reconnect();
    $this->pdo->beginTransaction();
} else {
    throw $e;
}

Additionally, I moved the ++$this->transactions; to the end of the method to simplify the code (so that we no longer need to --$this->transactions; if there was an error).

Excluding tests and PHPDoc fixes, this PR is really just 11 additions and 8 deletions.

[fredsted]

We've been having this issue on workers, they sometimes fail in beginTransaction(), and because we're using the --daemon flag, the workers stay up, continuing to make exceptions. So sometimes we'll have to go and restart all the jobs, and do a queue:retry all, then they'll run because the connection was reestablished. Right now we've tried fixing this by reconnecting before every job, but that seems excessive to me.

I think this would solve our problems!

[taylorotwell]

I'll reopen it so I can take a fresh look.

[iget-esoares]

@taylorotwell Will this fix be implemented on L5.2?

[GrahamCampbell]

Unlikely. Only 5.1 is LTS, not 5.2.

[lopn]

what's time to revised release version? I found that the lasted version of 5.1 not include this change!

[barryvdh]

Is this related to #17785 ?
Should commit etc also reconnect if failed?