fix(dogstatsd): decouple datagram reads from decoding and increase buffer limit to 256MiB - #2079
Conversation
Binary Size Analysis (Agent Data Plane)Baseline: 42b3b21 · Comparison: 94cad96 · diff ✅ Binary size difference within thresholdChanges by Module
Detailed Symbol Changes |
This comment has been minimized.
This comment has been minimized.
Regression Detector (Agent Data Plane)Run ID: Optimization Goals: ❌ 3 regressions detected
Fine details of change detection per experiment (2)Experiments configured
Bounds Checks: ✅ Passed (5)
ExplanationA change is flagged as a regression when |Δ mean %| > 5.00% in the regressing direction for its optimization goal AND SMP marks the experiment as a regression ( |
webern
left a comment
There was a problem hiding this comment.
This stood up to scrutiny...
Some interesting findings:
Potential future performance work, only if profiling justifies it:
- recvmmsg to receive multiple datagrams per syscall,
Why run the new reader/channel/oneshot architecture for connection-oriented streams when it provides no pipelining? Is the added unbounded per-connection task/channel overhead acceptable, or should connection-oriented streams retain a specialized inline path?
|
dadp150-pr2079-benchmark-bundle.zip Modified benchmark script |
d6f7ac4 to
fa4e191
Compare
There was a problem hiding this comment.
Connectionless readers no longer retain their promised reserved buffer while idle. A set of connection-oriented clients that leave partial frames open can consume the shared pool ceiling, preventing UDP/UDS datagram reads and causing the packet loss this change is intended to reduce.
🤖 Datadog Autotest · Commit fa4e191 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
|
|
||
| let mut buffer = select! { | ||
| _ = datagram_sender.closed() => break, | ||
| buffer = buffer_manager.take_buffer() => buffer, |
There was a problem hiding this comment.
Reserve buffers for connectionless readers
Mixed UDP/UDS-datagram and stream deployments can lose all connectionless DogStatsD traffic under partial-frame stream pressure.
Assertion details
- Input: A deployment enables a UDP or Unix-datagram listener alongside a TCP or Unix-stream listener and has its pool ceiling occupied by connection-oriented clients that each send an unterminated partial frame.
- Expected:
Each connectionless listener retains at least one pool slot, so it can continue draining its socket regardless of stalled connected streams. - Actual: After an idle datagram read completes, its buffer is returned to the shared pool. Connected readers can hold every buffer while awaiting more bytes, so the next datagram read blocks at
take_bufferand the kernel socket queue eventually drops packets.
Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
There was a problem hiding this comment.
With the default 32,768 buffers, this would require roughly 32,000 stalled connections so it's unlikely enough that I don’t consider it a blocker.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa4e1919d5
ℹ️ 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".
| let queued = QueuedDatagram { | ||
| result, | ||
| socket_context: socket_context.clone(), | ||
| }; | ||
| if datagram_sender.send(queued).await.is_err() { |
There was a problem hiding this comment.
Resolve UDS datagram origins before queueing
When UDS datagram origin detection or traffic capture is enabled, this queues only the payload plus peer_addr credentials and defers capture_uds_traffic/handle_frame until a decoder worker runs. Under bursts, a short-lived sender can exit while its packet is waiting in this channel; the later live-PID lookup can then lose or misattribute the container/origin tags even though the kernel supplied the PID at receive time. Resolve/cache the origin or capture container ID in the reader before enqueueing so queued datagrams do not depend on the sender still being alive.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Resolved. The socket reader now resolves and pins the process origin before enqueueing, and decoder/capture paths use that pinned entity. Added a PID-reuse regression test.
33341fa to
19e6ce9
Compare
DogStatsD RSS Comparison (Core Agent vs ADP)Run ID: Optimization Goals: ✅ No significant changes detectedFine details of change detection per experiment (5)Experiments configured
ExplanationA change is flagged as a regression when |Δ mean %| > 5.00% in the regressing direction for its optimization goal AND SMP marks the experiment as a regression ( |
Resolve and pin UDS sender origins before deferred decoding only when socket origin detection or traffic capture needs them. Cache unresolved PID lookups and make capture-state checks lock-free so the default datagram receive path stays fast. Also remove listener attribution from shared event-dispatch logs because one decoder buffer can contain events from multiple listeners.
Use 26,624 receive buffers by default, providing 208 MiB of payload capacity at the default 8 KiB buffer size. This is the smallest locally tested ceiling that consistently matched or outperformed the Core Agent after the UDS origin-resolution fix.
The traffic-capture origin test is cross-platform, but its yield_now import was gated to Unix alongside Unix socket types. Import it unconditionally so Windows unit tests compile.
Restore the default maximum to 32,768 receive buffers, providing 256 MiB of payload capacity at the default 8 KiB buffer size and matching the Core Agent default.
Raise the steady-load RSS bounds for the larger burst buffer and remove the temporary Core Agent comparison suite after collecting its data.
7ebc754
into
main
…ffer limit to 256MiB (#2079) ## Summary [Issue](https://datadoghq.atlassian.net/browse/DADP-150) Improve ADP's UDP and UDS datagram burst handling. Socket reads were coupled to decoding, and ADP's default elastic buffer ceiling was about 2 MiB and raised to match the Core Agent's default of roughly 256 MiB. This PR: - Queues connectionless socket reads in a bounded global buffer pool and decodes them in parallel. - Uses a one-item acknowledged channel for connected streams, preserving order without building a backlog. - Supports `dogstatsd_workers_count`. When unset or zero, ADP uses `max(vCPUs - 2, 2)`, matching the Core Agent's default formula. This count configures one global decoder worker pool shared by all connectionless UDP and UDS listeners. Connected streams continue to use one decoder per connection. - Raises the elastic ceiling from 256 to 32,768 buffers of about 8 KiB each—approximately 2 MiB to 256 MiB—while keeping the initial allocation at 128 buffers. - Shares the elastic buffer pool across connectionless listeners and connected readers. Connectionless datagrams wait in the bounded global queue, while connected streams use one-item acknowledged channels. - Reclaims excess idle buffers after bursts while retaining a small warm pool. ## Benchmark ### Original benchmark: baseline ADP The original DADP-150 benchmark used the pre-PR ADP behavior: sequential decoding with the original ~2 MiB buffer ceiling. It ran for 180 seconds with 60 clients, estimated 32 MiB bursts, client aggregation, and background sending. It reported `bytes_dropped_writer / (bytes_dropped_writer + bytes_sent)`. | Burst interval | Core datagram | Core stream | ADP datagram | ADP stream | |---|---:|---:|---:|---:| | 15s | 3.9% | 0.5% | 13.9% | 3.9% | | 30s | 5.1% | 0.5% | 9.7% | 2.2% | | 60s | 4.4% | 0.6% | 17.9% | 6.2% | ### Original benchmark: decoupled reads from decoding The original benchmark was rerun on four CPUs against the Core Agent, baseline ADP, and an early PR build that decoupled socket reads from decoding but retained one decoder and the original ~2 MiB buffer ceiling. Datagram results were: | Burst interval | Core Agent | Baseline ADP | Decoupled reads from decoding (~2 MiB) | |---|---:|---:|---:| | 15s | 11.4% | 59.8% | 52.7% | | 30s | 9.2% | 51.5% | 48.5% | | 60s | 14.2% | 52.8% | 51.6% | | Unweighted mean | 11.6% | 54.7% | 50.9% | So decoupling socket reads from decoding was an incremental change but did not close the gap between ADP and the agent ### Original benchmark: parallel decoding with larger buffers The original benchmark was rerun on four CPUs against the Core Agent, baseline ADP, and a PR build that combined parallel decoding with the increased ~256 MiB buffer ceiling. Datagram results were: | Burst interval | Core Agent | Baseline ADP | Parallel decoding + ~256 MiB | |---|---:|---:|---:| | 15s | 41.8% | 82.5% | 6.8% | | 30s | 20.9% | 48.5% | 4.1% | | 60s | 28.3% | 48.0% | 7.7% | | Unweighted mean | 30.3% | 59.7% | 6.2% | The larger buffer produced a substantial improvement in this run. However, five of the nine final counter snapshots were mathematically inconsistent, and the 15s PR result required one retry after the first run produced no telemetry. These results are directional evidence only. ### Why the benchmark changed The benchmark was changed because: - It was time-based, so contenders could complete different amounts of work. In the 30s case, [the original logs](https://github.com/user-attachments/files/30310804/dadp150-pr2079-benchmark-bundle.zip) captured 45,982,301 attempted bytes for ADP versus 36,695,095 for Core—25.3% more traffic during the same 180-second window. Attempted bytes are calculated as `bytes_sent + bytes_dropped_writer`. - The [original collector and logs](https://github.com/user-attachments/files/30310804/dadp150-pr2079-benchmark-bundle.zip) queried counters sequentially while traffic continued, producing non-atomic snapshots. One sample reported `bytes_dropped=2,541,636` below `bytes_dropped_writer=2,762,874`, which violates [the counter definition](https://github.com/DataDog/datadogpy/blob/v0.52.1/datadog/dogstatsd/base.py#L1275-L1277). - The [original collector](https://github.com/user-attachments/files/30310804/dadp150-pr2079-benchmark-bundle.zip) forcibly terminated workers without a drain step or verifying that their background queues were empty. - Each case ran once, so variance was unknown. ### Fixed workload: parallel decoding with larger buffers The revised benchmark tested parallel decoding with the buffer ceiling increased from ~2 MiB to ~256 MiB. It used a fixed workload instead of a fixed duration. Each contender received the same load: 60 clients sending three 32 MiB bursts over UDS datagrams, with four CPUs, client aggregation, and background sending. After the workload completed, client queues were drained before lifetime counters were collected. The reported percentage is the share of attempted packet bytes that clients failed to send. These failures are consistent with receiver backpressure, although the harness does not capture the specific socket errors. | Burst interval | Core Agent | Parallel decoding + ~256 MiB | Core elapsed | ADP elapsed | |---|---:|---:|---:|---:| | 15s | 18.1% | 8.7% | 43.7s | 42.6s | | 30s | 8.6% | 2.5% | 70.7s | 70.6s | | 60s | 8.7% | 2.6% | 130.5s | 130.2s | | Unweighted mean | 11.8% | 4.6% | — | — | With the fixed-workload harness, parallel decoding with a ~32 MiB ceiling averaged 39.8%. Increasing the ceiling to ~256 MiB reduced the average to 4.6%. Each case was run once, so variance remains unknown. These fixed-workload results were produced at commit [`b0a95a65`](b0a95a6), before the [idle-buffer reclamation change](c04f48f). They are directional evidence, not direct validation of the current head. [Download the benchmark scripts, configuration, pinned dependencies, logs, and JSON summaries](https://github.com/user-attachments/files/30310804/dadp150-pr2079-benchmark-bundle.zip). ## Memory trade-off Decoupling socket reads from decoding lets ADP continue draining the socket instead of making senders wait for capacity. This moves more of the backlog into ADP's userspace buffer pool, trading higher memory usage for less backpressure and fewer packet drops during bursts. The pool allocates 128 buffers initially and can grow on demand to 32,768 buffers, about 256 MiB of payload backing capacity. After five seconds without growth, it shrinks toward 256 idle buffers, so the full ceiling is not normally retained. The [performance report](#2079 (comment)) shows the expected trade-off: medium-load RSS increased 22.31% to 83.3 MiB, above its 75 MiB bound, while low and idle loads remained within bounds. To test whether reduced backpressure explains the higher RSS, three isolated three-minute `quality_gates_rss_dsd_medium` runs measured Lading's cumulative [`send().await` duration](https://github.com/DataDog/lading/blob/ed99f5f1329ce90511b579129afa15dd89570544/lading/src/generator/unix_datagram.rs#L283-L294). Lading waits for the UDS socket to accept the datagram, while the [original benchmark client](https://github.com/user-attachments/files/30310804/dadp150-pr2079-benchmark-bundle.zip) used 10 ms socket and queue timeouts that could turn backpressure into counted drops. Longer send duration therefore indicates more sender backpressure. Each run sent about 487,000 packets. | Case | Buffer ceiling | Send duration | Average RSS | Average pool size | |---|---:|---:|---:|---:| | Baseline ADP (sequential decoding) | ~2 MiB | 5.91s | 58.9 MiB | ~1.0 MiB | | Parallel decoding only | ~2 MiB | 5.34s | 61.3 MiB | ~2.0 MiB | | Parallel decoding + larger buffer | ~256 MiB | 4.78s | 67.0 MiB | ~5.1 MiB | Compared with baseline ADP, parallel decoding with the ~256 MiB ceiling spent 1.13 seconds less in `send().await` and used 8.0 MiB more RSS. Running the same parallel-decoding build with the original ~2 MiB ceiling increased send duration by 0.57 seconds and reduced RSS by 5.7 MiB. This supports the explanation that reduced socket backpressure shifts backlog into ADP memory. Parallel decoding alone remained 2.3 MiB above baseline, so other factors also contribute. Each case was run once, so variance is unknown. ## Correctness test harness Decoupled datagram processing can leave packets [queued for separate decoder workers](https://github.com/DataDog/saluki/blob/278ff0c470a7c21773563e98bfdbb2299423e378/lib/saluki-components/src/sources/dogstatsd/mod.rs#L1455-L1472). Origin enrichment [uses the sender PID from UDS credentials](https://github.com/DataDog/saluki/blob/278ff0c470a7c21773563e98bfdbb2299423e378/lib/saluki-components/src/sources/dogstatsd/mod.rs#L2111-L2120), but the harness previously [exited Millstone before flushing and collecting](https://github.com/DataDog/saluki/blob/c04f48fb4ff74a8d51bb99da5f0e2660e69bfd6a/bin/correctness/panoramic/src/correctness/runner.rs#L438-L469). It now [reports completion while keeping Millstone alive](https://github.com/DataDog/saluki/blob/278ff0c470a7c21773563e98bfdbb2299423e378/bin/correctness/millstone/src/main.rs#L75-L105) until collection finishes. This prevents the sender-PID lifecycle race without changing the workload or assertions. ## Validation - `make fmt` - 139 DogStatsD tests passed - `cargo check --workspace` - `cargo check --workspace --tests` - `make check-all` Co-authored-by: andrew.qian <andrew.qian@datadoghq.com> 7ebc754
Summary
Issue
Improve ADP's UDP and UDS datagram burst handling. Socket reads were coupled to decoding, and ADP's default elastic buffer ceiling was about 2 MiB and raised to match the Core Agent's default of roughly 256 MiB.
This PR:
dogstatsd_workers_count. When unset or zero, ADP usesmax(vCPUs - 2, 2), matching the Core Agent's default formula. This count configures one global decoder worker pool shared by all connectionless UDP and UDS listeners. Connected streams continue to use one decoder per connection.Benchmark
Original benchmark: baseline ADP
The original DADP-150 benchmark used the pre-PR ADP behavior: sequential decoding with the original ~2 MiB buffer ceiling. It ran for 180 seconds with 60 clients, estimated 32 MiB bursts, client aggregation, and background sending. It reported
bytes_dropped_writer / (bytes_dropped_writer + bytes_sent).Original benchmark: decoupled reads from decoding
The original benchmark was rerun on four CPUs against the Core Agent, baseline ADP, and an early PR build that decoupled socket reads from decoding but retained one decoder and the original ~2 MiB buffer ceiling. Datagram results were:
So decoupling socket reads from decoding was an incremental change but did not close the gap between ADP and the agent
Original benchmark: parallel decoding with larger buffers
The original benchmark was rerun on four CPUs against the Core Agent, baseline ADP, and a PR build that combined parallel decoding with the increased ~256 MiB buffer ceiling. Datagram results were:
The larger buffer produced a substantial improvement in this run. However, five of the nine final counter snapshots were mathematically inconsistent, and the 15s PR result required one retry after the first run produced no telemetry. These results are directional evidence only.
Why the benchmark changed
The benchmark was changed because:
bytes_sent + bytes_dropped_writer.bytes_dropped=2,541,636belowbytes_dropped_writer=2,762,874, which violates the counter definition.Fixed workload: parallel decoding with larger buffers
The revised benchmark tested parallel decoding with the buffer ceiling increased from ~2 MiB to ~256 MiB. It used a fixed workload instead of a fixed duration. Each contender received the same load: 60 clients sending three 32 MiB bursts over UDS datagrams, with four CPUs, client aggregation, and background sending. After the workload completed, client queues were drained before lifetime counters were collected.
The reported percentage is the share of attempted packet bytes that clients failed to send. These failures are consistent with receiver backpressure, although the harness does not capture the specific socket errors.
With the fixed-workload harness, parallel decoding with a ~32 MiB ceiling averaged 39.8%. Increasing the ceiling to ~256 MiB reduced the average to 4.6%. Each case was run once, so variance remains unknown.
These fixed-workload results were produced at commit
b0a95a65, before the idle-buffer reclamation change. They are directional evidence, not direct validation of the current head.Download the benchmark scripts, configuration, pinned dependencies, logs, and JSON summaries.
Memory trade-off
Decoupling socket reads from decoding lets ADP continue draining the socket instead of making senders wait for capacity. This moves more of the backlog into ADP's userspace buffer pool, trading higher memory usage for less backpressure and fewer packet drops during bursts.
The pool allocates 128 buffers initially and can grow on demand to 32,768 buffers, about 256 MiB of payload backing capacity. After five seconds without growth, it shrinks toward 256 idle buffers, so the full ceiling is not normally retained. The performance report shows the expected trade-off: medium-load RSS increased 22.31% to 83.3 MiB, above its 75 MiB bound, while low and idle loads remained within bounds.
To test whether reduced backpressure explains the higher RSS, three isolated three-minute
quality_gates_rss_dsd_mediumruns measured Lading's cumulativesend().awaitduration. Lading waits for the UDS socket to accept the datagram, while the original benchmark client used 10 ms socket and queue timeouts that could turn backpressure into counted drops. Longer send duration therefore indicates more sender backpressure. Each run sent about 487,000 packets.Compared with baseline ADP, parallel decoding with the ~256 MiB ceiling spent 1.13 seconds less in
send().awaitand used 8.0 MiB more RSS. Running the same parallel-decoding build with the original ~2 MiB ceiling increased send duration by 0.57 seconds and reduced RSS by 5.7 MiB. This supports the explanation that reduced socket backpressure shifts backlog into ADP memory. Parallel decoding alone remained 2.3 MiB above baseline, so other factors also contribute. Each case was run once, so variance is unknown.Correctness test harness
Decoupled datagram processing can leave packets queued for separate decoder workers. Origin enrichment uses the sender PID from UDS credentials, but the harness previously exited Millstone before flushing and collecting. It now reports completion while keeping Millstone alive until collection finishes. This prevents the sender-PID lifecycle race without changing the workload or assertions.
Validation
make fmtcargo check --workspacecargo check --workspace --testsmake check-all