----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51638/#review147837 -----------------------------------------------------------
I don't think this is the right fix. Aborting the connection is a nasty hack for backwards compatibility with *old clients* that don't have any features for re-sending released messages. It doesn't make sense to put this feature in the client, that doesn't help with *old* clients. The abort hack is a job for the router. Ideally the router would check connection properties for "I-can-resend-released-messages", and if it doesn't find that, abort instead of sending a release. We also probably want config to enable/disable the feature per listner/connector. That means unmodified old clients (and clients from other vendors) can work with dispatch. I'm not sure that we even want to touch messaging clients with the above router fix. The desirable behavior for new clients is to *not* tear down the connection or link on a released message, but just to re-send the released messages. We'll need a new set of configuration controls for how this works and we'll need to track re-sending per-link, not per-connection or session. I think we should allow the following configured possibilities: 1. same behavior as now 2. inform the user of the release, similar to what you propose for reject. 3. auto re-send on the same link according to configured intervals, retry limits etc. 1+2 are essential, we can take more time to design 3 since 2 allows the user to implement their own strategy till we have a good general-purpose automated strategy. Message retry config will likely be similar to connection retry but I expect we'll find some important differences when we start to design it. src/qpid/messaging/amqp/SenderContext.h (line 44) <https://reviews.apache.org/r/51638/#comment215122> If we do adopt the exception strategy for communicating relase/reject then I think we need a clearly marked exception hierarchy so the user can tell which exceptions destroy the link/session/connection and which don't. - Alan Conway On Sept. 5, 2016, 7:58 p.m., Gordon Sim wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51638/ > ----------------------------------------------------------- > > (Updated Sept. 5, 2016, 7:58 p.m.) > > > Review request for qpid and Alan Conway. > > > Repository: qpid-cpp > > > Description > ------- > > Rejected messages cause an exception to be thrown. However this does not > invalidate the session in anyway. More messages can be sent after > catching the exception. The original behaviour - i.e. simply ignoring > the rejected messages - can be obtained by setting the connection option > 'ignore_delivery_refused' to true. > > Released messages cause the transport to be aborted, triggering the usual > resending logic (whether defined by the application or using that defined > in the library itself). Again, released messages can be simply ignored as > they were prior to this change by setting 'ignore_undelivered' to true. > > For modified messages, if undeliverable-here is set to true, the message > is > treated as if it had been rejected, otheriwse if delivery-failed is set to > true it is treated as a released message. If neither is set it is simply > ignored with a warning (i.e. treated as accepted). > > > Diffs > ----- > > src/qpid/messaging/ConnectionOptions.h c8c8798 > src/qpid/messaging/ConnectionOptions.cpp d956e9a > src/qpid/messaging/amqp/ConnectionContext.cpp 25dd68d > 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 > >
