Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.

Enable cache time-based expiry by default #11849

Merged
merged 18 commits into from
Feb 11, 2022
Merged

Enable cache time-based expiry by default #11849

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
f798aba
make using cache expiry the default behavior
H-Shay Jan 27, 2022
adbd8ed
newsfragment
H-Shay Jan 27, 2022
bdeaf52
update comment
H-Shay Jan 27, 2022
83661c5
newsfragment and update sample config
H-Shay Jan 27, 2022
4eb3e6a
add boolean flag, rename expiry time
H-Shay Feb 3, 2022
652c620
regenerate sample config
H-Shay Feb 3, 2022
6784e1e
requested changes
H-Shay Feb 4, 2022
253edca
Merge branch 'develop' into shay/default_expiry
H-Shay Feb 4, 2022
775cbf7
add note in upgrade notes about changes
H-Shay Feb 4, 2022
6dddbc7
lint
H-Shay Feb 4, 2022
8efac83
renew sample config
H-Shay Feb 4, 2022
c86e536
follow config code style
H-Shay Feb 9, 2022
327ea5a
follow config code style
H-Shay Feb 9, 2022
8d06aac
regenerate sample config
H-Shay Feb 9, 2022
f37e95f
requested changes
H-Shay Feb 10, 2022
3299583
d'oh! lints
H-Shay Feb 10, 2022
77ac0d7
cleanup v1 header and reword flags
H-Shay Feb 11, 2022
604145a
Merge branch 'develop' into shay/default_expiry
H-Shay Feb 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/11849.misc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Enable cache time-based expiry by default.
Copy link
Member

Choose a reason for hiding this comment

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

It may be worth mentioning the new configuration options that are available, or otherwise pointing to the relevant section of the sample config for the full details.

This could also be done via our upgrading docs, where we have more room.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would likely still sneak a keyword or two in there: expiry_time will be superseded by expire_caches and cache_entry_ttl.
Helps perk people's ears up and helps if you ever need to do a Ctrl+F for one of the config option names.

12 changes: 8 additions & 4 deletions docs/sample_config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -737,11 +737,15 @@ caches:
per_cache_factors:
#get_users_who_share_room_with_user: 2.0

# Controls how long an entry can be in a cache without having been
# accessed before being evicted. Defaults to None, which means
# entries are never evicted based on time.
# Controls whether cache entries are evicted after a specified time
# period. Defaults to true.
# expire_caches: false

# If expire_caches is enabled, this flag controls how long an entry can
# be in a cache without having been accessed before being evicted.
# Defaults to 30m. Uncomment to set a different time to live for cache entries.
#
#expiry_time: 30m
# cache_entry_ttl: 30m

# Controls how long the results of a /sync request are cached for after
# a successful response is returned. A higher duration can help clients with
Expand Down
25 changes: 18 additions & 7 deletions synapse/config/cache.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,11 +148,15 @@ def generate_config_section(self, **kwargs) -> str:
per_cache_factors:
#get_users_who_share_room_with_user: 2.0

# Controls how long an entry can be in a cache without having been
# accessed before being evicted. Defaults to None, which means
# entries are never evicted based on time.
# Controls whether cache entries are evicted after a specified time
# period. Defaults to true.
# expire_caches: false

# If expire_caches is enabled, this flag controls how long an entry can
# be in a cache without having been accessed before being evicted.
# Defaults to 30m. Uncomment to set a different time to live for cache entries.
#
#expiry_time: 30m
# cache_entry_ttl: 30m

# Controls how long the results of a /sync request are cached for after
# a successful response is returned. A higher duration can help clients with
Expand Down Expand Up @@ -217,9 +221,16 @@ def read_config(self, config, **kwargs) -> None:
e.message # noqa: B306, DependencyException.message is a property
)

expiry_time = cache_config.get("expiry_time")
if expiry_time:
self.expiry_time_msec: Optional[int] = self.parse_duration(expiry_time)
expire_caches = cache_config.get("expire_caches", True)
cache_entry_ttl = cache_config.get("cache_entry_ttl")

Copy link
Member

Choose a reason for hiding this comment

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

Since we're changing config options, we should include backwards-compatibility support for the previous config option name. Thus if someone set expiry_time previously, that same value should continue to work even after this update. At some point ™️ in the future we can remove support for the old option name.

if expire_caches:
if cache_entry_ttl:
self.expiry_time_msec: Optional[int] = self.parse_duration(
cache_entry_ttl
)
else:
self.expiry_time_msec = 1800000
else:
self.expiry_time_msec = None

Expand Down