On 6 March 2017 at 12:23, Robbie Gemmell <[email protected]> wrote:
> On 3 March 2017 at 19:31, Alan Conway <[email protected]> wrote: > > On Wed, 2017-03-01 at 15:45 +0000, Robbie Gemmell wrote: > >> > That is a correct understanding of the AMQP 1.0 spec (section 2.6.7 > >> > specifically deals with this case of credit being revoked: "If the > >> > link-credit is less than or equal to zero, i.e., the delivery-count > >> > is the > >> > same as or greater than the delivery-limit, a sender MUST NOT send > >> > more > >> > messages. If the link-credit is reduced by the receiver when > >> > transfers are > >> > in-flight, the receiver MAY either handle the excess messages > >> > normally or > >> > detach the link with a transfer-limit-exceeded error code.") > >> > > >> > As Gordon suggested separately, the logical thing to do through the > >> > Proton > >> > API would be to try to flow negative credits... > > > > I would *strongly* suggest that if you want to stop a sender with too > > much credit you should close the link. The inconvenience of re- > > establishing the link will be nothing to the hell you will enter if you > > try to implement negative credit. It is theoretically possible and > > allowed by the spec, but the chances that any application you are > > connected to will implement it properly are slim to none. > > > > This mail weirdly turned up a days late :) > Indeed, I'd assumed it had got stuck in moderation and you'd pushed it through... obviously not :-) > > > The concept of credit is clear and simple when you see it as a promise > > between sender and receiver that must not be broken *in either > > direction*. Revoking credit is breaking half the promise. The more > > elaborate AMQP view of "credit" where it can be negative (debit?) is > > mathematically sound but pragmatically a bad idea. > > > >> > however I'm not sure that > >> > Proton-J is going to handle this correctly, > > > > See? See? I am fairly certain that almost nobody will handle this > > correctly. Proton-C provides the tools, but you have to get the book- > > keeping right, and have faith that every application you connect to > > will as well. That is a bad bet. > > > > I'm still not particularly confident that proton-c can handle this > fully either. From some testing Rob did, proton-j definitely doesnt > and as I suspected can do bad things like wrapping credit values and > end up sending massive uint values thinking they are negative (you > cant actually represent negative credit on the wire since its > unsigned). > So there are, I think two distinct things here: 1) Proton[-J] should absolutely be able to handle the peer it is communicating with following the AMQP spec and reducing credit. Proton[-j] should also handle the case where the amount of link credit granted on a link is greater than the largest values able to be stored in a signed int. Currently in Proton-J it looks to me like a) Receivers do absolutely no checking to see if the sender is sending beyond the amount of credit the receiver has allocated, and that any such checking is left to the application layer. Is this a deliberate API choice b) Very large credit values granted to senders will actually be interpreted as negative values and the sender will not send. This is clearly a bug. 2) Proton[-J] has no API that explicitly allows the reducing of granted credit. The documentation for (Proton-J) Receiver.flow() talks about "adding credit", and does not explicitly rule out the idea that the number to add may be negative... but it doesn't state it as a valid use case either. For point 2) there are essentially two API choices - either explicitly allowing -ve adds (and pushing the responsibility to the application layer to ensure that credit is granted retrospectively for messages sent after reduction has been initiation, but before processing at the sender); or we add a "stop()" method to the receiver. In the case of the latter the notion of "adding" credit is hard to define until the sender and receiver have a common view of the shared credit state. (Unless we take the view that stop() is simply an alias for flow(- current outstanding credit), and that, again, the application needs to take responsibility for retrospectively flowing credit for messages that were sent between the receiver reducing credit and the sender processing this change of credit state. -- Rob > That said, while in many cases it will behave no differently than a > drain, I do think we should implement support for this eventually, > thats why I raised the JIRA. Peers that dont handle it are one thing, > proton not being able to do it to begin with is another. > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > >
