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 2 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
2 changes: 1 addition & 1 deletion changelog.d/11849.misc
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Enable cache time-based expiry by default.
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"?

14 changes: 8 additions & 6 deletions docs/upgrade.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,6 @@ 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.

## Stablisation of MSC3231
Copy link
Contributor

Choose a reason for hiding this comment

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

I know it's not your mess, but as you're here, please could you clean this up by removing the 'v1.(next)' heading and bringing the announcement into the v1.53 section?
I personally think a 'v1.(next)' section would be easily forgotten as it's not in our release notes. Or at least, I'd forget! :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure thing


Expand All @@ -109,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
14 changes: 14 additions & 0 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 @@ -232,7 +235,18 @@ def read_config(self, config, **kwargs) -> 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."
)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is very nitty, but I would call these options rather than flags.
At least in my mind, a flag is a boolean.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a useful distinction, thank you!

self.expiry_time_msec = self.parse_duration(expiry_time)

self.sync_response_cache_duration = self.parse_duration(
Expand Down