-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Gracefully stopped Jetty not flushing all response data #4047
Comments
This issue is similar to #2788 for what the user seems to would like (i.e. having the server wait for all the in-flight requests to finish before closing all connections). However, we don't have such feature yet although a few bits and pieces are there - but we need to come up with a clear API and do all the things that needs to be done for a graceful shutdown.
|
Stopping the acceptor would result in clients able to start the connect, bu not actually succeed to connect, right? We've had others ask that all new requests during shutdown actually be responded to with 503 Unavailable during shutdown. |
Not only not accept new connections, but also rejecting requests on existing connections. |
In this case the connection is accepted before the shutdown starts and the request The problem is that not all data make it to the client. One observation: when content length is set AFTER the data is written, the problem disappears: ...
@Override
public void handle(String s, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException {
response.getOutputStream().write("a".getBytes());
try {
Thread.sleep(10_000);
}
catch (InterruptedException e)
{
e.printStackTrace();
}
response.getOutputStream().write("b".getBytes());
response.setContentLength(2);
}
... |
@sbordet The intention of the graceful stop of Statistics handler was that it would work with the Server.stop to provide more or less the sequence you describe above. This is a feature we do have and was working. If it is not working now it is because it is broken, not because it is not implemented. |
It looks like we are being overzealous with our async close handling. When the handler tries to write "b" it is getting a:
|
Interesting observation about the content-length. Having that set before writing "b" causes the response to be committed within the write! I note that if I don't set the content length at all and let the commit happen after the return, it also works? interesting! |
There appears to be a problem in our aggregation / generation! When the "b" is written, we see that it is the last byte of the content and thus trigger a flush. But in the aggregate buffer we only have the "a". So we flush HEADERS+"a" and then the generator says DONE! only after that do we flush the "b" in a separate write... which should not work... but does somehow because normally we are persistent... but when doing a graceful shutdown we do: case DONE:
{
// If shutdown after commit, we can still close here.
if (getConnector().isShutdown())
_shutdownOut = true;
return Action.SUCCEEDED;
} Thus the late write of "b" fails because we have shutdown the output!!! So why are we signalling DONE before we have written the "b" ? |
I think we broke this in #2772 |
Added test to reproduce issue Fixed bug from #2772 where output was shutdown on DONE without checking for END. Fixed aggregation logic to aggregate last write if aggregation already started. Signed-off-by: Greg Wilkins <gregw@webtide.com>
Added test to reproduce issue Fixed bug from #2772 where output was shutdown on DONE without checking for END. Fixed aggregation logic to aggregate last write if aggregation already started Improved comments and clarify conditions Signed-off-by: Greg Wilkins <gregw@webtide.com>
PR #4094 merged. |
This should run an embedded Jetty server with configured graceful shutdown and a simulated long running request.
Reproduction (with Jetty 9.4.14.v20181114):
curl -vvv http://localhost:8080
jps | grep NotGraceful | cut -d' ' -f1 | xargs kill
Expected:
Jetty will let the request finish gracefully, with "ab" being sent to the client.
Actual:
The second byte isn't sent to the client.
The processing of the response hit the
HttpGenerator.Result result = _generator.generateResponse(_info, _head, _header, chunk, _content, _lastContent);
line inorg.eclipse.jetty.server.HttpConnection.SendCallback#process
5 times - once withNEED_HEADER
result and then twice withFLUSH
andDONE
results. The firstDONE
then signaled that the socket can be closed:so the second flush with the remaining data wasn't able to send it.
The text was updated successfully, but these errors were encountered: