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... Cheers, Andrew. -- -- andrew d kennedy ? edinburgh : +44 7941 197 134 On 26 July 2010 16:03, Rafael Schloming <[email protected]> wrote: > Andrew Kennedy wrote: >> >> Hi. >> >> I have been looking at the 0-10 session close semantics, and been >> meaning to ask this for a while... >> >> There is no explicit close message in 0-10, rather the session timeout >> is supposed to be set to 0 seconds and then a session detach message >> is sent. I have implemented this, since it requires simply creating a >> handler for sessionRequestTimeout messages that actually sets expiry >> to 0, as discussed on QPID-2586 and in this message on the dev list: >> >> http://apache-qpid-developers.2158895.n2.nabble.com/Java-0-10-Session-closure-expiry-timeout-td4865744.html#a4865744 >> >> Also, see comments in the code (in o.a.q.t.Session.java): >> >> // XXX: when the broker and client support full session >> // recovery we should use expiry as the requested timeout >> >> // XXX: we manually set the expiry to zero here to >> // simulate full session recovery in brokers that don't >> // support it, we should remove this line when there is >> // broker support for full session resume: >> expiry = 0; >> >> which is exactly what I have done, I use the expiry field (therefore >> on session create the timeout is set to 1 on both sides now) and (in >> o.a.q.t.SessionDelegate.java) we have: >> >> // XXX: we ignore this right now, we should uncomment this >> // when full session resume is supported: >> >> where I have uncommented the line following, which just calls >> ssn.setExpiry(t.getTimeout()) and done the same in the new handler >> for sessionRequestTimeout. >> >> I believe that this is a fairly simple and non-contentious change. One >> thing that I noticed from other dev list messages is that this could >> affect the failover mechanism used by the C++ clients. I ran the cpp >> test suite with my changes and noticed no difference, but then I >> couldn't see what tests would actually be affected. Can anyone shged >> more light on the problems I might expect or be missing? > > Sorry for not commenting on this sooner, but until being prompted by a few > conversations, I only had a very vague recollection of this area. > > I think this change is going to be problematic for the C++ broker (and > possibly for the Java broker too). > > The reason the 0-10 session only ever advertises zero as the timeout is > because it does not do frame level replay. It does a semantic resume. If it > advertises a non-zero timeout then it will end up doing a semantic resume > against a session that is expecting frame level replay, and that won't be > happy. > > --Rafael > > --------------------------------------------------------------------- > Apache Qpid - AMQP Messaging Implementation > Project: http://qpid.apache.org > Use/Interact: mailto:[email protected] > > --------------------------------------------------------------------- Apache Qpid - AMQP Messaging Implementation Project: http://qpid.apache.org Use/Interact: mailto:[email protected]
