Only try to backfill event if we haven't tried before recently (exponential backoff)

[MadLittleMods]

Only try to backfill event if we haven't tried before recently (exponential backoff). No need to keep trying the same backfill point that fails over and over.

Fix #13622
Fix #8451

Follow-up to #13589

Part of #13356

---

The get_oldest_event_ids_with_depth_in_room tests are using this DAG layout:

flowchart BT

    3 --> A
    subgraph backfillPointsA
        b1:::not_backfilled_yet
        b2:::not_backfilled_yet
        b3:::not_backfilled_yet
    end

    subgraph main shoot
        5 --> 4 --> 3 --> 2:::not_backfilled_yet --> 1:::not_backfilled_yet
    end

    4 --> B
    subgraph backfillPointsB
        b4:::not_backfilled_yet
        b5:::not_backfilled_yet
        b6:::not_backfilled_yet
    end


    A --> b1 --> 2
    A --> b2 --> 2
    A --> b3 --> 2

    B --> b4 --> 3
    B --> b5 --> 3
    B --> b6 --> 3


    classDef not_backfilled_yet opacity: 0.75,stroke-dasharray: 5 5;

Loading

Dev notes

SYNAPSE_POSTGRES=1 SYNAPSE_TEST_LOG_LEVEL=DEBUG python -m twisted.trial tests.storage.test_event_federation.EventFederationWorkerStoreTestCase.test_get_backfill_points_in_room

SYNAPSE_TEST_LOG_LEVEL=DEBUG python -m twisted.trial tests.storage.test_event_federation.EventFederationWorkerStoreTestCase.test_get_backfill_points_in_room

---

Previous different graph

flowchart BT

    3 --> A
    subgraph backfillPointsA
        b1:::not_backfilled_yet
        b2:::not_backfilled_yet
        b3:::not_backfilled_yet
    end
    
    subgraph main shoot
        5 --> 4 --> 3 --> 2:::not_backfilled_yet --> 1:::not_backfilled_yet
    end

    4 --> B
    subgraph backfillPointsB
        b4:::not_backfilled_yet
        b5:::not_backfilled_yet
        b6:::not_backfilled_yet
    end


    A --> b1 --> 2
    A --> b2 --> 2
    A --> b3 --> 2

    B --> b4 ------> 2
    B --> b5 ------> 2
    B --> b6 ------> 2


    classDef not_backfilled_yet opacity: 0.75,stroke-dasharray: 5 5;

    %% alignment links
    B --- alignment1[ ]
    %% make the alignment links/nodes invisible
    style alignment1 visibility: hidden,color:transparent;
    linkStyle 18 stroke-width:2px,fill:none,stroke:none;

Loading

---

• Complement Docker SQLite version: 3.34.1 sqlite3.sqlite_version_info=(3, 34, 1)
• My local sqlite3 --version on macOS machine -> 3.37.0 2021-12-09 01:34:53 9ff244ce0739f8ee52a3e9671adb4ee54c83c640b02e3f9d185fd2f9a179aapl

---

‹
{
´
`
,
⹁

Todo

• Do same depth stuff to get_insertion_event_backward_extremities_in_room
  • Add tests
• Remove the depth filter from _maybe_backfill_inner itself because it's already done in the queries
• Future PR: Do we need to consider it a failed backfill attempt when we ask for backfill for a certain event and don't receive it in the response? -> Generalize tracking whenever we fail to fetch or process a federated event #13700
  • This isn't really a direct concern of this PR. We can iterate on this later.
• Figure out how to handle power support in SQLite, Only try to backfill event if we haven't tried before recently (exponential backoff) #13635 (comment)
• Fix sqlite3.OperationalError: no such column: failed_backfill_attempt_info, Only try to backfill event if we haven't tried before recently (exponential backoff) #13635 (comment)
• Future PR: Add a limit to both get_oldest_event_ids_with_depth_in_room and get_insertion_event_backward_extremities_in_room so we only try as much as we need to

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]

I'm not sure why we had this before? There is only ever one backward extremity with a given event_id (or one event with a given event_id)

Removing it and the query seems to behave just as good as before.

[MadLittleMods]

Thanks for the review @reivilibre and @erikjohnston! Also thanks to @squahtx and @richvdh for the tips! 🦆

[DMRobertson]

Two things here.

1. If num_attempts is 10 then (1 << 10) ** 3600000 overflows a 32bit signed int and postgres errors. See CS-API /messages requests fail on develop when using postgres due to NumericValueOutOfRange if some events have repeatedly failed to backfill #13929 for the investigation.
2. (Minor): the name "step" for the constant 3600000 (1 hour in ms) suggests that the first backoff period should be 1 hour: but as you noted above the first backoff period is 2 horus.

I'm attempting to PR a fix for (1).

[MadLittleMods]

PR to fix this: #13936