[Super Cache] Prevent deleting freshly preloaded files when GC time set to 0

[thingalon]

Fixes https://wordpress.org/support/topic/preload-mode-skips-caching-pages-or-deletes-already-cached-pages

When preloads finish, prune_super_cache is called to clean out leftovers. Unfortunately, if $cache_max_time is set to 0, preload will delete any files at least one second old - which frequently includes preloaded files. It causes intermittent post-preload disappearances.

This PR fixes this by ensuring the prune process will not delete files based on mtime if $cache_max_time is 0.

Proposed changes:

• Prevent prune_super_cache from deleting files if $cache_max_time is set to 0.

Other information:

• Have you written new tests for your changes, if applicable?
• Have you checked the E2E test CI results, and verified that your changes do not break them?
• Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

n/a

Does this pull request change what data or activity we track or use?

no

Testing instructions:

• Set cache timeout to 0
• Manually preload over and over
• Without this patch, sometimes files created near the start of the preload will get deleted at the end
• With this patch, all files created during the preload should remain. :)

[github-actions]

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

• ✅ Include a description of your PR changes.
  
• ✅ All commits were linted before commit.
  
• ✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
  
• ✅ Add testing instructions.
  
• ✅ Specify whether this PR includes any changes to data or privacy.
  
• ✅ Add changelog entries to affected projects
  

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

---

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

---

Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.

---

Super Cache plugin:

• Next scheduled release: June 6, 2023.
• Scheduled code freeze: May 29, 2023.

[pyronaur]

Maybe the code should be updated to match the debug message?

[donnchawp]

Good catch. I wonder if it would be better to put the $cache_max_time check at the top of prune_super_cache instead? It would save traversing down the directories. But we'd have to check everywhere else we use prune_super_cache().

Or in wp_cron_preload_cache we set the global $cache_max_time variable to the current time minus the filemtime of "preload_mutex.tmp". That file is created when preloading starts. Set that before calling prune_super_cache() at line 3248 and preloaded files should be safe.

[thingalon]

Good catch. I wonder if it would be better to put the $cache_max_time check at the top of prune_super_cache instead? It would save traversing down the directories. But we'd have to check everywhere else we use prune_super_cache().

I thought about doing this, but I wasn't sure about the $rename references I found throughout that code. I wasn't sure if there was any possibility of preloaded content getting renamed, and didn't want to risk causing new issues.

If that's not a consideration, then I would advocate for this. :)

Or in wp_cron_preload_cache we set the global $cache_max_time variable to the current time minus the filemtime of "preload_mutex.tmp". That file is created when preloading starts. Set that before calling prune_super_cache() at line 3248 and preloaded files should be safe.

This is also a very clever idea. I like that this would cause the preload system to clean out irrelevant files if you (for example) deleted a category since the last refresh.

We'd want to add a buffer when calculating a $cache_max_time override in case the system clock ticks over a second between starting on the prune job, and examining a particular file for pruning.

However, this would mean that we'd delete any cached content that isn't preloaded. So cached search results with a GET param of ?s= which may be fairly recent will get wiped. Is that a problem?

[thingalon]

I made a couple of changes in b1532bb based on @donnchawp 's suggestions:

1. I found some pre-existing logic for setting a max cache time before calling prune in the preload code already - there were just some code paths which could reach the prune call before setting an appropriate max cache time. So I moved that logic up, and
2. I updated the prune method to exit early if max time is set to 0.

Based on my testing, this fixes the issue, and preload should still clean out unused files after a long time. :-)

[donnchawp]

I think we can close this. The mass deletion of files seemed to be caused in part by how slow that person's server was, but we've made changes elsewhere to speed up preload or at least make it more reliable on slow servers.