You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Rainer Jung <ra...@kippdata.de> on 2010/07/19 00:50:48 UTC
Re: svn commit: r965150 - in /tomcat/trunk: java/org/apache/catalina/connector/
java/org/apache/tomcat/util/net/ webapps/docs/ webapps/docs/config/
On 18.07.2010 02:02, Mark Thomas wrote:
> On 18/07/2010 00:57, markt@apache.org wrote:
>> Author: markt
>> Date: Sat Jul 17 23:57:23 2010
>> New Revision: 965150
>>
>> URL: http://svn.apache.org/viewvc?rev=965150&view=rev
>> Log:
>> Restore pero's timeout fix for the BIO connector. Add configuration of the timeout.
>
> Servlet TCK (with BIO) and unit tests pass as of this commit.
>
> It is looking like timeouts are going to be required to fix
> https://issues.apache.org/bugzilla/show_bug.cgi?id=49567. The complete
> fix is going to require some refactoring and I'm not quite there. It
> does look like most of the fix for
> https://issues.apache.org/bugzilla/show_bug.cgi?id=49528 is going to end
> up being reverted.
>
> Also, need to check the NIO and APR timeout async requests.
I'm not totally sure, but after JFC's heads up that the TCK fails I had
a short look at the async timeout runnable. My impression was, that the
default value for the timeout is "-1" and this would let the timeout
condition in the runnable always trigger.
Regards,
Rainer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r965150 - in /tomcat/trunk:
java/org/apache/catalina/connector/ java/org/apache/tomcat/util/net/
webapps/docs/ webapps/docs/config/
Posted by Remy Maucherat <re...@apache.org>.
On Wed, 2010-07-21 at 21:36 +0100, Mark Thomas wrote:
> I take it that this means you think there is room for simplification.
> I'm still trying to get my head around the Async implementation so if
> you have any pointers on how it could be simplified, they would be much
> appreciated.
Ok, so for this particular topic, the argument I gave to the expert
group to avoid having to specify a timeout every time was that the
connector timeout was a decent default. This did convince enough people.
I don't see how putting another specific constant is a good idea here.
Rémy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r965150 - in /tomcat/trunk: java/org/apache/catalina/connector/
java/org/apache/tomcat/util/net/ webapps/docs/ webapps/docs/config/
Posted by Mark Thomas <ma...@apache.org>.
On 21/07/2010 10:47, Remy Maucherat wrote:
> On Wed, 2010-07-21 at 11:18 +0200, Rainer Jung wrote:
>> IMHO for any serious application of async the developer will need to
>> adjust the timeout.
>
> I am responsible for the fact that setting the timeout is separate, as I
> did not understand how it was meaningful in most cases to have it be
> anything other than the usual timeout set by the connector.
>
> I am impressed with the creativity that has been displayed so far on the
> async API implementation in Tomcat 7, so I guess that arbitrary 10s
> value goes along fine with that.
I take it that this means you think there is room for simplification.
I'm still trying to get my head around the Async implementation so if
you have any pointers on how it could be simplified, they would be much
appreciated.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r965150 - in /tomcat/trunk:
java/org/apache/catalina/connector/ java/org/apache/tomcat/util/net/
webapps/docs/ webapps/docs/config/
Posted by Remy Maucherat <re...@apache.org>.
On Wed, 2010-07-21 at 11:18 +0200, Rainer Jung wrote:
> IMHO for any serious application of async the developer will need to
> adjust the timeout.
I am responsible for the fact that setting the timeout is separate, as I
did not understand how it was meaningful in most cases to have it be
anything other than the usual timeout set by the connector.
I am impressed with the creativity that has been displayed so far on the
async API implementation in Tomcat 7, so I guess that arbitrary 10s
value goes along fine with that.
Rémy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r965150 - in /tomcat/trunk: java/org/apache/catalina/connector/
java/org/apache/tomcat/util/net/ webapps/docs/ webapps/docs/config/
Posted by Mark Thomas <ma...@apache.org>.
On 21/07/2010 10:18, Rainer Jung wrote:
> On 21.07.2010 09:40, Mark Thomas wrote:
>> On 20/07/2010 11:14, Rainer Jung wrote:
>>> So: the default we use is 10000, but if the timeout were<=0 then I
>>> think the condition in the AsyncTimeout runnable would fail.
>>
>> Agreed. That matches with what I was observing.
>>
>> I have no idea if a default timeout of 10s is appropriate for Async
>> requests or not. It seems like a good place to start but I'd be fine
>> with 60s too. The advantage of a shorter timeout is that when tests
>> fail, they fail relatively quickly.
>
> IMHO for any serious application of async the developer will need to
> adjust the timeout. A very long default is of no real use as a timeout,
> so I'd be fine with the 10 seconds we have now. Yes, many use cases will
> fire the timeout though they want to wait longer, but at least they get
> aware they need to adjust.
+1
> Once we've finalized this we need to add the asyncTimeout description to
> the connectors config docs (though IMHO when people start to use
> different async use cases in parallel, a connector configuration will no
> longer be enough and they have to set the timeout via the AsyncContext
> API).
The docs were updated when I added the configuration option. Note that
only HTTP BIO has an implementation at the moment. Once I have a fix for
bug 49567 that passes the TCK then I'll extend the fix to cover the
other connectors.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r965150 - in /tomcat/trunk: java/org/apache/catalina/connector/
java/org/apache/tomcat/util/net/ webapps/docs/ webapps/docs/config/
Posted by Rainer Jung <ra...@kippdata.de>.
On 21.07.2010 09:40, Mark Thomas wrote:
> On 20/07/2010 11:14, Rainer Jung wrote:
>> So: the default we use is 10000, but if the timeout were<=0 then I
>> think the condition in the AsyncTimeout runnable would fail.
>
> Agreed. That matches with what I was observing.
>
> I have no idea if a default timeout of 10s is appropriate for Async
> requests or not. It seems like a good place to start but I'd be fine
> with 60s too. The advantage of a shorter timeout is that when tests
> fail, they fail relatively quickly.
IMHO for any serious application of async the developer will need to
adjust the timeout. A very long default is of no real use as a timeout,
so I'd be fine with the 10 seconds we have now. Yes, many use cases will
fire the timeout though they want to wait longer, but at least they get
aware they need to adjust.
Once we've finalized this we need to add the asyncTimeout description to
the connectors config docs (though IMHO when people start to use
different async use cases in parallel, a connector configuration will no
longer be enough and they have to set the timeout via the AsyncContext API).
Regards,
Rainer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r965150 - in /tomcat/trunk: java/org/apache/catalina/connector/
java/org/apache/tomcat/util/net/ webapps/docs/ webapps/docs/config/
Posted by Mark Thomas <ma...@apache.org>.
On 20/07/2010 11:14, Rainer Jung wrote:
> On 20.07.2010 08:35, jean-frederic clere wrote:
>> On 07/19/2010 07:16 PM, Mark Thomas wrote:
>>> On 18/07/2010 23:50, Rainer Jung wrote:
>>>> On 18.07.2010 02:02, Mark Thomas wrote:
>>>>> On 18/07/2010 00:57, markt@apache.org wrote:
>>>>>> Author: markt
>>>>>> Date: Sat Jul 17 23:57:23 2010
>>>>>> New Revision: 965150
>>>>>>
>>>>>> URL: http://svn.apache.org/viewvc?rev=965150&view=rev
>>>>>> Log:
>>>>>> Restore pero's timeout fix for the BIO connector. Add configuration
>>>>>> of the timeout.
>>>>>
>>>>> Servlet TCK (with BIO) and unit tests pass as of this commit.
>>>>>
>>>>> It is looking like timeouts are going to be required to fix
>>>>> https://issues.apache.org/bugzilla/show_bug.cgi?id=49567. The complete
>>>>> fix is going to require some refactoring and I'm not quite there. It
>>>>> does look like most of the fix for
>>>>> https://issues.apache.org/bugzilla/show_bug.cgi?id=49528 is going
>>>>> to end
>>>>> up being reverted.
>>>>>
>>>>> Also, need to check the NIO and APR timeout async requests.
>>>>
>>>> I'm not totally sure, but after JFC's heads up that the TCK fails
>>>
>>> What heads up where?
>>
>> That was more than 2 weeks ago. I need to retest with the actual code.
>>
>>> This stuff should be on the dev list. A simple "The
>
> As JFC said: it was an old message (on the dev list):
>
> http://apache.markmail.org/message/7t7l2oijwtsn6gp6
That wasn't clear from your original message. Further, I don't think
those failures are relevant. Peter made a bunch of changes back then.
This is one change to the BIO connector and I have tested that the
Servlet TCK still passes with this change. I'm pretty sure it was his
other changes that caused the TCK to fail.
>>> Servlet 3.0 TCK fails with the XXX connector." is fine on the dev list.
>>>
>>> Which test with which connector?
>>
>> It was with APR.
Peter's previous changes caused failures with BIO as well.
>>
>>> I did only check with BIO.
>>>
>>>> I had
>>>> a short look at the async timeout runnable. My impression was, that the
>>>> default value for the timeout is "-1" and this would let the timeout
>>>> condition in the runnable always trigger.
>>>
>>> > From the top of page 14 of the servlet 3.0 spec:
>>>
>>> AsyncContext.setTimeout(long)... A value of 0 or less indicates that the
>>> asynchronous operation will never time out. ...
>
>>> I couldn't find an explicit default.
>>
>> Can't find it too.. Why not using 0 (for ever) or 1 minutes (to prevent
>> bad applications).
A default timeout of forever causes problems when Runnables are used.
The original implementation didn't take into account that the Runnable
could be used to call AsyncContext.complete(). This was bug 49528. My
first attempt to fix this handled this one special case - it did not
handle the general case (bug 49528). I am currently working on a fix for
the general case. The general case fix needs timeouts since that is the
only way to detect errors in the applications where complete() is not
called.
> I was slightly confused by two possibly different notions of timeout,
> the one in AsyncContext and the one in the AsyncTimeout runnable. I was
> looking at the AsyncTimeout runnable in JIoEndpoint which contains:
>
> if ((now-access)>socket.getTimeout()) {
> processSocket(socket,SocketStatus.TIMEOUT);
> }
>
> So I thought since now>=access if the timeout is <=0 it will run
> processSocket(socket,SocketStatus.TIMEOUT). This seems to end up in
> CoyoteAdapter.asyncDispatch(*, *, SocketStatus.TIMEOUT), which calls
> asyncContextImpl.setTimeoutState(). That in turn sets an internal state
> that shortcuts doInternalDispatch() to fire a timeout event and return
> an error.
>
> The async context timeout seems to be set from the connector
> asyncTimeout attribute (default 10000) and never gets used directly.
>
> But it is all tied together via the listener which is called when the
> async context timeout is set. The listener is implemented in the
> processor, which then sets the new timeout as a timeout to the socket,
> so the two timeouts are actually the same at the end.
>
> So: the default we use is 10000, but if the timeout were <=0 then I
> think the condition in the AsyncTimeout runnable would fail.
Agreed. That matches with what I was observing.
I have no idea if a default timeout of 10s is appropriate for Async
requests or not. It seems like a good place to start but I'd be fine
with 60s too. The advantage of a shorter timeout is that when tests
fail, they fail relatively quickly.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r965150 - in /tomcat/trunk: java/org/apache/catalina/connector/
java/org/apache/tomcat/util/net/ webapps/docs/ webapps/docs/config/
Posted by Rainer Jung <ra...@kippdata.de>.
On 20.07.2010 08:35, jean-frederic clere wrote:
> On 07/19/2010 07:16 PM, Mark Thomas wrote:
>> On 18/07/2010 23:50, Rainer Jung wrote:
>>> On 18.07.2010 02:02, Mark Thomas wrote:
>>>> On 18/07/2010 00:57, markt@apache.org wrote:
>>>>> Author: markt
>>>>> Date: Sat Jul 17 23:57:23 2010
>>>>> New Revision: 965150
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=965150&view=rev
>>>>> Log:
>>>>> Restore pero's timeout fix for the BIO connector. Add configuration
>>>>> of the timeout.
>>>>
>>>> Servlet TCK (with BIO) and unit tests pass as of this commit.
>>>>
>>>> It is looking like timeouts are going to be required to fix
>>>> https://issues.apache.org/bugzilla/show_bug.cgi?id=49567. The complete
>>>> fix is going to require some refactoring and I'm not quite there. It
>>>> does look like most of the fix for
>>>> https://issues.apache.org/bugzilla/show_bug.cgi?id=49528 is going to end
>>>> up being reverted.
>>>>
>>>> Also, need to check the NIO and APR timeout async requests.
>>>
>>> I'm not totally sure, but after JFC's heads up that the TCK fails
>>
>> What heads up where?
>
> That was more than 2 weeks ago. I need to retest with the actual code.
>
>> This stuff should be on the dev list. A simple "The
As JFC said: it was an old message (on the dev list):
http://apache.markmail.org/message/7t7l2oijwtsn6gp6
>> Servlet 3.0 TCK fails with the XXX connector." is fine on the dev list.
>>
>> Which test with which connector?
>
> It was with APR.
>
>> I did only check with BIO.
>>
>>> I had
>>> a short look at the async timeout runnable. My impression was, that the
>>> default value for the timeout is "-1" and this would let the timeout
>>> condition in the runnable always trigger.
>>
>> > From the top of page 14 of the servlet 3.0 spec:
>>
>> AsyncContext.setTimeout(long)... A value of 0 or less indicates that the
>> asynchronous operation will never time out. ...
>> I couldn't find an explicit default.
>
> Can't find it too.. Why not using 0 (for ever) or 1 minutes (to prevent
> bad applications).
I was slightly confused by two possibly different notions of timeout,
the one in AsyncContext and the one in the AsyncTimeout runnable. I was
looking at the AsyncTimeout runnable in JIoEndpoint which contains:
if ((now-access)>socket.getTimeout()) {
processSocket(socket,SocketStatus.TIMEOUT);
}
So I thought since now>=access if the timeout is <=0 it will run
processSocket(socket,SocketStatus.TIMEOUT). This seems to end up in
CoyoteAdapter.asyncDispatch(*, *, SocketStatus.TIMEOUT), which calls
asyncContextImpl.setTimeoutState(). That in turn sets an internal state
that shortcuts doInternalDispatch() to fire a timeout event and return
an error.
The async context timeout seems to be set from the connector
asyncTimeout attribute (default 10000) and never gets used directly.
But it is all tied together via the listener which is called when the
async context timeout is set. The listener is implemented in the
processor, which then sets the new timeout as a timeout to the socket,
so the two timeouts are actually the same at the end.
So: the default we use is 10000, but if the timeout were <=0 then I
think the condition in the AsyncTimeout runnable would fail.
Regards,
Rainer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r965150 - in /tomcat/trunk: java/org/apache/catalina/connector/
java/org/apache/tomcat/util/net/ webapps/docs/ webapps/docs/config/
Posted by jean-frederic clere <jf...@gmail.com>.
On 07/19/2010 07:16 PM, Mark Thomas wrote:
> On 18/07/2010 23:50, Rainer Jung wrote:
>> On 18.07.2010 02:02, Mark Thomas wrote:
>>> On 18/07/2010 00:57, markt@apache.org wrote:
>>>> Author: markt
>>>> Date: Sat Jul 17 23:57:23 2010
>>>> New Revision: 965150
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=965150&view=rev
>>>> Log:
>>>> Restore pero's timeout fix for the BIO connector. Add configuration
>>>> of the timeout.
>>>
>>> Servlet TCK (with BIO) and unit tests pass as of this commit.
>>>
>>> It is looking like timeouts are going to be required to fix
>>> https://issues.apache.org/bugzilla/show_bug.cgi?id=49567. The complete
>>> fix is going to require some refactoring and I'm not quite there. It
>>> does look like most of the fix for
>>> https://issues.apache.org/bugzilla/show_bug.cgi?id=49528 is going to end
>>> up being reverted.
>>>
>>> Also, need to check the NIO and APR timeout async requests.
>>
>> I'm not totally sure, but after JFC's heads up that the TCK fails
>
> What heads up where?
That was more than 2 weeks ago. I need to retest with the actual code.
> This stuff should be on the dev list. A simple "The
> Servlet 3.0 TCK fails with the XXX connector." is fine on the dev list.
>
> Which test with which connector?
It was with APR.
> I did only check with BIO.
>
>> I had
>> a short look at the async timeout runnable. My impression was, that the
>> default value for the timeout is "-1" and this would let the timeout
>> condition in the runnable always trigger.
>
>>>From the top of page 14 of the servlet 3.0 spec:
>
> AsyncContext.setTimeout(long)... A value of 0 or less indicates that the
> asynchronous operation will never time out. ...
>
> I couldn't find an explicit default.
Can't find it too.. Why not using 0 (for ever) or 1 minutes (to prevent
bad applications).
Cheers
Jean-Frederic
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r965150 - in /tomcat/trunk: java/org/apache/catalina/connector/
java/org/apache/tomcat/util/net/ webapps/docs/ webapps/docs/config/
Posted by Mark Thomas <ma...@apache.org>.
On 18/07/2010 23:50, Rainer Jung wrote:
> On 18.07.2010 02:02, Mark Thomas wrote:
>> On 18/07/2010 00:57, markt@apache.org wrote:
>>> Author: markt
>>> Date: Sat Jul 17 23:57:23 2010
>>> New Revision: 965150
>>>
>>> URL: http://svn.apache.org/viewvc?rev=965150&view=rev
>>> Log:
>>> Restore pero's timeout fix for the BIO connector. Add configuration
>>> of the timeout.
>>
>> Servlet TCK (with BIO) and unit tests pass as of this commit.
>>
>> It is looking like timeouts are going to be required to fix
>> https://issues.apache.org/bugzilla/show_bug.cgi?id=49567. The complete
>> fix is going to require some refactoring and I'm not quite there. It
>> does look like most of the fix for
>> https://issues.apache.org/bugzilla/show_bug.cgi?id=49528 is going to end
>> up being reverted.
>>
>> Also, need to check the NIO and APR timeout async requests.
>
> I'm not totally sure, but after JFC's heads up that the TCK fails
What heads up where? This stuff should be on the dev list. A simple "The
Servlet 3.0 TCK fails with the XXX connector." is fine on the dev list.
Which test with which connector? I did only check with BIO.
> I had
> a short look at the async timeout runnable. My impression was, that the
> default value for the timeout is "-1" and this would let the timeout
> condition in the runnable always trigger.
>From the top of page 14 of the servlet 3.0 spec:
AsyncContext.setTimeout(long)... A value of 0 or less indicates that the
asynchronous operation will never time out. ...
I couldn't find an explicit default.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org