You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Pavel Moravec <pm...@redhat.com> on 2014/01/31 13:14:51 UTC

Review Request 17587: [legacystore] store_chk raises "Operation on non-existent record: operation=unlock; rid=.." on aborted DTX transaction in TplStore

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17587/
-----------------------------------------------------------

Review request for qpid and Kim van der Riet.


Bugs: https://issues.apache.org/jira/browse/QPID-5530
    https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/QPID-5530


Repository: qpid


Description
-------

Processing DTX enqueue, dequeue and abort, self.__emap is empty map everytime. Hence executing:

     def _abort(self, xid):
         """Perform an abort operation for the given xid record"""
         for fid, hdr, lock in self.__map[xid]:
             if isinstance(hdr, jrnl.DeqRec):
                 self.__emap.unlock(hdr.deq_rid)
         del self.__map[xid]

raises error in unmap.

Though I did not understand full logic of emap / map in janal.py, the extra condition in _abort prevents raising the error and allows proper processing of the abort.


Diffs
-----

  /trunk/qpid/tools/src/py/qpidstore/janal.py 1562703 

Diff: https://reviews.apache.org/r/17587/diff/


Testing
-------


Thanks,

Pavel Moravec


Re: Review Request 17587: [legacystore] store_chk raises "Operation on non-existent record: operation=unlock; rid=.." on aborted DTX transaction in TplStore

Posted by Kim van der Riet <ki...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17587/#review33330
-----------------------------------------------------------

Ship it!


Ship It!

- Kim van der Riet


On Jan. 31, 2014, 3:50 p.m., Pavel Moravec wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17587/
> -----------------------------------------------------------
> 
> (Updated Jan. 31, 2014, 3:50 p.m.)
> 
> 
> Review request for qpid and Kim van der Riet.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/QPID-5530
>     https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/QPID-5530
> 
> 
> Repository: qpid
> 
> 
> Description
> -------
> 
> Processing DTX enqueue, dequeue and abort, self.__emap is empty map everytime. Hence executing:
> 
>      def _abort(self, xid):
>          """Perform an abort operation for the given xid record"""
>          for fid, hdr, lock in self.__map[xid]:
>              if isinstance(hdr, jrnl.DeqRec):
>                  self.__emap.unlock(hdr.deq_rid)
>          del self.__map[xid]
> 
> raises error in unmap.
> 
> Though I did not understand full logic of emap / map in janal.py, the extra condition in _abort prevents raising the error and allows proper processing of the abort.
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/tools/src/py/qpidstore/janal.py 1562852 
> 
> Diff: https://reviews.apache.org/r/17587/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Pavel Moravec
> 
>


Re: Review Request 17587: [legacystore] store_chk raises "Operation on non-existent record: operation=unlock; rid=.." on aborted DTX transaction in TplStore

Posted by Pavel Moravec <pm...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17587/
-----------------------------------------------------------

(Updated Jan. 31, 2014, 3:50 p.m.)


Review request for qpid and Kim van der Riet.


Changes
-------

New patch version uploaded (in fact verified Kim's patch I received with corrected typo).

It applies Kim's comment and works well.


Bugs: https://issues.apache.org/jira/browse/QPID-5530
    https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/QPID-5530


Repository: qpid


Description
-------

Processing DTX enqueue, dequeue and abort, self.__emap is empty map everytime. Hence executing:

     def _abort(self, xid):
         """Perform an abort operation for the given xid record"""
         for fid, hdr, lock in self.__map[xid]:
             if isinstance(hdr, jrnl.DeqRec):
                 self.__emap.unlock(hdr.deq_rid)
         del self.__map[xid]

raises error in unmap.

Though I did not understand full logic of emap / map in janal.py, the extra condition in _abort prevents raising the error and allows proper processing of the abort.


Diffs (updated)
-----

  /trunk/qpid/tools/src/py/qpidstore/janal.py 1562852 

Diff: https://reviews.apache.org/r/17587/diff/


Testing
-------


Thanks,

Pavel Moravec


Re: Review Request 17587: [legacystore] store_chk raises "Operation on non-existent record: operation=unlock; rid=.." on aborted DTX transaction in TplStore

Posted by Kim van der Riet <ki...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17587/#review33323
-----------------------------------------------------------


if (isinstance(hdr, jrnl.DeqRec)) and (hdr.deq_rid in self.__map):

I don't think this change is correct, you cannot search for rids in the transaction map self.__map like this. Better would be to catch the exception and handle it:

1. Attempt to unlock the dequeue rid in the enqueue map (as is currently the case)
2. If a NonExistentRecordError is thrown, then:
   a. Check for the dequeue rid in any enqueue records in the current xid being aborted. If found, ignore, as the entire xid is being deleted. This should happen for the TPL only.
   b. If not part of the current transaction being aborted, rethrow the exception.

- Kim van der Riet


On Jan. 31, 2014, 12:14 p.m., Pavel Moravec wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17587/
> -----------------------------------------------------------
> 
> (Updated Jan. 31, 2014, 12:14 p.m.)
> 
> 
> Review request for qpid and Kim van der Riet.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/QPID-5530
>     https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/QPID-5530
> 
> 
> Repository: qpid
> 
> 
> Description
> -------
> 
> Processing DTX enqueue, dequeue and abort, self.__emap is empty map everytime. Hence executing:
> 
>      def _abort(self, xid):
>          """Perform an abort operation for the given xid record"""
>          for fid, hdr, lock in self.__map[xid]:
>              if isinstance(hdr, jrnl.DeqRec):
>                  self.__emap.unlock(hdr.deq_rid)
>          del self.__map[xid]
> 
> raises error in unmap.
> 
> Though I did not understand full logic of emap / map in janal.py, the extra condition in _abort prevents raising the error and allows proper processing of the abort.
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/tools/src/py/qpidstore/janal.py 1562703 
> 
> Diff: https://reviews.apache.org/r/17587/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Pavel Moravec
> 
>