You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geronimo.apache.org by Rick McGuire <ri...@gmail.com> on 2008/01/14 20:00:09 UTC
Is there a problem with AsyncHttpClient connection reuse?
I haven't convinced myself that this is a problem yet, but it's worth
asking the question. The AsyncHttpClient has support for connection
reuse when making repeated connections to sites. Since these
connnections persist between instantiations of AsyncHttpClient
instances, there's a single static cache of the reusable connections
that is maintained. Selection from the session cache is made using the
host/port combination only, which means it's possible that a connection
that is reused by a client will get processed using the thread pool
configuration of the original connection client and not the current
requesting client. I suspect this is not an issue that can be that can
be easily detected or protected against, so the "fix" might just be a
word of caution added to the documentation on enabling connection reuse.
rick
Re: Is there a problem with AsyncHttpClient connection reuse?
Posted by Sangjin Lee <sj...@gmail.com>.
It looks good to me. A couple of minor things.
- AsyncHttpClient.getCachedConnection() takes a SessionCache as an argument.
I don't think this is necessary as the SessionCache is an instance variable
of AsyncHttpClient?
- We will need to reflect this API change to the Geronimo sample code that
we have...
Thanks,
Sangjin
On 1/15/08, Rick McGuire <ri...@gmail.com> wrote:
>
> I opened a Jira on this issue:
>
> https://issues.apache.org/jira/browse/GERONIMO-3749
>
> And attached a patch that I worked up to address this. This is
> implemented by plugging a SessionCache instance into the AsyncHttpClient
> instance. A couple notes about this:
>
> 1. I chose to use a set method instead of the a constructor argument
> because the number of permutations of the constructors is already
> a bit high. It might be better as a constructor argument, but I
> don't think we want to try to add a permutations for all of the
> possibilities.
> 2. The reuseConnection attribute became redundant when this was
> added. The cache is now used to trigger the reuse behavior.
>
> Please comment on the patch. I'll not commit anything until it appears
> we have consensus.
>
> Rick
>
> Sangjin Lee wrote:
> > Yes, if you used a different configuration for the SSL, then it would
> > be another issue.
> >
> > The questions are:
> > - Do we want to even allow an option for using a shared connection
> cache?
> > The benefit of a shared connection cache is ... just that; connection
> > reuse will be maximized for the given process. However, the drawback
> > is that you may run into unexpected socket configuration/SSL
> > configuration behavior when you hand the connections to a different
> > client instance.
> > - If we do allow it, what should be the default?
> > I think *not* sharing the connection cache might be a better default
> > behavior.
> >
> > Thanks,
> > Sangjin
> >
> > On 1/14/08, *Jarek Gawor* <jgawor@gmail.com <ma...@gmail.com>>
> > wrote:
> >
> > On 1/14/08, Sangjin Lee <sjlee0@gmail.com
> > <ma...@gmail.com>> wrote:
> > > It's a good point... Currently the session cache is global, and
> all
> > > AsyncHttpClient instances are forced to share it. The main
> > thing that
> > > concerns me is that as a result the connections will retain all
> > the socket
> > > properties that came from the AsyncHttpClient instance that
> > opened the
> > > connection. This might not be intuitive to say the least. For
> > example, one
> > > AsyncHttpClient instance opens a connection with TCP delay
> > disabled, and
> > > then another instance (this time with TCP delay enabled) comes
> > along and
> > > picks up this connection for reuse. Contrary to what it
> > expects, it would
> > > get a connection with TCP delay disabled.
> >
> > Right and this might be a bigger issue for SSL connections where you
> > might need to distinguish between cached connections based on the
> > client certs, CA certs, or enabled cipher suites (but I'm not sure
> if
> > these options can be set on the AsyncHttpClient).
> >
> > Jarek
> >
> >
> >
>
>
Re: Is there a problem with AsyncHttpClient connection reuse?
Posted by Rick McGuire <ri...@gmail.com>.
I opened a Jira on this issue:
https://issues.apache.org/jira/browse/GERONIMO-3749
And attached a patch that I worked up to address this. This is
implemented by plugging a SessionCache instance into the AsyncHttpClient
instance. A couple notes about this:
1. I chose to use a set method instead of the a constructor argument
because the number of permutations of the constructors is already
a bit high. It might be better as a constructor argument, but I
don't think we want to try to add a permutations for all of the
possibilities.
2. The reuseConnection attribute became redundant when this was
added. The cache is now used to trigger the reuse behavior.
Please comment on the patch. I'll not commit anything until it appears
we have consensus.
Rick
Sangjin Lee wrote:
> Yes, if you used a different configuration for the SSL, then it would
> be another issue.
>
> The questions are:
> - Do we want to even allow an option for using a shared connection cache?
> The benefit of a shared connection cache is ... just that; connection
> reuse will be maximized for the given process. However, the drawback
> is that you may run into unexpected socket configuration/SSL
> configuration behavior when you hand the connections to a different
> client instance.
> - If we do allow it, what should be the default?
> I think *not* sharing the connection cache might be a better default
> behavior.
>
> Thanks,
> Sangjin
>
> On 1/14/08, *Jarek Gawor* <jgawor@gmail.com <ma...@gmail.com>>
> wrote:
>
> On 1/14/08, Sangjin Lee <sjlee0@gmail.com
> <ma...@gmail.com>> wrote:
> > It's a good point... Currently the session cache is global, and all
> > AsyncHttpClient instances are forced to share it. The main
> thing that
> > concerns me is that as a result the connections will retain all
> the socket
> > properties that came from the AsyncHttpClient instance that
> opened the
> > connection. This might not be intuitive to say the least. For
> example, one
> > AsyncHttpClient instance opens a connection with TCP delay
> disabled, and
> > then another instance (this time with TCP delay enabled) comes
> along and
> > picks up this connection for reuse. Contrary to what it
> expects, it would
> > get a connection with TCP delay disabled.
>
> Right and this might be a bigger issue for SSL connections where you
> might need to distinguish between cached connections based on the
> client certs, CA certs, or enabled cipher suites (but I'm not sure if
> these options can be set on the AsyncHttpClient).
>
> Jarek
>
>
>
Re: Is there a problem with AsyncHttpClient connection reuse?
Posted by Rick McGuire <ri...@gmail.com>.
Sangjin Lee wrote:
> Yes, if you used a different configuration for the SSL, then it would
> be another issue.
>
> The questions are:
> - Do we want to even allow an option for using a shared connection cache?
> The benefit of a shared connection cache is ... just that; connection
> reuse will be maximized for the given process. However, the drawback
> is that you may run into unexpected socket configuration/SSL
> configuration behavior when you hand the connections to a different
> client instance.
> - If we do allow it, what should be the default?
> I think *not* sharing the connection cache might be a better default
> behavior.
Perhaps we should handle this the same way as the thread pools.
Externalize the session cache and if you wish to share connections,
provide the appropriate session cache to the AsyncHttpClient instance
when it's instantiated. If no cache is provided, then connection reuse
is disabled for that client instance.
Rick
>
> Thanks,
> Sangjin
>
> On 1/14/08, *Jarek Gawor* <jgawor@gmail.com <ma...@gmail.com>>
> wrote:
>
> On 1/14/08, Sangjin Lee <sjlee0@gmail.com
> <ma...@gmail.com>> wrote:
> > It's a good point... Currently the session cache is global, and all
> > AsyncHttpClient instances are forced to share it. The main
> thing that
> > concerns me is that as a result the connections will retain all
> the socket
> > properties that came from the AsyncHttpClient instance that
> opened the
> > connection. This might not be intuitive to say the least. For
> example, one
> > AsyncHttpClient instance opens a connection with TCP delay
> disabled, and
> > then another instance (this time with TCP delay enabled) comes
> along and
> > picks up this connection for reuse. Contrary to what it
> expects, it would
> > get a connection with TCP delay disabled.
>
> Right and this might be a bigger issue for SSL connections where you
> might need to distinguish between cached connections based on the
> client certs, CA certs, or enabled cipher suites (but I'm not sure if
> these options can be set on the AsyncHttpClient).
>
> Jarek
>
>
>
Re: Is there a problem with AsyncHttpClient connection reuse?
Posted by Sangjin Lee <sj...@gmail.com>.
Yes, if you used a different configuration for the SSL, then it would be
another issue.
The questions are:
- Do we want to even allow an option for using a shared connection cache?
The benefit of a shared connection cache is ... just that; connection reuse
will be maximized for the given process. However, the drawback is that you
may run into unexpected socket configuration/SSL configuration behavior when
you hand the connections to a different client instance.
- If we do allow it, what should be the default?
I think *not* sharing the connection cache might be a better default
behavior.
Thanks,
Sangjin
On 1/14/08, Jarek Gawor <jg...@gmail.com> wrote:
>
> On 1/14/08, Sangjin Lee <sj...@gmail.com> wrote:
> > It's a good point... Currently the session cache is global, and all
> > AsyncHttpClient instances are forced to share it. The main thing that
> > concerns me is that as a result the connections will retain all the
> socket
> > properties that came from the AsyncHttpClient instance that opened the
> > connection. This might not be intuitive to say the least. For example,
> one
> > AsyncHttpClient instance opens a connection with TCP delay disabled, and
> > then another instance (this time with TCP delay enabled) comes along and
> > picks up this connection for reuse. Contrary to what it expects, it
> would
> > get a connection with TCP delay disabled.
>
> Right and this might be a bigger issue for SSL connections where you
> might need to distinguish between cached connections based on the
> client certs, CA certs, or enabled cipher suites (but I'm not sure if
> these options can be set on the AsyncHttpClient).
>
> Jarek
>
Re: Is there a problem with AsyncHttpClient connection reuse?
Posted by Jarek Gawor <jg...@gmail.com>.
On 1/14/08, Sangjin Lee <sj...@gmail.com> wrote:
> It's a good point... Currently the session cache is global, and all
> AsyncHttpClient instances are forced to share it. The main thing that
> concerns me is that as a result the connections will retain all the socket
> properties that came from the AsyncHttpClient instance that opened the
> connection. This might not be intuitive to say the least. For example, one
> AsyncHttpClient instance opens a connection with TCP delay disabled, and
> then another instance (this time with TCP delay enabled) comes along and
> picks up this connection for reuse. Contrary to what it expects, it would
> get a connection with TCP delay disabled.
Right and this might be a bigger issue for SSL connections where you
might need to distinguish between cached connections based on the
client certs, CA certs, or enabled cipher suites (but I'm not sure if
these options can be set on the AsyncHttpClient).
Jarek
Re: Is there a problem with AsyncHttpClient connection reuse?
Posted by Sangjin Lee <sj...@gmail.com>.
It's a good point... Currently the session cache is global, and all
AsyncHttpClient instances are forced to share it. The main thing that
concerns me is that as a result the connections will retain all the socket
properties that came from the AsyncHttpClient instance that opened the
connection. This might not be intuitive to say the least. For example, one
AsyncHttpClient instance opens a connection with TCP delay disabled, and
then another instance (this time with TCP delay enabled) comes along and
picks up this connection for reuse. Contrary to what it expects, it would
get a connection with TCP delay disabled.
I see a couple of options:
- Relax the current SessionCache implementation so it's no longer a
singleton.
- Make each AsyncHttpClient create its own SessionCache or make it an option
of using the default session cache or its own
I am thinking we want to give it an option of using a default session cache
or its own. What do you think?
Thanks,
Sangjin
On 1/14/08, Rick McGuire <ri...@gmail.com> wrote:
>
> I haven't convinced myself that this is a problem yet, but it's worth
> asking the question. The AsyncHttpClient has support for connection
> reuse when making repeated connections to sites. Since these
> connnections persist between instantiations of AsyncHttpClient
> instances, there's a single static cache of the reusable connections
> that is maintained. Selection from the session cache is made using the
> host/port combination only, which means it's possible that a connection
> that is reused by a client will get processed using the thread pool
> configuration of the original connection client and not the current
> requesting client. I suspect this is not an issue that can be that can
> be easily detected or protected against, so the "fix" might just be a
> word of caution added to the documentation on enabling connection reuse.
>
> rick
>