Doctrine DBAL
  1. Doctrine DBAL
  2. DBAL-335

Is MasterSlaveConnection implemented correctly - seems to overwrite master connection on transaction methods?

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3
    • Component/s: None
    • Labels:
      None

      Description

      Forgive me to doubt, but I think there may be a bug in MasterSlaveConnection.

      It's easier to understand what I'm saying by debugging and tracing the flow, but I'll illustrate it with gists.

      First, here is a simple service method to create a user. It opens up a transaction, persists the user, commits and returns. On error, if there is an active transaction, rollback. Here is the gist:

      https://gist.github.com/3547674

      The "$conn->beginTransaction();" line is where we trace through (the remainder of the service method is now irrelevant). Looking into MasterSlaveConnection.php, we see the method tries to connect to the master connection (call this point ###):

      https://gist.github.com/3547720

      Now looking in the next gist, we see what happens when "$this->connect('master');" is called. At this point it's not that interesting, the internal "$this->_conn" property is set to "master".

      https://gist.github.com/3547750

      Now here lies the bug I believe. "parent::beginTransaction();" is called. When looking into this method, we see that another call is made to connect but this time without "master" as the argument (i.e. connect to slave). This call to connect is made before incrementing the transaction nesting level.

      https://gist.github.com/3547808

      Now, I won't do another gist for "MasterSlaveConnection::connect", but if you refer to the file at line 13 https://gist.github.com/3547750#file_master_slave_connection.php, you will see that it checks the transaction nesting level and if it is there, forces master. However, we don't increment the level until after the method returns, so the slave is used. Ultimately, this results in the internal "$this->_conn" property set to the "slave" connection which violates our original action at ### above where we said we want to connect to "master".

      Am I missing something here? Here is a gist the is a basic attempt at fixing this one method. It simply copies the code from the parent method except does not connect twice. I believe the same would have to occur for all the other methods unless it can be fixed once at the "MasterSlaveConnection::connect" level.

      https://gist.github.com/3547880

      I've just fleshed out "beginTransaction", "commit" and "rollBack" in "MasterSlaveConnection" by basically copying and pasting the code from the parent class and for my failing use case, this fixes the issue. However, it did require updating "Connection" slightly so that I had access to some private variables.

        Issue Links

          Activity

          Jonathan Ingram created issue -
          Benjamin Eberlei made changes -
          Field Original Value New Value
          Status Open [ 1 ] Resolved [ 5 ]
          Fix Version/s 2.3 [ 10184 ]
          Resolution Fixed [ 1 ]
          Marco Pivetta made changes -
          Link This issue is required for DBAL-1006 [ DBAL-1006 ]

            People

            • Assignee:
              Benjamin Eberlei
              Reporter:
              Jonathan Ingram
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: