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

Add support for configuring the blocking timeout for Jetty connectors #1795

Merged
merged 1 commit into from Nov 2, 2016

Conversation

Projects
None yet
5 participants
@arteam
Member

arteam commented Nov 2, 2016

The issue reported in #1794.

Jetty 9.3.2.v20150730 introduced a new option for the configuration of HTTP connection factories. The option is called blockingTimeout and allows to configure the timeout which applies to all blocking operations in addition to idleTimeout. The rationale behind it is to allow to set a timeout
for blocking operations which perform I/O activity, but with a slow rate (for example serving a big static file). Such tasks won't exhaust the pool of worker threads which should improve the QoS for a Jetty server.

See:
https://dev.eclipse.org/mhonarc/lists/jetty-announce/msg00085.html
https://bugs.eclipse.org/bugs/show_bug.cgi?id=472621

Add support for configuring the blocking timeout for Jetty connectors
Jetty 9.3.2.v20150730 introduced a new option for the configuration of HTTP
connection factories. The option is called `blockingTimeout` and allows to
configure the timeout which applies to all blocking operations in addition
to `idleTimeout`. The rationale behind it is to allow to set a timeout
for blocking operations which perform I/O activity, but with a slow rate
(for example serving a big static file). Such tasks won't exhaust the pool
of worker threads which should improve the QoS for a Jetty server.

See:

https://dev.eclipse.org/mhonarc/lists/jetty-announce/msg00085.html
https://bugs.eclipse.org/bugs/show_bug.cgi?id=472621
@coveralls

This comment has been minimized.

coveralls commented Nov 2, 2016

Coverage Status

Coverage increased (+0.02%) to 82.325% when pulling 150af6c on http_connector_blocking_timeout into 4541594 on master.

@arteam arteam added this to the 1.1.0 milestone Nov 2, 2016

@arteam arteam added the improvement label Nov 2, 2016

@CodingFabian

This comment has been minimized.

CodingFabian commented Nov 2, 2016

@arteam thanks should be sufficient - i tried to come up with a pr on my own, but I was unable to decide how to deal with the -1 setting. Looks like you decided that one would not want to manually set it?

@arteam

This comment has been minimized.

Member

arteam commented Nov 2, 2016

Yes, I think making the field nullable is a better way to represent its absence than using -1. We use the Duration class for time durations, so we would need to specify the unit of time along with the number. I find quite strange from an outsiders perspective to specify -1ms as an indicator of the default value.

@CodingFabian

This comment has been minimized.

CodingFabian commented Nov 2, 2016

agreed. in my opinion this is good to be merged.

@joschi joschi self-assigned this Nov 2, 2016

@joschi

joschi approved these changes Nov 2, 2016

LGTM. 👍

@joschi joschi merged commit 7696a91 into master Nov 2, 2016

@joschi joschi deleted the http_connector_blocking_timeout branch Nov 2, 2016

@nickbabcock

This comment has been minimized.

Contributor

nickbabcock commented Nov 2, 2016

Would this be a better solution for #1674, or are these two different problems?

@CodingFabian

This comment has been minimized.

CodingFabian commented Nov 2, 2016

In my case it is @nickbabcock

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment