Doctrine 1
  1. Doctrine 1
  2. DC-185

The pessimistic offline locking manager locks the entire table

    Details

    • Type: Bug Bug
    • Status: Reopened
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 1.1.4, 1.1.5, 1.2.3
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None
    • Environment:
      Windows XP, WampServer Version 2.0

      Description

      Scenario:
      $entity = Doctrine::getTable('Steps')->find($pID);
      $lockingManager = new Doctrine_Locking_Manager_Pessimistic( Doctrine_Manager::connection() );
      $lockingManager->releaseAgedLocks(300);
      $gotLock = $lockingManager->getLock($entity, 'user1' );

      Running this code locks the entire table "Steps", and not just the record.

      in the table "doctrine_lock_tracking", in the fields: "object_type" and "object_key" are saved in this case: "Steps" and "IDStep".
      I think that here must be saved "Steps" and "120" (the value of IDStep).

      1. row_based_locking.patch
        3 kB
        Florian Zumkeller-Quast
      2. DC185TestCase.php
        2 kB
        Fabian Brussa

        Activity

        Hide
        Markus Wößner added a comment -

        Having a look at "Doctrine_Locking_Manager_Pessimistic::getLock()" it becomes clear what causes this misbehaviour:

            public function getLock(Doctrine_Record $record, $userIdent)
            {
                $objectType = $record->getTable()->getComponentName();
                $key        = $record->getTable()->getIdentifier();
        
                $gotLock = false;
                $time = time();
        
                if (is_array($key)) {
                    // Composite key
                    $key = implode('|', $key);
                }
        
                try {
                    $dbh = $this->conn->getDbh();
                    $this->conn->beginTransaction();
        
                    $stmt = $dbh->prepare('INSERT INTO ' . $this->_lockTable
                                          . ' (object_type, object_key, user_ident, timestamp_obtained)'
                                          . ' VALUES (:object_type, :object_key, :user_ident, :ts_obtained)');
        
                    $stmt->bindParam(':object_type', $objectType);
                    $stmt->bindParam(':object_key', $key);
                    $stmt->bindParam(':user_ident', $userIdent);
                    $stmt->bindParam(':ts_obtained', $time);
        

        There is NO hint about the Record's identifier VALUE but only about the identifier's NAME (mostly "id") which appears to be redundant information. Instead of ...

                $key = $record->getTable()->getIdentifier();
        

        ..there should be something like ..

                $key = $record->get($record->getTable()->getIdentifier());
        

        In case of composite keys a string concatenation, prefixed by identifier's name might work but I would recommend using "md5()" on resulting value to limit its length since field "object_key" is limited to 250 chars.

        Show
        Markus Wößner added a comment - Having a look at "Doctrine_Locking_Manager_Pessimistic::getLock()" it becomes clear what causes this misbehaviour: public function getLock(Doctrine_Record $record, $userIdent) { $objectType = $record->getTable()->getComponentName(); $key = $record->getTable()->getIdentifier(); $gotLock = false ; $time = time(); if (is_array($key)) { // Composite key $key = implode('|', $key); } try { $dbh = $ this ->conn->getDbh(); $ this ->conn->beginTransaction(); $stmt = $dbh->prepare('INSERT INTO ' . $ this ->_lockTable . ' (object_type, object_key, user_ident, timestamp_obtained)' . ' VALUES (:object_type, :object_key, :user_ident, :ts_obtained)'); $stmt->bindParam(':object_type', $objectType); $stmt->bindParam(':object_key', $key); $stmt->bindParam(':user_ident', $userIdent); $stmt->bindParam(':ts_obtained', $time); There is NO hint about the Record's identifier VALUE but only about the identifier's NAME (mostly "id") which appears to be redundant information. Instead of ... $key = $record->getTable()->getIdentifier(); ..there should be something like .. $key = $record->get($record->getTable()->getIdentifier()); In case of composite keys a string concatenation, prefixed by identifier's name might work but I would recommend using "md5()" on resulting value to limit its length since field "object_key" is limited to 250 chars.
        Hide
        Florian Zumkeller-Quast added a comment -

        Based on the previous comment by Markus Wößner i created a patch for row based locking.

        It concatenates the PK fields and their values to a string and calculates the sha-1 hash as a unique string representing that record. This string is then used as key so that we'll only lock the single Record and not the whole table.

        I hope you'll give this patch a try - It solved this problem for me.

        Show
        Florian Zumkeller-Quast added a comment - Based on the previous comment by Markus Wößner i created a patch for row based locking. It concatenates the PK fields and their values to a string and calculates the sha-1 hash as a unique string representing that record. This string is then used as key so that we'll only lock the single Record and not the whole table. I hope you'll give this patch a try - It solved this problem for me.
        Hide
        Markus Wößner added a comment -

        I applied patch from Mr. Florian Zumkeller-Quast and provided testcase didn't fail anymore. I think this should do it. By the way I find it strange that this issue isn't already fixed. I guess locking is not very much used by Doctrine users.

        Show
        Markus Wößner added a comment - I applied patch from Mr. Florian Zumkeller-Quast and provided testcase didn't fail anymore. I think this should do it. By the way I find it strange that this issue isn't already fixed. I guess locking is not very much used by Doctrine users.
        Hide
        Jérôme Weber added a comment -

        I applied patch too and it works now. I guess too that nobody use Lockings but when you use it ... without the patch it fails.

        Show
        Jérôme Weber added a comment - I applied patch too and it works now. I guess too that nobody use Lockings but when you use it ... without the patch it fails.
        Hide
        Grégoire Paris added a comment -
        Show
        Grégoire Paris added a comment - Duplicate of http://www.doctrine-project.org/jira/browse/DC-984

          People

          • Assignee:
            Jonathan H. Wage
            Reporter:
            Fabian Brussa
          • Votes:
            7 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated: