-
Notifications
You must be signed in to change notification settings - Fork 39
fix(materialize): bound the full read a source falls back to when its watermark expires #1663
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
4b334e3
4eaec8f
af8269e
82317e7
d674f0d
c5cbdd7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -134,7 +134,7 @@ async fn local_table_end_to_end() { | |
|
|
||
| // Full unpinned streaming read: all 5 rows. | ||
| let scan = provider | ||
| .scan_for_materialize_stream(gs, "silver.people", &[], None, None) | ||
| .scan_for_materialize_stream(gs, "silver.people", &[], None, None, None) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Observation, not a demand. The unit coverage here is genuinely strong (and I verified all six mutations die by name — the three you claimed plus three more), but both production surprises in this PR's own history ( |
||
| .await | ||
| .expect("full scan"); | ||
| assert_eq!(scan.to_snapshot_id, Some(current)); | ||
|
|
@@ -170,7 +170,7 @@ async fn local_table_end_to_end() { | |
| }; | ||
|
|
||
| let scan = provider | ||
| .scan_for_materialize_stream(gs, "silver.people", &[], Some(first), None) | ||
| .scan_for_materialize_stream(gs, "silver.people", &[], Some(first), None, None) | ||
| .await | ||
| .expect("incremental scan"); | ||
| assert!(scan.incremental, "append-only window scans incrementally"); | ||
|
|
@@ -184,7 +184,7 @@ async fn local_table_end_to_end() { | |
| // PINNED read: to = the FIRST snapshot → only the first append's rows, | ||
| // and the resolved watermark is the pin, not current. | ||
| let scan = provider | ||
| .scan_for_materialize_stream(gs, "silver.people", &[], None, Some(first)) | ||
| .scan_for_materialize_stream(gs, "silver.people", &[], None, Some(first), None) | ||
| .await | ||
| .expect("pinned scan"); | ||
| assert_eq!(scan.to_snapshot_id, Some(first), "pin is honored"); | ||
|
|
@@ -198,7 +198,7 @@ async fn local_table_end_to_end() { | |
| // An expired/unknown pin is the typed error, never a fall-forward. | ||
| // (`MaterializeScan` has no Debug — a stream field — so match manually.) | ||
| match provider | ||
| .scan_for_materialize_stream(gs, "silver.people", &[], None, Some(999)) | ||
| .scan_for_materialize_stream(gs, "silver.people", &[], None, Some(999), None) | ||
| .await | ||
| { | ||
| Ok(_) => panic!("unknown pin must fail, not fall forward"), | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Praise. The cursor-XOR-snapshot encoding, with the ratchet documented in the field's own doc-comment and pinned by
a_partial_pass_writes_the_cursor_and_never_advances_the_snapshot, is exactly how an invariant that once discarded 80% of a deployment's entities should be nailed down. Same for the round-down checkpoint walk and its severed-chain test — the "note what is absent from its tests" framing ind674f0d0dis the kind of commit-message reviewers dream about.