Page MenuHomeElementl

Revert "Load execution plan from the snapshot on the run when resume retrying"
ClosedPublic

Authored by prha on Jul 29 2021, 1:44 AM.

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

I tried plumbing the subset selection through (via get_subset_external_pipeline_result), but there's a couple calls that only passes through solids_to_execute and not solid_selection, and both are used to generate the original snapshot id.

If you think it's worth it, I can keep at it, but we're not saving gRPC calls since we have to call out to subset the pipeline (and generate the valid snapshot id) anyway. Not sure if the main part of the optimization was not calling to gRPC or if the difference between constructing the plan and constructing the pipeline subset snapshot is significant.

prha requested review of this revision.Jul 29 2021, 2:26 AM

No worries. If there’s a test you’re adding to backfills for the thing you’re working on that caught this, that would break if I ever tried to I revert this later, that would be cool.

I can do this too later but we could also split out the backfill path from the non-backfill path, I mostly cared about the ‘retry a single run’ case

This revision is now accepted and ready to land.Jul 29 2021, 2:29 AM

Or I wonder if we had the same issue in the non-backfill case.. I’ll need to check