On 26 July 2010 19:04, Rafael Schloming <[email protected]> wrote: > Andrew Kennedy wrote: >> >> Ok, >> >> So, the result of what I changed is that the 0-10 session expiry is >> correctly propagated via sessionRequestTimeout and sessionTimeout >> messages, however you are saying that this will not work correctly? If >> I understand things correctly, when the client creates a session and >> negotiates an N > 0 second expiry (or timeout) with the broker, this >> successful negitiation will mislead a well-behaved client into >> thinking it can resume a session in a way that is currently not >> possible? >> >> I can certainly revert the change if it is likely to cause any issues; >> I think I was misled by there not being a handy test case that I could >> look at, and I'm not sure it would be possible to create one using the >> Java broker. As people have mentioned, more CI would certainly make >> this kind of thing easier - It took me longer than I would have >> expected just to get the Java client tests running against the C++ >> broker... > > I'm not suggesting that it was entirely correct the way it was before > either. At the very least the code was/is confusing. I'm just saying that > the client never does frame replay and so should never advertise a non-zero > session timeout. > > It does however still end up going into the detached state prior to doing a > semantic resume. This is purely a client side state though, and does not > imply that the session still exists on the server. > > Really expiry as it is currently used is just a boolean that indicates > whether semantic resume is enabled. I think a proper fix might be to rename > expiry to resume and change the type to boolean. With that done you could > (re)introduce a timeout/expiry field that corresponds to the protocol value > if needed for your original fix. I'm not sure exactly why this would be > necessary though given that it should never have a non-zero value, but I > don't know the details of the bug you're trying to fix. > > --Rafael
The 'bug' is as I described in an earlier email, the desire to make it so that the client would respond to a broker request to close to the session (e.g. as invoked via management) by actually transitioning to the closed state instead of the detached state (failure to do so leads to further attempts to use the session spending 60s doing nothing before simply throwing an exception reporting a timeout waiting for the session to become open, rather than why it was 'closed'). That previously didnt happen because the client advertised a timeout of 0 to the broker but set the expiry value to be 1 locally (the value of which is then used to determine what state to transition into when the detach is recieved) and then ignored the sessionRequestTimout value of 0 sent by the broker before the close detach. Previously it was attempted to just set the expiry to 0 on the client but the issue seemed to be that failover depended on the client going into the detached state and so that caused the failover tests to fail. In actually implementing the sessionRequestTimeout() handling I believe Andrew was aiming to have the client respond to the brokers session close request by first setting the expiry to 0 and then detaching, thus transitioning the client Session to the closed state. However, as you mention the changes also had the effect of negotiating the value with the broker to be 1, which it didnt previously. Robbie --------------------------------------------------------------------- Apache Qpid - AMQP Messaging Implementation Project: http://qpid.apache.org Use/Interact: mailto:[email protected]
