Process previously failed backfill events in the background

[MadLittleMods]

Process previously failed backfill events in the background because they are bound to fail again and we don't need to waste time holding up the request for something that is bound to fail again.

Fix #13623

Follow-up to #13621 and #13622

Part of making /messages faster: #13356

---

For example, in this /messages request, this PR would have saved 15s because the event being processed by the _process_pulled_event with a ❗ has already failed 4 times previously and we would just process it in the background.

Full Jaeger trace JSON

Dev notes

backfill
_process_pulled_events
_process_pulled_event

---

Testing:

SYNAPSE_POSTGRES=1 SYNAPSE_POSTGRES_USER=postgres SYNAPSE_TEST_LOG_LEVEL=INFO python -m twisted.trial tests.storage.test_event_federation.EventFederationWorkerStoreTestCase.test_separate_event_ids_with_failed_pull_attempts

COMPLEMENT_ALWAYS_PRINT_SERVER_LOGS=1 COMPLEMENT_DIR=../complement ./scripts-dev/complement.sh -run TestImportHistoricalMessages

---

Related PRs

• Keep track when we try and fail to process a pulled event #13589
• Stop getting missing prev_events after we already know their signature is invalid #13816
• Only try to backfill event if we haven't tried before recently (exponential backoff) #13635

Todo

• Add test to sanity check that events are still processed even if it was in the background (discussed in this comment)

Pull Request Checklist

• Pull request is based on the develop branch
• Pull request includes a changelog file. The entry should:
  • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
  • Use markdown where necessary, mostly for code blocks.
  • End with either a period (.) or an exclamation mark (!).
  • Start with a capital letter.
  • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
• Pull request includes a sign off
• Code style is correct
  (run the linters)

[MadLittleMods]

Continuing from previous comment,

The test passes even without this pump(). I'm uncertain about the exact intricacies and details of run_as_background_process(...) that allow it to pass. It is possible that the test passes due to its time-sensitive nature, and without the pump(), it might become flakey. However, these are merely theoretical assumptions.

But I want to ensure that this test is robust to any work that may happen in run_as_background_process(...) in the future. Ideally, we would have an idiomatic wait_for_background_processes() that would indicate clear intentions and handle the complexity of whatever waiting is necessary.

[clokep]

Looks good! Thanks for working through it with me.

[clokep]

Although I see now that CI is unhappy...

[MadLittleMods]

Thanks for the review and extra push today @clokep to get this across the line 🦮