> On Aug. 21, 2014, 11:13 a.m., Robbie Gemmell wrote: > > Looking at the client code (I'll leave the code for people familiar with > > it), I can see where it (o.a.q.transport.Session#complete) would be aware > > of the gap and make the sync() method hold awaiting it being filled, and I > > think what you describe sounds like it should work in that as the > > completion for the transfer is issued before the timeout period elapses > > then the client should be happy and return from the commit() call. > > > > That said, I think the 0-10 spec does says that what the client is doing is > > ok and what the broker is [going to be] doing [still] isn't: for tx.commit > > it has the statement that "the commit will not be completed until all > > preceding commands which it affects have been completed." The tx.rollback > > description adds a few more words in its related statement, saying > > "notification of completion for all such commands will be issued before the > > issuance of the completion for the rollback". > > Robbie Gemmell wrote: > * I'll leave the *broker* code for people familiar with it, obviously ;)
Agreed, I can modify this fix slightly to get the broker to do the right thing - complete the commit only when the entire TX is complete. Finally it makes sense! - Alan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23096/#review51165 ----------------------------------------------------------- On Aug. 20, 2014, 9:35 p.m., Alan Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23096/ > ----------------------------------------------------------- > > (Updated Aug. 20, 2014, 9:35 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. With HA enabled, both transfer and > commit > are asynchronously completed and it is possible for the commit to > complete *before* the preceeding transfer. In that case the C++ broker sends > completions immediately for the commit (because it has the sync flag), but > there > is a hole for the incomplete transfer. The transfer does *not* have the sync > flag so when it completes the broker does not immediately send a completion > and > we hang. > > Its not completely clear which of the broker or the client is strictly correct > in this case, but it seems prudent to fix it on the broker. The fix: when a > command with the sync flag completes, if there are any "holes" (earlier > commands > not yet complete) then the broker records the current command ID as if it were > an execution.sync. Thus when the holes are filled the broker will send the > completions immediately. > > > Diffs > ----- > > trunk/qpid/cpp/src/qpid/broker/SessionState.h 1619236 > trunk/qpid/cpp/src/qpid/broker/SessionState.cpp 1619236 > > Diff: https://reviews.apache.org/r/23096/diff/ > > > Testing > ------- > > Passes reproducer and ctest > > > Thanks, > > Alan Conway > >
