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

Reload cache factors from disk on SIGHUP #12673

Merged
merged 17 commits into from
May 11, 2022
Merged

Reload cache factors from disk on SIGHUP #12673

Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
d942823
Fix minor typos in cache config docstring
May 9, 2022
077ce90
Add drive-by annotations in s.config._base
May 9, 2022
1f668be
Define `RootConfig.reload_config_section`
May 9, 2022
2a5ce2f
Move `format_config_error` to s.config._base
May 9, 2022
69da4ca
Reload cache config on sighup
May 9, 2022
85f4385
Changelog
May 9, 2022
1febfd6
If `global_factor` is omitted, use 0.5 on reload
May 9, 2022
4039b2d
Annotate CacheConfig's instance vars at the top
May 10, 2022
b7a6ec5
Separate reading and applying cache config
May 10, 2022
519f98b
Remove redundant assert
May 10, 2022
b9595ad
Create a new config object when reloading
May 10, 2022
1118b9f
Fix tests oops
May 10, 2022
f6e1394
Document the ability to reload cache config
May 10, 2022
55963ac
Master process needs to apply cache config too.
May 11, 2022
f199d82
Update TRACK_MEMORY_USAGE on cache config reload
May 11, 2022
412414d
Overly comprehensive docstring
May 11, 2022
a7418c0
Don't update TRACK_MEMORY_USAGE; update docs
May 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
2 changes: 2 additions & 0 deletions synapse/app/_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
from twisted.protocols.tls import TLSMemoryBIOFactory
from twisted.python.threadpool import ThreadPool

import synapse.util.caches
from synapse.api.constants import MAX_PDU_SIZE
from synapse.app import check_bind_error
from synapse.app.phone_stats_home import start_phone_stats_home
Expand Down Expand Up @@ -499,6 +500,7 @@ def reload_cache_config(config: HomeServerConfig) -> None:
previous_cache_config.__dict__,
config.caches.__dict__,
)
synapse.util.caches.TRACK_MEMORY_USAGE = config.caches.track_memory_usage
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, so this is safe to update to False, but I'm not 100% sure its safe to update this to True. At the very least the metrics will be wrong for a while, as we won't track memory usage for anything that is already in the cache.

Happy for us to do one of:

  1. Not update this
  2. Only update this to False
  3. Check its safe to update this to True and add a big comment about the fact it will underestimate the cache size for a while

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll go for option 1 and update the docstring.

I'll also update the docs to only say that SIGHUP reloads cache factors.

Copy link
Member

Choose a reason for hiding this comment

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

Fair!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One more look over please?

config.caches.resize_all_caches()


Expand Down