Add a cache around server ACL checking #16360
Conversation
rust/src/acl/mod.rs
Outdated
| // check for ipv6 literals. These start with '['. | ||
| if server_name.starts_with("[") { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
This attempts to stay close to the Python code, maybe it would make more sense to do Ipv6Addr. 🤷
There was a problem hiding this comment.
I didn't fully follow the stuff about having to go via storage controllers or not. Defer to @erikjohnston on whether or not this makes sense (it probably does?)
I've left some more comments too.
I guess we already have existing tests for this, and we're just changing the impl so don't have any new behaviours to worry about. It might be worth a test which changes the ACLs and checks that the new set of ACLs apply, if we haven't got such a test already. (The thinking is to sanity-check the cache invalidation works!)
There was a problem hiding this comment.
From 50 feet away, I'm just a little anxious that we now have three places to remember to invalidate the cache. In general, I find it hard to know when to invalidate caches other than "put it in a sensible place to make the bug go away". But maybe this is fine.
It might be worth a test which changes the ACLs and checks that the new set of ACLs apply, if we haven't got such a test already. (The thinking is to sanity-check the cache invalidation works!)
Any thoughts on this---do you think it'd be worthwhile?
It is tested by sytest. I had removed the cache invalidation and tests fail. I'll double check though. |
Aha, that works for me! |
|
@erikjohnston Any last thoughts? |
Tests fail when I remove the invalidation. 😄 |
Ship it! |
No significant changes since 1.94.0rc1. - Render plain, CSS, CSV, JSON and common image formats in the browser (inline) when requested through the /download endpoint. ([\matrix-org#15988](matrix-org#15988)) - Add experimental support for [MSC4028](matrix-org/matrix-spec-proposals#4028) to push all encrypted events to clients. ([\matrix-org#16361](matrix-org#16361)) - Minor performance improvement when sending presence to federated servers. ([\matrix-org#16385](matrix-org#16385)) - Minor performance improvement by caching server ACL checking. ([\matrix-org#16360](matrix-org#16360)) - Add developer documentation concerning gradual schema migrations with column alterations. ([\matrix-org#15691](matrix-org#15691)) - Improve documentation of the user directory search algorithm. ([\matrix-org#16320](matrix-org#16320)) - Fix rendering of user admin API documentation around deactivation. This was broken in Synapse 1.91.0. ([\matrix-org#16355](matrix-org#16355)) - Update documentation around message retention policies. ([\matrix-org#16382](matrix-org#16382)) - Add note to `federation_domain_whitelist` config option to clarify its usage. ([\matrix-org#16416](matrix-org#16416)) - Improve legacy release notes. ([\matrix-org#16418](matrix-org#16418)) - Remove Python version from `/_synapse/admin/v1/server_version`. ([\matrix-org#16380](matrix-org#16380)) - Avoid running CI steps when the files they check have not been changed. ([\matrix-org#14745](matrix-org#14745), [\matrix-org#16387](matrix-org#16387)) - Improve type hints. ([\matrix-org#14911](matrix-org#14911), [\matrix-org#16350](matrix-org#16350), [\matrix-org#16356](matrix-org#16356), [\matrix-org#16395](matrix-org#16395)) - Added support for pydantic v2 in addition to pydantic v1. Contributed by Maxwell G (@gotmax23). ([\matrix-org#16332](matrix-org#16332)) - Get CI to check PRs have been signed-off. ([\matrix-org#16348](matrix-org#16348)) - Add missing licence header. ([\matrix-org#16359](matrix-org#16359)) - Improve type hints, and bump types-psycopg2 from 2.9.21.11 to 2.9.21.14. ([\matrix-org#16381](matrix-org#16381)) - Improve comments in `StateGroupBackgroundUpdateStore`. ([\matrix-org#16383](matrix-org#16383)) - Update maturin configuration. ([\matrix-org#16394](matrix-org#16394)) - Downgrade replication stream time out error log lines to warning. ([\matrix-org#16401](matrix-org#16401)) * Bump actions/checkout from 3 to 4. ([\matrix-org#16250](matrix-org#16250)) * Bump cryptography from 41.0.3 to 41.0.4. ([\matrix-org#16362](matrix-org#16362)) * Bump dawidd6/action-download-artifact from 2.27.0 to 2.28.0. ([\matrix-org#16374](matrix-org#16374)) * Bump docker/setup-buildx-action from 2 to 3. ([\matrix-org#16375](matrix-org#16375)) * Bump gitpython from 3.1.35 to 3.1.37. ([\matrix-org#16376](matrix-org#16376)) * Bump msgpack from 1.0.5 to 1.0.6. ([\matrix-org#16377](matrix-org#16377)) * Bump msgpack from 1.0.6 to 1.0.7. ([\matrix-org#16412](matrix-org#16412)) * Bump phonenumbers from 8.13.19 to 8.13.22. ([\matrix-org#16413](matrix-org#16413)) * Bump psycopg2 from 2.9.7 to 2.9.8. ([\matrix-org#16409](matrix-org#16409)) * Bump pydantic from 2.3.0 to 2.4.2. ([\matrix-org#16410](matrix-org#16410)) * Bump regex from 1.9.5 to 1.9.6. ([\matrix-org#16408](matrix-org#16408)) * Bump sentry-sdk from 1.30.0 to 1.31.0. ([\matrix-org#16378](matrix-org#16378)) * Bump types-netaddr from 0.8.0.9 to 0.9.0.1. ([\matrix-org#16411](matrix-org#16411)) * Bump types-psycopg2 from 2.9.21.11 to 2.9.21.14. ([\matrix-org#16381](matrix-org#16381)) * Bump urllib3 from 1.26.15 to 1.26.17. ([\matrix-org#16422](matrix-org#16422)) # -----BEGIN PGP SIGNATURE----- # # iQIzBAABCAAdFiEE8SRSDO7gYkSP4chELS76LzL74EcFAmUlINwACgkQLS76LzL7 # 4EdvExAAgjk6+/Fu45MRG7u5kFmFzoZWLOPD10XROANaSeqW1l/pBhFh+XvwR4TZ # l/FdkSfS9YpHnw3aof13TclLu6IVWDM+vqYFuY2HSY/yzbcGvJFHqr26kOccpTTd # 2r9m/AkguyHEBECDW8qJLXb8M7dqNa2SydTBu1+IrKfj6nq+fRxVyQhyAJXrI1Ta # Dnz0XJ4TcwTrMPVk4MYrAcYjID6IV89dtp7ttH4DwXKDeSjMtxM/46EIg4u+VXDz # fzK25JHVFYJA5+/rOn/RslmxjJHQfEIEB6NYxQwLeMeZuGSZooTebKn1odwogvhI # Srtfsytum+twgSHD1s+7KldM+EjTiu7ouKi8VcfOlFuLnuBiROEc5WUljcL5K63F # kVx2bXGU/eNkPp6ntNhYfgswx+yk2rXFqkTjz+xZQIZcOBqehHBDy8VhtwlRkTUw # bzocdKkLMA4nfSlq5fFOAErMqJKsPS8aN9yYPShqEUiSUOKle8eHfA1cTXJuK0MS # K2/YcDDZmJBrwVADyNDk5GKaDx39rR752OSuJb57Sp/edwUg6+H1I6lIN6YTeoJw # FzJwGMzuMCktOQRW2enxQiA6RZjXFCwvD1LoWMjyO4YTXQwXxNCXsb0kLKUqfwsy # qMGphWEl3rdzVSuFapNAgOLF0RfFNYZdhQnk+3fNEwxumxoqgho= # =hx8G # -----END PGP SIGNATURE----- # gpg: Signature made Tue Oct 10 11:01:00 2023 BST # gpg: using RSA key F124520CEEE062448FE1C8442D2EFA2F32FBE047 # gpg: Can't check signature: No public key # Conflicts: # .github/workflows/docker.yml # .github/workflows/docs-pr-netlify.yaml # .github/workflows/docs-pr.yaml # .github/workflows/docs.yaml # .github/workflows/latest_deps.yml # .github/workflows/poetry_lockfile.yaml # .github/workflows/push_complement_image.yml # .github/workflows/release-artifacts.yml # .github/workflows/tests.yml # .github/workflows/twisted_trunk.yml # poetry.lock # rust/src/push/base_rules.rs
We were hitting this block frequently causing use to throw redo work unnecessarily. Just throw a cache on it.
I selfishly also converted it to Rust while I was here because it uses many of the same primitives as our push rules code, but I can back this out if it seems over the top.