Skip to content

Commit

Permalink
Sliding Sync: Make PerConnectionState immutable (#17600)
Browse files Browse the repository at this point in the history
This is so that we can cache it.

We also move the sliding sync types to
`synapse/types/handlers/sliding_sync.py`. This is mainly in-prep for
#17599 to avoid circular imports.

The only change in behaviour is that
`RoomSyncConfig.combine_sync_config(..)` now returns a new room sync
config rather than mutating in-place.

Reviewable commit-by-commit.

---------

Co-authored-by: Eric Eastwood <eric.eastwood@beta.gouv.fr>
  • Loading branch information
erikjohnston and MadLittleMods authored Aug 29, 2024
1 parent 1a6b718 commit 2999a14
Show file tree
Hide file tree
Showing 9 changed files with 441 additions and 444 deletions.
1 change: 1 addition & 0 deletions changelog.d/17600.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Make the sliding sync `PerConnectionState` class immutable.
19 changes: 18 additions & 1 deletion scripts-dev/mypy_synapse_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
NoneType,
TupleType,
TypeAliasType,
TypeVarType,
UninhabitedType,
UnionType,
)
Expand Down Expand Up @@ -233,6 +234,7 @@ def check_is_cacheable(
"synapse.synapse_rust.push.FilteredPushRules",
# This is technically not immutable, but close enough.
"signedjson.types.VerifyKey",
"synapse.types.StrCollection",
}

# Immutable containers only if the values are also immutable.
Expand Down Expand Up @@ -298,7 +300,7 @@ def is_cacheable(

elif rt.type.fullname in MUTABLE_CONTAINER_TYPES:
# Mutable containers are mutable regardless of their underlying type.
return False, None
return False, f"container {rt.type.fullname} is mutable"

elif "attrs" in rt.type.metadata:
# attrs classes are only cachable iff it is frozen (immutable itself)
Expand All @@ -318,6 +320,9 @@ def is_cacheable(
else:
return False, "non-frozen attrs class"

elif rt.type.is_enum:
# We assume Enum values are immutable
return True, None
else:
# Ensure we fail for unknown types, these generally means that the
# above code is not complete.
Expand All @@ -326,6 +331,18 @@ def is_cacheable(
f"Don't know how to handle {rt.type.fullname} return type instance",
)

elif isinstance(rt, TypeVarType):
# We consider TypeVars immutable if they are bound to a set of immutable
# types.
if rt.values:
for value in rt.values:
ok, note = is_cacheable(value, signature, verbose)
if not ok:
return False, f"TypeVar bound not cacheable {value}"
return True, None

return False, "TypeVar is unbound"

elif isinstance(rt, NoneType):
# None is cachable.
return True, None
Expand Down
16 changes: 7 additions & 9 deletions synapse/handlers/sliding_sync/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,6 @@
_RoomMembershipForUser,
)
from synapse.handlers.sliding_sync.store import SlidingSyncConnectionStore
from synapse.handlers.sliding_sync.types import (
HaveSentRoomFlag,
MutablePerConnectionState,
PerConnectionState,
RoomSyncConfig,
StateValues,
)
from synapse.logging.opentracing import (
SynapseTags,
log_kv,
Expand All @@ -57,10 +50,15 @@
StreamKeyType,
StreamToken,
)
from synapse.types.handlers import (
SLIDING_SYNC_DEFAULT_BUMP_EVENT_TYPES,
from synapse.types.handlers import SLIDING_SYNC_DEFAULT_BUMP_EVENT_TYPES
from synapse.types.handlers.sliding_sync import (
HaveSentRoomFlag,
MutablePerConnectionState,
PerConnectionState,
RoomSyncConfig,
SlidingSyncConfig,
SlidingSyncResult,
StateValues,
)
from synapse.types.state import StateFilter
from synapse.util.async_helpers import concurrently_execute
Expand Down
14 changes: 8 additions & 6 deletions synapse/handlers/sliding_sync/extensions.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,6 @@

from synapse.api.constants import AccountDataTypes, EduTypes
from synapse.handlers.receipts import ReceiptEventSource
from synapse.handlers.sliding_sync.types import (
HaveSentRoomFlag,
MutablePerConnectionState,
PerConnectionState,
)
from synapse.logging.opentracing import trace
from synapse.storage.databases.main.receipts import ReceiptInRoom
from synapse.types import (
Expand All @@ -35,7 +30,14 @@
StrCollection,
StreamToken,
)
from synapse.types.handlers import OperationType, SlidingSyncConfig, SlidingSyncResult
from synapse.types.handlers.sliding_sync import (
HaveSentRoomFlag,
MutablePerConnectionState,
OperationType,
PerConnectionState,
SlidingSyncConfig,
SlidingSyncResult,
)

if TYPE_CHECKING:
from synapse.server import HomeServer
Expand Down
34 changes: 17 additions & 17 deletions synapse/handlers/sliding_sync/room_lists.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,6 @@
)
from synapse.events import StrippedStateEvent
from synapse.events.utils import parse_stripped_state_event
from synapse.handlers.sliding_sync.types import (
HaveSentRoomFlag,
PerConnectionState,
RoomSyncConfig,
)
from synapse.logging.opentracing import start_active_span, trace
from synapse.storage.databases.main.state import (
ROOM_UNKNOWN_SENTINEL,
Expand All @@ -61,7 +56,14 @@
StreamToken,
UserID,
)
from synapse.types.handlers import OperationType, SlidingSyncConfig, SlidingSyncResult
from synapse.types.handlers.sliding_sync import (
HaveSentRoomFlag,
OperationType,
PerConnectionState,
RoomSyncConfig,
SlidingSyncConfig,
SlidingSyncResult,
)
from synapse.types.state import StateFilter

if TYPE_CHECKING:
Expand Down Expand Up @@ -279,15 +281,11 @@ async def compute_interested_rooms(
room_id
)
if existing_room_sync_config is not None:
existing_room_sync_config.combine_room_sync_config(
room_sync_config = existing_room_sync_config.combine_room_sync_config(
room_sync_config
)
else:
# Make a copy so if we modify it later, it doesn't
# affect all references.
relevant_room_map[room_id] = (
room_sync_config.deep_copy()
)

relevant_room_map[room_id] = room_sync_config

room_ids_in_list.append(room_id)

Expand Down Expand Up @@ -351,11 +349,13 @@ async def compute_interested_rooms(
# and need to fetch more info about.
existing_room_sync_config = relevant_room_map.get(room_id)
if existing_room_sync_config is not None:
existing_room_sync_config.combine_room_sync_config(
room_sync_config
room_sync_config = (
existing_room_sync_config.combine_room_sync_config(
room_sync_config
)
)
else:
relevant_room_map[room_id] = room_sync_config

relevant_room_map[room_id] = room_sync_config

# Filtered subset of `relevant_room_map` for rooms that may have updates
# (in the event stream)
Expand Down
8 changes: 4 additions & 4 deletions synapse/handlers/sliding_sync/store.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@
import attr

from synapse.api.errors import SlidingSyncUnknownPosition
from synapse.handlers.sliding_sync.types import (
from synapse.logging.opentracing import trace
from synapse.types import SlidingSyncStreamToken
from synapse.types.handlers.sliding_sync import (
MutablePerConnectionState,
PerConnectionState,
SlidingSyncConfig,
)
from synapse.logging.opentracing import trace
from synapse.types import SlidingSyncStreamToken
from synapse.types.handlers import SlidingSyncConfig

if TYPE_CHECKING:
pass
Expand Down
Loading

0 comments on commit 2999a14

Please sign in to comment.