Skip to content

Commit 2b9da23

Browse files
Fix off-by-one that leaves one stale point in the graph buffer (#580)
drain(0..idx) skips the element at idx, but idx is the index of the last point that's actually older than the buffer window (it matched the filter right above). So every update() call quietly leaves exactly one out-of-window sample sitting in the data, forever. Switched it to drain(0..=idx) and added a test that seeds a few stale/fresh points and checks nothing older than the window survives after update().
1 parent 443c73d commit 2b9da23

1 file changed

Lines changed: 35 additions & 1 deletion

File tree

gping/src/plot_data.rs

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,10 @@ impl PlotData {
4545
.map(|(idx, _)| idx)
4646
.next_back();
4747
if let Some(idx) = last_idx {
48-
self.data.drain(0..idx).for_each(drop)
48+
// `idx` itself is still stale (it matched the filter above), so it must be
49+
// included in the drained range too, otherwise one out-of-window point is
50+
// always left behind.
51+
self.data.drain(0..=idx).for_each(drop)
4952
}
5053
}
5154

@@ -139,6 +142,37 @@ impl<'a> From<&'a PlotData> for Dataset<'a> {
139142
mod tests {
140143
use super::*;
141144

145+
// Regression test for the buffer trim in `PlotData::update`: every point older than
146+
// `buffer` seconds should be dropped, including the single oldest stale point, which
147+
// used to survive because `drain(0..idx)` excluded the boundary index itself.
148+
#[test]
149+
fn update_drops_all_points_outside_the_buffer_window() {
150+
let buffer_secs = 5u64;
151+
let mut plot = PlotData::new("host".to_string(), buffer_secs, Style::default(), false);
152+
153+
let now = Local::now().timestamp_millis() as f64 / 1_000f64;
154+
155+
// Seed with a mix of stale (older than the buffer window) and fresh points,
156+
// pushed in ascending timestamp order like the real update loop would.
157+
plot.data.push((now - 10.0, 100.0)); // stale
158+
plot.data.push((now - 8.0, 200.0)); // stale
159+
plot.data.push((now - 6.0, 300.0)); // stale, closest to the boundary
160+
plot.data.push((now - 2.0, 400.0)); // fresh
161+
plot.data.push((now - 1.0, 500.0)); // fresh
162+
163+
// Triggers the trim logic; also appends one brand-new point.
164+
plot.update(Some(Duration::from_millis(42)));
165+
166+
let earliest_allowed = now - buffer_secs as f64;
167+
for &(timestamp, _) in &plot.data {
168+
assert!(
169+
timestamp >= earliest_allowed,
170+
"found a point at {timestamp}, which is older than the buffer window start {earliest_allowed}; data: {:?}",
171+
plot.data
172+
);
173+
}
174+
}
175+
142176
#[test]
143177
fn test_jitter_uses_chronological_order() {
144178
// Oscillating latencies: sorted-order "jitter" would telescope down to

0 commit comments

Comments
 (0)