You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Mark Thomas <ma...@apache.org> on 2016/12/13 11:24:47 UTC
Incoming refactoring
Hi,
Just a heads up that - assuming the currently running tests pass - I'll
be committing a batch of connector refactoring changes. The general aim
of these changes is:
- Reduce the duplication of configuration between Protocol and Processor
In addition to removing a chunk of duplicate getters and setters, it
should make it easier to make dynamic configuration changes.
- Reduce the visibility of the Endpoint.
Processor no longer accesses Endpoint directly.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Incoming refactoring
Posted by Mark Thomas <ma...@apache.org>.
On 14/12/2016 09:33, R�my Maucherat wrote:
> 2016-12-14 10:07 GMT+01:00 Mark Thomas <ma...@apache.org>:
>
>> On 14/12/2016 08:54, R�my Maucherat wrote:
>>> 2016-12-14 9:50 GMT+01:00 Mark Thomas <ma...@apache.org>:
>>>
>>>> The failure happens on current trunk too so I don't think this is
>>>> related to the refactoring. However, I am going to investigate this
>>>> failure first - before I apply the refactoring.
>>>>
>>>> What is the intermittent failure ?
>>> https://ci.apache.org/builders/tomcat-trunk seems happy (usually it's
>> not).
>>
>> TestHttp11Processor.test57621b
>>
>> Fails maybe 1 time in 10. It looks like something is going wrong
>> resetting the input buffer between requests.
>>
>> Ah this test is very specific. The runnable is doing what a dispatch would
> do but it doesn't have the code to wait until the container thread has
> returned:
>
> public synchronized boolean asyncDispatch() {
> if (!ContainerThreadMarker.isContainerThread() && state ==
> AsyncState.STARTING) {
> state = AsyncState.DISPATCH_PENDING;
> return false;
> } else {
> return doDispatch();
> }
> }
>
> vs:
>
> public synchronized void asyncRun(Runnable runnable) {
> ... [set env]
> processor.getExecutor().execute(runnable);
> ... [unset env]
> }
>
> So if this is allowed, it would run into the same concurrency issues that
> were fixed for dispatch.
That might be an issue but I am seeing something different. Still
digging into this...
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Incoming refactoring
Posted by Rémy Maucherat <re...@apache.org>.
2016-12-14 10:07 GMT+01:00 Mark Thomas <ma...@apache.org>:
> On 14/12/2016 08:54, Rémy Maucherat wrote:
> > 2016-12-14 9:50 GMT+01:00 Mark Thomas <ma...@apache.org>:
> >
> >> The failure happens on current trunk too so I don't think this is
> >> related to the refactoring. However, I am going to investigate this
> >> failure first - before I apply the refactoring.
> >>
> >> What is the intermittent failure ?
> > https://ci.apache.org/builders/tomcat-trunk seems happy (usually it's
> not).
>
> TestHttp11Processor.test57621b
>
> Fails maybe 1 time in 10. It looks like something is going wrong
> resetting the input buffer between requests.
>
> Ah this test is very specific. The runnable is doing what a dispatch would
do but it doesn't have the code to wait until the container thread has
returned:
public synchronized boolean asyncDispatch() {
if (!ContainerThreadMarker.isContainerThread() && state ==
AsyncState.STARTING) {
state = AsyncState.DISPATCH_PENDING;
return false;
} else {
return doDispatch();
}
}
vs:
public synchronized void asyncRun(Runnable runnable) {
... [set env]
processor.getExecutor().execute(runnable);
... [unset env]
}
So if this is allowed, it would run into the same concurrency issues that
were fixed for dispatch.
Rémy
Re: Incoming refactoring
Posted by Mark Thomas <ma...@apache.org>.
On 14/12/2016 08:54, R�my Maucherat wrote:
> 2016-12-14 9:50 GMT+01:00 Mark Thomas <ma...@apache.org>:
>
>> The failure happens on current trunk too so I don't think this is
>> related to the refactoring. However, I am going to investigate this
>> failure first - before I apply the refactoring.
>>
>> What is the intermittent failure ?
> https://ci.apache.org/builders/tomcat-trunk seems happy (usually it's not).
TestHttp11Processor.test57621b
Fails maybe 1 time in 10. It looks like something is going wrong
resetting the input buffer between requests.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Incoming refactoring
Posted by Rémy Maucherat <re...@apache.org>.
2016-12-14 9:50 GMT+01:00 Mark Thomas <ma...@apache.org>:
> The failure happens on current trunk too so I don't think this is
> related to the refactoring. However, I am going to investigate this
> failure first - before I apply the refactoring.
>
> What is the intermittent failure ?
https://ci.apache.org/builders/tomcat-trunk seems happy (usually it's not).
Rémy
Re: Incoming refactoring
Posted by Mark Thomas <ma...@apache.org>.
On 13/12/2016 21:14, Mark Thomas wrote:
> On 13/12/2016 15:21, Mark Thomas wrote:
>> On 13/12/2016 15:09, Christopher Schultz wrote:
>>> Mark,
>>>
>>> On 12/13/16 6:24 AM, Mark Thomas wrote:
>>>> Hi,
>>>>
>>>> Just a heads up that - assuming the currently running tests pass - I'll
>>>> be committing a batch of connector refactoring changes. The general aim
>>>> of these changes is:
>>>>
>>>> - Reduce the duplication of configuration between Protocol and Processor
>>>> In addition to removing a chunk of duplicate getters and setters, it
>>>> should make it easier to make dynamic configuration changes.
>>>>
>>>> - Reduce the visibility of the Endpoint.
>>>> Processor no longer accesses Endpoint directly.
>>>
>>> +1
>>>
>>> Any simplification we can make in that area will be good.
>>>
>>> Are you thinking of back-porting to 8.5 as well, or is this likely to
>>> stay in 9.0?
>>
>> Not sure. 8.5.x is meant to be stable and I'm changing APIs. Then again,
>> they are internal APIs. Config isn't changing. Also, we'd need to back
>> port the Acceptor changes first.
>>
>> Overall I'm neutral on this. I'd like to see the refactoring in 8.5.x
>> but I'm concerned about any instability or regressions that might slip in.
>
> An intermittent test failure has appeared. I need to investigate further
> before pushing the commit.
The failure happens on current trunk too so I don't think this is
related to the refactoring. However, I am going to investigate this
failure first - before I apply the refactoring.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Incoming refactoring
Posted by Mark Thomas <ma...@apache.org>.
On 13/12/2016 15:21, Mark Thomas wrote:
> On 13/12/2016 15:09, Christopher Schultz wrote:
>> Mark,
>>
>> On 12/13/16 6:24 AM, Mark Thomas wrote:
>>> Hi,
>>>
>>> Just a heads up that - assuming the currently running tests pass - I'll
>>> be committing a batch of connector refactoring changes. The general aim
>>> of these changes is:
>>>
>>> - Reduce the duplication of configuration between Protocol and Processor
>>> In addition to removing a chunk of duplicate getters and setters, it
>>> should make it easier to make dynamic configuration changes.
>>>
>>> - Reduce the visibility of the Endpoint.
>>> Processor no longer accesses Endpoint directly.
>>
>> +1
>>
>> Any simplification we can make in that area will be good.
>>
>> Are you thinking of back-porting to 8.5 as well, or is this likely to
>> stay in 9.0?
>
> Not sure. 8.5.x is meant to be stable and I'm changing APIs. Then again,
> they are internal APIs. Config isn't changing. Also, we'd need to back
> port the Acceptor changes first.
>
> Overall I'm neutral on this. I'd like to see the refactoring in 8.5.x
> but I'm concerned about any instability or regressions that might slip in.
An intermittent test failure has appeared. I need to investigate further
before pushing the commit.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Incoming refactoring
Posted by Mark Thomas <ma...@apache.org>.
On 13/12/2016 15:09, Christopher Schultz wrote:
> Mark,
>
> On 12/13/16 6:24 AM, Mark Thomas wrote:
>> Hi,
>>
>> Just a heads up that - assuming the currently running tests pass - I'll
>> be committing a batch of connector refactoring changes. The general aim
>> of these changes is:
>>
>> - Reduce the duplication of configuration between Protocol and Processor
>> In addition to removing a chunk of duplicate getters and setters, it
>> should make it easier to make dynamic configuration changes.
>>
>> - Reduce the visibility of the Endpoint.
>> Processor no longer accesses Endpoint directly.
>
> +1
>
> Any simplification we can make in that area will be good.
>
> Are you thinking of back-porting to 8.5 as well, or is this likely to
> stay in 9.0?
Not sure. 8.5.x is meant to be stable and I'm changing APIs. Then again,
they are internal APIs. Config isn't changing. Also, we'd need to back
port the Acceptor changes first.
Overall I'm neutral on this. I'd like to see the refactoring in 8.5.x
but I'm concerned about any instability or regressions that might slip in.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Incoming refactoring
Posted by Christopher Schultz <ch...@christopherschultz.net>.
Mark,
On 12/13/16 6:24 AM, Mark Thomas wrote:
> Hi,
>
> Just a heads up that - assuming the currently running tests pass - I'll
> be committing a batch of connector refactoring changes. The general aim
> of these changes is:
>
> - Reduce the duplication of configuration between Protocol and Processor
> In addition to removing a chunk of duplicate getters and setters, it
> should make it easier to make dynamic configuration changes.
>
> - Reduce the visibility of the Endpoint.
> Processor no longer accesses Endpoint directly.
+1
Any simplification we can make in that area will be good.
Are you thinking of back-porting to 8.5 as well, or is this likely to
stay in 9.0?
-chris