----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23096/#review51270 -----------------------------------------------------------
Ship it! Looks good to me; nice work! - Gordon Sim On Aug. 21, 2014, 10:56 p.m., Alan Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23096/ > ----------------------------------------------------------- > > (Updated Aug. 21, 2014, 10:56 p.m.) > > > Review request for qpid, Gordon Sim, rajith attapattu, and Robbie Gemmell. > > > Bugs: QPID-5855 > https://issues.apache.org/jira/browse/QPID-5855 > > > Repository: qpid > > > Description > ------- > > The problem: the java client sets the sync flag on tx.commit and then waits > for > completion of the entire transaction. According to the 0-10 spec, this is > correct, the commit (or rollback) will not complete until all of the > transactional commands have completed. However the C++ broker was sometimes > completing a commit *before* one of the the corresponding enqueues. It issued > the completions up to the commit (because the commit is makred sync) but there > is a "hole" for the incomplete enqueue. The enqueue is not marked sync so when > this hole is filled no completion is sent and the client hangs. > > Fix: make tx.commit a "sync point", that is it behaves like execution.sync and > is not completed till all preceeding commands are complete. Note tx.rollback > does not need modification as it is never completed asynchronously. > > > Diffs > ----- > > trunk/qpid/cpp/src/qpid/broker/AsyncCommandCallback.h 1619584 > trunk/qpid/cpp/src/qpid/broker/AsyncCommandCallback.cpp 1619584 > trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1619584 > trunk/qpid/cpp/src/qpid/broker/SessionContext.h 1619584 > trunk/qpid/cpp/src/qpid/broker/SessionState.h 1619584 > trunk/qpid/cpp/src/qpid/broker/SessionState.cpp 1619584 > trunk/qpid/cpp/src/tests/AsyncCompletion.cpp 1619584 > > Diff: https://reviews.apache.org/r/23096/diff/ > > > Testing > ------- > > Passes reproducer and ctest > > > Thanks, > > Alan Conway > >
