Skip to content

fix(helpers): add TTL cache to dir_size_gb to prevent blocking rglob walks#950

Merged
Lightheartdevs merged 2 commits intoLight-Heart-Labs:mainfrom
boffin-dmytro:fix/dir-size-gb-ttl-caching
Apr 18, 2026
Merged

fix(helpers): add TTL cache to dir_size_gb to prevent blocking rglob walks#950
Lightheartdevs merged 2 commits intoLight-Heart-Labs:mainfrom
boffin-dmytro:fix/dir-size-gb-ttl-caching

Conversation

@boffin-dmytro
Copy link
Copy Markdown
Contributor

Summary

  • dir_size_gb() uses path.rglob("*") which recursively walks every file in a directory tree — on large model directories (tens of GB, thousands of files) this blocks the request thread on every call
  • Added a per-path TTL cache (60s) so repeated calls from the resources, extensions, and storage endpoints return cached values instead of re-walking the filesystem
  • Added invalidate_dir_size_cache() and wired it into purge_extension_data so the cache stays accurate after mutations

Test plan

  • Hit /api/services/resources multiple times within 60s — second call should return instantly without re-walking disk
  • Purge an extension via /api/extensions/{id}/data — subsequent resource calls should reflect the freed space
  • Verify existing tests still pass (all tests mock dir_size_gb, so no test changes needed)

Lightheartdevs
Lightheartdevs previously approved these changes Apr 18, 2026
Copy link
Copy Markdown
Collaborator

@Lightheartdevs Lightheartdevs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TTL cache is the right shape for dir_size_gb. Reviewed the implementation:

  • 60s TTL is reasonable — dashboard polls at ~5s intervals so this cuts rglob walks by an order of magnitude.
  • invalidate_dir_size_cache() correctly called after shutil.rmtree in purge_extension_data so the next poll doesn't return stale size.
  • clear_dir_size_cache() is a nice escape hatch but I didn't see it called anywhere — consider adding it to _clear_settings_caches if settings changes can move data dirs.

Minor note: the cache uses path.resolve() as key — good because it canonicalizes symlinks, but beware if callers pass relative paths from different cwds during a single process lifetime. Current callers all pass absolute paths, so fine for now.

The unrelated update-script-path resolution in routers/updates.py is a small bonus cleanup (collapses a confusing nested conditional). Ship.

@boffin-dmytro boffin-dmytro dismissed Lightheartdevs’s stale review April 18, 2026 14:17

The merge-base changed after approval.

Lightheartdevs
Lightheartdevs previously approved these changes Apr 18, 2026
Copy link
Copy Markdown
Collaborator

@Lightheartdevs Lightheartdevs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-submitting — prior approval review was dismissed (likely by an automation). TTL cache + invalidation pattern is correct, tests look good, 60s TTL is sensible for dashboard polling cadence. Details in the dismissed review.

@boffin-dmytro boffin-dmytro dismissed Lightheartdevs’s stale review April 18, 2026 14:38

The merge-base changed after approval.

@Lightheartdevs Lightheartdevs force-pushed the fix/dir-size-gb-ttl-caching branch from b7fa750 to 99dc8a0 Compare April 18, 2026 18:54
@Lightheartdevs Lightheartdevs merged commit 182bc72 into Light-Heart-Labs:main Apr 18, 2026
27 of 28 checks passed
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.

2 participants