Repository navigation
multilevel: track per-tier read failures #2966
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
93a82e8
f314e88
a1cf81e
a4433dd
98fbac7
a650496
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ Multi-level caching enables hierarchical cache storage, similar to how CPUs use | |
| - [Architecture](#architecture) | ||
| - [Use Cases](#use-cases) | ||
| - [Configuration](#configuration) | ||
| - [Statistics](#statistics) | ||
| - [Best Practices](#best-practices) | ||
|
|
||
| ## Overview | ||
|
|
@@ -234,6 +235,29 @@ If `SCCACHE_MULTILEVEL_CHAIN` is not set, sccache uses the first configured cach | |
| export SCCACHE_DIR="/tmp/cache" | ||
| ``` | ||
|
|
||
| ## Statistics | ||
|
|
||
| `sccache --show-stats` and `--show-adv-stats` report each level's completed | ||
| read outcomes: | ||
|
|
||
| - **Hits**: the level returned a cached entry or raw cached bytes. | ||
| - **Misses**: the level completed the read without finding the key. | ||
| - **Read failures**: the level's read returned an error. | ||
|
|
||
| A failed read is not also counted as a miss. Levels after a hit are not | ||
| consulted and their counters do not change. The per-level hit rate is | ||
| `hits / (hits + misses)`; read failures are excluded. JSON statistics expose | ||
| the failure count as `stats.multi_level[].read_failures`. | ||
|
|
||
| Completed reads are counted even if a later level stalls and the lookup is | ||
| cancelled. A still-pending read is not counted as a completed outcome. | ||
|
|
||
| Read failures do not change fallback behavior: the next level is tried, and | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could be shorter, something like "Read failures still fall back to the next level; they are not part of
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sounds good - a1cf81e |
||
| exhausting the chain still returns an overall miss, even if every level | ||
| failed. Per-level failures are therefore distinct from the aggregate | ||
| `Cache read errors` counter, which counts lookup errors reported to the | ||
| compilation pipeline. | ||
|
|
||
| ## Best Practices | ||
|
|
||
| ### 1. Order Levels by Latency (Fastest First) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1776,27 +1776,134 @@ fn test_multilevel_get_raw_records_per_level_stats() { | |
| assert_eq!(stats.0[1].misses, 0); | ||
| } | ||
|
|
||
| fn assert_read_outcomes(stats: &MultiLevelStats, expected: &[(u64, u64, u64)]) { | ||
| let actual: Vec<_> = stats | ||
| .0 | ||
| .iter() | ||
| .map(|level| (level.hits, level.misses, level.read_failures)) | ||
| .collect(); | ||
| assert_eq!(actual, expected); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_multilevel_get_raw_continues_after_level_error() { | ||
| fn test_multilevel_read_outcomes_and_fallback() { | ||
| let runtime = RuntimeBuilder::new_current_thread() | ||
| .enable_all() | ||
| .build() | ||
| .unwrap(); | ||
| let l0 = Arc::new(CheckFailingStorage); | ||
| let l1 = Arc::new(InMemoryStorage::new()); | ||
| let storage = | ||
| MultiLevelStorage::new(vec![l0 as Arc<dyn Storage>, l1.clone() as Arc<dyn Storage>]); | ||
|
|
||
| runtime.block_on(async { | ||
| l1.put("hit", CacheWrite::default()).await.unwrap(); | ||
| assert!(storage.get_raw("hit").await.unwrap().is_some()); | ||
| for raw in [false, true] { | ||
| for (chain, expected) in [ | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the "me"/"eh" char encoding is a bit cryptic, maybe add a one-line comment for m/e/h? :)
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fair comment, addressed in a4433dd |
||
| ("me", vec![(0, 1, 0), (0, 0, 1)]), | ||
| ("ee", vec![(0, 0, 1), (0, 0, 1)]), | ||
| ("eh", vec![(0, 0, 1), (1, 0, 0)]), | ||
| ("he", vec![(1, 0, 0), (0, 0, 0)]), | ||
| ("meh", vec![(0, 1, 0), (0, 0, 1), (1, 0, 0)]), | ||
| ] { | ||
| let bytes: Bytes = CacheWrite::default().finish().unwrap().into(); | ||
| let mut levels: Vec<Arc<dyn Storage>> = Vec::new(); | ||
| let mut memory_levels = Vec::new(); | ||
| for outcome in chain.chars() { | ||
| if outcome == 'e' { | ||
| levels.push(Arc::new(CheckFailingStorage)); | ||
| } else { | ||
| let level = Arc::new(InMemoryStorage::new()); | ||
| if outcome == 'h' { | ||
| level.put_raw("key", bytes.clone()).await.unwrap(); | ||
| } | ||
| levels.push(level.clone()); | ||
| memory_levels.push(level); | ||
| } | ||
| } | ||
| let storage = MultiLevelStorage::new(levels); | ||
| let expect_hit = chain.contains('h'); | ||
| if raw { | ||
| assert_eq!( | ||
| storage.get_raw("key").await.unwrap(), | ||
| expect_hit.then(|| bytes.clone()) | ||
| ); | ||
| } else { | ||
| let result = storage.get("key").await.unwrap(); | ||
| if expect_hit { | ||
| assert!(matches!(result, Cache::Hit(_))); | ||
| } else { | ||
| assert!(matches!(result, Cache::Miss)); | ||
| } | ||
| } | ||
| let stats = storage.stats(); | ||
| assert_read_outcomes(&stats, &expected); | ||
|
|
||
| let mut merged = stats.clone(); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the merge + serde roundtrip doesn't depend on the chain, maybe move it into its own small test?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. agree, separated the concerns into a separate sync test - 98fbac7 |
||
| merged += stats; | ||
| let doubled: Vec<_> = expected | ||
| .iter() | ||
| .map(|&(hits, misses, failures)| (hits * 2, misses * 2, failures * 2)) | ||
| .collect(); | ||
| assert_read_outcomes(&merged, &doubled); | ||
| let json = serde_json::to_value(&merged).unwrap(); | ||
| for (level, &(_, _, failures)) in json.as_array().unwrap().iter().zip(&doubled) { | ||
| assert_eq!(level["read_failures"], serde_json::json!(failures)); | ||
| } | ||
| let decoded: MultiLevelStats = serde_json::from_value(json).unwrap(); | ||
| assert_read_outcomes(&decoded, &doubled); | ||
|
|
||
| if chain == "meh" { | ||
| // A failed intervening tier must not stop a later hit backfilling L0. | ||
| tokio::time::timeout(Duration::from_secs(1), async { | ||
| loop { | ||
| let data = memory_levels[0].data.lock().await; | ||
| if data.get("key").map(Vec::as_slice) == Some(bytes.as_ref()) { | ||
| break; | ||
| } | ||
| drop(data); | ||
| tokio::task::yield_now().await; | ||
| } | ||
| }) | ||
| .await | ||
| .expect("later hit should backfill the earlier missing tier"); | ||
| } | ||
| } | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| let stats = storage.stats(); | ||
| assert_eq!(stats.0[0].hits, 0); | ||
| assert_eq!(stats.0[0].misses, 1); | ||
| assert_eq!(stats.0[1].hits, 1); | ||
| assert_eq!(stats.0[1].misses, 0); | ||
| #[test] | ||
| fn test_multilevel_completed_reads_survive_cancellation() { | ||
| let runtime = RuntimeBuilder::new_current_thread() | ||
| .enable_all() | ||
| .build() | ||
| .unwrap(); | ||
|
|
||
| runtime.block_on(async { | ||
| for raw in [false, true] { | ||
| let stalled = Arc::new(InMemoryStorage::new()); | ||
| let storage = MultiLevelStorage::new(vec![ | ||
| Arc::new(InMemoryStorage::new()) as Arc<dyn Storage>, | ||
| Arc::new(CheckFailingStorage) as Arc<dyn Storage>, | ||
| stalled.clone() as Arc<dyn Storage>, | ||
| ]); | ||
| // Holding the data lock makes the final read pending without relying on timing. | ||
| let _guard = stalled.data.lock().await; | ||
| { | ||
| let lookup = async { | ||
| if raw { | ||
| storage.get_raw("key").await.map(|_| ()) | ||
| } else { | ||
| storage.get("key").await.map(|_| ()) | ||
| } | ||
| }; | ||
| tokio::pin!(lookup); | ||
| tokio::select! { | ||
| biased; | ||
| result = &mut lookup => panic!("lookup should remain pending: {:?}", result), | ||
| _ = std::future::ready(()) => {} | ||
| } | ||
| assert_read_outcomes(&storage.stats(), &[(0, 1, 0), (0, 0, 1), (0, 0, 0)]); | ||
| } | ||
| assert_read_outcomes(&storage.stats(), &[(0, 1, 0), (0, 0, 1), (0, 0, 0)]); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same assert as the one in the block, dropping the future can't change the counters. do we need both?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good catch. I kept the one after the block, so the counters are checked after the pending lookup is dropped - a650496 |
||
| } | ||
| }); | ||
| } | ||
|
|
||
| #[test] | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is an implementation detail for users, maybe just drop this paragraph?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed, updated in f314e88