#9275 refactor to batch compare and append for one shard in persist sink
Motivation
https://github.com/MaterializeInc/database-issues/issues/9275
Checklist
- [x] This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
- [x] This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
- [x] If this PR evolves an existing
$T ⇔ Proto$Tmapping (possibly in a backwards-incompatible way), then it is tagged with aT-protolabel. - [x] If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
- [x] If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.
nightly run for 787b0e6a2b: https://buildkite.com/materialize/nightly/builds/12218
nightly build: https://buildkite.com/materialize/nightly/builds/12226
Thanks for all the followup! Persist-side stuff looks good to me.
It would be good to get some sources-side signoff for this. (For example, I think this will now retry in some cases where it previously would have bailed, when bail_on_concurrent_modification is set - but that may be fine. A sources expert would know!)
Also makes sense to get this behaviour behind a flag. But I understand that's in the works also!
running nightly for b7feb76 please note this does not test the batching change but tests that the existing implementation still works.
apologies, rebased to latest main.
two nightly builds:
- random feature flags - https://buildkite.com/materialize/nightly/builds/12294/steps/waterfall
- default flags with persist sink batching enabled - https://buildkite.com/materialize/nightly/builds/12295/steps/waterfall
Awesome, thanks for triggering Nightly! Note that with random feature flags we currently expect to see a bunch of test failures (OoMs, perf. regressions, different plans), but not wrong results or panics. I see a bunch of scary stuff in there:
- With random flags there are wrong results in 🏎️ testdrive with SIZE 1 and testdrive with :azure: blob store:
fetch-select-during-ingest.td:73:1: non-matching rows: expected:
[["123"], ["234"]]
got:
[["123"]]
Poor diff:
- 234
I'm not sure if this is related to your PR, but I haven't seen it on main yet! Doing some more test runs just in case though: 1, 2, 3 Edit: Couldn't repro with those, trying locally with your PR and the specific set of flags now. 2. With default flags two tests panic: CRDB rolling restarts & Zero downtime:
0dt-mz_new-1 | 2025-06-11T23:44:26.527793Z thread 'persist:0009' panicked at src/persist-client/src/fetch.rs:1214:13: key=n0014700/p6dda475f-9bd9-466b-a941-fc3190577c91 inline=Description { lower: Antichain { elements: [1749685442001] }, upper: Antichain { elements: [] }, since: Antichain { elements: [0] } } registered=Description { lower: Antichain { elements: [1749685441001] }, upper: Antichain { elements: [1749685444001] }, since: Antichain { elements: [0] } }
- With default flags Zippy Postgres CDC has an OoM:
Jun 11 22:38:21 hetzner-aarch64-8cpu-16gb-07668cf2 kernel: Memory cgroup out of memory: Killed process 389025 (clusterd) total-vm:28756204kB, anon-rss:12667528kB, file-rss:47520kB, shmem-rss:0kB, UID:999 pgtables:48544kB oom_score_adj:0
I have not seen this flaking on main, is it expected that your PR increases memory usage?
The unused dependencies and Kafka failures are also on main, I'll look into them, feel free to ignore for your PR.
brilliant! thanks so much for deep diving. I will take a look at the memory issue. I was not expecting a significant bump in memory usage as a result of the PR so will look into it.
A rerun of the Zippy Postgres CDC didn't OoM, so the signal is not very strong.
yeah, I will still triple check. by:
- soak test - running a long running source with a limited memory footprint for the materialized.
- pushing a large changeset through a source (write large batch to postgres table that has a source in materialized).
hoping that either large amount of data in a short time or over a long time would expose a memory issue.
The wrong result in Testdrive reproduces!
bin/mzcompose --find testdrive down && CI_SYSTEM_PARAMETERS_SEED=019760fd-11f6-4a42-a9a1-1fe7ef26c8bf CI_SYSTEM_PARAMETERS=random bin/mzcompose --find testdrive run default --default-size=1 fetch-select-during-ingest.td
But also on main, I'll open a bug, not related to your PR. Edit: https://github.com/MaterializeInc/database-issues/issues/9378
nice! thanks for tracking that down!