Bug 56713

Summary: Limit time that incoming request waits while webapp is reloading
Product: Tomcat 8 Reporter: Konstantin Kolinko <knst.kolinko>
Component: CatalinaAssignee: Tomcat Developers Mailing List <dev>
Status: NEW ---    
Severity: enhancement    
Priority: P2    
Version: 8.0.9   
Target Milestone: ----   
Hardware: PC   
OS: All   
Attachments: 2014-07-15_tc8_56713_v1.patch

Description Konstantin Kolinko 2014-07-12 15:52:04 UTC
(Originally raised in bug 56710)

There is an old feature that when web application is being reloaded, any incoming requests for that application are paused until reload completes.

This feature dates back to Tomcat 4.1.x or earlier. See also bug 53024 for fixing (reimplementing) this feature in Tomcat 7.

The waiting loop is implemented in CoyoteAdapter.postParseRequest().

While this feature is useful, the wait time shall not be infinite.

(1) How long is the allowed wait time?
(2) What to do if wait time expires?
(3) What to do if Connector state changes, e.g Tomcat shuts down?

Regarding (1):
a) I thought that maybe "connectionTimeout" of a Connector is the good time to wait.
b) Make it configurable on Host?
c) Make it configurable on Context?
d) Hard-code something?

There are caveats for a):
- There may be several Connectors in the same Service with different value for this attribute.
- For an AJP connector this attribute is "-1" by default.

Regarding (2):
I think the best would be to unregister that context in the Mapper and let whatever other (ROOT) application there to handle the request. The ROOT application may respond with 503, 404, 302 or whatever is suitable.

(Rejecting the request in CoyoteAdapter would be unfriendly, as it happens at a lower level and there is no ErrorReportValve to render an error page. The en-user will see an empty page).

I think this needs update to the Mapper API.
There will be race condition between this forced de-registration of context, and its re-registration after successful startup. To avoid it,

a) MapperListener shall notify Mapper that the context has been paused. The Mapper shall remember the "paused" state.
b) The forced de-registration shall be a separate method in the Mapper and it shall check the "paused" state of a context before removing it.
Comment 1 Konstantin Kolinko 2014-07-13 14:58:58 UTC
(In reply to Konstantin Kolinko from comment #0)
> a) MapperListener shall notify Mapper that the context has been paused. The
> Mapper shall remember the "paused" state.

That part of API was implemented r1610220, as it was needed to fix bug 56710.
Comment 2 Mark Thomas 2014-07-14 07:04:40 UTC
Remy highlighted the potential problems of pausing requests during reload (large numbers of threads blocking waiting for the reload) and I agree that this could be a problem. However, the only user complaints we have had about this feature is that it wasn't working or that it could work better. No-one has complained about large numbers of threads blocking. Givent that no-one has complained about large numbers of blocked threads on reloads, I am wondering if there is really a problem here that needs fixing.

Regarding the questions on proposed solution:

How is this going to work if it is the ROOT context that is being reloaded?

(1) a) and d) have obvious issues. I'd suggest configuration on the host with the ability to override it on the context if desired

(2) There might not be a ROOT application (there should be but there might not). How is the Context going to be re-registered once it has started? I don't think any user is not going to want the context to be available eventually.

(3) Presumably the Context eventually starts and then is shutdown.

I was about to say that there are lots of complexities here - just like automatic deployment that led to producing this page:

Reading that it occurred to me that really what we are talking about here is the difference between reloading and redeploying. Would it not be simpler to leave this in the hands of the system administrator? They can choose whether to redploy or reload (if reload is an option for the change they want to make).

One thing we could do is add some logging to notify the system admin that threads have been blocked and that they can use redeploy to avoid this. I was thinking along the lines of one log message the first time a thread was blocked and then further messages every TBD (15?, background thread?) seconds giving the current number of waiting threads until the context is reloaded.
Comment 3 Konstantin Kolinko 2014-07-15 13:29:57 UTC
Created attachment 31814 [details]

An idea of a patch to illustrate the proposal.
The wait time is hard-coded to be 2 minutes.
This one is for the current trunk (at 1610400)
Comment 4 Remy Maucherat 2014-07-15 13:32:42 UTC
Reload is a development oriented feature, and the wait feature in that case is a convinience (basically, you can hit the browser's reload button without having to check the reload is finished). I doubt this feature is in meaninglful use in any other situation.