compute: delay retracting a dropped export's lifecycle rows - #38417
Draft
antiguru wants to merge 2 commits into
Draft
compute: delay retracting a dropped export's lifecycle rows#38417antiguru wants to merge 2 commits into
antiguru wants to merge 2 commits into
Conversation
The lifecycle log retracts an export's rows the moment it is dropped, so an
object's history vanishes with the object. That loses exactly the episodes worth
looking at: a dataflow that was dropped before it hydrated, or one whose
hydration is the reason someone is reading the log at all. It also means a short
lived dataflow can come and go inside one introspection interval and leave no
trace.
Delay the retraction instead, and record the drop as a stage of its own.
Two delays, because the two populations churn at completely different rates. A
transient export is created per peek and per subscribe, so retaining those for
minutes costs hundreds of megabytes on a busy replica: at eight workers and two
hundred peeks per second, a five minute window holds around 1.4 million rows.
A few seconds is enough for a reader to observe them and costs single digit
megabytes. A user object is dropped by DDL, so even a thousand drops inside the
window is under ten megabytes, and there the history is worth keeping.
compute_lifecycle_retraction_delay default 5 min
compute_lifecycle_retraction_delay_transient default 5 s
Both are floored at the logging interval in code rather than by convention. The
demux rounds update timestamps up to that interval, so a shorter delay can round
to the same timestamp as the insertion and leave the rows never separately
visible, which would defeat the point.
The delays are read per batch rather than at construction, so an
`UpdateConfiguration` command takes effect without recreating the logging
dataflow, and a per-replica override applies to a replica under investigation
without touching the rest.
The `dropped` stage is what makes the delay readable. Without it a lingering row
says only that an export reached some stage, not whether it still exists, so
"hydrated but never written" could not be told apart from "dropped before it
wrote". With it the object's last event names its fate and the delay is pure
retention, and a row whose export id no longer appears in `mz_objects` is
explained by its own `dropped` event rather than reading as a leak.
Part of CPU-226
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018ZVCMBSLdxzGus78ZKWhZz
The design doc says `catalog_server_explain.slt` needs no change when a builtin log is added, on the grounds that its query filters `o.id NOT LIKE 'si%'` and so never enumerates a per-replica introspection index. The filter is real, but the conclusion does not follow. The plans already in the file embed the inlined builtin `VALUES` sets as `Constant (N rows)` nodes, so every count over a catalog relation that gained a row moves, and adding an ontology entity and link moves two more. Record the question that catches this: not whether a new plan appears, but whether the existing plans change. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018ZVCMBSLdxzGus78ZKWhZz
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.
Stacked on #38403. Base is
claude/hydration-visibility-compute-js1ycm, so the diff here is just the two commits on top. Review #38403 first.Motivation
The lifecycle log as it stands retracts an export's rows the moment it is dropped, so an object's history vanishes with the object. That loses exactly the episodes worth looking at: a dataflow dropped before it hydrated, or one whose hydration is the reason someone opened the log. It also means a short-lived dataflow can come and go inside one introspection interval and leave no trace at all.
Part of CPU-226.
Description
Delay the retraction, and record the drop as a stage of its own.
Two delays, because the two populations churn at rates too different to share one value.
compute_lifecycle_retraction_delaycompute_lifecycle_retraction_delay_transientA transient export's rate is the query rate. At eight workers and two hundred peeks per second, a five minute window holds around 1.4 million rows, hundreds of megabytes, which is not a reasonable thing to hold in a replica's memory for introspection. A few seconds costs single-digit megabytes and is enough for a subscribe to observe them. A user object is dropped by DDL, so a thousand drops inside a five minute window is under ten megabytes, and there the history is the thing worth keeping. So the split is required rather than a nicety.
Both are floored at the logging interval in code, not by convention. The demux rounds update timestamps up to that interval, so a delay shorter than one interval can round to the same timestamp as the insertion, and the rows are then never separately visible, which defeats the point.
The
droppedstage is what makes the delay readable. Without it a lingering row says only that an export reached some stage, not whether it still exists, so "hydrated but never written" could not be told apart from "dropped before it wrote". With it the object's last event names its fate, the delay is pure retention, and a row whoseexport_idno longer appears inmz_objectsis explained by its owndroppedevent rather than reading as a leak. It is a per-worker event, since each worker's demux handles the drop of its own export.Replica scoping. The delays are read per batch inside the demux rather than at construction, following
prometheus.rs.ComputeCommand::UpdateConfigurationapplies updates in place to the sameConfigSetthe logging dataflow holds, so a change takes effect without recreating the dataflow, andCreateInstancecarries aninitial_configsnapshot so a fresh replica starts with the right values.specialize_command_for_replicamerges per-replicaConfigUpdatesinto both, with no per-flag allowlist, so a replica under investigation can be told to retain longer than the rest without any additional plumbing.The second commit is unrelated to the delay and corrects a claim I put in the design doc in #38403: that
catalog_server_explain.sltneeds no change when a builtin log is added. Theo.id NOT LIKE 'si%'filter is real, so no new EXPLAIN entry appears, but the existing plans embed the inlined builtinVALUESsets asConstant (N rows)nodes, so every count over a catalog relation that gained a row moves. #38403 has the golden fix; this records the question that catches it.Verification
cargo check -p mz-compute -p mz-compute-types --all-targetsis clean.bin/fmt --checkpasses for rustfmt, black and ruff (bufis not installed in this environment, and there are no proto changes).test/testdrive/compute-lifecycle-events.tdgains coverage for the new behaviour: it sets both delays to'2s'viaALTER SYSTEM SETrather than disabling them, so the delayed path is the one under test, asserts adroppedevent appears, and then asserts the rows are retracted.droppedis added to the closedeventvocabulary asserted underset-max-tries max-tries=1.One thing CI is the first to prove. The testdrive file sets the two delays through
ALTER SYSTEM SET, which assumes dyncfgs are settable that way.compute_temporal_bucketing_summaryis aConfig<Duration>registered in the sameUNINTERESTING_SYSTEM_PARAMETERSlist, so the shape matches a working precedent, but I have not executed this file. If it turns out not to work, the fix is to pass the values via--system-parameter-defaultinstead.Both flag names are registered in
misc/python/materialize/mzcompose/__init__.pyandmisc/python/materialize/parallel_workload/action.py, without whichcheck-test-flagsfails.Generated by Claude Code