qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gordon Sim" <g...@redhat.com>
Subject Re: Review Request 17592: QPID-5531 [C++ broker] Set timeout for every DTX transaction
Date Mon, 03 Feb 2014 09:15:16 GMT

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

Ship it!



/trunk/qpid/cpp/src/qpid/broker/DtxManager.h
<https://reviews.apache.org/r/17592/#comment62863>

    If you gave this a default value of 0 and added a one line check for that (meaning no
timeout) that would (a) mean you didn't need to change the tests and (b) would allow users
to turn of the default timeout completely if desired (not that I can think of any good reason
for doing so).
    
    However this is perfectly acceptable as is also.


- Gordon Sim


On Feb. 1, 2014, 2:47 p.m., Pavel Moravec wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17592/
> -----------------------------------------------------------
> 
> (Updated Feb. 1, 2014, 2:47 p.m.)
> 
> 
> Review request for qpid, Chug Rolke, Cliff Jansen, Kim van der Riet, and Steve Huston.
> 
> 
> Bugs: QPID-5531
>     https://issues.apache.org/jira/browse/QPID-5531
> 
> 
> Repository: qpid
> 
> 
> Description
> -------
> 
> If a rogue external Transaction Manager forgets to commit/rollback a prepared DTX, Tpl
store keeps an orphaned enqueue record. To prevent it, every DTX transaction should have a
default timeout after that the broker automatically aborts the transaction.
> 
> QPID-5531 adds broker option dtx-default-timeout for that.
> 
> My concerns for review:
> - is 3600 seconds as default value proper? Isn't it too high?
> - ms-sql and/or ms-clfs store part of the patch (recoverTransaction method) has not been
even compiled
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/qpid/broker/Broker.h 1563386 
>   /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1563386 
>   /trunk/qpid/cpp/src/qpid/broker/DtxManager.h 1563386 
>   /trunk/qpid/cpp/src/qpid/broker/DtxManager.cpp 1563386 
>   /trunk/qpid/cpp/src/tests/legacystore/OrderingTest.cpp 1563386 
>   /trunk/qpid/cpp/src/tests/legacystore/SimpleTest.cpp 1563386 
>   /trunk/qpid/cpp/src/tests/legacystore/TransactionalTest.cpp 1563386 
>   /trunk/qpid/cpp/src/tests/legacystore/TwoPhaseCommitTest.cpp 1563386 
> 
> Diff: https://reviews.apache.org/r/17592/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Pavel Moravec
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message