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

Time out busy presence status & test multi-device busy #16174

Merged
merged 2 commits into from
Sep 5, 2023
Merged

Time out busy presence status & test multi-device busy #16174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
405d326
Add multi-device tests with busy status.
clokep Aug 23, 2023
2ce32cc
Newsfragment
clokep Aug 23, 2023
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
1 change: 1 addition & 0 deletions changelog.d/16174.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a long-standing bug where multi-device accounts could cause high load due to presence.
19 changes: 18 additions & 1 deletion synapse/handlers/presence.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@
# How long to wait until a new /events or /sync request before assuming
# the client has gone.
SYNC_ONLINE_TIMEOUT = 30 * 1000
# Busy status waits longer, but does eventually go offline.
BUSY_ONLINE_TIMEOUT = 60 * 60 * 1000

# How long to wait before marking the user as idle. Compared against last active
IDLE_TIMER = 5 * 60 * 1000
Expand Down Expand Up @@ -2066,7 +2068,15 @@ def handle_timeout(
device_state.last_sync_ts, device_state.last_active_ts
)

if now - sync_or_active > SYNC_ONLINE_TIMEOUT:
# Implementations aren't meant to timeout a device with a busy
# state, but it needs to timeout *eventually* or else the user
# will be stuck in that state.
online_timeout = (
BUSY_ONLINE_TIMEOUT
if device_state.state == PresenceState.BUSY
else SYNC_ONLINE_TIMEOUT
)
if now - sync_or_active > online_timeout:
# Mark the device as going offline.
offline_devices.append(device_id)
device_changed = True
Expand Down Expand Up @@ -2166,6 +2176,13 @@ def handle_update(
new_state = new_state.copy_and_replace(last_federation_update_ts=now)
federation_ping = True

if new_state.state == PresenceState.BUSY:
wheel_timer.insert(
now=now,
obj=user_id,
then=new_state.last_user_sync_ts + BUSY_ONLINE_TIMEOUT,
)

else:
wheel_timer.insert(
now=now,
Expand Down
104 changes: 101 additions & 3 deletions tests/handlers/test_presence.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
from synapse.events.builder import EventBuilder
from synapse.federation.sender import FederationSender
from synapse.handlers.presence import (
BUSY_ONLINE_TIMEOUT,
EXTERNAL_PROCESS_EXPIRY,
FEDERATION_PING_INTERVAL,
FEDERATION_TIMEOUT,
Expand Down Expand Up @@ -912,6 +913,13 @@ def test_set_presence_from_syncing_is_set(self) -> None:
for cases in [
# If both devices have the same state, online should eventually idle.
# Otherwise, the state doesn't change.
(
PresenceState.BUSY,
PresenceState.BUSY,
PresenceState.BUSY,
PresenceState.BUSY,
PresenceState.BUSY,
),
(
PresenceState.ONLINE,
PresenceState.ONLINE,
Expand All @@ -933,7 +941,29 @@ def test_set_presence_from_syncing_is_set(self) -> None:
PresenceState.OFFLINE,
PresenceState.OFFLINE,
),
# If the second device has a "lower" state it should fallback to it.
# If the second device has a "lower" state it should fallback to it,
# except for "busy" which overrides.
(
PresenceState.BUSY,
PresenceState.ONLINE,
PresenceState.BUSY,
PresenceState.BUSY,
PresenceState.BUSY,
),
(
PresenceState.BUSY,
PresenceState.UNAVAILABLE,
PresenceState.BUSY,
PresenceState.BUSY,
PresenceState.BUSY,
),
(
PresenceState.BUSY,
PresenceState.OFFLINE,
PresenceState.BUSY,
PresenceState.BUSY,
PresenceState.BUSY,
),
(
PresenceState.ONLINE,
PresenceState.UNAVAILABLE,
Expand All @@ -956,6 +986,27 @@ def test_set_presence_from_syncing_is_set(self) -> None:
PresenceState.UNAVAILABLE,
),
# If the second device has a "higher" state it should override.
(
PresenceState.ONLINE,
PresenceState.BUSY,
PresenceState.BUSY,
PresenceState.BUSY,
PresenceState.BUSY,
),
(
PresenceState.UNAVAILABLE,
PresenceState.BUSY,
PresenceState.BUSY,
PresenceState.BUSY,
PresenceState.BUSY,
),
(
PresenceState.OFFLINE,
PresenceState.BUSY,
PresenceState.BUSY,
PresenceState.BUSY,
PresenceState.BUSY,
),
(
PresenceState.UNAVAILABLE,
PresenceState.ONLINE,
Expand Down Expand Up @@ -1114,6 +1165,12 @@ def test_set_presence_from_syncing_multi_device(
for workers in (False, True)
for cases in [
# If both devices have the same state, nothing exciting should happen.
(
PresenceState.BUSY,
PresenceState.BUSY,
PresenceState.BUSY,
PresenceState.BUSY,
),
(
PresenceState.ONLINE,
PresenceState.ONLINE,
Expand All @@ -1132,7 +1189,26 @@ def test_set_presence_from_syncing_multi_device(
PresenceState.OFFLINE,
PresenceState.OFFLINE,
),
# If the second device has a "lower" state it should fallback to it.
# If the second device has a "lower" state it should fallback to it,
# except for "busy" which overrides.
(
PresenceState.BUSY,
PresenceState.ONLINE,
PresenceState.BUSY,
PresenceState.BUSY,
),
(
PresenceState.BUSY,
PresenceState.UNAVAILABLE,
PresenceState.BUSY,
PresenceState.BUSY,
),
(
PresenceState.BUSY,
PresenceState.OFFLINE,
PresenceState.BUSY,
PresenceState.BUSY,
),
(
PresenceState.ONLINE,
PresenceState.UNAVAILABLE,
Expand All @@ -1152,6 +1228,24 @@ def test_set_presence_from_syncing_multi_device(
PresenceState.OFFLINE,
),
# If the second device has a "higher" state it should override.
(
PresenceState.ONLINE,
PresenceState.BUSY,
PresenceState.BUSY,
PresenceState.BUSY,
),
(
PresenceState.UNAVAILABLE,
PresenceState.BUSY,
PresenceState.BUSY,
PresenceState.BUSY,
),
(
PresenceState.OFFLINE,
PresenceState.BUSY,
PresenceState.BUSY,
PresenceState.BUSY,
),
(
PresenceState.UNAVAILABLE,
PresenceState.ONLINE,
Expand Down Expand Up @@ -1266,7 +1360,11 @@ def test_set_presence_from_non_syncing_multi_device(

# 8. Advance such that the second device should be discarded (the sync timeout),
# then pump so _handle_timeouts function to called.
self.reactor.advance(SYNC_ONLINE_TIMEOUT / 1000)
if dev_1_state == PresenceState.BUSY or dev_2_state == PresenceState.BUSY:
timeout = BUSY_ONLINE_TIMEOUT
else:
timeout = SYNC_ONLINE_TIMEOUT
self.reactor.advance(timeout / 1000)
self.reactor.pump([5])

# 9. There are no more devices, should be offline.
Expand Down
Loading