fix: prevent dead letters from Write commands racing with TCP connection close#32905
Draft
sebastian-alfers wants to merge 1 commit intomainfrom
Draft
fix: prevent dead letters from Write commands racing with TCP connection close#32905sebastian-alfers wants to merge 1 commit intomainfrom
sebastian-alfers wants to merge 1 commit intomainfrom
Conversation
johanandren
reviewed
Mar 23, 2026
Contributor
johanandren
left a comment
johanandren
left a comment
There was a problem hiding this comment.
Did you try it out with a reproducer and saw it silence the logs?
| case Unregistered => | ||
| // NIO cleanup done; linger briefly so any in-flight WriteCommands that race with | ||
| // the connection close (e.g. from an Akka HTTP stream) are answered with CommandFailed | ||
| // instead of silently landing in dead letters. |
Contributor
There was a problem hiding this comment.
Suggested change
| // instead of silently landing in dead letters. | |
| // instead of ending up as dead letters. |
The problem you are trying to solve is that it is not silent, but noisy in logs, no?
| val doNothing: () => Unit = () => () | ||
|
|
||
| /** How long the connection actor lingers after close to absorb in-flight writes. */ | ||
| val DrainTimeout: FiniteDuration = 100.milliseconds |
Contributor
There was a problem hiding this comment.
Should not be hardcoded but come from config
Contributor
There was a problem hiding this comment.
With 0 as complete opt out the lingering check, and probably the default value.
| def draining: Receive = { | ||
| case write: WriteCommand => | ||
| if (TraceLogging) log.debug("Dropping write to already-closed connection") | ||
| sender() ! CommandFailed(write) |
Contributor
There was a problem hiding this comment.
Will this not just move the noisy logging to the writer?
Contributor
|
Not convinced on the benefit vs risk/performance overhead of this, but maybe if it can be opt in through config. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
When a peer closes a TCP connection (e.g. an HAProxy SSL health check that opens and immediately closes the socket), the connection actor's close notification was sent from postStop — after the actor was fully terminated.
Any WriteCommand sent by a higher-level stream (e.g. Akka HTTP) between the peer close and the actor's death would land in dead letters with no way to intercept them.
Fix:
before entering the unregistering state, so they receive the close event
while the actor is still alive.
draining state instead of stopping immediately. The draining state answers
any in-flight WriteCommands with CommandFailed and stops after a short
idle timeout (100 ms).
while still delivering the close event to a pending write's commander if
it differs from the connection handler.