On Fri, 2008-07-11 at 00:25 -0400, Sam Berlin wrote: > For being able to implement something like keep-alive timeouts without > needing an API change, these are my thoughts.. > > ManagedClientConnection is OK -- the open(...) method takes an > HttpContext, and that context is pretty safe to reuse over the > lifetime of that connection.
Sam, Do you intent to create a separate Httpcontext for each connection or do you intent to pass the current execution context? I am fine with both approaches as long as the current execution context does not get cached in the connection, as execution contexts tend to have a very short life span, usually that of a single request execution. > You could pretty easily define a > contract that the user of the connection puts a value in the context, > ManagedClientConnection could keep a handle on it from the open method > and then look for the value whenever needed (like releaseConnection). > > ClientConnectionManager is a little more tricky. You could capture > the HttpContext the same way as ManagedClientConnection could do it > internally, except also expose it -- since ClientConnectionManager > determines the ManagedClientConnection implementation, it can make > sure it exposes the HttpContext however it needs it. (One example > being setting parameters before calling connectionPool.freeEntry from > ClientConnectionManager.releaseConnection.) > > AbstractConnPool doesn't really have any arbitrary map-like construct > in its PoolEntries. It's OK though, because it's an abstract class, > and methods can be added when needed (and method signatures overloaded > when needed). > > The whole thing is similiar to the changes required for adding state > to the connections. The data could be pulled directly from the > HttpContext if wanted, so adding a get/set state to > ManagedClientConnection is a useful addition, but not a strictly > required one. Then to pass the information through to the PoolEntry > so that the ClientConnectionManager could use it, there absolutely > needed to be a get/set state on AbstractPoolEntry -- no other way > around it. > > So, in summary... It definitely does seem like state (for auth tokens) > and keep-alive timeouts could have been added without API changes, so > everything looks to me. Still, I think it'd be useful to maybe change > get/setState in AbstractPoolEntry to > get/setAttribute(TSCCMContext.USER_TOKEN, ..). This works perfectly > (with alloc entry nulling the whole context before closing it, and the > check for the state being same [to reuse the connection] looks just as > the user token attribute). But then again, since AbstractConnPool is > abstract, that can be done whenever it's needed too. > > Sam > I think it would be easier if you put together a patch, maybe initially just a very simple one for public interfaces only Oleg > > On Thu, Jul 10, 2008 at 5:57 PM, Sam Berlin <[EMAIL PROTECTED]> wrote: > > That's the question, really. I'll dig into it a bit deeper tonight > > and try to come up with some more formalized thoughts -- I just > > remember having this nagging feeling while doing the keep-alive that > > it was only possible to add because the API wasn't frozen. Ideally, > > it would have been possible (if a little less elegant) even if the API > > was frozen. > > > > Sam > > > > On Thu, Jul 10, 2008 at 5:52 PM, Oleg Kalnichevski <[EMAIL PROTECTED]> > > wrote: > >> On Thu, 2008-07-10 at 17:32 -0400, Sam Berlin wrote: > >> > >> ... > >> > >>> While adding keep-alive support, I noticed that code-changes that need > >>> to provide the ClientConnectionManager with more information aren't > >>> possible without API changes. In the rest of HttpClient's code, > >>> there's usually an HttpContext being passed around that can hold > >>> key/value pairs so things can easily be plugged in without requiring > >>> full-blown API changes. I can't see any way to do this with > >>> ClientConnectionManager currently. Maybe there should be some Context > >>> that that uses? Or share the HttpContext itself? This will likely > >>> become an issue after an API freeze if there's more data we find > >>> ourselves suddenly wanting. > >>> > >>> Sam > >> > >> What methods of ClientConnectionManager should take a context as an > >> additional parameter? > >> > >> Oleg > >> > >> > >> --------------------------------------------------------------------- > >> To unsubscribe, e-mail: [EMAIL PROTECTED] > >> For additional commands, e-mail: [EMAIL PROTECTED] > >> > >> > > --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
