Uploaded image for project: 'Doctrine DBAL'
  1. Doctrine DBAL
  2. DBAL-335

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


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


      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:


      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 ###):


      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".


      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.


      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.


      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



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


              • Created: