Include cache size in manifest segment size#87
Merged
Conversation
The active-chunk cache (.arrows) can persist across a crash — it holds rows not yet flushed to the main segment file and is read back by Segment::iter() on the next open. It is only safe to remove after Writer::end() flushes its contents into the main file. Previously size_estimate() excluded the cache, so any segment that survived a crash with a live .arrows file was under-counted in the manifest (and therefore in retention's accounting). On a busy cluster these orphaned cache files accumulated to several GiB of invisible disk usage. Fix: include cache_path() in Segment::size_estimate(). Remove the now- redundant cache.size() addition in Writer::size_estimate() (cache.size() also reads the same .arrows file via fs::metadata, so it would have double-counted). Segment::destroy() already removes cache_path(), so retention cleans it up automatically. Co-Authored-By: Claude <noreply@anthropic.com>
segment_disk_size() added cache_path() to tracked_files (so orphan detection didn't flag it) but excluded it from the size total via an explicit guard. This meant UpdateManifestSizes never updated manifest entries to reflect the cache bytes, leaving existing rows under-counted even after the size_estimate() fix. Remove the guard so the cache file is counted the same as other parts. For sealed segments this corrects the manifest size; for active segments it gives a more accurate informational delta (process_active_segment never mutates the manifest anyway). Co-Authored-By: Claude <noreply@anthropic.com>
imp
approved these changes
Jun 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The cache is "ephemeral" yes, but there are situations where it actually survives and is e.g. used in the recovery process. It should not be ignored; until it is physically removed from disk, it must be included in the size of its associated segment.