Retry to sync out of sync device lists

[babolivier]

When a call to user_device_resync fails, we don't currently mark the remote user's device list as out of sync, nor do we retry to sync it.

#6776 introduced some code infrastructure to mark device lists as stale/out of sync.

This PR uses that code infrastructure to mark device lists as out of sync if processing an incoming device list update makes the device handler realise that the device list is out of sync, but we can't resync right now.

It also adds a looping call to retry all failed resync every 30s. I'm not entirely sure about that retry logic right now so would like some opinions on it. It shouldn't cause too much spam in the logs as I removed the "Failed to handle device list update for..." warning logs when catching NotRetryingDestination.

Fixes #7418

TODO: Write some tests

[babolivier]

Just tried this PR on my HS, and at least half of it seems to work well, in that inserting rows manually with device_lists_remote_resync using:

INSERT INTO device_lists_remote_resync VALUES ('@user:example.com', (EXTRACT(epoch FROM NOW()) * 1000)::BIGINT);

seems to result in the users' device lists being resynced automatically, and having their shield turned back to green in Riot.

Another point is that I'll need to see if this PR doesn't clash with #6786, it shouldn't be the case but maybe there's be something that needs to be done to smooth them together and make the whole thing more maintainable.

[babolivier]

Alright, the other half of this (the bit that does the insertion if resync fails) seems to also be working, given I'm now seeing Successfully resynced the device list for... lines in my logs for users I haven't manually inserted \o/

[babolivier]

So I think this PR should be ready for review. I don't believe there's anything that needs to be done wrt #6786.

Also, I'm not sure if it's good enough that, if a remote is unreachable, retry it every 30s and fail silently on a NotRetryingDestination error or if the signature of user_device_resync should be changed so that it also returns how long to wait until the next attempt, and _maybe_retry_device_resync would then keep track of which resync should be retried when (but that may be too much faff).

Another question/issue I have is that it would be neat to cache get_user_ids_requiring_device_list_resync since we might be calling the function every 30s, however the way that function is used outside of this PR makes invalidating that cache quite tricky, as we can't know all of the lists of user IDs that function has been called with.

[babolivier]

Known shortcomings of this PR:

• It doesn't do the right thing wrt logging contexts, c.f.:

2020-05-13 11:23:04,595 - synapse.storage.database - 525 - WARNING - - Starting db txn 'update_remote_device_list_cache' from sentinel context
2020-05-13 11:23:04,595 - synapse.storage.database - 564 - WARNING - - Starting db connection from sentinel context: metrics will be lost

• It doesn't update device_lists_outbound_last_success (and maybe other tables) upon successful /devices/... response. see Retry to sync out of sync device lists #7453 (comment)

[richvdh]

@babolivier please could you fix #7498 while you're here

[babolivier]

Sure

[babolivier]

So I'm not so sure about

It doesn't update device_lists_outbound_last_success (and maybe other tables) upon successful /devices/... response.

being a shortcoming of this PR anymore.

This is only needed if the remote server is completely unreachable, since Synapse will always 200 device list updates, even if it failed to process them.

AFAICT, when responding to a request, we can't really be 100% sure response will be reached and fully processed by the remote server (especially if that server or its connection is wobbly). In that case, I'd rather have the sending server wrongly think it has failed to send the update and resend it with the next transaction (so the remote server can simply ignore it if it's already received it) than have it wrongly think it's succeeded and never send it again.

[clokep]

Looks good if CI passes!