Do not keep going if there are 5 back-to-back background update failures.

[reivilibre]

Fixes #12780 — see that issue for context.

Up for debate whether this is a fair solution or not.

At least it makes tests fail rather than hang (personally confirmed).

[squahtx]

If we do fail 5 times, would there be any indication in CI?

[reivilibre]

If we do fail 5 times, would there be any indication in CI?

Yeah, the test will fail. (Before this, it would spin, seemingly forever)

[clokep]

Will this cause user's homeservers to potentially shutdown after running for a bit? That seems quite confusing.

Additionally -- will this stop all background processes from running or will the ones after the failed one continue?

[reivilibre]

Will this cause user's homeservers to potentially shutdown after running for a bit? That seems quite confusing.

No.

The logs have an error along the lines of

StopIteration: [{'update_name': 'add_rooms_room_version_column', 'depends_on': None}, {'update_name': 'current_state_events_membership', 'depends_on': None}, {'update_name': 'delete_old_current_state_events', 'depends_on': 'current_state_events_membership'}, {'update_name': 'devices_last_seen', 'depends_on': None}, {'update_name': 'event_store_labels', 'depends_on': None}, {'update_name': 'insert_room_retention', 'depends_on': None}, {'update_name': 'populate_stats_process_users', 'depends_on': 'populate_stats_process_rooms'}, {'update_name': 'redactions_have_censored_ts_idx', 'depends_on': None}, {'update_name': 'redactions_received_ts', 'depends_on': None}, {'update_name': 'remove_tombstoned_rooms_from_directory', 'depends_on': None}, {'update_name': 'room_membership_forgotten_idx', 'depends_on': None}, {'update_name': 'state_groups_room_id_idx', 'depends_on': None}, {'update_name': 'users_set_deactivated_flag', 'depends_on': None}, {'update_name': 'remove_dup_outbound_pokes', 'depends_on': None}, {'update_name': 'populate_stats_process_rooms', 'depends_on': None}, {'update_name': 'users_have_local_media', 'depends_on': None}, {'update_name': 'e2e_cross_signing_keys_idx', 'depends_on': None}, {'update_name': 'user_external_ids_user_id_idx', 'depends_on': None}, {'update_name': 'rejected_events_metadata', 'depends_on': None}, {'update_name': 'chain_cover', 'depends_on': 'rejected_events_metadata'}, {'update_name': 'remove_deactivated_pushers', 'depends_on': None}, {'update_name': 'remove_stale_pushers', 'depends_on': None}, {'update_name': 'purged_chain_cover', 'depends_on': None}, {'update_name': 'populate_rooms_creator_column', 'depends_on': None}, {'update_name': 'remove_deleted_email_pushers', 'depends_on': None}, {'update_name': 'presence_stream_not_offline_index', 'depends_on': None}, {'update_name': 'remove_hidden_devices_from_device_inbox', 'depends_on': None}, {'update_name': 'local_group_updates_index', 'depends_on': None}, {'update_name': 'remove_deleted_devices_from_device_inbox', 'depends_on': None}, {'update_name': 'event_arbitrary_relations', 'depends_on': None}, {'update_name': 'remove_dead_devices_from_device_inbox', 'depends_on': None}, {'update_name': 'delete_account_data_for_deactivated_users', 'depends_on': None}, {'update_name': 'event_search_sqlite_delete_non_strings', 'depends_on': None}, {'update_name': 'cache_invalidation_index_by_instance', 'depends_on': None}]

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/rei/work/synapse/synapse/storage/background_updates.py", line 291, in run_background_updates
    result = await self.do_next_background_update(sleep)
  File "/home/rei/work/synapse/synapse/storage/background_updates.py", line 413, in do_next_background_update
    update_info = self._background_update_handlers[self._current_background_update + 'x']
KeyError: 'add_rooms_room_version_columnx'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/rei/work/synapse/synapse/metrics/background_process_metrics.py", line 243, in run
    return await func(*args, **kwargs)
  File "/home/rei/work/synapse/synapse/storage/background_updates.py", line 296, in run_background_updates
    raise RuntimeError(
RuntimeError: 5 back-to-back background update failures; aborting.

but the homeserver otherwise continues to operate.

Additionally -- will this stop all background processes from running or will the ones after the failed one continue?

Yes, I think so. But that's no worse than before as no later background updates will get any time if the first one spins.
(Also note that in general it's not necessarily safe to run them out of order...)

This is essentially the same failure mode as before, except:

• no 100% CPU usage
• no disk filling
• Trial tests will actually report a failure rather than spinning (N.B. This is because the tests ensure all the background updates have finished before proceeding)

[clokep]

Additionally -- will this stop all background processes from running or will the ones after the failed one continue?

Yes, I think so. But that's no worse than before as no later background updates will get any time if the first one spins.
(Also note that in general it's not necessarily safe to run them out of order...)

This is what we want! Sorry if it sounded like I was implying that was bad. This is a good thing since updates are ordered.

[squahtx]

lgtm