Skip to content
This repository has been 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
Show file tree
Hide file tree
Changes from 14 commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Enable cache time-based expiry by default.
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved
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.

13 changes: 9 additions & 4 deletions docs/sample_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -751,11 +751,16 @@ 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. Uncomment to disable this feature.
#
#expiry_time: 30m
#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.
#
#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
7 changes: 7 additions & 0 deletions docs/upgrade.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,13 @@ process, for example:
```
# Upgrading to v1.(next)

## Time-based cache expiry is now enabled by default

Formerly, entries in the cache were not evicted regardless of whether they were accessed after storing.
This behavior has now changed. By default entries in the cache are now evicted after 30m of not being accessed.
To change the default behavior, go to the `caches` section of the config and change the `expire_caches` and
`cache_entry_ttl` flags as necessary. Please note that these flags replace the `expiry_time` flag in the config.

H-Shay marked this conversation as resolved.
Show resolved Hide resolved
## Stablisation of MSC3231

The unstable validity-check endpoint for the
Expand Down
Empty file.
26 changes: 19 additions & 7 deletions synapse/config/cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,11 +148,16 @@ 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. Uncomment to disable this feature.
#
#expiry_time: 30m
#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.
#
#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,12 +222,19 @@ 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", "30m")

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:
self.expiry_time_msec: Optional[int] = self.parse_duration(cache_entry_ttl)
else:
self.expiry_time_msec = None

# Backwards compatibility support for the now-removed "expiry_time" config flag.
expiry_time = cache_config.get("expiry_time")
if expiry_time:
self.expiry_time_msec = self.parse_duration(expiry_time)
H-Shay marked this conversation as resolved.
Show resolved Hide resolved

self.sync_response_cache_duration = self.parse_duration(
cache_config.get("sync_response_cache_duration", 0)
)
Expand Down