perf(reingest): batch dataset loading and remove N+1 queries#750
Conversation
reconstruct_execution_definition walked execution.datasets then dataset.files lazily, issuing O(M*D) queries per reingest. Load the linked datasets in one query using with_polymorphic(Dataset, "*") (eager subclass columns) plus selectinload(.files), collapse the per-row dataset-link inserts into a single batched insert, and warm the ExecutionGroup.executions identity map once in get_executions_for_reingest instead of lazy-loading per group. Adds a query-count regression test asserting reconstruction issues a constant number of SELECTs regardless of dataset/file count. (cherry picked from commit ebd3dcd3a3da6d16404c7e74c2bcd07f20b36968)
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthrough
ChangesReingest N+1 Query Optimisation
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Description
ref executions reingestreconstructs each completed execution'sExecutionDefinitionfrom the database before re-runningbuild_execution_result(). Reconstruction walkedexecution.datasetsand thendataset.fileslazily, and_extract_dataset_attributestriggered a per-dataset load of the polymorphic subclass table, so a bulk reingest issued O(M × D) queries (M executions, D datasets each). The dataset links were also copied with oneINSERTper row, and the eligibility scan lazy-loaded each group's fullexecutionscollection just to read[0].This makes reingest scale with the number of executions rather than the number of files, with no behaviour change:
reconstruct_execution_definitionnow loads the linked datasets in a single query usingwith_polymorphic(Dataset, "*")(eager-loads the polymorphic subclass columns) plusselectinload(.files), so neither thefor file in dataset.filesloop nor_extract_dataset_attributesissues a per-dataset query.session.execute(insert(), rows), mirroringExecution.register_datasets.get_executions_for_reingestwarms theExecutionGroup.executionsidentity map once before the loop instead of lazy-loading each group's collection.On a synthetic execution with 8 datasets and 3 files each, reconstruction drops from 18 SELECTs to a constant 3, independent of dataset count. The reconstructed definition, the set of
(execution_id, dataset_id)links, and the reingest failure-handling path are all unchanged. A new regression test asserts the reconstruction SELECT count is constant in the dataset count (it grows, and fails, if the eager load is reverted).Checklist
Please confirm that this pull request has done the following:
changelog/Summary by CodeRabbit
Performance Improvements