Skip to content

Commit c4c2ee8

Browse files
mtabebeclaude
andcommitted
adapter: exclude dependents of every new builtin, and bound the weak caught-up path
Two gaps in the 0dt caught-up gate, both found in review of MaterializeInc#38399. Seed the transitive-dependent walk from all of `new_builtin_collections`, not just the materialized views among them. A brand-new builtin table or source has no read-only writer either, since `register_table_collections` retains only migrated tables while read-only, so a `Replacement`-migrated MV reading one can never advance past that empty frontier. With migrated MVs now kept in the gate, such an MV would block promotion until an operator intervened. No builtin table or source has been added since v26.38.0, so this is latent today. Require the write frontier to be within the allowed lag of `now` on the no-live-frontier path, the same bound the live-frontier path applies. Hydration is a one-shot property, so a collection that hydrated and then stalled satisfied that branch forever. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent ed1200d commit c4c2ee8

2 files changed

Lines changed: 50 additions & 18 deletions

File tree

‎src/adapter/src/coord.rs‎

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5172,32 +5172,34 @@ pub fn serve(
51725172

51735173
// A collection that can't advance its write frontier in read-only mode
51745174
// stalls its transitive dependents too, so exclude those from the caught-up
5175-
// check as well. That's new builtin MVs, whose fresh shard has no writer until
5176-
// this deployment promotes, plus migrated MVs whenever the leader is too old for
5177-
// them to write. An excluded dependent may still be hydrating right after
5178-
// promotion, a brief blip we accept because these MVs are small and get a writer
5179-
// at cut-over.
5175+
// check as well. That's every *new* builtin collection, whose fresh shard has no
5176+
// writer until this deployment promotes, plus migrated MVs whenever the leader is
5177+
// too old for them to write. An excluded dependent may still be hydrating right
5178+
// after promotion, a brief blip we accept because these collections are small and
5179+
// get a writer at cut-over.
51805180
//
5181-
// A migrated builtin *table* needs no such treatment even though a builtin MV can
5181+
// The walk is seeded from all of `new_builtin_collections`, not just the MVs among
5182+
// them. A brand-new builtin table or source has no read-only writer either
5183+
// (`register_table_collections` retains only *migrated* tables while read-only),
5184+
// so an MV reading one can never advance past that empty frontier.
5185+
//
5186+
// A *migrated* builtin table is the opposite case, even though a builtin MV can
51825187
// read one (`mz_clusters` joins `mz_cluster_replica_size_internal`):
51835188
// `read_only_mode_table_worker` keeps advancing the uppers of migrated tables, so
51845189
// an MV over one still catches up.
5185-
let new_builtin_mvs = new_builtin_collections
5186-
.iter()
5187-
.map(|global_id| {
5188-
catalog
5189-
.state()
5190-
.try_get_entry_by_global_id(global_id)
5191-
.expect("new builtin collections have catalog entries")
5192-
})
5193-
.filter(|entry| entry.is_materialized_view())
5194-
.map(|entry| entry.id());
5190+
let new_builtin_items = new_builtin_collections.iter().map(|global_id| {
5191+
catalog
5192+
.state()
5193+
.try_get_entry_by_global_id(global_id)
5194+
.expect("new builtin collections have catalog entries")
5195+
.id()
5196+
});
51955197
let frozen_migrated_mvs = migrated_storage_collections_0dt
51965198
.iter()
51975199
.copied()
51985200
.filter(|_| !hydrate_migrated_mvs)
51995201
.filter(|id| catalog.state().get_entry(id).is_materialized_view());
5200-
let mut todo: Vec<_> = new_builtin_mvs.chain(frozen_migrated_mvs).collect();
5202+
let mut todo: Vec<_> = new_builtin_items.chain(frozen_migrated_mvs).collect();
52015203
while let Some(item_id) = todo.pop() {
52025204
let entry = catalog.state().get_entry(&item_id);
52035205
exclude_collections.extend(entry.global_ids());

‎src/adapter/src/coord/caught_up.rs‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -634,12 +634,42 @@ impl Coordinator {
634634
self.controller.storage.collection_hydrated(id)?
635635
}
636636
};
637+
638+
// Also require the write frontier to be within the allowed lag of `now`, the
639+
// same bound the live-frontier path applies. Hydration alone is a one-shot
640+
// property: a collection that hydrated and then stalled keeps satisfying the
641+
// gate forever. With no live frontier to compare against, `now` stands in for
642+
// it. An empty write frontier means the collection is done advancing, which
643+
// trivially satisfies this.
644+
//
645+
// NOTE: unlike the live-frontier path there is no `cutoff` escape hatch for a
646+
// collection that is hopelessly behind, and there deliberately cannot be: a
647+
// frontier frozen at the minimum is exactly the state this gate exists to
648+
// catch, so ignoring the worst offenders would defeat it. A collection stuck
649+
// here blocks promotion until `with_0dt_deployment_max_wait` elapses or an
650+
// operator intervenes.
651+
let write_frontier_plus_allowed_lag = Antichain::from_iter(
652+
write_frontier
653+
.iter()
654+
.map(|t| t.step_forward_by(&allowed_lag)),
655+
);
656+
let within_lag = PartialOrder::less_equal(
657+
&Antichain::from_elem(now),
658+
&write_frontier_plus_allowed_lag,
659+
);
660+
637661
tracing::info!(
638662
?write_frontier,
639663
%collection_hydrated,
664+
%within_lag,
665+
?allowed_lag,
666+
?now,
640667
"collection {id} not in live frontiers"
641668
);
642-
if write_frontier.less_equal(&Timestamp::minimum()) || !collection_hydrated {
669+
if write_frontier.less_equal(&Timestamp::minimum())
670+
|| !collection_hydrated
671+
|| !within_lag
672+
{
643673
all_caught_up = false;
644674
}
645675
continue;

0 commit comments

Comments
 (0)