mirror of
https://github.com/GreptimeTeam/greptimedb.git
synced 2026-10-19 10:22:31 +00:00
refactor: simplify nullable_wave fixture and remove redundant test machinery
- Make null_every NonZeroUsize; remove zero-disable semantics.
- Remove false SST-alignment restriction; test cross-SST with unaligned boundary.
- Replace duplicate sorting oracle with compact NULL-position checks.
- Remove redundant increase perf query (same indexed counter work as rate).
- Trim repeated fixture explanation in case.toml and fixture-format.md.
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
(cherry picked from commit f5a333174a)
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
This commit is contained in:
discord9
@@ -549,17 +549,13 @@ pub(super) enum Distribution {
|
||||
#[serde(rename = "deterministic_wave")]
|
||||
DeterministicWave { min: f64, max: f64 },
|
||||
/// Wave values with periodic NULLs: every `null_every`-th sample of each
|
||||
/// series is NULL, and `0` writes no NULLs.
|
||||
///
|
||||
/// The cadence is counted in samples of the series rather than in generated
|
||||
/// rows, so every series carries the same null ratio on every layout; a row
|
||||
/// cadence stays constant per series whenever `series_count` is a multiple of
|
||||
/// `null_every`.
|
||||
/// series is NULL. The cadence is counted in samples of the series rather
|
||||
/// than in generated rows.
|
||||
#[serde(rename = "nullable_wave")]
|
||||
NullableWave {
|
||||
min: f64,
|
||||
max: f64,
|
||||
null_every: usize,
|
||||
null_every: NonZeroUsize,
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
@@ -266,7 +266,7 @@ fn generate_record_batch(
|
||||
}) => {
|
||||
columns.push(Arc::new(Float64Array::from_iter((0..rows).map(|r| {
|
||||
let sample_index = series_sample_index(layout, sst_idx, base_row, r);
|
||||
if *null_every > 0 && sample_index.is_multiple_of(*null_every) {
|
||||
if sample_index.is_multiple_of(null_every.get()) {
|
||||
None
|
||||
} else {
|
||||
Some(wave_value(*min, *max, base_row + r))
|
||||
@@ -774,96 +774,27 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// The nullable range-function case must write NULL on every fourth sample of
|
||||
/// every series and wave values elsewhere, while its dense control table stays
|
||||
/// NULL-free.
|
||||
/// The nullable range-function case keeps per-series NULL positions and wave
|
||||
/// values distinct from its dense control.
|
||||
#[test]
|
||||
fn nullable_wave_nulls_one_sample_in_every_four_per_series() {
|
||||
let case: CaseFile = toml::from_str(include_str!(
|
||||
fn nullable_wave_keeps_null_positions_and_dense_control() {
|
||||
let mut case: CaseFile = toml::from_str(include_str!(
|
||||
"../../../../../tests/perf/query_cases/promql_nullable_range_fn/case.toml"
|
||||
))
|
||||
.expect("nullable range-function case should parse");
|
||||
let scenario = match &case.scenario {
|
||||
let scenario = match &mut case.scenario {
|
||||
Scenario::DirectReadableSst(scenario) => scenario,
|
||||
_ => unreachable!("nullable range-function case is a direct SST case"),
|
||||
};
|
||||
assert_eq!(2, scenario.tables.len());
|
||||
scenario.layout.series_count = NonZeroUsize::new(2).expect("2 is nonzero");
|
||||
scenario.layout.rows_per_sst = 8;
|
||||
scenario.layout.sst_count = 1;
|
||||
|
||||
let sst_idx = 3;
|
||||
let sequence = 1003;
|
||||
let base_row = sst_idx * scenario.layout.rows_per_sst;
|
||||
let series_count = scenario.layout.series_count.get();
|
||||
let samples_per_series = scenario.layout.rows_per_sst / series_count;
|
||||
for table in &scenario.tables {
|
||||
let metadata = Arc::new(build_region_metadata(table, RegionId::from(43)));
|
||||
let batch =
|
||||
generate_record_batch(table, &metadata, &scenario.layout, sst_idx, sequence);
|
||||
let value = table
|
||||
.columns
|
||||
.iter()
|
||||
.find(|column| column.name == "value")
|
||||
.expect("fixture case has a value field");
|
||||
let (min, max, null_every) = match value.distribution.as_ref() {
|
||||
Some(Distribution::NullableWave {
|
||||
min,
|
||||
max,
|
||||
null_every,
|
||||
}) => (*min, *max, *null_every),
|
||||
Some(Distribution::DeterministicWave { min, max }) => (*min, *max, 0),
|
||||
other => panic!("unexpected value distribution in {}: {other:?}", table.name),
|
||||
};
|
||||
let host = table
|
||||
.columns
|
||||
.iter()
|
||||
.find(|column| column.name == "host")
|
||||
.expect("fixture case has a host tag");
|
||||
let instance = table
|
||||
.columns
|
||||
.iter()
|
||||
.find(|column| column.name == "instance")
|
||||
.expect("fixture case has an instance tag");
|
||||
|
||||
let mut expected = (0..scenario.layout.rows_per_sst)
|
||||
.map(|row| {
|
||||
let series = series_for_row(&scenario.layout, sst_idx, base_row, row);
|
||||
let mut tags = HashMap::new();
|
||||
tags.insert(host.name.clone(), tag_value(host, series));
|
||||
tags.insert(instance.name.clone(), tag_value(instance, series));
|
||||
let is_null = null_every > 0
|
||||
&& series_sample_index(&scenario.layout, sst_idx, base_row, row)
|
||||
.is_multiple_of(null_every);
|
||||
(
|
||||
encode_dense_primary_key(table, &tags),
|
||||
timestamp_for_row(&scenario.layout, base_row, row),
|
||||
sequence,
|
||||
tag_value(host, series),
|
||||
tag_value(instance, series),
|
||||
(!is_null).then(|| wave_value(min, max, base_row + row)),
|
||||
)
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
expected.sort_by(|left, right| {
|
||||
left.0
|
||||
.cmp(&right.0)
|
||||
.then_with(|| left.1.cmp(&right.1))
|
||||
.then_with(|| right.2.cmp(&left.2))
|
||||
});
|
||||
|
||||
let timestamps = batch
|
||||
.column(batch.num_columns() - 4)
|
||||
.as_any()
|
||||
.downcast_ref::<TimestampNanosecondArray>()
|
||||
.expect("nanosecond fixture timestamp")
|
||||
.values()
|
||||
.to_vec();
|
||||
let sequences = batch
|
||||
.column(batch.num_columns() - 2)
|
||||
.as_any()
|
||||
.downcast_ref::<UInt64Array>()
|
||||
.expect("fixture sequence column")
|
||||
.values()
|
||||
.to_vec();
|
||||
let hosts = string_values(&batch, 0);
|
||||
let instances = string_values(&batch, 1);
|
||||
let batch = generate_record_batch(table, &metadata, &scenario.layout, sst_idx, 1003);
|
||||
let values = batch
|
||||
.column(2)
|
||||
.as_any()
|
||||
@@ -871,48 +802,38 @@ mod tests {
|
||||
.expect("fixture value column")
|
||||
.iter()
|
||||
.collect::<Vec<Option<f64>>>();
|
||||
let actual = encoded_primary_keys(&batch)
|
||||
.into_iter()
|
||||
.zip(timestamps)
|
||||
.zip(sequences)
|
||||
.zip(hosts)
|
||||
.zip(instances)
|
||||
.zip(values.iter().cloned())
|
||||
.map(
|
||||
|(((((primary_key, timestamp), sequence), host), instance), value)| {
|
||||
(primary_key, timestamp, sequence, host, instance, value)
|
||||
},
|
||||
)
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(expected, actual, "{}", table.name);
|
||||
let (min, max, null_positions) = match table
|
||||
.columns
|
||||
.iter()
|
||||
.find(|column| column.name == "value")
|
||||
.expect("fixture case has a value field")
|
||||
.distribution
|
||||
.as_ref()
|
||||
{
|
||||
Some(Distribution::NullableWave { min, max, .. }) => (*min, *max, &[0][..]),
|
||||
Some(Distribution::DeterministicWave { min, max }) => (*min, *max, &[][..]),
|
||||
other => panic!("unexpected value distribution in {}: {other:?}", table.name),
|
||||
};
|
||||
|
||||
// Sorted by primary key, every series owns `samples_per_series`
|
||||
// consecutive rows ordered by timestamp, so its NULLs must sit on a
|
||||
// single residue class of its own sample positions.
|
||||
assert_eq!(series_count, values.len() / samples_per_series);
|
||||
for (chunk_index, chunk) in values.chunks(samples_per_series).enumerate() {
|
||||
if null_every == 0 {
|
||||
assert!(
|
||||
chunk.iter().all(Option::is_some),
|
||||
"{} series chunk {chunk_index} must stay dense",
|
||||
table.name
|
||||
);
|
||||
continue;
|
||||
}
|
||||
let residue = chunk.iter().position(Option::is_none).unwrap_or_else(|| {
|
||||
panic!("{} series chunk {chunk_index} has no NULL", table.name)
|
||||
});
|
||||
for (series, samples) in values.chunks(4).enumerate() {
|
||||
assert_eq!(
|
||||
samples_per_series / null_every,
|
||||
chunk.iter().filter(|value| value.is_none()).count(),
|
||||
"{} series chunk {chunk_index} NULL count",
|
||||
null_positions,
|
||||
samples
|
||||
.iter()
|
||||
.enumerate()
|
||||
.filter_map(|(position, value)| value.is_none().then_some(position))
|
||||
.collect::<Vec<_>>(),
|
||||
"{} series {series} NULL positions",
|
||||
table.name
|
||||
);
|
||||
for (position, value) in chunk.iter().enumerate() {
|
||||
for (sample, value) in samples.iter().enumerate() {
|
||||
if null_positions.contains(&sample) {
|
||||
continue;
|
||||
}
|
||||
assert_eq!(
|
||||
value.is_none(),
|
||||
position % null_every == residue,
|
||||
"{} series chunk {chunk_index} sample {position}",
|
||||
Some(wave_value(min, max, base_row + sample * 2 + series)),
|
||||
*value,
|
||||
"{} series {series} sample {sample} wave value",
|
||||
table.name
|
||||
);
|
||||
}
|
||||
@@ -920,19 +841,21 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// The case's window arithmetic counts the NULL slots of every window, so the
|
||||
/// cadence must also stay on one residue class of a series across SSTs, not
|
||||
/// only inside a single SST.
|
||||
/// NULL cadence must continue across unaligned SST boundaries rather than
|
||||
/// restarting in each SST.
|
||||
#[test]
|
||||
fn nullable_wave_keeps_one_null_residue_per_series_across_ssts() {
|
||||
let case: CaseFile = toml::from_str(include_str!(
|
||||
fn nullable_wave_continues_across_unaligned_ssts() {
|
||||
let mut case: CaseFile = toml::from_str(include_str!(
|
||||
"../../../../../tests/perf/query_cases/promql_nullable_range_fn/case.toml"
|
||||
))
|
||||
.expect("nullable range-function case should parse");
|
||||
let scenario = match &case.scenario {
|
||||
let scenario = match &mut case.scenario {
|
||||
Scenario::DirectReadableSst(scenario) => scenario,
|
||||
_ => unreachable!("nullable range-function case is a direct SST case"),
|
||||
};
|
||||
scenario.layout.series_count = NonZeroUsize::new(2).expect("2 is nonzero");
|
||||
scenario.layout.rows_per_sst = 6;
|
||||
scenario.layout.sst_count = 3;
|
||||
let table = scenario
|
||||
.tables
|
||||
.iter()
|
||||
@@ -944,14 +867,10 @@ mod tests {
|
||||
.find(|column| column.name == "value")
|
||||
.expect("fixture case has a value field");
|
||||
let null_every = match value.distribution.as_ref() {
|
||||
Some(Distribution::NullableWave { null_every, .. }) => *null_every,
|
||||
Some(Distribution::NullableWave { null_every, .. }) => null_every.get(),
|
||||
other => panic!("nullable_table must use nullable_wave, found {other:?}"),
|
||||
};
|
||||
// Without aligned SST boundaries the per-series cadence would shift by one
|
||||
// slot per SST, and the expected window counts documented in the case would
|
||||
// stop holding.
|
||||
let samples_per_series = scenario.layout.rows_per_sst / scenario.layout.series_count.get();
|
||||
assert_eq!(0, samples_per_series % null_every);
|
||||
|
||||
let metadata = Arc::new(build_region_metadata(table, RegionId::from(43)));
|
||||
let mut null_positions: HashMap<(String, String), Vec<usize>> = HashMap::new();
|
||||
@@ -991,15 +910,11 @@ mod tests {
|
||||
null_positions.len(),
|
||||
"every series must be represented"
|
||||
);
|
||||
let expected_nulls = scenario.layout.sst_count * samples_per_series / null_every;
|
||||
for (series, positions) in &null_positions {
|
||||
assert_eq!(expected_nulls, positions.len(), "{series:?} null count");
|
||||
assert!(
|
||||
positions
|
||||
.iter()
|
||||
.all(|position| position % null_every == positions[0] % null_every),
|
||||
"{series:?} nulls leave their residue class: {:?}",
|
||||
&positions[..positions.len().min(6)]
|
||||
assert_eq!(
|
||||
&[0, null_every, null_every * 2],
|
||||
positions.as_slice(),
|
||||
"{series:?} NULL positions must continue across SSTs"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -73,18 +73,10 @@ or a wave with periodic NULLs:
|
||||
distribution = { kind = "nullable_wave", min = 0.0, max = 100.0, null_every = 4 }
|
||||
```
|
||||
|
||||
`nullable_wave` also accepts `null_every = 0` to disable NULLs. It writes NULL on
|
||||
every `null_every`-th sample of each series and wave values everywhere else, so a
|
||||
NULL is one missing sample of one series rather than a missing row. The cadence
|
||||
is counted in samples of the series, not in generated rows, so every series
|
||||
carries the same null ratio under any `series_layout`; when one SST holds a
|
||||
multiple of `null_every` samples per series (`rows_per_sst / series_count`), the
|
||||
pattern also repeats with the same period in every SST. Counting the cadence in
|
||||
generated rows instead would make it constant per series whenever `series_count`
|
||||
is a multiple of `null_every` (128 and 4, for example), leaving series either
|
||||
entirely NULL or entirely NULL-free. Counting per series keeps NULL and non-NULL
|
||||
samples mixed inside range-function windows, which is what the nullable branches
|
||||
of the PromQL range functions observe.
|
||||
`null_every` must be greater than zero. `nullable_wave` writes NULL on every
|
||||
`null_every`-th sample of each series and wave values everywhere else; the cadence
|
||||
is counted in per-series samples, not generated rows. Do not use a row-based
|
||||
cadence: it can leave individual series entirely NULL or entirely NULL-free.
|
||||
|
||||
### Query kinds
|
||||
|
||||
|
||||
@@ -1,24 +1,6 @@
|
||||
# PromQL nullable range-function benchmark: regression evidence for the indexed
|
||||
# valid-sample lookup in nullable range functions (#9234).
|
||||
#
|
||||
# `rate`/`increase`/`delta` and the presence evaluator behind
|
||||
# `count_over_time`/`last_over_time` fall back to a per-window validity scan once
|
||||
# any sample of a series is NULL. `nullable_table` writes NULL on every fourth
|
||||
# sample of every series, so 25% of its samples are missing (one missing sample
|
||||
# per minute at the 15s cadence) and every window of every series contains both
|
||||
# NULL and non-NULL samples instead of being entirely NULL or entirely dense.
|
||||
# `dense_table` is the same fixture without NULLs; it keeps the no-NULL fast path
|
||||
# measured next to the nullable path.
|
||||
#
|
||||
# 128 series x 780 timestamps at 15s cadence = 99,840 rows per table, written as
|
||||
# 13 SSTs x 7,680 rows. Data starts one hour before evaluation; 481 evaluations
|
||||
# span two hours. Range windows are left-open and every series is dense on the
|
||||
# 15s grid, so a fully covered 5m window holds 20 slots and a 1m window holds 4.
|
||||
# Five of the 20 slots and one of the 4 slots are NULL, which makes
|
||||
# `count_over_time(nullable_table[5m])` return exactly 15 and
|
||||
# `count_over_time(nullable_table[1m])` return exactly 3 at every one of the 481
|
||||
# evaluations: no window is entirely NULL and none is NULL-free, so every window
|
||||
# of every series takes the nullable branch.
|
||||
# Benchmarks nullable PromQL range functions against a dense control. Every
|
||||
# 15-second series has NULL every fourth sample; fully covered windows therefore
|
||||
# retain 15 valid samples at [5m] and 3 at [1m].
|
||||
|
||||
[case]
|
||||
name = "promql_nullable_range_fn"
|
||||
@@ -115,16 +97,6 @@ iterations = 9
|
||||
[scenario.queries.thresholds]
|
||||
max_candidate_latency_regression_pct = 20
|
||||
|
||||
[[scenario.queries]]
|
||||
name = "increase_nullable_5m"
|
||||
kind = "tql"
|
||||
query = "TQL ANALYZE VERBOSE (1704070800, 1704078000, '15s') increase(nullable_table{host=~'host.*'}[5m])"
|
||||
warmup = 3
|
||||
iterations = 9
|
||||
|
||||
[scenario.queries.thresholds]
|
||||
max_candidate_latency_regression_pct = 20
|
||||
|
||||
# `delta` shares the nullable window-bounds path but skips the counter correction.
|
||||
[[scenario.queries]]
|
||||
name = "delta_nullable_5m"
|
||||
|
||||
Reference in New Issue
Block a user