mirror of
https://github.com/openobserve/openobserve.git
synced 2026-11-06 00:09:22 +00:00
fix(search): clamp auto histogram interval to 1s for sub-10s ranges (#13916)
## 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.
This commit is contained in:
@@ -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<i64, Error> {
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user