On Tue, Feb 14, 2012 at 9:36 AM, Mark Thomas <ma...@apache.org> wrote:
> On 14/02/2012 08:29, Costin Manolache wrote: > > Ok, took a bit to get the Apr polling to work and add some minimal tests. > > > > Please take another look - in particular to > > > https://github.com/costinm/tomcat/blob/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java > > > > The spdy implementation seems to work with chrome, and the client seems > to > > work with google. > > Probably still has plenty of bugs - but it's a start. > > > > If no objections - I'll start merging the LightProtocol/util.net changes > > first, than add the spdy server implementation. The spdy client and > tests - > > probably later, I want to clean them up a bit more. > > I am all for adding SPDY support to Tomcat asap including if practical > to the 7.0.x branch (noting that there may be some API changes that may > prevent this). > So far I don't think I modified any API in incompatible ways. > > I think the patch needs more discussion before it is committed to trunk. > There are several areas where I am uncomfortable. My key areas of > concern are: > > 1. The patch places Protocol like code (the code that works with the > processors) in the endpoints. This significantly muddies the waters > between Endpoint, Protocol and Processor which will make future > maintenance more difficult. > I can rename LightProtocol to LightHandler (extends Endpoint.Handler ), and Processor to LightConnectionHandler and extend Endpoint.Handler. I don't care much about the name :-), but it is important ( for my and probably other use cases ) for the LightProcessor to be associated with a socket. The current Protocol/Processor are very tied to HTTP and one-request-at-a-time. > 2. Generic support for upgraded protocols is now available in trunk and > SPDY should use this rather than adding SPDY specific upgrade handling. > The generic upgrade process supports all three endpoints. The 'upgrade' process is for accepting a HTTP request and upgrading the protocol - like websocket-over-http does. SPDY is different - the protocol is negotiated in the TLS handshake, there is no HTTP request associated with it. Also: the current upgrade is quite heavy, it holds Requet/Response and a bunch of buffers. I want SPDY to scale to very large number of connections (100k+), so minimal memory is a requirement. Note that there is a 'websocket over SPDY' proposal, so we may need to merge some things. But even with normal websocket - I think there would be value in using the LightProcessor instead of the current upgrade, so you could scale to far more connections. > Unless I am > missing something the current SPDY implementation does not support NIO. > Yes, NIO doesn't make sense for SPDY - we can add the hooks, but I don't see a use case. SPDY ( as implemented by chrome/firefox or servers ) requires a special TLS extension, "next protocol negotiation", which is not implemented in JSSE/NIO. It is available as a patch to current openssl, and is part of the latest openssl. So you must use APR to use SPDY. The second use case for SPDY is as a proxy protocol, between apache ( there is a mod_spdy in progress ) or other NPN-speaking frontend and tomcat, similar with AJP. In this case JIO is more than adequate - SPDY is multiplexing, so a single JIO TCP connection can hold as many HTTP requests ( including comet, websocket, etc ). NIO would be overkill - there is no need for it's non-blocking stuff. > > 3. Having spent a considerable amount of effort cleaning up the > connector code, removing duplicate code, resolving inconsistencies and > making it easier to maintain, this patch feels like a step backwards. > Let me know how I can address your concerns :-) > > I am more than happy to see changes to the current generic HTTP upgrade > support if it is not currently able to support SPDY. > As I mentioned - there are completely different beasts. However the 'light protocol' stuff may be a better interface for websocket connections, in particular if you want extreme scalability. > > I also have some more cosmetic concerns but those are easily resolved. > Let me know :-) Costin > > Mark > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > >
