Re: Incoming refactoring

2016-12-14 Thread Mark Thomas 
On 14/12/2016 09:33, Rémy Maucherat wrote:
> 2016-12-14 10:07 GMT+01:00 Mark Thomas :
> 
>> On 14/12/2016 08:54, Rémy Maucherat wrote:
>>> 2016-12-14 9:50 GMT+01:00 Mark Thomas :
>>>
 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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Re: Incoming refactoring

2016-12-14 Thread Rémy Maucherat 
2016-12-14 10:07 GMT+01:00 Mark Thomas :

> On 14/12/2016 08:54, Rémy Maucherat wrote:
> > 2016-12-14 9:50 GMT+01:00 Mark Thomas :
> >
> >> 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

2016-12-14 Thread Mark Thomas 
On 14/12/2016 08:54, Rémy Maucherat wrote:
> 2016-12-14 9:50 GMT+01:00 Mark Thomas :
> 
>> 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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Re: Incoming refactoring

2016-12-14 Thread Rémy Maucherat 
2016-12-14 9:50 GMT+01:00 Mark Thomas :

> 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

2016-12-14 Thread Mark Thomas 
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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Re: Incoming refactoring

2016-12-13 Thread Mark Thomas 
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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Re: Incoming refactoring

2016-12-13 Thread Mark Thomas 
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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Re: Incoming refactoring

2016-12-13 Thread Christopher Schultz 
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



signature.asc
Description: OpenPGP digital signature