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 16 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. The `expiry_time` config flag will be superseded by `expire_caches` and `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.

this should probably be a .feature?

and "will be" should be "has been"?

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
9 changes: 9 additions & 0 deletions docs/upgrade.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ process, for example:
```
# Upgrading to v1.(next)


## Stablisation of MSC3231

The unstable validity-check endpoint for the
Expand All @@ -102,6 +103,14 @@ Please update any relevant reverse proxy or firewall configurations appropriatel

# Upgrading to v1.53.0

## 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.
Copy link
Member

Choose a reason for hiding this comment

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

Might be worth mentioning that expiry_time will still work (for now), but is deprecated?



## Dropping support for `webclient` listeners and non-HTTP(S) `web_client_location`

Per the deprecation notice in Synapse v1.51.0, listeners of type `webclient`
Expand Down
Empty file.
40 changes: 33 additions & 7 deletions synapse/config/cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

import logging
import os
import re
import threading
Expand All @@ -23,6 +24,8 @@

from ._base import Config, ConfigError

logger = logging.getLogger(__name__)

# The prefix for all cache factor-related environment variables
_CACHE_PREFIX = "SYNAPSE_CACHE_FACTOR"

Expand Down Expand Up @@ -148,11 +151,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.
#
#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,12 +225,30 @@ 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 and expire_caches:
logger.warning(
"You have set two incompatible flags, expiry_time and expire_caches. Please only use the "
"expire_caches and cache_entry_ttl flags and delete the expiry_time flag as it is "
"deprecated."
)
if expiry_time:
logger.warning(
"Expiry_time is a deprecated flag, please use the expire_caches and cache_entry_ttl flags "
"instead."
)
self.expiry_time_msec = self.parse_duration(expiry_time)

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