On 14/02/2012 20:47, Costin Manolache wrote: > 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.
That was more a reference to the generic HTTP upgrade that I thought SPDY should be using. >> 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 :-), I don't care about the name either. Renaming them does not remove the concern. > but it is important ( for my and > probably other > use cases ) for the LightProcessor to be associated with a socket. Does it need the socket or are input and output streams sufficient? > The current Protocol/Processor are very tied to HTTP and > one-request-at-a-time. Yes and no. The AbstractProtocol has only a minimal understanding of HTTP and AJP and that is a trade-off of not quite ideal design for significantly greater shared code and therefore much less duplication. AbstractProtocol uses only the Processor interface. The concrete implementations are obviously tied to specific protocols and endpoints. A lot of work has gone into trying to separate the generic and protocol / endpoint specific code. It isn't perfect but trunk is a lot cleaner than 7.0.x in this respect and I'd like to see it stay that way. >> 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. Ah. I need to better understand SPDY then. My instinct is still that the implementation should be more like the upgrade implementation than your current proposal but I obviously need to read up on SPDY to see how realistic that is. > Also: the current upgrade is quite heavy, it holds Requet/Response and a > bunch of > buffers. No it doesn't. Take another look at the code. The original implementation did but as I stated at the time my intention was to do some further refactoring to fix that. That refactoring has been complete (and is what resulted in the API changes I was referring to previously). > I want SPDY to scale to very large number of connections (100k+), > so minimal memory is a requirement. No issue there. > Note that there is a 'websocket over SPDY' proposal, so we may need to > merge some things. Having skimmed the latest SPDY draft spec at this point, it looks more like a protocol switch would be required rather than running WebSocket over SPDY. The protocols look to have a lot in common and I'm not sure I see the point of running WebSocket over SPDY. > 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. Already done. See above. >> 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. Isn't the use case for NIO and SPDY the same as for HTTP? Lots of clients using SPDY would look a lot like lots of HTTP clients using a very long keep-alive wouldn't it? > 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. I assume this restriction only applies if you wish to use SPDY over TLS. > 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. That assumes that there is only a single TCP connection between HTTPD and Tomcat. I'm not sure how valid that would be given that HTTPD is typically multiple processes and that most production configurations I see have multiple HTTPDs and multiple Tomcats that all talk to each other. Whether there would be enough connections to justify NIO I don't know. >> 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 :-) It isn't completely clear to me - even after looking through your patch - how a SPDY connection is initiated. Could you provide pointers to the starting points for non-TLS and TLS connections? The more I read, the more I expect SPDY to look more like org.apache.coyote.spdy and make use of the AbstractProtocol in a similar manner to HTTP and AJP - even if it is just to initiate the connection before passing it off to a lighter-weight processor. >> 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. I understand this much better now. Thanks for the pointers. > However the 'light protocol' stuff may be a better interface for websocket > connections, in particular if you want extreme scalability. I think there is some likely commonality between WebSocket and SPDY around what the 'light protocol' and what currently exists as the 'UpgradeProcessor'. >> I also have some more cosmetic concerns but those are easily resolved. > > Let me know :-) Lets get the important stuff sorted out, then we can start the pointless flame war over the trivia ;) Cheers, Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
