Skip to content
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

Enhance logic of LowCacheTime to minimize noise. #408

Open
mdbitz opened this issue Apr 10, 2019 · 1 comment · May be fixed by #445
Open

Enhance logic of LowCacheTime to minimize noise. #408

mdbitz opened this issue Apr 10, 2019 · 1 comment · May be fixed by #445

Comments

@mdbitz
Copy link

mdbitz commented Apr 10, 2019

❗️ Warning: Low cache expiry time of "wp_rand( 5 * MINUTE_IN_SECONDS, 10 * MINUTE_IN_SECONDS )", it is recommended to have 300 seconds or more (WordPressVIPMinimum.Cache.LowExpiryCacheTime.LowCacheTime).

wp_cache_set( $latest_blocks_cache_key, $latest_blocks, null, wp_rand( 5 * MINUTE_IN_SECONDS, 10 * MINUTE_IN_SECONDS ) );

What problem would the enhancement address for VIP?

It is very common for clients to use constants HOUR_IN_SECONDS, DAY_IN_SECONDS, MINUTE_IN_SECONDS and to also vary the duration to not have cache stampedes.

Describe the solution you'd like

Can we enhance the logic of the ruleset to not flag for common usages?

@GaryJones
Copy link
Contributor

GaryJones commented Apr 11, 2019

We already handle time constants.

What we don't handle yet are calls to wp_rand() / rand() / random_int() / mt_rand(), - but we can check that the first arg meets the 300-second minimum (actually, it would have to be either arg for wp_rand(), as wp_rand() can accept args in either order).

@GaryJones GaryJones added this to the Future Release milestone Apr 11, 2019
@rebeccahum rebeccahum self-assigned this Aug 29, 2019
@GaryJones GaryJones removed this from the Future Release milestone Jul 23, 2020
@GaryJones GaryJones added this to the 3.x milestone Aug 26, 2023
@rebeccahum rebeccahum modified the milestones: 3.0.1, 3.x May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants