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

Reduce the cache entry expiration TTL #1481

Merged
merged 1 commit into from
Jul 13, 2021
Merged

Conversation

dralley
Copy link
Contributor

@dralley dralley commented Jul 12, 2021

If Redis goes down then its persisted data can get out of sync with
Pulp, because cache invalidation events cannot occur. We should mitigate
the impact by letting cache entries expire after a shorter period of
time.

closes: #8996
https://pulp.plan.io/issues/8996

@@ -242,7 +242,7 @@
# https://docs.pulpproject.org/pulpcore/configuration/settings.html#pulp-cache
CACHE_ENABLED = True
CACHE_SETTINGS = {
"EXPIRES_TTL": 86400,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

24 hours.

@@ -242,7 +242,7 @@
# https://docs.pulpproject.org/pulpcore/configuration/settings.html#pulp-cache
CACHE_ENABLED = True
CACHE_SETTINGS = {
"EXPIRES_TTL": 86400,
"EXPIRES_TTL": 480, # 8 minutes
Copy link
Contributor Author

@dralley dralley Jul 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to @jlsherrill , most large users would have their clusters updating over a fairly short timeframe, so an expiration time measured in minutes is reasonable and would still result in high utilization.

We want to reduce the impact / risk of any significant cache-invalidation events. (e.g. Redis goes down, Pulp keeps going and invalidates cache, Redis comes back)

The ideal amount of time is up for debate though, and perhaps we should consider adding a little bit of randomness to amortize the cost of cache expiration events.

@pulpbot
Copy link
Member

pulpbot commented Jul 12, 2021

Attached issue: https://pulp.plan.io/issues/8996

Copy link
Member

@ipanova ipanova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'd probably lead towards 10 mins

@dralley dralley mentioned this pull request Jul 13, 2021
@@ -0,0 +1 @@
Reduced the default expiration TTL of entries in the Pulp content app cache.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this is maybe worth exposing to users.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We never advertised the default to begin with so I don't really think it's necessary. However I've just added it to the configuration docs.

If Redis goes down then its persisted data can get out of sync with
Pulp, because cache invalidation events cannot occur. We should mitigate
the impact by letting cache entries expire after a shorter period of
time.

closes: #8996
https://pulp.plan.io/issues/8996
@dralley dralley merged commit 017b0b1 into pulp:master Jul 13, 2021
@dralley dralley deleted the cache-expiration branch July 13, 2021 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants