fix: restore Arrow download path, decouple delete list, detect stale step-01 - #18
Merged
Merged
Conversation
…step-01 Three fixes surfaced by the 2025-08 backfill. 1. google-cloud-bigquery-storage was neither declared nor installed, so every tomography download fell back to REST and built result frames from JSON rows instead of Arrow buffers. On 2025-08-01 that meant 13.7M node-edge rows in 19.5 min, and the retained frame plus Phase 4's allocations OOM-killed the worker at 22.1 GB inside a 28 GB cgroup — failing the whole chunk. 2026-05-27 survived the same path at 11.2M rows, so the pipeline has been running just under this ceiling. Also pins pyarrow explicitly rather than relying on it arriving transitively via pandas/db-dtypes. This affects every nightly, not just backfills. 2. SQL_FILES and OUTPUT_TABLES were zipped positionally, which forced one list to serve both the per-step resume map and the --delete-first delete list. That capped the delete list at "one table per SQL file", so the three Phase-D correlation tables — written by Python, with no 1:1 SQL file — could never be added without raising. Every re-run this week needed a manual pre-delete or it appended a second copy of that date's rows. Split into an explicit SQL_FILE_TO_OUTPUT_TABLE dict, DERIVED_OUTPUT_TABLES, and DELETE_TABLES, with an import-time check replacing strict=True. giga_meter_measurements is deliberately excluded: it accumulates history under first-writer-wins and part of it was recovered by hand, so a delete not followed by a successful 04 loses traces permanently. 3. find_stale_merged_partitions() detects step-01 partitions that are present and non-empty — invisible to step_already_done() — but hold a fraction of the available measurements. Three such dates (2026-05-27, 2026-07-13, 2026-07-17, at 8.1%/4.5%/13.7% of source) were found only by hand; nothing in the pipeline saw them. Compares each partition to its neighbour median, excluding itself. Reads INFORMATION_SCHEMA.PARTITIONS only, so it is free and runs after Phase A on every invocation. Warns rather than fails: a real traffic dip is indistinguishable from under-capture using metadata alone. Validated against live metadata — flags all three known-bad dates when their pre-repair counts are replayed (ratios 0.144/0.084/0.241 against a 0.5 cutoff) and zero false positives across ~350 current partitions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Burdantes
added a commit
that referenced
this pull request
Aug 9, 2026
* fix: REST fallback must not re-create the BigQuery Storage client _read_df catches a Storage Read API failure and retries with a bare query_job.to_dataframe(). That argument defaults to create_bqstorage_client=True, so whenever google-cloud-bigquery-storage is importable the fallback silently builds its own Storage client — re-attempting the exact thing it is falling back from and re-raising the identical error. This was latent while the package was absent: the import failed first, so the retry had nothing to auto-create and REST genuinely worked. Installing the package in #18 exposed it, and every Phase D died with 403 ... does not have 'bigquery.readsessions.create' permission logged immediately after "using REST download". Verified against the live identity: to_dataframe() fails, to_dataframe(create_bqstorage_client=False) succeeds. Note this is a per-identity IAM question as well as a packaging one. The pipeline's ADC identity is loqman@measurementlab.net, which lacks roles/bigquery.readSessionUser on mlab-collaboration. Until that is granted the Arrow path is unavailable and this degrades to REST — which works, but is slower and much heavier: a 13.7M-row download OOM-killed the worker at 22.1 GB inside a 28 GB cgroup and failed the 2025-08 backfill chunk. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: make _read_df fallback tests independent of ambient credentials The first version sequenced job.to_dataframe side effects assuming BigQueryReadClient() would construct successfully. That holds on a machine with application-default credentials but not in CI, where construction raises first and the Arrow call never happens — so the side_effect list lined up differently and three tests failed. They passed locally for the wrong reason. Drive both branches explicitly with a fake google.cloud.bigquery_storage module whose client constructor can be made to raise, covering the two real-world cases (missing ADC, and the 403 when the identity lacks bigquery.readsessions.create). Verified passing both with ambient ADC and with credentials stripped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three fixes surfaced by the 2025-08 backfill, which failed in Phase D last night.
1. Missing
google-cloud-bigquery-storage→ tomography OOMgoogle-cloud-bigquery-storagewas neither declared inpyproject.tomlnor installed in the image, so every tomography download loggedand built result frames from JSON rows instead of Arrow buffers.
On 2025-08-01 that meant 13.7M node-edge rows in 19.5 minutes, and the retained frame plus Phase 4's allocations OOM-killed the worker at 22.1 GB inside a 28 GB cgroup, failing the whole chunk:
2026-05-27 survived the identical path at 11.2M rows, so the pipeline has been running just under this ceiling for a while. This affects every nightly, not just backfills — ~20 min of each Phase D is REST download.
Also pins
pyarrowexplicitly (it was only indevextras and arriving transitively).2.
--delete-firstcould not clear the Phase-D correlation tablesSQL_FILESandOUTPUT_TABLESwere zipped positionally, forcing one list to serve two purposes:SQL_FILE_TO_OUTPUT_TABLEresume mapdelete_dates()delete listThe three correlation tables are written by Phase-D Python with no 1:1 SQL file, so adding them raised on
strict=True. Consequence: every re-run needed a manual pre-delete or it appended a second copy of that date's rows (they useinsert_rows_json, which has no self-delete).Split into an explicit
SQL_FILE_TO_OUTPUT_TABLEdict,DERIVED_OUTPUT_TABLES, andDELETE_TABLES, with an import-time check replacingstrict=True.giga_meter_measurementsis deliberately excluded — it accumulates history under first-writer-wins and part of it was recovered by hand, so a delete not followed by a successful04loses traces permanently. A test pins that exclusion so adding it has to be a conscious decision.3. Nothing detected a stunted
merged_download_uploadStep 01 can write a partition that is present and non-empty — invisible to
step_already_done()— while holding a fraction of the available measurements. Everything downstream scales down with it.Three such dates were found by hand this week; nothing in the pipeline saw them:
find_stale_merged_partitions()compares each partition to its neighbour median, excluding itself so a stale date cannot drag down its own baseline. ReadsINFORMATION_SCHEMA.PARTITIONSonly — free — so it runs after Phase A on every invocation. Warns rather than fails, because from metadata alone a genuine traffic dip is indistinguishable from under-capture; the log line says how to confirm.Validated against live metadata:
Testing
55 passed, 2 skippedon Python 3.14, including 14 new tests acrosstest_output_table_wiring.pyandtest_stale_merged_check.py.ruff checkandruff format --checkclean.🤖 Generated with Claude Code