=htc #851 fix leaking AbsorbCancellation #852
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
akka-ci
added
validating
|
Test FAILed. |
viktorklang
reviewed
Feb 8, 2017
| # If the network level buffers (including the Akka Stream / Akka IO networking stack buffers) | ||
| # contains more data than can be transfered to the client in the given time when the server-side considers | ||
| # to be finished with this connection, the client may encounter a connection reset. | ||
| linger-timeout = 1 min |
There was a problem hiding this comment.
Should probably mention how to set it to infinite?
jrudolph
force-pushed
the
w/fix-absorb-cancellation
branch
from
eca65c2
to
2361612
Compare
akka-ci
added
validating
|
Test FAILed. |
jrudolph
force-pushed
the
w/fix-absorb-cancellation
branch
from
2361612
to
f33e163
Compare
akka-ci
added
validating
|
Test FAILed. |
ktoso
reviewed
Feb 9, 2017
| log.debug(s"Stage was canceled after delay of $cancelAfter") | ||
| completeStage() | ||
| } | ||
| case _ ⇒ |
There was a problem hiding this comment.
I always like to add a // do nothing so it's clear the empty case is intentional
ktoso
reviewed
Feb 9, 2017
| case finite: FiniteDuration ⇒ | ||
| scheduleOnce(finite) { | ||
| log.debug(s"Stage was canceled after delay of $cancelAfter") | ||
| completeStage() |
ktoso
reviewed
Feb 9, 2017
| def absorbCancellation[T]: Flow[T, T, NotUsed] = Flow.fromGraph(new AbsorbCancellationStage) | ||
| final class AbsorbCancellationStage[T] extends SimpleLinearGraphStage[T] { | ||
| def createLogic(inheritedAttributes: Attributes): GraphStageLogic = new GraphStageLogic(shape) with InHandler with OutHandler with StageLogging { | ||
| def delayCancellation[T](cancelAfter: Duration): Flow[T, T, NotUsed] = Flow.fromGraph(new DelayCancellationStage(cancelAfter)) |
ktoso
reviewed
Feb 9, 2017
| # | ||
| # Set to 'infinite' to disable automatic connection closure (which will risk to leak connections). | ||
| linger-timeout = 1 min | ||
|
|
There was a problem hiding this comment.
good docs, I'm curious if 1 minute is a good value... I guess it's ok
|
We have a docs page on timeouts in http, document this there too, so it's the "go-to" place for all timeouts, can be basically a copy of the reference.conf docs. LGTM once added to docs |
jrudolph
force-pushed
the
w/fix-absorb-cancellation
branch
from
f33e163
to
62a6046
Compare
|
Added to docs, will merge after validation. |
akka-ci
added
validating
ktoso
reviewed
Feb 16, 2017
| contains more data than can be transferred to the client in the given time when the server-side considers | ||
| to be finished with this connection, the client may encounter a connection reset. | ||
|
|
||
| Set to 'infinite' to disable automatic connection closure (which will risk to leak connections). |
ktoso
reviewed
Feb 16, 2017
| to be finished with this connection, the client may encounter a connection reset. | ||
|
|
||
| Set to 'infinite' to disable automatic connection closure (which will risk to leak connections). | ||
|
|
akka-ci
added
needs-attention
|
Test FAILed. |
Under circumstances that I could not reproduce so far, the AbsorbCancellation stage introduced in akka#459 may keep the graph running if data was already buffered but never read. The reason is that the stage so far requires completion to reach the stage after cancellation was absorbed which might be blocked by incoming elements that were never pulled. The solution is two-fold. After cancellation: 1) pull in and ignore all incoming elements to eventually fetch completion 2) complete the stage after a configurable time In the HTTP use case, we delay the cancellation by no more than the time specified in the new linger-timeout setting.
jrudolph
force-pushed
the
w/fix-absorb-cancellation
branch
from
62a6046
to
dfa7dd9
Compare
akka-ci
added
validating
|
Test PASSed. |
|
Is there an update on this? I've upgraded to Akka-Http 10.1.8 and Akka 2.5.22 but still facing the same issue. |
Hi @jadhavj, can you explain what issue you are still facing? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Under circumstances that I could not reproduce so far,
the AbsorbCancellation stage introduced in #459 may keep the graph running
if data was already buffered but never read. The reason is that the stage
so far requires completion to reach the stage after cancellation was
absorbed which might be blocked by incoming elements that were never
pulled.
The solution is two-fold. After cancellation:
In the HTTP use case, we delay the cancellation by no more than the time specified
in the new linger-timeout setting.