metro-cache: Cleanup FileStore caches, deprecate AutoCleanFileStore#1570
Closed
metro-cache: Cleanup FileStore caches, deprecate AutoCleanFileStore#1570
Conversation
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D81051149 |
facebook-github-bot
pushed a commit
that referenced
this pull request
Sep 8, 2025
…1570) Summary: Modernise `FileStore` and `AutoCleanFileStore` tests and implementations, using built-in recursive directory scans, `memfs` over `metro-memory-fs`, and `#`-private over `_`-private. ## Deprecating `AutoCleanFileStore` Deprecate `AutoCleanFileStore` because the current implementation isn't used by default and has a number of flaws that should prevent us recommending it: - It re-scans all files every interval, even though it's in a position to know when files are read and written by the cache. - It will delete recently used entries just because they weren't recently written. - It's fully synchronous (event-loop blocking) The current mechanism could easily exist outside Metro, e.g., as a simple cron of: ``` find /cache/root -mtime +3 -exec rm {} \; ``` Typically, caches are written to OS "temp" directories whose lifetimes are managed by the OS. A better *Metro* cache solution would be an LRU cache, but this could be fully implemented in userland and we haven't had any requests to include one in core. Changelog: ``` - **[Deprecated]**: metro-cache: Deprecate AutoCleanFileStore ``` Rollback Plan: Differential Revision: D81051149
2ab8d08 to
fe4b548
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D81051149 |
robhogan
added a commit
that referenced
this pull request
Sep 9, 2025
…1570) Summary: Modernise `FileStore` and `AutoCleanFileStore` tests and implementations, using built-in recursive directory scans, `memfs` over `metro-memory-fs`, and `#`-private over `_`-private. ## Deprecating `AutoCleanFileStore` Deprecate `AutoCleanFileStore` because the current implementation isn't used by default and has a number of flaws that should prevent us recommending it: - It re-scans all files every interval, even though it's in a position to know when files are read and written by the cache. - It will delete recently used entries just because they weren't recently written. - It's fully synchronous (event-loop blocking) The current mechanism could easily exist outside Metro, e.g., as a simple cron of: ``` find /cache/root -mtime +3 -exec rm {} \; ``` Typically, caches are written to OS "temp" directories whose lifetimes are managed by the OS. A better *Metro* cache solution would be an LRU cache, but this could be fully implemented in userland and we haven't had any requests to include one in core. Changelog: ``` - **[Deprecated]**: metro-cache: Deprecate AutoCleanFileStore ``` Reviewed By: huntie Differential Revision: D81051149
fe4b548 to
6004288
Compare
…1570) Summary: Pull Request resolved: #1570 Modernise `FileStore` and `AutoCleanFileStore` tests and implementations, using built-in recursive directory scans, `memfs` over `metro-memory-fs`, and `#`-private over `_`-private. ## Deprecating `AutoCleanFileStore` Deprecate `AutoCleanFileStore` because the current implementation isn't used by default and has a number of flaws that should prevent us recommending it: - It re-scans all files every interval, even though it's in a position to know when files are read and written by the cache. - It will delete recently used entries just because they weren't recently written. - It's fully synchronous (event-loop blocking) The current mechanism could easily exist outside Metro, e.g., as a simple cron of: ``` find /cache/root -mtime +3 -exec rm {} \; ``` Typically, caches are written to OS "temp" directories whose lifetimes are managed by the OS. A better *Metro* cache solution would be an LRU cache, but this could be fully implemented in userland and we haven't had any requests to include one in core. Changelog: ``` - **[Deprecated]**: metro-cache: Deprecate AutoCleanFileStore ``` Reviewed By: huntie Differential Revision: D81051149
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D81051149 |
6004288 to
2de8322
Compare
Contributor
|
This pull request has been merged in a264732. |
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
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.
Summary:
Modernise
FileStoreandAutoCleanFileStoretests and implementations, using built-in recursive directory scans,memfsovermetro-memory-fs, and#-private over_-private.Deprecating
AutoCleanFileStoreDeprecate
AutoCleanFileStorebecause the current implementation isn't used by default and has a number of flaws that should prevent us recommending it:The current mechanism could easily exist outside Metro, e.g., as a simple cron of:
Typically, caches are written to OS "temp" directories whose lifetimes are managed by the OS.
A better Metro cache solution would be an LRU cache, but this could be fully implemented in userland and we haven't had any requests to include one in core.
Changelog:
Rollback Plan:
Differential Revision: D81051149