Skip to content

cache: exclude staged blocks from the cached block count - #7419

Open
Looka149 wants to merge 3 commits into
juicedata:mainfrom
Looka149:fix/blockcache-blocks-exclude-staging
Open

cache: exclude staged blocks from the cached block count#7419
Looka149 wants to merge 3 commits into
juicedata:mainfrom
Looka149:fix/blockcache-blocks-exclude-staging

Conversation

@Looka149

@Looka149 Looka149 commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Resolves #7418

The cache now keeps its own block count, updated wherever used is updated. Neither counts staged blocks.

blockcache_blocks reports this count, so it now matches blockcache_bytes.

--cache-items uses it too, so the limit now applies to read cache blocks only. Staged blocks still take disk space, and --free-space-ratio still limits that.

Tested on a kind cluster with --writeback, 200 staged blocks and --cache-items 50: reading 100 files now keeps 50 read cache blocks instead of 1, and staging_blocks stays 200.

stats() reported the block count from the key index while used skips staging
blocks, so with --writeback the two metrics described different sets: 200
staged blocks reported blockcache_blocks=200 and blockcache_bytes=0, and the
same blocks were double-counted alongside staging_blocks.

Track the count next to used so both are derived from the same decision.
full() still counts staging blocks against cache-items: they occupy inodes.
The row says the metric counts bytes in the staging path but the unit column
said 秒 (second), likely copied from the staging_block_delay_seconds row below
it. The English table already says byte.
Staged blocks cannot be evicted, so counting them against --cache-items makes
cleanupFull chase a target it can never reach and drop read cache blocks
instead: with --cache-items 50 and 200 staged blocks, 99 of 100 freshly cached
blocks were evicted right away and blockcache_hits stayed at 0.

The byte limit already excludes them through used, and their inode usage stays
bounded by --free-space-ratio, which isFull() applies to staging as well.
@Looka149
Looka149 requested a review from CaitinChen as a code owner August 19, 2026 15:14
@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 38.09524% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 27.89%. Comparing base (d178e3d) to head (14d6715).

Files with missing lines Patch % Lines
pkg/chunk/disk_cache.go 31.57% 13 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #7419       +/-   ##
===========================================
- Coverage   61.92%   27.89%   -34.04%     
===========================================
  Files          31      179      +148     
  Lines       25295    59205    +33910     
===========================================
+ Hits        15663    16513      +850     
- Misses       7769    40533    +32764     
- Partials     1863     2159      +296     

☔ 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.

@CaitinChen

Copy link
Copy Markdown
Collaborator

@jiefenghuang PTAL

@Looka149

Copy link
Copy Markdown
Contributor Author

@CaitinChen @jiefenghuang I updated the issue and the PR description. Thank you in advance for your review.

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.

staged blocks are counted as cache blocks, but not as cache bytes

2 participants