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.

Sign up for GitHub

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

Jump to bottom

Improve timeout support for HTTP #1456

Merged

bondolo merged 11 commits into apple:main from bondolo:timeout-http

Apr 2, 2021

Contributor

bondolo commented Mar 25, 2021 •

edited by tkountis

Loading

Motivation:
Some uses of the HTTP API require progress or completion within specified time limits.

Modifications:
New or enhanced filters are provided for client and server to manage timeout events.
Existing client filter TimeoutHttpRequesterFilter is enhanced to support additional
timeout options for the entire HTTP request/response transaction. A new server filter
TimeoutHttpServiceFilter is provided for managing timeouts by services.

Result:
HTTP API provides request/response transaction timeout capabilities

bondolo requested review from tkountis, idelpivnitskiy and Scottmitch

March 25, 2021 00:13

bondolo self-assigned this

Contributor

tkountis commented Mar 25, 2021

Minor comments.

tkountis reviewed

View reviewed changes

...ternal/src/main/groovy/io/servicetalk/gradle/plugin/internal/ServiceTalkLibraryPlugin.groovy Outdated Show resolved Hide resolved

servicetalk-http-utils/src/main/java/io/servicetalk/http/utils/TimeoutHttpRequesterFilter.java Outdated Show resolved Hide resolved

servicetalk-http-utils/src/main/java/io/servicetalk/http/utils/TimeoutHttpRequesterFilter.java Show resolved Hide resolved

servicetalk-http-utils/src/main/java/io/servicetalk/http/utils/TimeoutHttpRequesterFilter.java Outdated Show resolved Hide resolved

servicetalk-http-netty/src/test/java/io/servicetalk/http/netty/ResponseTimeoutTest.java Show resolved Hide resolved

tkountis reviewed

View reviewed changes

servicetalk-http-utils/src/main/java/io/servicetalk/http/utils/TimeoutHttpServiceFilter.java Show resolved Hide resolved

bondolo force-pushed the timeout-http branch from 0bfcc1d to f436583 Compare

March 26, 2021 23:12

Scottmitch reviewed

View reviewed changes

...helloworld/src/main/java/io/servicetalk/examples/http/helloworld/async/HelloWorldClient.java Outdated Show resolved Hide resolved

servicetalk-http-utils/src/main/java/io/servicetalk/http/utils/TimeoutHttpRequesterFilter.java Outdated Show resolved Hide resolved

servicetalk-http-utils/src/main/java/io/servicetalk/http/utils/TimeoutHttpRequesterFilter.java Outdated Show resolved Hide resolved

servicetalk-http-utils/src/main/java/io/servicetalk/http/utils/TimeoutHttpRequesterFilter.java Outdated Show resolved Hide resolved

idelpivnitskiy reviewed

View reviewed changes

servicetalk-http-utils/src/main/java/io/servicetalk/http/utils/TimeoutHttpRequesterFilter.java Outdated Show resolved Hide resolved

servicetalk-http-utils/src/main/java/io/servicetalk/http/utils/TimeoutHttpRequesterFilter.java Outdated Show resolved Hide resolved

servicetalk-http-utils/src/main/java/io/servicetalk/http/utils/TimeoutHttpRequesterFilter.java Show resolved Hide resolved

servicetalk-http-utils/src/main/java/io/servicetalk/http/utils/TimeoutHttpRequesterFilter.java Show resolved Hide resolved

servicetalk-http-utils/src/main/java/io/servicetalk/http/utils/TimeoutHttpRequesterFilter.java Outdated Show resolved Hide resolved

servicetalk-http-utils/src/main/java/io/servicetalk/http/utils/TimeoutHttpRequesterFilter.java Outdated Show resolved Hide resolved

servicetalk-http-utils/src/main/java/io/servicetalk/http/utils/TimeoutHttpServiceFilter.java Outdated Show resolved Hide resolved

servicetalk-http-utils/src/main/java/io/servicetalk/http/utils/TimeoutHttpRequesterFilter.java Outdated Show resolved Hide resolved

servicetalk-http-utils/src/main/java/io/servicetalk/http/utils/TimeoutHttpRequesterFilter.java Outdated Show resolved Hide resolved

servicetalk-http-utils/src/main/java/io/servicetalk/http/utils/TimeoutHttpServiceFilter.java Outdated Show resolved Hide resolved

idelpivnitskiy reviewed

View reviewed changes

servicetalk-http-utils/src/main/java/io/servicetalk/http/utils/TimeoutHttpServiceFilter.java Show resolved Hide resolved

bondolo force-pushed the timeout-http branch 2 times, most recently from 2f6e0c9 to c690727 Compare

March 30, 2021 21:02

idelpivnitskiy reviewed

View reviewed changes

Member

idelpivnitskiy left a comment

Couple polishing comments and ship it. Can't wait to use these enhancements instead of a custom filters

...ternal/src/main/groovy/io/servicetalk/gradle/plugin/internal/ServiceTalkLibraryPlugin.groovy Outdated Show resolved Hide resolved

...helloworld/src/main/java/io/servicetalk/examples/http/helloworld/async/HelloWorldClient.java Outdated Show resolved Hide resolved

servicetalk-http-utils/src/main/java/io/servicetalk/http/utils/TimeoutHttpRequesterFilter.java Outdated Show resolved Hide resolved

servicetalk-http-utils/src/main/java/io/servicetalk/http/utils/TimeoutHttpRequesterFilter.java Outdated Show resolved Hide resolved

servicetalk-http-utils/src/main/java/io/servicetalk/http/utils/TimeoutHttpRequesterFilter.java Outdated Show resolved Hide resolved

servicetalk-http-utils/src/main/java/io/servicetalk/http/utils/TimeoutHttpServiceFilter.java Outdated Show resolved Hide resolved

servicetalk-http-utils/src/main/java/io/servicetalk/http/utils/TimeoutHttpServiceFilter.java Outdated Show resolved Hide resolved

servicetalk-examples/http/timeout/src/main/resources/log4j2.xml Outdated Show resolved Hide resolved

...-examples/http/timeout/src/main/java/io/servicetalk/examples/http/timeout/TimeoutClient.java Outdated Show resolved Hide resolved

...-examples/http/timeout/src/main/java/io/servicetalk/examples/http/timeout/TimeoutServer.java Outdated Show resolved Hide resolved

idelpivnitskiy reviewed

View reviewed changes

servicetalk-http-utils/src/main/java/io/servicetalk/http/utils/TimeoutHttpServiceFilter.java Outdated Show resolved Hide resolved

idelpivnitskiy reviewed

View reviewed changes

servicetalk-http-utils/src/main/java/io/servicetalk/http/utils/TimeoutHttpServiceFilter.java Outdated Show resolved Hide resolved

idelpivnitskiy reviewed

View reviewed changes

...-examples/http/timeout/src/main/java/io/servicetalk/examples/http/timeout/TimeoutClient.java Outdated Show resolved Hide resolved

servicetalk-http-utils/src/main/java/io/servicetalk/http/utils/TimeoutHttpRequesterFilter.java Outdated Show resolved Hide resolved

...ternal/src/main/groovy/io/servicetalk/gradle/plugin/internal/ServiceTalkLibraryPlugin.groovy Outdated Show resolved Hide resolved

servicetalk-http-utils/src/main/java/io/servicetalk/http/utils/TimeoutHttpServiceFilter.java Show resolved Hide resolved

bondolo added 10 commits

April 1, 2021 11:15


          Improve timeout support for HTTP

b08671d

Motivation:
Some uses of the HTTP API require progress or completion within specified time limits.

Modifications:
New or enhanced filters are proivded for client and server to manage timeout events.
Existing client filter `TimeoutHttpRequesterFilter` is enhanced to support additional
timeout options for the entire HTTP request/response transaction. A new server filter
`TimeoutHttpServiceFilter` is provided for managing timeouts by services.

Result:
HTTP API provides request/response transaction timeout capabilities


          Review feedback

f31ce6f


          remove debugging utils from test

90cee49


          Improve javadoc to mention filter ordering

be523b9

Also adds more details about AsyncContext use with defaultTimeout()


          Adds test contributed by @idelpivnitskiy

ecb7289


          Implement review feedback.

a3bbd9c


          Implement review feedback

cadd601


          review feedback

e40e661


          revert stray ignoreFailures change

f98029b


          implement review feedback

3b7a4f1

Add support for fullRequestResponse timeout to ServiceFilter
More common code in filters
Move simpleDurationTimeout() to TimeoutFromRequest

bondolo force-pushed the timeout-http branch from b0965b8 to 3b7a4f1 Compare

April 1, 2021 20:42

idelpivnitskiy approved these changes

View reviewed changes

Member

idelpivnitskiy left a comment

Let's merge this PR after simpleDurationTimeout is made pkg-private. The PR looks correct from the implementation and API point of view. Code sharing can be done in a follow-up PR, if ever necessary.
Thank you for the great improvement!

servicetalk-http-utils/src/main/java/io/servicetalk/http/utils/TimeoutHttpRequesterFilter.java Outdated Show resolved Hide resolved

servicetalk-http-utils/src/main/java/io/servicetalk/http/utils/TimeoutFromRequest.java Outdated Show resolved Hide resolved


          final nits. Move simpleDurationTimeout back to package private

bc730a2

bondolo merged commit a953261 into apple:main

bondolo deleted the timeout-http branch

April 2, 2021 00:28

idelpivnitskiy pushed a commit to idelpivnitskiy/servicetalk that referenced this pull request


          Improve timeout support for HTTP (apple#1456)

366680a

Motivation:
Some uses of the HTTP API require progress or completion within specified time limits.

Modifications:
New or enhanced filters are proivded for client and server to manage timeout events.
Existing client filter `TimeoutHttpRequesterFilter` is enhanced to support additional
timeout options for the entire HTTP request/response transaction. A new server filter
`TimeoutHttpServiceFilter` is provided for managing timeouts by services.

Result:
HTTP API provides request/response transaction timeout capabilities

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

Labels

None yet