Skip to content

Conversation

@robhogan
Copy link
Contributor

@robhogan robhogan commented Sep 8, 2025

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

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 8, 2025
@facebook-github-bot
Copy link
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
@facebook-github-bot
Copy link
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
…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
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D81051149

@facebook-github-bot
Copy link
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants