We don't cache the fact that a user has no devices

[DMRobertson]

Spotted by @squahtx.

Some users, e.g. appservice users, won't have any devices whatsoever. If we query their homeserver to retrieve that user's devices, it should respond with an empty list of devices. In the processing of that response, by the time we reach this line we have a devices list from a successful response whose size is at most 999.

We transform devices into a dictionary {d["device_id"]: d for d in devices}. Assuming there are no repeated device_ids, this is a reversible transformation.

synapse/synapse/handlers/device.py

Lines 951 to 958 in e5cdb9e

	cached_devices = await self.store.get_cached_devices_for_user(user_id)
	if cached_devices == {d["device_id"]: d for d in devices}:
	logging.info(
	"Skipping device list resync for %s, as our cache matches already",
	user_id,
	)
	devices = []
	ignore_devices = True

Here, cached_devices is:

synapse/synapse/storage/databases/main/devices.py

Lines 555 to 565 in 5305a5e

	@cached()
	async def get_cached_devices_for_user(self, user_id: str) -> Dict[str, JsonDict]:
	devices = await self.db_pool.simple_select_list(
	table="device_lists_remote_cache",
	keyvalues={"user_id": user_id},
	retcols=("device_id", "content"),
	desc="get_cached_devices_for_user",
	)
	return {
	device["device_id"]: db_to_json(device["content"]) for device in devices
	}

Suppose we have never cached devices for this user, and that the devices list is empty. Then cached_devices will be an empty dictionary and so too will the transformed version of devices. Thus we'll set ignore_devices = True and not cache the fact that this user has no devices.

Then, whenever someone wants this user's device again, we'll repeat the federation request again.

[DMRobertson]

The problem here is that the DB doesn't have a way to distinguish "this user has no devices" from "I've not got a cache entry for this user".

[DMRobertson]

We suspect this contributed to performance problems on the master today, after a room on t2bot.io containing bridged (deviceless!) users had encryption enabled.

[DMRobertson]

the DB doesn't have a way to distinguish "this user has no devices" from "I've not got a cache entry for this user".

This may be incorrect. Sean noted that there's a device_lists_remote_extremeties table which tracks username and stream ids. When is this written to?

• mark_remote_user_device_list_as_unsubscribed deletes entries from this table for a given user
  • called when this user leaves all rooms that this server can see. (Even if it's a local user?)
• update_remote_device_list_cache_entry inserts, replaces or deletes an entry for a single device
  • called by incoming_device_list_update from federation
• update_remote_device_list_cache replaces the entries for a given user
  • this is the one that's being skipped in the code samples in the description of this issue

It looks like every write to the cache is matched by a write to the extremities table. Promising!