Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions crates/memtrack/src/ebpf/memtrack/maps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ impl MemtrackBpf {
))
}

/// Callback that resumes every pressure-stopped process.
pub(super) fn on_ring_drained(&self) -> Box<dyn Fn() + Send> {
/// Callback that resumes pressure-stopped processes at the low watermark.
pub(super) fn on_ring_low_fill(&self) -> Box<dyn Fn() + Send> {
let stopped = self.stopped.clone();
Box::new(move || {
if let Err(error) = stopped.release_pressure() {
Expand Down
2 changes: 1 addition & 1 deletion crates/memtrack/src/ebpf/memtrack/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ impl MemtrackBpf {
resolve,
tx,
poll_interval_ms,
Some(self.on_ring_drained()),
Some(self.on_ring_low_fill()),
))
}

Expand Down
41 changes: 28 additions & 13 deletions crates/memtrack/src/ebpf/poller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ fn consume_all(ringbuf: &RingBuffer, ring: *mut libbpf_sys::ring) {
}
}

/// Records read per regular-tick chunk. Bounds the work between fill checks.
const CONSUME_CHUNK_RECORDS: usize = BATCH_ITEMS;

fn poll_iteration<T>(
control: std::result::Result<Sender<()>, RecvTimeoutError>,
consume: impl FnOnce(),
Expand Down Expand Up @@ -72,8 +75,9 @@ fn poll_iteration<T>(
/// Polls a BPF ring buffer in a background thread, parsing raw entries with a
/// user-supplied closure and forwarding them to an mpsc channel in batches.
///
/// The poll thread runs until the poller is dropped, doing a final full
/// `consume()` on shutdown so no buffered entries are lost.
/// Regular ticks consume bounded chunks and check the low-fill release
/// watermark between chunks. Shutdown performs a final full `consume()` so no
/// buffered entries are lost.
pub struct RingBufferPoller {
ctl: Option<Sender<Sender<()>>>,
poll_thread: Option<JoinHandle<()>>,
Expand All @@ -85,7 +89,7 @@ impl RingBufferPoller {
parse: F,
tx: Sender<Vec<T>>,
poll_interval_ms: u64,
on_drained: Option<Box<dyn Fn() + Send>>,
on_low_fill: Option<Box<dyn Fn() + Send>>,
) -> Result<Self>
where
M: MapCore,
Expand Down Expand Up @@ -123,23 +127,34 @@ impl RingBufferPoller {
// SAFETY: the built `RingBuffer` holds exactly the one ring added above.
let ring =
unsafe { libbpf_sys::ring_buffer__ring(ringbuf.as_libbpf_object().as_ptr(), 0) };
// Resume below 1/4 fill; BPF stops at 3/4, which leaves hysteresis.
let release_if_low = || {
if let Some(on_low_fill) = &on_low_fill
&& unsafe { libbpf_sys::ring__avail_data_size(ring) }
< unsafe { libbpf_sys::ring__size(ring) } / 4
{
on_low_fill();
}
};
while poll_iteration(
ctl_rx.recv_timeout(Duration::from_millis(poll_interval_ms)),
|| consume_all(&ringbuf, ring),
|| {
let _ = ringbuf.poll(Duration::ZERO);
// A short or failed chunk ends the tick; the loop body below
// checks the fill after it.
while ringbuf.consume_raw_n(CONSUME_CHUNK_RECORDS)
== CONSUME_CHUNK_RECORDS as i32
{
release_if_low();
}
Comment on lines +145 to +149

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Full chunks delay drain requests If producers keep supplying at least 1,024 records per read, this loop keeps consuming without checking the control channel. The attach worker can request a synchronous drain() while processes are still running, but its request cannot be acknowledged until the loop ends, so it can block indefinitely. Limit the work per tick or check for control requests between chunks.

Knowledge Base Used: eBPF memory tracker

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/memtrack/src/ebpf/poller.rs
Line: 145-149

Comment:
**Full chunks delay drain requests** If producers keep supplying at least 1,024 records per read, this loop keeps consuming without checking the control channel. The attach worker can request a synchronous `drain()` while processes are still running, but its request cannot be acknowledged until the loop ends, so it can block indefinitely. Limit the work per tick or check for control requests between chunks.

**Knowledge Base Used:** [eBPF memory tracker](https://app.greptile.com/codspeed/-/custom-context/knowledge-base/codspeedhq/codspeed/-/docs/ebpf-memory-tracker.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code Fix in Codex

},
&batch,
&tx,
) {
if let Some(on_drained) = &on_drained
&& unsafe { libbpf_sys::ring__avail_data_size(ring) } == 0
{
on_drained();
}
release_if_low();
}
if let Some(on_drained) = &on_drained {
on_drained();
if let Some(on_low_fill) = &on_low_fill {
on_low_fill();
}
});

Expand Down Expand Up @@ -192,7 +207,7 @@ impl ThreadedRingBufferPoller {
resolve: R,
tx: Sender<Vec<U>>,
poll_interval_ms: u64,
on_drained: Option<Box<dyn Fn() + Send>>,
on_low_fill: Option<Box<dyn Fn() + Send>>,
) -> Result<Self>
where
M: MapCore,
Expand All @@ -202,7 +217,7 @@ impl ThreadedRingBufferPoller {
R: Fn(T) -> U + Send + 'static,
{
let (parsed_tx, parsed_rx) = mpsc::channel::<Vec<T>>();
let ring = RingBufferPoller::new(rb_map, parse, parsed_tx, poll_interval_ms, on_drained)?;
let ring = RingBufferPoller::new(rb_map, parse, parsed_tx, poll_interval_ms, on_low_fill)?;
let resolver = std::thread::spawn(move || {
for batch in parsed_rx {
let resolved = batch.into_iter().map(&resolve).collect();
Expand Down
Loading