fix: Invalidate space hierarchy cache on room join/leave events #1524

javif89 · 2026-03-10T03:58:53Z

javif89 commented

2026-03-10 03:58:53 +00:00

First-time contributor

This pull request adds SpaceRoomHierarchy cache invalidation on room join/leave events.

This change should prevent potentially dead rooms from showing up in clients, and make it easier
to make decisions based on a room's member count returned from the /[room id]/hierarchy endpoint.

I moved the roomid_spacehierarchy_cache up a level to the state_cache so it could be accessed in update_joined_count.
Otherwise we would have to invalidate the cache in any other spot where the member count might change.

Closes #1522

Pull request checklist:

This pull request targets the main branch, and the branch is named something other than
main.
I have written an appropriate pull request title and my description is clear.
I understand I am responsible for the contents of this pull request.
I have followed the contributing guidelines:
- My contribution follows the code style, if applicable.
- I ran pre-commit checks before opening/drafting this pull request.
- I have tested my contribution (or proof-read it for documentation-only changes)
  myself, if applicable. This includes ensuring code compiles.
- My commit messages follow the commit message format and are descriptive.
- I have written a news fragment for this PR, if applicable.

This pull request adds `SpaceRoomHierarchy` cache invalidation on room join/leave events. This change should prevent potentially dead rooms from showing up in clients, and make it easier to make decisions based on a room's member count returned from the `/[room id]/hierarchy` endpoint. I moved the `roomid_spacehierarchy_cache` up a level to the `state_cache` so it could be accessed in `update_joined_count`. Otherwise we would have to invalidate the cache in any other spot where the member count might change. Closes #1522 **Pull request checklist:** - [x] This pull request targets the `main` branch, and the branch is named something other than `main`. - [x] I have written an appropriate pull request title and my description is clear. - [x] I understand I am responsible for the contents of this pull request. - I have followed the [contributing guidelines][c1]: - [x] My contribution follows the [code style][c2], if applicable. - [x] I ran [pre-commit checks][c1pc] before opening/drafting this pull request. - [x] I have [tested my contribution][c1t] (or proof-read it for documentation-only changes) myself, if applicable. This includes ensuring code compiles. - [x] My commit messages follow the [commit message format][c1cm] and are descriptive. - [ ] I have written a [news fragment][n1] for this PR, if applicable. [c1]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md [c2]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/docs/development/code_style.mdx [c1pc]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md#pre-commit-checks [c1t]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md#running-tests-locally [c1cm]: https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/CONTRIBUTING.md#commit-messages [n1]: https://towncrier.readthedocs.io/en/stable/tutorial.html#creating-news-fragments

ginger requested changes

2026-03-11 12:51:50 +00:00

ginger left a comment

a few preliminary thoughts

.pre-commit-config.yaml Outdated

					
				@ -40,7 +40,7 @@ repos:

				    hooks:

				      - id: cargo-fmt

				        name: cargo fmt

				        entry: cargo +nightly fmt --

this is supposed to use nightly

Reverted 👍

nex marked this conversation as resolved

nix/shells/default.nix Outdated

					
				@ -21,8 +21,28 @@

				          pkgs.liburing

				          pkgs.rust-jemalloc-sys-unprefixed

				          rocksdbAllFeatures

				          # Pre commit support

these nix changes should go in their own PR

Reverted

nex marked this conversation as resolved

javif89 changed title from ~~WIP: fix: Invalidate space hierarchy cache on room join/leave events~~ to fix: Invalidate space hierarchy cache on room join/leave events

2026-03-11 21:07:14 +00:00

javif89 requested review from ginger

2026-03-12 16:10:58 +00:00

nex added the

Bug

Matrix/Client

labels

2026-03-13 21:20:41 +00:00

javif89 force-pushed fix/invalidate-hierarchy-cache-on-room-event from f402aaa84d

Documentation / Build and Deploy Documentation (pull_request) Has been skipped

Details

Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 2m56s

Details

Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 16m4s

Details

to 582e2aa9a8

Documentation / Build and Deploy Documentation (pull_request) Has been skipped

Details

Checks / Prek / Pre-commit & Formatting (pull_request) Successful in 3m4s

Details

Checks / Prek / Clippy and Cargo Tests (pull_request) Successful in 14m52s

Details

2026-03-15 00:55:27 +00:00

Compare

ginger requested changes

2026-03-17 13:17:55 +00:00

src/service/rooms/spaces/mod.rs Outdated

					
				@ -39,4 +38,3 @@

				pub struct Service {

					services: Services,

					pub roomid_spacehierarchy_cache: Mutex<Cache>,

why was this cache moved to the state_cache service? I think it's more relevant to this service

why was this cache moved to the `state_cache` service? I think it's more relevant to this service

nevermind, I can't read

ginger marked this conversation as resolved

src/service/rooms/state_cache/mod.rs Outdated

					
				@ -54,12 +63,18 @@ struct Data {

				}

				type AppServiceInRoomCache = SyncRwLock<HashMap<OwnedRoomId, HashMap<String, bool>>>;

				type SpaceHierarchyCache = Mutex<LruCache<OwnedRoomId, Option<CachedSpaceHierarchySummary>>>;

this may not need to be a Tokio mutex -- see https://docs.rs/tokio/latest/tokio/sync/struct.Mutex.html#which-kind-of-mutex-should-you-use

You might be right here. However, it was a tokio mutex before and I just moved the same logic, so I wouldn't want to change it as part of this PR since I don't know what downstream implications it would have.

Happy to work on that as part of a different task though if it would yield better performance.

You might be right here. However, [it was a tokio mutex before](https://forgejo.ellis.link/continuwuation/continuwuity/src/branch/main/src/service/rooms/spaces/mod.rs#L35) and I just moved the same logic, so I wouldn't want to change it as part of this PR since I don't know what downstream implications it would have. Happy to work on that as part of a different task though if it would yield better performance.