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

[5.3] Allow reconnect in transaction by setting transactions to 0 #15931

Merged
merged 1 commit into from
Oct 18, 2016

Conversation

halaei
Copy link
Contributor

@halaei halaei commented Oct 15, 2016

I am not sure if I apply this to 5.1 as well. The tests and codes changed. Maybe need another PR.

Also here is a history you may check #5944 #5937

Thanks.

@taylorotwell
Copy link
Member

So is this to replace all your other PRs? I'm confused by all these PRs.

@taylorotwell
Copy link
Member

Can you provide description of initial problem and how this fixes it?

@halaei
Copy link
Contributor Author

halaei commented Oct 16, 2016

Code history check

In Laravel 4.2, this line were added to Connection::setPdo():

if ($this->transactions >= 1) throw new \RuntimeException("Attempt to change PDO inside running transaction");

As mentioned in #5937, the motivation was to prevent Connection::run() from breaking transaction logic by retrying part of a transaction in a lost connection inside another connection.

The initial proposed solutions in #5937 was this

(1) skip Connection::reconnectIfMissingConnection and Connection::tryAgainIfCausedByLostConnection calls if Connection::$transactions > 0
or
(2) modify them to throw exceptions and reset Connection::$transactions to 0 when they called.

Then (2) was implemented (without resetting $transactions to 0) in #5944, which I think was wrong, and some time later (1) is also done.

The behavior after this PR

I think by having solution (1) in the code, there is no need to throw exception in setPdo() when Connection::transactions > 0. Just reseting the value to 0 is enough.

The three new added tests shows the new behavior:

  1. Connection::setPdo(), which may be called by DB::reconnect(), will not throw exception if a transaction is open in the current connection. Instead, it just resets Connection::transactions to 0.
  2. Connection::run() attempts to reconnect(swap PDO) on lost connection event if and only if there is no open transaction in the connection. If there is an open transaction, it re-throws the exception so the client code decides what to do; It may give up and die, but only after this PR it can call DB::reconnect() and retry the transaction or pick the next job from the queue.

Thanks.

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