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

Add metrics tracking for eviction to ResponseCache #16028

Merged
merged 4 commits into from
Aug 1, 2023
Merged

Add metrics tracking for eviction to ResponseCache #16028

merged 4 commits into from
Aug 1, 2023

Conversation

realtyem
Copy link
Contributor

@realtyem realtyem commented Jul 29, 2023 •
edited
Loading

It appears to be a slight oversight that tracking time based removal was not added. Here you go.

response_cache_eviction_results

I think it looks pretty good. I used the query:

rate(synapse_util_caches_response_cache_evicted_size{instance=~"$instance", job=~"$job", index=~"$index"}[$bucket_size])

Pull Request Checklist

Signed-off-by: Jason Little realtyem@gmail.com

@realtyem realtyem force-pushed the resp-cache-eviction-metrics branch from 95ab0ad to 74aaf45 Compare July 29, 2023 23:03
@realtyem realtyem marked this pull request as ready for review July 29, 2023 23:03
@realtyem realtyem requested a review from a team as a code owner July 29, 2023 23:03
@realtyem realtyem force-pushed the resp-cache-eviction-metrics branch from 74aaf45 to 61ccf6c Compare July 29, 2023 23:05
realtyem
realtyem commented Jul 29, 2023
View reviewed changes
synapse/util/caches/response_cache.py Outdated

def entry_timeout(self, key: KV) -> None:
"""For the call_later to remove from the cache"""
logger.debug(f"Expiring from [{self._name}] - {key}")
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 am open to removing the debugging log lines, but it's up to you.

@clokep
Copy link
Member

clokep commented Jul 31, 2023

@realtyem You mentioned to me that we do this for "regular" caches; can you link to that code? It should help with review.

@realtyem
Copy link
Contributor Author

@realtyem You mentioned to me that we do this for "regular" caches; can you link to that code? It should help with review.

By "regular" caches, I meant like LruCache which uses a completely different set of mechanisms for evicting 'entries'. Consider: jemalloc, background loops, cache-invalidation-over-replication, etc. The ResponseCache is by nature a much, much simpler implementation. I can try to pin it down, which one would you like to see?

clokep
clokep reviewed Jul 31, 2023
View reviewed changes
synapse/util/caches/response_cache.py Outdated
self._metrics.inc_evictions(reason)
self.unset(key)

def entry_timeout(self, key: KV) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

This seems reasonable, but I have a few suggestions:

  • Let's make this private (add a _ in front of it).
  • Get rid of the evict_because method -- it is trivial and doesn't add much IMO / only adds indirection.
  • Stop calling unset and inline it.
  • Update unset to include self._metrics.inc_evictions(EvicationReason.invalidation).

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 think I got it all into 59563dd

@realtyem
Address review feedback
59563dd 
1. Change entry_timeout to _entry_timeout
2. Inline popping the cache item instead of using unset
3. Inline evict_because into _entry_timeout
4. Add EvictionReason.invalidate metrics counting to unset, so it is counted too
@realtyem realtyem requested a review from clokep August 1, 2023 08:58
@realtyem realtyem changed the title Add metrics tracking for eviction based on time to ResponseCache Add metrics tracking for eviction to ResponseCache Aug 1, 2023
@clokep clokep merged commit 7cbb2a0 into matrix-org:develop Aug 1, 2023
37 checks passed
@clokep
Copy link
Member

clokep commented Aug 1, 2023

Thanks!

yingziwu added a commit to yingziwu/synapse that referenced this pull request Aug 21, 2023
@yingziwu
Merge tag 'v1.90.0' into develop
046021b 
No significant changes since 1.90.0rc1.

- Scope transaction IDs to devices (implement [MSC3970](matrix-org/matrix-spec-proposals#3970)). ([\matrix-org#15629](matrix-org#15629))
- Remove old rows from the `cache_invalidation_stream_by_instance` table automatically (this table is unused in SQLite). ([\matrix-org#15868](matrix-org#15868))

- Fix a long-standing bug where purging history and paginating simultaneously could lead to database corruption when using workers. ([\matrix-org#15791](matrix-org#15791))
- Fix a long-standing bug where profile endpoint returned a 404 when the user's display name was empty. ([\matrix-org#16012](matrix-org#16012))
- Fix a long-standing bug where the `synapse_port_db` failed to configure sequences for application services and partial stated rooms. ([\matrix-org#16043](matrix-org#16043))
- Fix long-standing bug with deletion in dehydrated devices v2. ([\matrix-org#16046](matrix-org#16046))

- Add `org.opencontainers.image.version` labels to Docker containers [published by Matrix.org](https://hub.docker.com/r/matrixdotorg/synapse). Contributed by Mo Balaa. ([\matrix-org#15972](matrix-org#15972), [\matrix-org#16009](matrix-org#16009))

- Add a internal documentation page describing the ["streams" used within Synapse](https://matrix-org.github.io/synapse/v1.90/development/synapse_architecture/streams.html). ([\matrix-org#16015](matrix-org#16015))
- Clarify comment on the keys/upload over replication enpoint. ([\matrix-org#16016](matrix-org#16016))
- Do not expose Admin API in caddy reverse proxy example. Contributed by @NilsIrl. ([\matrix-org#16027](matrix-org#16027))

- Remove support for legacy application service paths. ([\matrix-org#15964](matrix-org#15964))
- Move support for application service query parameter authorization behind a configuration option. ([\matrix-org#16017](matrix-org#16017))

- Update SQL queries to inline boolean parameters as supported in SQLite 3.27. ([\matrix-org#15525](matrix-org#15525))
- Allow for the configuration of the backoff algorithm for federation destinations. ([\matrix-org#15754](matrix-org#15754))
- Allow modules to check whether the current worker is configured to run background tasks. ([\matrix-org#15991](matrix-org#15991))
- Update support for [MSC3958](matrix-org/matrix-spec-proposals#3958) to match the latest revision of the MSC. ([\matrix-org#15992](matrix-org#15992))
- Allow modules to schedule delayed background calls. ([\matrix-org#15993](matrix-org#15993))
- Properly overwrite the `redacts` content-property for forwards-compatibility with room versions 1 through 10. ([\matrix-org#16013](matrix-org#16013))
- Fix building the nix development environment on MacOS systems. ([\matrix-org#16019](matrix-org#16019))
- Remove leading and trailing spaces when setting a display name. ([\matrix-org#16031](matrix-org#16031))
- Combine duplicated code. ([\matrix-org#16023](matrix-org#16023))
- Collect additional metrics from `ResponseCache` for eviction. ([\matrix-org#16028](matrix-org#16028))
- Fix endpoint improperly declaring support for MSC3814. ([\matrix-org#16068](matrix-org#16068))
- Drop backwards compat hack for event serialization. ([\matrix-org#16069](matrix-org#16069))

* Update PyYAML to 6.0.1. ([\matrix-org#16011](matrix-org#16011))
* Bump cryptography from 41.0.2 to 41.0.3. ([\matrix-org#16048](matrix-org#16048))
* Bump furo from 2023.5.20 to 2023.7.26. ([\matrix-org#16077](matrix-org#16077))
* Bump immutabledict from 2.2.4 to 3.0.0. ([\matrix-org#16034](matrix-org#16034))
* Update certifi to 2023.7.22 and pygments to 2.15.1. ([\matrix-org#16044](matrix-org#16044))
* Bump jsonschema from 4.18.3 to 4.19.0. ([\matrix-org#16081](matrix-org#16081))
* Bump phonenumbers from 8.13.14 to 8.13.18. ([\matrix-org#16076](matrix-org#16076))
* Bump regex from 1.9.1 to 1.9.3. ([\matrix-org#16073](matrix-org#16073))
* Bump serde from 1.0.171 to 1.0.175. ([\matrix-org#15982](matrix-org#15982))
* Bump serde from 1.0.175 to 1.0.179. ([\matrix-org#16033](matrix-org#16033))
* Bump serde from 1.0.179 to 1.0.183. ([\matrix-org#16074](matrix-org#16074))
* Bump serde_json from 1.0.103 to 1.0.104. ([\matrix-org#16032](matrix-org#16032))
* Bump service-identity from 21.1.0 to 23.1.0. ([\matrix-org#16038](matrix-org#16038))
* Bump types-commonmark from 0.9.2.3 to 0.9.2.4. ([\matrix-org#16037](matrix-org#16037))
* Bump types-jsonschema from 4.17.0.8 to 4.17.0.10. ([\matrix-org#16036](matrix-org#16036))
* Bump types-netaddr from 0.8.0.8 to 0.8.0.9. ([\matrix-org#16035](matrix-org#16035))
* Bump types-opentracing from 2.4.10.5 to 2.4.10.6. ([\matrix-org#16078](matrix-org#16078))
* Bump types-setuptools from 68.0.0.0 to 68.0.0.3. ([\matrix-org#16079](matrix-org#16079))
Fizzadar added a commit to beeper/synapse-legacy-fork that referenced this pull request Sep 18, 2023
@Fizzadar
Merge tag 'v1.90.0' into merge-1.90
abee71f 
No significant changes since 1.90.0rc1.

- Scope transaction IDs to devices (implement [MSC3970](matrix-org/matrix-spec-proposals#3970)). ([\matrix-org#15629](matrix-org#15629))
- Remove old rows from the `cache_invalidation_stream_by_instance` table automatically (this table is unused in SQLite). ([\matrix-org#15868](matrix-org#15868))

- Fix a long-standing bug where purging history and paginating simultaneously could lead to database corruption when using workers. ([\matrix-org#15791](matrix-org#15791))
- Fix a long-standing bug where profile endpoint returned a 404 when the user's display name was empty. ([\matrix-org#16012](matrix-org#16012))
- Fix a long-standing bug where the `synapse_port_db` failed to configure sequences for application services and partial stated rooms. ([\matrix-org#16043](matrix-org#16043))
- Fix long-standing bug with deletion in dehydrated devices v2. ([\matrix-org#16046](matrix-org#16046))

- Add `org.opencontainers.image.version` labels to Docker containers [published by Matrix.org](https://hub.docker.com/r/matrixdotorg/synapse). Contributed by Mo Balaa. ([\matrix-org#15972](matrix-org#15972), [\matrix-org#16009](matrix-org#16009))

- Add a internal documentation page describing the ["streams" used within Synapse](https://matrix-org.github.io/synapse/v1.90/development/synapse_architecture/streams.html). ([\matrix-org#16015](matrix-org#16015))
- Clarify comment on the keys/upload over replication enpoint. ([\matrix-org#16016](matrix-org#16016))
- Do not expose Admin API in caddy reverse proxy example. Contributed by @NilsIrl. ([\matrix-org#16027](matrix-org#16027))

- Remove support for legacy application service paths. ([\matrix-org#15964](matrix-org#15964))
- Move support for application service query parameter authorization behind a configuration option. ([\matrix-org#16017](matrix-org#16017))

- Update SQL queries to inline boolean parameters as supported in SQLite 3.27. ([\matrix-org#15525](matrix-org#15525))
- Allow for the configuration of the backoff algorithm for federation destinations. ([\matrix-org#15754](matrix-org#15754))
- Allow modules to check whether the current worker is configured to run background tasks. ([\matrix-org#15991](matrix-org#15991))
- Update support for [MSC3958](matrix-org/matrix-spec-proposals#3958) to match the latest revision of the MSC. ([\matrix-org#15992](matrix-org#15992))
- Allow modules to schedule delayed background calls. ([\matrix-org#15993](matrix-org#15993))
- Properly overwrite the `redacts` content-property for forwards-compatibility with room versions 1 through 10. ([\matrix-org#16013](matrix-org#16013))
- Fix building the nix development environment on MacOS systems. ([\matrix-org#16019](matrix-org#16019))
- Remove leading and trailing spaces when setting a display name. ([\matrix-org#16031](matrix-org#16031))
- Combine duplicated code. ([\matrix-org#16023](matrix-org#16023))
- Collect additional metrics from `ResponseCache` for eviction. ([\matrix-org#16028](matrix-org#16028))
- Fix endpoint improperly declaring support for MSC3814. ([\matrix-org#16068](matrix-org#16068))
- Drop backwards compat hack for event serialization. ([\matrix-org#16069](matrix-org#16069))

* Update PyYAML to 6.0.1. ([\matrix-org#16011](matrix-org#16011))
* Bump cryptography from 41.0.2 to 41.0.3. ([\matrix-org#16048](matrix-org#16048))
* Bump furo from 2023.5.20 to 2023.7.26. ([\matrix-org#16077](matrix-org#16077))
* Bump immutabledict from 2.2.4 to 3.0.0. ([\matrix-org#16034](matrix-org#16034))
* Update certifi to 2023.7.22 and pygments to 2.15.1. ([\matrix-org#16044](matrix-org#16044))
* Bump jsonschema from 4.18.3 to 4.19.0. ([\matrix-org#16081](matrix-org#16081))
* Bump phonenumbers from 8.13.14 to 8.13.18. ([\matrix-org#16076](matrix-org#16076))
* Bump regex from 1.9.1 to 1.9.3. ([\matrix-org#16073](matrix-org#16073))
* Bump serde from 1.0.171 to 1.0.175. ([\matrix-org#15982](matrix-org#15982))
* Bump serde from 1.0.175 to 1.0.179. ([\matrix-org#16033](matrix-org#16033))
* Bump serde from 1.0.179 to 1.0.183. ([\matrix-org#16074](matrix-org#16074))
* Bump serde_json from 1.0.103 to 1.0.104. ([\matrix-org#16032](matrix-org#16032))
* Bump service-identity from 21.1.0 to 23.1.0. ([\matrix-org#16038](matrix-org#16038))
* Bump types-commonmark from 0.9.2.3 to 0.9.2.4. ([\matrix-org#16037](matrix-org#16037))
* Bump types-jsonschema from 4.17.0.8 to 4.17.0.10. ([\matrix-org#16036](matrix-org#16036))
* Bump types-netaddr from 0.8.0.8 to 0.8.0.9. ([\matrix-org#16035](matrix-org#16035))
* Bump types-opentracing from 2.4.10.5 to 2.4.10.6. ([\matrix-org#16078](matrix-org#16078))
* Bump types-setuptools from 68.0.0.0 to 68.0.0.3. ([\matrix-org#16079](matrix-org#16079))

# -----BEGIN PGP SIGNATURE-----
#
# iQIzBAABCAAdFiEE8SRSDO7gYkSP4chELS76LzL74EcFAmTbUOEACgkQLS76LzL7
# 4Efskw/+J4X30PoqSvBWbilr8kTzwNSDXrkefYXR2sLburgowCyuAtKtCdbvnZUX
# 3KRwii5/GDsduXiNY836oRxO/KWE43b1ce9C9qJM7V6NmgkJBgHRvnh69wdlmBqt
# 6b6TQHoEByYS7yK90+QsRm1Bqrw7eoVO9oxcZ+4lb7Mjswf491Pga8kFJqdvjtTX
# UXo4vAqYyP6Yn7sUrQmXy0N8gZ5ZFHhZEvZZ8+iEsNjPO468cSVGq8/iPB1EwBm2
# nbfZWMDnD2p7plJezXOPEBxnVR3RrWbCbK08SiiNMcQynCvBgAUfkd3GnsO726jb
# 19i8p6tjuj2r41UgqYCTG2i2ij6uJquA/qq3rIiVNQVKG9aPHQ8hJfu9XOdEvaJe
# Je9H6QFrU/hR640tFvb5Hdc/4ThabvtC5xgl4ZGT6y6I0s5LNwk8fJiz3sDFt0i1
# XKsqGVemBGopZnwjQIPFJaHjPT7of33PXLE/hf1vX+oXU/6MNbFYkDLY9nnnQeOx
# 0GEbiYaxrj8SfxNmEykMLNCfxwJ719cSR1q8vPYn6r6TOS1pJMV0SgciXoaQ/VW6
# WlRpZolvXYSye34JW8Rg4ojAz0oYfJ2IiUpwY7eSEq4DtuosTjEECKrgB8DLqKlM
# +qDd4/yeqVN5/sui5oGsR71aTMy/jdnzqmdmuFvsSwz9/7PfMEU=
# =FWp5
# -----END PGP SIGNATURE-----
# gpg: Signature made Tue Aug 15 11:18:09 2023 BST
# gpg:                using RSA key F124520CEEE062448FE1C8442D2EFA2F32FBE047
# gpg: Can't check signature: No public key

# Conflicts:
#	.github/workflows/docker.yml
#	CHANGES.md
#	Cargo.lock
#	debian/changelog
#	docs/upgrade.md
#	flake.lock
#	poetry.lock
#	pyproject.toml
#	rust/src/push/base_rules.rs
#	synapse/events/utils.py
#	synapse/handlers/pagination.py
#	synapse/http/site.py
#	synapse/rest/client/devices.py
#	synapse/storage/databases/main/roommember.py
#	synapse/storage/schema/__init__.py
#	tests/rest/client/test_devices.py
#	tests/rest/client/test_redactions.py
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@clokep clokep clokep approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@realtyem @clokep