Skip to content

Commit 160d23a

Browse files
committed
fix(cost): reject unsupported cache and storage evidence combinations
1 parent aa486bf commit 160d23a

4 files changed

Lines changed: 48 additions & 3 deletions

File tree

‎crates/asap-aware-mapping/src/physical_plan_cost_model.rs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,16 @@ impl<'a> PhysicalPlanCostModel<'a> {
156156
target: &TargetSubDAG<'_>,
157157
) -> Result<PhysicalPlanComparison, AnalyticalCostError> {
158158
let (snapshot, raw) = self.target_evidence(target)?;
159+
// Aggregate cache hit ratios do not identify which independently rounded
160+
// extents issue requests. Do not mix cache-adjusted bytes/CPU with
161+
// uncached request counts until cache-aware extent evidence is available.
162+
if snapshot.storage_io.is_some()
163+
&& matches!(&snapshot.cache_profile, CacheProfile::Evidence(_))
164+
{
165+
return Err(AnalyticalCostError::InvalidCacheEvidence(
166+
"storage operation costs require an explicit no-cache profile; cache-aware extent evidence is unavailable",
167+
));
168+
}
159169
let scope = &snapshot.scope;
160170
let evidence = QueryEvidence {
161171
provider: self.provider,

‎crates/devtools/src/bin/dag_export.rs‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1675,6 +1675,18 @@ mod tests {
16751675
.as_ref()
16761676
.unwrap()
16771677
.contains(STORAGE_IO_MODEL_VERSION));
1678+
assert_eq!(selected.cache_profile.as_deref(), Some("no-cache-v1"));
1679+
for name in ["result_cache_hit_ratio", "buffer_cache_hit_ratio"] {
1680+
assert_eq!(
1681+
selected
1682+
.inputs
1683+
.iter()
1684+
.find(|term| term.name == name)
1685+
.unwrap()
1686+
.value,
1687+
0.0
1688+
);
1689+
}
16781690
assert_eq!(
16791691
selected.evidence_version.as_deref(),
16801692
Some("test-evidence-v1")
@@ -1693,6 +1705,24 @@ mod tests {
16931705
let (baseline, selected, _) = model.annotations(&candidate, &root);
16941706
assert_eq!(baseline.value, Some(160.0));
16951707
assert_eq!(selected.value, Some(10.0));
1708+
// Aggregate cache hit assumptions cannot locate cached extents or
1709+
// reconstruct independently rounded physical storage requests.
1710+
let mut cached = serde_json::to_value(&requests_only).unwrap();
1711+
cached["targets"][0]["scope"]["cache_profile"] = serde_json::json!({
1712+
"profile": "evidence", "version": "cached-with-storage-v1",
1713+
"distinct_evaluations": 1, "repeated_identical_evaluations": 9,
1714+
"result_cache": {"working_set_bytes": 100, "capacity_bytes": 100},
1715+
"buffer_cache": {"working_set_bytes": 100, "capacity_bytes": 100},
1716+
"result_invalidation_ratio": null,
1717+
});
1718+
let cached = parse_planner_cost_document(&cached.to_string()).unwrap();
1719+
let cached_model = ExportPlannerCostModel { document: &cached };
1720+
assert!(cached_model.candidate_cost(&candidate, &target).is_none());
1721+
assert!(cached_model
1722+
.annotations(&candidate, &root)
1723+
.0
1724+
.value
1725+
.is_none());
16961726
document
16971727
.storage_io
16981728
.as_mut()

‎crates/types/src/resources.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ pub mod cpu;
1111
pub mod measurement;
1212
pub mod physical;
1313
pub mod storage;
14-
pub use storage::StorageResources;
1514
pub use cache::{CacheCapacityEvidence, CacheEvidence, CacheProfile};
15+
pub use storage::StorageResources;
1616

1717
pub use cpu::{MeasuredCpu, ModeledCpu};
1818
pub use measurement::Measurement;

‎docs/developer_docs/storage-operation-costs.md‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,13 @@ Missing entries, expired/future evidence, incompatible node snapshots, zero
7272
request sizes, invalid calibration, and overflow return typed analytical
7373
errors. When used by plan ranking/export they make that comparison unavailable.
7474
This profile extends the physical-plan adapter; the separate summary-maintenance
75-
lifecycle estimator retains its existing dimensions. Cache behavior is a
76-
separate model input and is not inferred here.
75+
lifecycle estimator retains its existing dimensions. Combined physical-plan
76+
ranking currently supports storage profiles only with an explicit `NoCache`
77+
profile. `CacheProfile::Evidence` together with storage evidence makes the
78+
comparison unavailable: aggregate cache hit ratios cannot identify which
79+
independently rounded extents issue requests. Supporting that combination
80+
requires cache-aware extent/request evidence; the adapter does not mix
81+
cache-adjusted bytes and CPU with uncached operation counts.
7782

7883
Verification:
7984

0 commit comments

Comments
 (0)