Bug 60319 - Executor limits reported via JMX can be inconsistent
Summary: Executor limits reported via JMX can be inconsistent
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 9
Classification: Unclassified
Component: Connectors (show other bugs)
Version: 9.0.0.M11
Hardware: All All
: P2 normal (vote)
Target Milestone: -----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-10-28 20:43 UTC by Mark Thomas
Modified: 2016-10-31 20:44 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Thomas 2016-10-28 20:43:38 UTC
When a connector is configured with an external executor JMX allows the min/max threads to be set either directly via the executor or via the connector/protocol/endpoint. If set via the connector/protocol/endpoint the reported current settings can be inconsistent.

I'm not convinced that allowing the executor settings to be set via the connector/protocol/endpoint was a good idea. I'm currently considering changing the implementation along the following lines:
- calls to setters only change settings for internal exectuor
- if an external executor is configured, getters always return '-1' or similar
Comment 1 Christopher Schultz 2016-10-29 17:36:36 UTC
I might even throw an exception.

For TC9, I think we can even remove these getters/setters on the <Connector> itself, unless the consensus is that configuration-convenience trumps simplicity (of the code).

If we opt for continued support for min/max settings directly on the <Connector> only for an internal connector, I would vote for throwing an exception (IllegalStateException?) when using an external <Executor> and calling an associated internal-executor-only method.
Comment 2 Mark Thomas 2016-10-31 09:14:12 UTC
The problem with the exception approach is that we don't necessarily know that an external executor is going to be configured at the point the setters are called.

I agree that simpler is good. I want to look at this some more today with that in mind.
Comment 3 Mark Thomas 2016-10-31 16:52:09 UTC
I've committed a fix to trunk. The code isn't much simpler but the behaviour should be simpler for users to follow. Back-ports to follow shortly.
Comment 4 Mark Thomas 2016-10-31 20:44:16 UTC
Fixed in trunk for 9.0.0.M12 onwards, 8.5.x for 8.5.7 onwards, 8.0.x for 8.0.39 onwards and 7.0.x for 7.0.73 onwards.

Not back-ported to 6.0.x since the lack of refactoring to AbstractEndpoint makes it a lot more work, 6.0.x is close to EOL and this is a minor issue.