Repository navigation
Conversation
Signed-off-by: Jesse Hodges <hodges.jesse@gmail.com>
…ecks Signed-off-by: Jesse Hodges <hodges.jesse@gmail.com>
Signed-off-by: Jesse Hodges <hodges.jesse@gmail.com>
|
My concern is how much new behavior this adds to resolve the scheduler race. The option requires plan-time snapshot metadata, serialization through apply, scheduler-boundary counting, downstream coverage checks, and new rejection and recovery paths. For the hourly case, the replacement finishes its planned backfill through 10:00, but the incumbent has advanced through 11:00. This PR allows promotion with that hour missing. Production then serves less recent data until a later scheduler run repairs the replacement. Finalization does not mean that repair has completed. Could we handle this by backfilling the newly missing hour immediately before promotion, then retrying the existing strict check once? A second scheduler advance could still cause deployment to fail, but a successful deployment would retain the current coverage guarantee. I tried that approach in an isolated prototype. It worked for an hourly parent and child without replaying the whole plan or its plan-level hooks. It also exposed a downstream problem: if a child interval is already marked complete, filling the parent alone does not recompute the child. The overlap guard addresses that problem conservatively, but overlapping intervals can also be intentional. A forward-only child may reuse its production table and retain historical rows while its parent gets a replacement table. Our test confirmed that behavior, including the child using the new parent value on its next scheduled interval. So I’m not suggesting catch-up is a trivial replacement. I’d like to understand why accepting gaps is preferable to a bounded catch-up attempt, given that both approaches need to account for downstream coverage and table reuse. |
|
Thanks for working through this. We chose trailing-gap acceptance as a small, opt-in mitigation compared with coordinating deployments and schedulers through locking. It deliberately trades temporary freshness for deployment completion. Your bounded catch-up proposal raises a useful question about whether we can keep strict promotion semantics with a similarly scoped change. I see four approaches, with different tradeoffs:
The downstream distinction matters across these approaches: filling a parent does not automatically recompute a child interval already marked complete, but overlapping coverage is not necessarily incorrect when a child intentionally reuses its production table. The current guard rejects that ambiguity conservatively. Locking would prevent new interference during cutover, but would not by itself resolve coverage already recorded. Given that, I think a bounded automatic replan/apply retry deserves evaluation alongside the dedicated catch-up stage. It would automate the existing manual recovery path while retaining strict checks. “Retriable” would mean another attempt has a reasonable chance of succeeding, not that it must succeed; a second failure could still be returned to the operator. The current opt-in approach remains useful where completing deployment is worth accepting temporary freshness debt, but I agree we should compare it against those retry options rather than only against locking. |
Related to #5825.
A production plan can finish its planned backfill and then fail the no-gap check because a scheduled run advanced the incumbent snapshot while the replacement was being built. This adds an opt-in policy that allows promotion and finalization when the replacement is missing only a bounded, contiguous trailing suffix created after the plan was built. The suffix stays missing in interval state so the normal scheduler can repair it.
Strict no-gap behavior remains the default (
0). The GitHub bot can opt in withmax_trailing_gap_intervals_on_deploy_to_prod=1; Python callers can useContext.plan_builder(..., no_gaps=True, max_trailing_gap_intervals=1). The underlying plan/promotion policy is independent of GitHub. This PR does not add a CLI flag or notebook magic option.Safety and scope
This is a bounded freshness tradeoff rather than the model-level locking proposed in the issue. Subsequent scheduler selections must include upstream dependencies. The bound applies to drift observed by the promotion check; an already-running scheduler is not fenced from advancing the old snapshot afterward. This change does not add durable repair tracking.
Validation
make stylepassed: Ruff, Ruff formatting, mypy, and migration validation.The implementation has also run in our production fork. Two production deployment attempts successfully used the allowance. For one, operations metadata and retained interval state corroborate subsequent repair on the exact promoted snapshot: the covering scheduler batch completed about 4m18s after finalization. This verifies one model's repair; it does not establish all downstream row values or a durable repair-time guarantee.