From efdbd45f844e8ae66b0d6d8b1a2c487f0aee692b Mon Sep 17 00:00:00 2001 From: Hengfei Yang Date: Fri, 21 Aug 2026 13:31:02 +0000 Subject: [PATCH] fix(search): clamp auto histogram interval to 1s for sub-10s ranges (#13916) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What For time ranges narrower than 10 seconds, the auto-generated histogram interval now drops to `1 second` instead of the previous `10 second` floor. Implemented as a new `s` unit in the `intervals!` ladder in `generate_histogram_interval` plus a `(s, 10, "10 second")` tier, with the fall-through returning `1 second`. All tiers from 10s upward are unchanged, and intervals written explicitly in user SQL are untouched. ## Why Fixes #13896. With the 10s floor, a 1-second query range got a bucket wider than the whole range. The scan-start snap to the bucket boundary (`histogram_bucket_start`, introduced by #12312 to keep the first chart bucket complete) then pulled in up to 10s of data from before the selected range, so histogram bucket counts dwarfed the log search total — the UI event counter and pagination are derived from the bucket sum, producing e.g. "1 to 50 out of 195 events" with only 8 real rows and empty trailing pages. With 1s buckets, whole-second range starts sit exactly on the bucket grid, so the snap no longer widens the scan and the bucket sum matches the log search total. ## Verification - Unit tests added for the 1s/10s boundary; full `search` crate suite passes. - E2E reproduction (single node, 20 events/sec ingested, 1s mid-bucket range `[hh:mm:s4, hh:mm:s5)`): - before: log search `total: 20`, histogram bucket sum **100** (follower plan showed `FilterExec: _timestamp >= …s0` — start snapped back 4s) - after: log search `total: 20`, histogram bucket sum **20**, `FilterExec` covers exactly the requested 1s range ## Notes for reviewers - The snap behavior from #12312 itself is intentionally left untouched; this fix only ensures the derived bucket is never wider than the range. - The clamp is deliberately **not** added to `validate_and_adjust_histogram_interval`: that path validates caller-supplied intervals (explicit SQL intervals, and the per-partition re-validation of the streaming pre-computed interval), where range-dependence would make partitions disagree — see `test_preset_interval_is_consistent_across_partitions`. - Millisecond-precision range starts (relative time pickers) can still be snapped back by up to 1s; that residue is bounded by one 1s bucket versus up to 10s before. --- .../src/sql/visitor/histogram_interval.rs | 26 ++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/src/search/src/sql/visitor/histogram_interval.rs b/src/search/src/sql/visitor/histogram_interval.rs index 8d21bfcf0d..70e455ed5f 100644 --- a/src/search/src/sql/visitor/histogram_interval.rs +++ b/src/search/src/sql/visitor/histogram_interval.rs @@ -87,6 +87,7 @@ macro_rules! intervals { (@unit h) => { chrono::Duration::hours }; (@unit m) => { chrono::Duration::minutes }; + (@unit s) => { chrono::Duration::seconds }; } pub fn generate_histogram_interval(time_range: (i64, i64)) -> &'static str { @@ -106,7 +107,7 @@ pub fn generate_histogram_interval(time_range: (i64, i64)) -> &'static str { (h, 2, "1 minute"), (h, 1, "30 second"), (m, 30, "15 second"), - (m, 15, "10 second"), + (s, 10, "10 second"), ]; for (time, interval) in INTERVALS.iter() { @@ -114,7 +115,12 @@ pub fn generate_histogram_interval(time_range: (i64, i64)) -> &'static str { return interval; } } - "10 second" + // Ranges narrower than one 10s bucket drop to 1s buckets: a bucket wider + // than the whole range makes the scan-start snap (histogram_bucket_start) + // pull in up to 10s of data from before the range, so bucket counts dwarf + // the range total (#13896). With 1s buckets the snap widens the scan by + // less than a second (zero for whole-second range starts). + "1 second" } pub fn convert_histogram_interval_to_seconds(interval: &str) -> Result { @@ -526,12 +532,26 @@ mod tests { assert_eq!(generate_histogram_interval((0, 24 * 31 * hour)), "12 hour"); // > 24*28h but <= 24*30h → "6 hour" assert_eq!(generate_histogram_interval((0, 24 * 29 * hour)), "6 hour"); - // duration < 15 min → falls through all checks → "10 second" + // 10s <= duration < 30 min → "10 second" assert_eq!(generate_histogram_interval((0, 10 * minute)), "10 second"); // duration >= 30 min → "15 second" assert_eq!(generate_histogram_interval((0, 31 * minute)), "15 second"); } + #[test] + fn test_generate_histogram_interval_narrow_range() { + let second = 1_000_000_i64; + // a range narrower than the 10s floor drops to 1s buckets so the + // scan-start snap cannot widen the query past the range (#13896) + assert_eq!(generate_histogram_interval((0, second)), "1 second"); + assert_eq!( + generate_histogram_interval((0, 10 * second - 1)), + "1 second" + ); + // at exactly 10s the historical floor is kept + assert_eq!(generate_histogram_interval((0, 10 * second)), "10 second"); + } + #[test] fn test_convert_histogram_interval_abbreviations() { // Test single-letter and short abbreviations