Details

    • Type: Sub-task Sub-task
    • Status: Resolved Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.3.2
    • Fix Version/s: 1.3.3rc1
    • Component/s: JDO
    • Labels:
      None
    • Number of attachments :
      4

      Description

      In current implementation, if an object is deleted, then the _deleted will first true, when the transaction released the lock, the _deleted will be false. It is . It means counterintuitive. I think this also cause the bug in Castor-1044.
      I'll change the meaning of _deleted, so if the object is deleted, than the _deleted is true forever.

      1. patch-C3121-20110626.txt
        4 kB
        Wensheng Dou
      2. patch-C3121-20110703.txt
        20 kB
        Wensheng Dou
      3. patch-C3121-20110705.txt
        3 kB
        Wensheng Dou
      4. patch-C3121-20110713.txt
        37 kB
        Ralf Joachim

        Activity

        Hide
        Wensheng Dou added a comment -

        After I investigated the Castor-1044, I have not found the relation with Castor-1044 yet.

        Show
        Wensheng Dou added a comment - After I investigated the Castor-1044, I have not found the relation with Castor-1044 yet.
        Hide
        Wensheng Dou added a comment -

        What have been done in this patch:
        (1) change the meaning of _deleted, so if the object is deleted, then the deleted will be true forever.
        (2) Simplify the method acquireUpdateLock, this doesn't relate to (1).

        Show
        Wensheng Dou added a comment - What have been done in this patch: (1) change the meaning of _deleted, so if the object is deleted, then the deleted will be true forever. (2) Simplify the method acquireUpdateLock, this doesn't relate to (1).
        Hide
        Wensheng Dou added a comment - - edited

        I’m writing some tests to expose the meaning of the property _deleted. Because I can’t control the sequence of ObjectLock.delete and ObjectLock.release, in order to expose the meaning of _deleted, I have to add a Thread.sleep(1000) after the _delete becomes true in ObjectLock.delete. Then other transactions will grain the opportunity to execute before the ObjectLock.release. I’ll try to explain the complexity of _deleted. I think it is too, too, too complex.
        (1) Why I should add a Thread.sleep(1000) in the ObjectLock.delete?
        According to the comment in ObjectLock, if the object has been deleted, the transaction must call ObjectLock.delete instead of ObjectLock.release. But in fact, if the object has been deleted, the transaction first calls ObjectLock.delete, and then ObjectLock.release.I think the comment is wrong, and the code is right. Because the transaction first calls ObjectLock.delete, and then ObjectLock.release is intrusive and understandable.
        Ok, now, the transaction first calls ObjectLock.delete, then ObjectLock.release. We see what happens about the property _deleted in this sequence. In ObjectLock.delete, the _deleted becomes true, then notify other transactions to execute. In ObjectLock.release, the _deleted becomes false again.
        Ok, We know that the transaction which deletes the object, should first call ObjectLock.delete, then ObjectLock.release (in one thread), even through the transaction notify other thread to execute (which will first check _deleted property, if the _deleted is true, then throw ObjectDeletedWaitingForLockException (acquireLoadLock)), the other transactions will rarely gain opportunity to execute and throw ObjectDeletedWaitingForLockException. So I have to add a Thread.sleep(1000) to show the case.

        (2) Is it ok to change the _deleted to false in ObjectLock.release?
        If one transaction deletes one object, after the transaction call ObjectLock.delete, the _deleted is set to be true, then after the transaction call ObjectLock.release, the _deleted is set to be false. So if one transaction deletes the object, some subsequential transaction will get the ObjectDeletedWaitingForLockException (before the deleting transaction calls the ObjectLock.release), the other subsequential transaction will acquire the lock, then query the database to make sure whether the object exists (after the deleting transaction calls the ObjectLock.release).
        So this process will make it difficult to understand the ObjectDeletedWaitingForLockException, and the sequrence of instructions will affect the result of transaction, which make the result indeterminate.

        (3) Should the acquireCreateLock wait until the _deleted becomes false?
        In the ObjectLock.acquireCreateLock, If the _deleted is true, the acquireCreateLock will wait until the _deleted becomes false. What sequence of instructions will cause this happen? It is ObjectLock.delete, ObjectLock.acquireCreateLock, ObjectLock.release. But we know that, ObjectLock.acquireCreateLock rarely happens between ObjectLock.delete and ObjectLock.release, because of the schedule of threads.
        So I think the acquireCreateLock should not wait until the _deleted becomes false, if no other transactions are using the lock, the acquireCreateLock will succeed.

        (4) Should ObjectLock.removeWaiting change _delete to be false?
        If one transaction finds the _deleted is true, then throw ObjectDeletedWaitingForLockException. After that, the removeWaiting is called, then (if no other transactions are using the lock) the _deleted is set to be false. So other waiting transactions will throw ObjectDeletedWaitingForLockException.
        Ok, now, if removeWaiting doesn’t change the _deleted to be false, all waiting transaction will also throw ObjectDeletedWaitingForLockException. And It will has the same meaning with the current implementation.

        (5) Should the ObjectLock.confirm should change _deleted to be false if not succeed?
        In the ObjectLock.confirm, if it is not succeed, the _deleted is set to be true. But in fact, we don’t know whether the object is deleted or not.
        So we should keep the _delete to be false, and let the sequence process to check whether the object is deleted or not.

        (6) Should the ObjectLock.acquireUpdateLock wait until the _delete is set to be false?
        If one object is deleted, the update should throw Exception.

        (7) What happened when other transactions (not in the waiting list) try to acquire the lock?
        After the transaction deletes the object, it will expire the lock, then the other transactions (not in the waiting list) will not acquire the lock again. So it should create new lock related to the OID.

        After these tests and analysis, I think I get the whole meaning of the property _deleted. The current implementation make it too, too, too complex. So the simplification should be necessary. What should be changed?
        (1) We should keep the sequence of instructions in the current implementation, if the object has been deleted, the transaction first calls ObjectLock.delete, and then ObjectLock.release. So the comments will need to be corrected.
        (2) Simplify the meaning of _deleted, If the object is deleted, the _deleted will be true forever. After the waiting transactions are processed, the old lock will be disposed.
        (3) After the deleting transaction releases the lock, other waiting transactions could go on. The waiting transactions should not go on before the deleting transaction releases the lock. If the object is deleted, then all other waiting transaction will throw ObjectDeletedWaitingForLockException.

        Show
        Wensheng Dou added a comment - - edited I’m writing some tests to expose the meaning of the property _deleted. Because I can’t control the sequence of ObjectLock.delete and ObjectLock.release, in order to expose the meaning of _deleted, I have to add a Thread.sleep(1000) after the _delete becomes true in ObjectLock.delete. Then other transactions will grain the opportunity to execute before the ObjectLock.release. I’ll try to explain the complexity of _deleted. I think it is too, too, too complex. (1) Why I should add a Thread.sleep(1000) in the ObjectLock.delete? According to the comment in ObjectLock, if the object has been deleted, the transaction must call ObjectLock.delete instead of ObjectLock.release. But in fact, if the object has been deleted, the transaction first calls ObjectLock.delete, and then ObjectLock.release.I think the comment is wrong, and the code is right. Because the transaction first calls ObjectLock.delete, and then ObjectLock.release is intrusive and understandable. Ok, now, the transaction first calls ObjectLock.delete, then ObjectLock.release. We see what happens about the property _deleted in this sequence. In ObjectLock.delete, the _deleted becomes true, then notify other transactions to execute. In ObjectLock.release, the _deleted becomes false again. Ok, We know that the transaction which deletes the object, should first call ObjectLock.delete, then ObjectLock.release (in one thread), even through the transaction notify other thread to execute (which will first check _deleted property, if the _deleted is true, then throw ObjectDeletedWaitingForLockException (acquireLoadLock)), the other transactions will rarely gain opportunity to execute and throw ObjectDeletedWaitingForLockException. So I have to add a Thread.sleep(1000) to show the case. (2) Is it ok to change the _deleted to false in ObjectLock.release? If one transaction deletes one object, after the transaction call ObjectLock.delete, the _deleted is set to be true, then after the transaction call ObjectLock.release, the _deleted is set to be false. So if one transaction deletes the object, some subsequential transaction will get the ObjectDeletedWaitingForLockException (before the deleting transaction calls the ObjectLock.release), the other subsequential transaction will acquire the lock, then query the database to make sure whether the object exists (after the deleting transaction calls the ObjectLock.release). So this process will make it difficult to understand the ObjectDeletedWaitingForLockException, and the sequrence of instructions will affect the result of transaction, which make the result indeterminate. (3) Should the acquireCreateLock wait until the _deleted becomes false? In the ObjectLock.acquireCreateLock, If the _deleted is true, the acquireCreateLock will wait until the _deleted becomes false. What sequence of instructions will cause this happen? It is ObjectLock.delete, ObjectLock.acquireCreateLock, ObjectLock.release. But we know that, ObjectLock.acquireCreateLock rarely happens between ObjectLock.delete and ObjectLock.release, because of the schedule of threads. So I think the acquireCreateLock should not wait until the _deleted becomes false, if no other transactions are using the lock, the acquireCreateLock will succeed. (4) Should ObjectLock.removeWaiting change _delete to be false? If one transaction finds the _deleted is true, then throw ObjectDeletedWaitingForLockException. After that, the removeWaiting is called, then (if no other transactions are using the lock) the _deleted is set to be false. So other waiting transactions will throw ObjectDeletedWaitingForLockException. Ok, now, if removeWaiting doesn’t change the _deleted to be false, all waiting transaction will also throw ObjectDeletedWaitingForLockException. And It will has the same meaning with the current implementation. (5) Should the ObjectLock.confirm should change _deleted to be false if not succeed? In the ObjectLock.confirm, if it is not succeed, the _deleted is set to be true. But in fact, we don’t know whether the object is deleted or not. So we should keep the _delete to be false, and let the sequence process to check whether the object is deleted or not. (6) Should the ObjectLock.acquireUpdateLock wait until the _delete is set to be false? If one object is deleted, the update should throw Exception. (7) What happened when other transactions (not in the waiting list) try to acquire the lock? After the transaction deletes the object, it will expire the lock, then the other transactions (not in the waiting list) will not acquire the lock again. So it should create new lock related to the OID. After these tests and analysis, I think I get the whole meaning of the property _deleted. The current implementation make it too, too, too complex. So the simplification should be necessary. What should be changed? (1) We should keep the sequence of instructions in the current implementation, if the object has been deleted, the transaction first calls ObjectLock.delete, and then ObjectLock.release. So the comments will need to be corrected. (2) Simplify the meaning of _deleted, If the object is deleted, the _deleted will be true forever. After the waiting transactions are processed, the old lock will be disposed. (3) After the deleting transaction releases the lock, other waiting transactions could go on. The waiting transactions should not go on before the deleting transaction releases the lock. If the object is deleted, then all other waiting transaction will throw ObjectDeletedWaitingForLockException.
        Hide
        Wensheng Dou added a comment - - edited

        The patch-C3121-20110703.txt adds two tests for _deleted property. Because I have not add a Thread.sleep(1000) in the ObjectLock.delete, the testDeleteThenLoad will failed in almost all the executions.
        If we refactor the ObjectLock as the above comment, the tests will pass as excepted.

        Show
        Wensheng Dou added a comment - - edited The patch-C3121-20110703.txt adds two tests for _deleted property. Because I have not add a Thread.sleep(1000) in the ObjectLock.delete, the testDeleteThenLoad will failed in almost all the executions. If we refactor the ObjectLock as the above comment, the tests will pass as excepted.
        Hide
        Wensheng Dou added a comment - - edited

        After the CASTOR-3132 is fixed, I refactor the patch-C3121-20110626.txt and check it again.
        In patch-C3121-20110705.txt, I resubmit the patch for simplifying the meaning of the property _deleted. In this patch, the tests in patch-C3121-20110703.txt will pass.

        Show
        Wensheng Dou added a comment - - edited After the CASTOR-3132 is fixed, I refactor the patch-C3121-20110626.txt and check it again. In patch-C3121-20110705.txt, I resubmit the patch for simplifying the meaning of the property _deleted. In this patch, the tests in patch-C3121-20110703.txt will pass.
        Hide
        Ralf Joachim added a comment -

        Final patch including both patches of Wensheng and some improvements of code formating

        Show
        Ralf Joachim added a comment - Final patch including both patches of Wensheng and some improvements of code formating

          People

          • Assignee:
            Wensheng Dou
            Reporter:
            Wensheng Dou
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: