Doctrine 2 - ORM
  1. Doctrine 2 - ORM
  2. DDC-410

review all sql to ensure that it works with concurrent requests

    Details

    • Type: Task Task
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0-ALPHA4
    • Fix Version/s: 2.0-BETA3
    • Component/s: None
    • Security Level: All
    • Labels:
      None

      Description

      this is just a reminder ticket .. it is important that all SQL code in D2 takes concurrent requests into account. this means simply fetching data and then writing data back to the RDBMS needs to take into account that there could be concurrent requests that change the fetched data. this means employing necessary explicit locking or optimistic locking etc.

        Issue Links

          Activity

          Hide
          Lukas Kahwe added a comment -

          This is looking quite promising. I guess it would be wise to give each model class the choice if it wants to cause a lock or not, because especially in high concurrency scenarios locking a table can of course have some drastic performance effects. Actually if I for example have a sortable behavior and I am not messing with the sorting at all, I do not need to lock the table either. So probably the best approach would be that if a model method is called that requires locking. Each model could register an onFlush() event (theoretically it could try and be smart and only register the event when it knows it has something to do) and then given the value of the flag tell the lock class to issue a lock for the given underlying table.

          BTW: as for innodb .. like i said some RDBMS have this behavior .. but its nothing we can rely on at all ..

          Show
          Lukas Kahwe added a comment - This is looking quite promising. I guess it would be wise to give each model class the choice if it wants to cause a lock or not, because especially in high concurrency scenarios locking a table can of course have some drastic performance effects. Actually if I for example have a sortable behavior and I am not messing with the sorting at all, I do not need to lock the table either. So probably the best approach would be that if a model method is called that requires locking. Each model could register an onFlush() event (theoretically it could try and be smart and only register the event when it knows it has something to do) and then given the value of the flag tell the lock class to issue a lock for the given underlying table. BTW: as for innodb .. like i said some RDBMS have this behavior .. but its nothing we can rely on at all ..
          Hide
          Benjamin Eberlei added a comment -

          yeah of course, this code locks all the time, however some kind of configuration to check for models that need full table locks is probably necessary, but as you can see from the code, easy to implement.

          My idea would be to add a "TableLockRequired" marker interface and in the loops check if any dirty model implements this marker interface.

          Show
          Benjamin Eberlei added a comment - yeah of course, this code locks all the time, however some kind of configuration to check for models that need full table locks is probably necessary, but as you can see from the code, easy to implement. My idea would be to add a "TableLockRequired" marker interface and in the loops check if any dirty model implements this marker interface.
          Hide
          Jordi Boggiano added a comment -

          Just as a reminder, I think if some code attempts to lock and the current RDBMS/Engine doesn't allow the requested lock, D2 should (make it an option maybe, but default true imo) throw an exception saying locking doesn't work with the current engine. Silently doing nothing is good in production to avoid pages exploding, but in development I want to know when I lock if it's not really locking in the background. And if I can know that without reading all detailed specs about whatever engine is in use that's even better.

          Show
          Jordi Boggiano added a comment - Just as a reminder, I think if some code attempts to lock and the current RDBMS/Engine doesn't allow the requested lock, D2 should (make it an option maybe, but default true imo) throw an exception saying locking doesn't work with the current engine. Silently doing nothing is good in production to avoid pages exploding, but in development I want to know when I lock if it's not really locking in the background. And if I can know that without reading all detailed specs about whatever engine is in use that's even better.
          Hide
          Roman S. Borschel added a comment -

          Synchronizing schedule with DDC-178. This issue is considered resolved when the improved locking support from DDC-178 has been implemented. As Benjamin demonstrated more custom (and generic, reusable) locking strategies are possible through the use of the event system.

          Show
          Roman S. Borschel added a comment - Synchronizing schedule with DDC-178 . This issue is considered resolved when the improved locking support from DDC-178 has been implemented. As Benjamin demonstrated more custom (and generic, reusable) locking strategies are possible through the use of the event system.
          Hide
          Benjamin Eberlei added a comment -

          resolved.

          Show
          Benjamin Eberlei added a comment - resolved.

            People

            • Assignee:
              Benjamin Eberlei
              Reporter:
              Lukas Kahwe
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: