----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51638/#review148511 -----------------------------------------------------------
Fix it, then Ship it! I think this is good although we need a bit more control over retries: a time maxRetryInterval as well as/instead of maxRetryAttempts. The client has no way to predict how long retries will take so may have no reasonable way to set the count, but they may have reasonable expectations for max back-end recovery times. There is an issue of load on the router: this client will re-send as fast as the router can release. The router is not the problem, it is the back-end service *behind* the router that has failed. So hammering the router as fast as possible while the back-end recovers is not productive and a potential retry-storm at the router. However, the router should protect *itself* from naive clients and *not* assume the clients are well behaved. The router should manage exponentail retries and/or deliberately hold releases till back-ends come back up to make sure the client gets the right re-try semantics with the minimal useless traffic. Again this makes it important to code the client in terms of time delays not retry attempts because the router may deliberately hold of on releasing based on its knowledge of back-end conditions. src/qpid/messaging/amqp/SenderContext.cpp (line 50) <https://reviews.apache.org/r/51638/#comment215950> Does that mean: maxDeliveryAttempts==0 is infinite retries, 1 is try once (default), and N is one normal send and N-1 retries? - Alan Conway On Sept. 7, 2016, 11:01 a.m., Gordon Sim wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51638/ > ----------------------------------------------------------- > > (Updated Sept. 7, 2016, 11:01 a.m.) > > > Review request for qpid and Alan Conway. > > > Repository: qpid-cpp > > > Description > ------- > > Introduces two new connection options (AMQP 1.0 only): > > * max_delivery_count determines how many times we try to resend a 'released' > message. A value of 0, which is the default, retries indefinitely. > * raise_rejected determines whether an MessageRejected exception is raised > when a message is 'rejected', the default is true > > A message is considered 'released' if the outcome is relased, or if the > outcome as modified and the 'undeliverable-here' flag is not set. A message > is considered 'rejected' if the outcome is rejected, if the outcome is > modified *and* the 'undeliverable-here' flag is set, or if it was 'released' > but we have reached the maximum nuber of delivery attempts. > > Original behaviour can be obtained by setting max_delivery_count=1 and > raise_rejected=false. > > > Diffs > ----- > > src/qpid/messaging/ConnectionOptions.h c8c8798 > src/qpid/messaging/ConnectionOptions.cpp d956e9a > src/qpid/messaging/amqp/ConnectionContext.cpp 25dd68d > src/qpid/messaging/amqp/EncodedMessage.cpp cf60046 > src/qpid/messaging/amqp/SenderContext.h 467a8e0 > src/qpid/messaging/amqp/SenderContext.cpp fe8b4d3 > src/qpid/messaging/amqp/SessionContext.h 67b3c1e > src/qpid/messaging/amqp/SessionContext.cpp 92bdea7 > src/qpid/messaging/amqp/Transaction.cpp 754b00d > > Diff: https://reviews.apache.org/r/51638/diff/ > > > Testing > ------- > > > Thanks, > > Gordon Sim > >
