Skip to content

fix(data-modeling): keep materialization on when temporal is unreachable - #116509

Draft
posthog[bot] wants to merge 1 commit into
masterfrom
posthog-self-driving/fixdata-modeling-keep-materialization-aafb85
Draft

posthog[bot] wants to merge 1 commit into
masterfrom
posthog-self-driving/fixdata-modeling-keep-materialization-aafb85

Conversation

@posthog

@posthog posthog Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Problem

  • A short Temporal outage silently switches off materialization for saved queries and endpoints across many teams. Their data stops refreshing, with no message, until a person re-enables it.
  • schedule_materialization first asks Temporal which DAGs run on tiers. When the connect fails (for example, a TLS handshake error), the broad except Exception treats it as a permanent fault and sets is_materialized=False.
  • Nothing turns the flag back on. The managed viewset loop, the materialize action and the endpoints enable all reach this path.
  • Disable-on-failure for real scheduling faults is deliberate (#78259). This PR keeps it. It only takes the "Temporal could not answer" case out of it.

Origin

  • Error tracking: issue
  • First signal: 2026-10-13
  • Inbox report: open
  • Likely cause: a59815e
  • Task started by: auto-start, after the report was rated P1 and ready to fix

Changes

  • An outage no longer changes the flag. A failed connect, or an RPC error with status UNAVAILABLE, DEADLINE_EXCEEDED or RESOURCE_EXHAUSTED from the v2 lookup, now raises SchedulingUnavailableError. The error is captured, and is_materialized stays as it was.
  • All other failures still disable materialization, as before.
  • Each caller now handles the new error:
Caller On a Temporal outage
Saved query materialize action Restores the previous is_materialized and frequency, returns the same "Materialization failed" error as before
Endpoint enable Raises inside its atomic block, so the endpoint keeps its previous state
Managed viewset sync Already catches and logs per view. The flag stays on, and the next sync schedules again
  • Recovery: the new reenable_materialization command takes the saved query ids from the captured scheduling errors. It skips deleted and already materialized ones, and schedules the rest. It runs as a dry run unless you pass --apply. If a query fails to schedule again, the command reports it and leaves it off.

Note

A brand-new managed view whose first schedule hits an outage stays is_materialized=True until the next viewset sync schedules it. Before this change, the outage switched the view off permanently.

How did you test this code?

  • Automated tests only. No run against a real Temporal namespace.
  • Ran locally: the data modeling schedule, materialization and new command tests, test_endpoint_materialization.py, test_managed_viewset.py, plus ruff, uv run mypy --cache-fine-grained . and hogli product:lint data_modeling.

Test rationale:

  • test_temporal_outage_raises_and_keeps_materialization (TLS connect error, UNAVAILABLE RPC) covers the reported regression. Both cases fail when the model change is reverted. It sits next to test_disables_materialization_when_v2_lookup_fails, which is unchanged and still proves that an unknown error disables materialization.
  • The existing restore test in test_saved_query_materialization.py is now parameterized with an outage case. It catches the materialize action leaving is_materialized=True with no schedule after the new error.
  • test_reenable_materialization.py covers the dry run, and an apply run that must skip deleted queries and report a query with no node that cannot be scheduled.

Release status

  • No feature flag controls this change

Docs update

None. No existing doc covers this scheduling contract.

🤖 Agent context

Autonomy: Fully autonomous

Agent: Claude Code, claude-opus-5-5

  • Built from a PostHog inbox report. No customer material is in the code or tests. The test error strings are invented.
  • Rejected option: a retry loop inside schedule_materialization. An outage that lasts an hour outlives any in-request retry. Raising lets each caller keep the correct state, and the managed viewset sync already retries on its next run.
  • No open PR does the same thing. Searches for this scheduling path found only unrelated data-modeling PRs.
  • Not added: the daily "materialized to not materialized" metric that the report suggests. That is follow-up work.
  • Skills invoked: /writing-tests, /writing-pr-descriptions.

Created with PostHog Desktop from this inbox report.

🤖 Generated with Claude Code

A failed Temporal connect in the v2 schedule lookup now raises SchedulingUnavailableError and leaves is_materialized unchanged. Other scheduling failures still disable materialization. The materialize action restores the previous state, and the endpoints enable unwinds its atomic block. Adds reenable_materialization to recover saved queries a past outage switched off.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Generated-By: PostHog Desktop
Task-Id: 8bbe4016-6ae8-4d06-bb8e-854675d2c9b0
@posthog posthog Bot added the self-driving label Oct 13, 2026
@trunk-io

trunk-io Bot commented Oct 13, 2026

Copy link
Copy Markdown

Merging to master in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@posthog
posthog Bot requested a review from sakce October 13, 2026 18:44
@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

⚠️ Trunk lane — backend Python lane

This PR is assigned to the backend Python lane. It runs backend Python tests and may merge in parallel with PRs in other lanes.

✅ Duplication (Python) — clean

New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

✅ Duplication (TypeScript) — clean

New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

⚠️ Comment density — 4% of added code lines are comments (8 of 197)

This section warns when comments are more than 3% of the code lines a PR adds, and alerts above 6%. Before agent-assisted PRs, the typical share was about 2%. Only full-line comments count. Docstrings, generated files, snapshots, migrations, and workflow files are left out.

Comments that restate the code, record how the change came about, or narrate the next line add noise for the next reader. Keep the comments that explain a reason the code cannot show, and remove the rest. See .agents/skills/writing-code-comments/SKILL.md for the house rules.

Files with the most added comment lines:

File Comment lines Added lines
products/data_modeling/backend/logic/saved_query_materialization.py 2 11
products/data_modeling/backend/models/datawarehouse_saved_query.py 2 23
products/data_modeling/backend/schedule.py 2 13
products/data_modeling/backend/management/commands/reenable_materialization.py 1 66
products/endpoints/backend/logic/materialization.py 1 4

This check does not block merging. It updates on every push and clears when the share drops.

@stamphog stamphog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not approved — escalated to a human reviewer.

Re-add the stamphog label to request another review once you have addressed this.

The materialization-state changes enter risky territory without independent assurance; the merge-queue bot comment is not a review. Request a current-head review covering scheduling and recovery from @PostHog/team-data-modeling before re-requesting.

  • Risky territory without independent assurance: Data models: products/data_modeling/backend/models/datawarehouse_saved_query.py changes the persisted materialization-state failure contract, preserving the flag when the initial Temporal lookup has a recognized availability failure.; Data models: products/data_modeling/backend/management/commands/reenable_materialization.py adds an operator command that writes saved-query materialization flags and schedules selected queries across teams.
Gate mechanics and policy version
Gate Result
prerequisites ✓ all clear
deny-list ✓ no deny categories matched
size ✓ 143L, 6F substantive, 243L/9F incl. docs/generated/snapshots — within ceiling
tier ✓ T1-agent / T1c-medium (243L, 9F, single-area, fix)
stamphog 2.5.0 .stamphog/policy.yml @ e8cd392 · reviewed head e8cd392

@stamphog stamphog Bot added the posthog-review Run a Standard PostHog Review label Oct 13, 2026

@posthog posthog Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PostHog Review · Standard · reviewed e8cd392: 2 Should fix.

This Standard review started from the posthog-review label. Comment @posthog review --deep for a Deep review, or @posthog help to see all commands.

saved_query.is_materialized = True
saved_query.save(update_fields=["is_materialized"])
try:
saved_query.schedule_materialization()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 · Promote recovered view nodes before reporting success

When a query's first materialization enable failed during the Temporal lookup, its node remains VIEW because the enable action never reached _mark_nodes_materialized. This command restores the flag and reports success, but schedule_materialization does not change the node type, so execute_dag skips it as ephemeral. Promote recovered nodes to MAT_VIEW after successful scheduling.

Comment on lines +61 to +63
saved_query.refresh_from_db(fields=["is_materialized"])
if saved_query.is_materialized:
reenabled += 1

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 · Propagate reconciliation failures before counting recovery as successful

When recovering a failed first enable on an already-tiered DAG, the lookup can succeed while schedule reconciliation fails. _reconcile_dag_best_effort swallows that failure, so this flag check reports recovery although the tier never includes the query. Subsequent command runs skip it as already materialized. Use a reconciliation path that propagates errors before counting success.

@posthog posthog Bot removed the posthog-review Run a Standard PostHog Review label Oct 13, 2026
@posthog
posthog Bot requested a review from a team October 13, 2026 18:59

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant