Repository navigation
SLES-2913: prevent named pipe failures from eating the whole thread pool - #231
apiarian-datadog merged 6 commits into
Conversation
|
This will also hopefully take care of #219 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2fc929af02
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Let's hope this fixes #219 🤞 |
|
validated with https://github.com/DataDog/serverless-examples/pull/205 . works in isolation and in combination. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds safeguards to prevent runaway blocking/overlapping behavior in named-pipe sending and telemetry flushing, with new tests to validate the intended throttling and non-overlap behavior.
Changes:
- Add a cooldown gate to
NamedPipeTransportto fail fast after send/connect/write failures. - Change telemetry timer to one-shot re-arming to prevent overlapping flushes and thread pool growth.
- Add unit tests covering named pipe cooldown behavior and telemetry flush overlap/dispose scenarios.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| tests/StatsdClient.Tests/Transport/NamedPipeTransportTests.cs | Adds timing-based tests validating cooldown avoids repeated blocking on connect/write failures. |
| tests/StatsdClient.Tests/TelemetryTests.cs | Adds tests asserting flush callbacks don’t overlap and dispose doesn’t deadlock during an in-flight flush. |
| src/StatsdClient/Transport/NamedPipeTransport.cs | Implements failure cooldown gating and resets cooldown on successful send. |
| src/StatsdClient/Telemetry.cs | Switches to one-shot timer re-arm pattern with locking + disposed flag to prevent overlapping flushes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
c25b94a
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5631eca0d3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
## Release 9.2.1 Changelog and version bump for the 9.2.1 NuGet release. ### Included changes * [BUGFIX] Prevent named pipe write failures from causing unbounded thread growth. The telemetry timer now reschedules itself after each round instead of spawning a new thread every 10s, and backs off after send failures. See #231. ### Checklist - [x] Changelog entries are accurate - [x] Version numbers are correct - [x] PR approved and ready to merge Once merged, run `/publish-release` to tag, build, and push to NuGet.
dogstatsdin Azure App Services with the Windows Extension sometimes fail to correctly get a named pipe. This can happen when the app is hot-reloaded and the previous incarnation ofdogstatsdhasn't released the named pipe when the newdogstatsdis starting up. This causes the named pipe writes from the dogstatsd client to fail (permanently, until the app is restarted again).These failures cause a steady growth of threads as the telemetry timer spawns one every 10 seconds to try to write client telemetry, and the 2 second timeout for each of the 11 stats that the telemetry wants to send compounds to 22 seconds of work. Forever.
This change turns the telemetry timer into one that reschedules itself after every round, instead of spawning one every 10 seconds. It also adds a cooldown for sending telemetry, so that if there's a failure to send, we back off for a bit before trying to send any more.
This is one of a few different patches to address this issue. We also have DataDog/dd-trace-dotnet#8874 for the app service extension process manager and DataDog/datadog-agent#53328 for dogstatsd.