-
Notifications
You must be signed in to change notification settings - Fork 671
metro-cache: Cleanup FileStore caches, deprecate AutoCleanFileStore #1570
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Conversation
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
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. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
CLA Signed
This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
fb-exported
Merged
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