Skip to content

multilevel: track per-tier read failures - #2966

Open
aw1cks wants to merge 1 commit into
mozilla:mainfrom
aw1cks-forks:multilevel-read-failures
Open

aw1cks wants to merge 1 commit into
mozilla:mainfrom
aw1cks-forks:multilevel-read-failures

Conversation

@aw1cks

@aw1cks aw1cks commented Oct 28, 2026

Copy link
Copy Markdown

Previously, failed reads from individual cache tiers were counted as misses.
This PR introduces a per-tier read_failures metric.

Fallback behavior is unchanged.

@codecov-commenter

codecov-commenter commented Oct 28, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.04762% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 78.65%. Comparing base (dac6c80) to head (93a82e8).

Files with missing lines Patch % Lines
src/cache/multilevel_test.rs 98.92% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2966      +/-   ##
==========================================
+ Coverage   78.54%   78.65%   +0.10%     
==========================================
  Files          74       74              
  Lines       44039    44123      +84     
==========================================
+ Hits        34591    34704     +113     
+ Misses       9448     9419      -29     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread docs/MultiLevel.md
`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

Copy link
Copy Markdown
Collaborator

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?

Comment thread docs/MultiLevel.md
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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The 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 Cache read errors."

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 [

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The 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? :)

let stats = storage.stats();
assert_read_outcomes(&stats, &expected);

let mut merged = stats.clone();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The 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?
this test is already doing a lot

}
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)]);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The 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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants