Repository navigation
Bug 2080901 - [BMO-ETL] When any table fails to export, the entire snapshot should be rolled back to avoid incorrect results in charts/dashboards - #2784
Conversation
…apshot should be rolled back to avoid incorrect results in charts/dashboards
There was a problem hiding this comment.
🟡 Changes recommended
Locking and remote query lifecycle issues can still allow duplicate snapshots or interrupt valid exports.
0 open findings
What changed in this PR
This PR aims to prevent partial BMO-ETL snapshots from affecting charts and dashboards by staging exports before transactional publication.
Changes:
- Adds staging tables, transactional publication, and cleanup.
- Introduces lock heartbeats, stale-lock recovery, and query polling.
- Expands tests for retries, cleanup, locking, and invalid dates.
| File | Description |
|---|---|
| extensions/BMO/t/bmo/bmo_etl.t | Adds export lifecycle and validation tests. |
| extensions/BMO/Extension.pm | Adds the lock heartbeat column and migration. |
| extensions/BMO/bin/export_bmo_etl.pl | Implements staged publication and export lifecycle handling. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| use constant LOCK_REFRESH_EVERY => 60; # one minute | ||
| use constant LOCK_STALE_AFTER => 10 * 60; # ten minutes | ||
| use constant LOCK_MAX_WAIT => 15 * 60; # fifteen minutes | ||
| use constant LOCK_POLL_INTERVAL => 30; |
There was a problem hiding this comment.
Should we make these configurable with defaults, in case we need to tweak them in the future?
There was a problem hiding this comment.
I'd rather keep these as constants for now. They're coupled: LOCK_STALE_AFTER must stay well above the heartbeat gap, or a retry can steal a live lock and publish twice. A param would make that easy to break by accident. The one that's useful operationally, lock wait, is already exposed as --lock-wait. Happy to add a CLI flag for QUERY_MAX_WAIT if you think we'll need it.
| logger("$staging_table has $count of $sent_counts{$table} rows, waiting."); | ||
| sleep 10; | ||
| } | ||
| if ($count != $sent_counts{$table}) { |
There was a problem hiding this comment.
Should a count mismatch here be fatal? If a retried insertAll duplicates rows, the whole run fails even though staging has everything we need. Could we instead dedupe when copying out of staging (e.g. SELECT DISTINCT in the publish INSERT ... SELECT, since retried rows are identical) and relax this to $count >= $sent_counts{$table}?
No description provided.