----------------------------------------------------------- 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 > >
