Skip to content

fix(load): admit a csv overwrite's memory once, one share per partition - #1187

Merged
azimafroozeh merged 4 commits into
ModernRelay:mainfrom
azimafroozeh:load-admission-backpressure
Oct 28, 2026
Merged

azimafroozeh merged 4 commits into
ModernRelay:mainfrom
azimafroozeh:load-admission-backpressure

Conversation

@azimafroozeh

Copy link
Copy Markdown
Collaborator

Closes #1160. A csv overwrite's parse partitions charge one shared pool as they decode, so which charge crosses query_memory_bytes and the actual the refusal reports depend on the order the partition threads reached the pool: the same case and seed reported actual: 26677 on one machine and 26285 on a loaded runner, which the DST can only call a replay mismatch, and after #1159 pinned one pair of charges into a fixed order and #1182 admitted the key sets before any partition starts, a busier runner finds the next pair.

ValidateExec now admits the load's memory once, after the key sets are charged and before any partition is polled: the room the limit leaves past the pool's total is carved into one share per parse partition and one for the unique holders, and every charge of a partition (its source pieces, its decoding, each decoded batch, the copy of that batch the fragment sink's chunker will hold, and the batch's validation facts, the last two charged once the slice's decode scratch is released) is made by the partition's own reader in source order against its share, so a refusal and its actual, the smallest limit whose share would have admitted the charge, follow that thread's own sequence and never another's progress; the fragment sink's own holds, lease metadata and the registry's growth are counted on the pool and never refused. The guard is the admission floor: a share under one partition's source admission plus one slice refuses at the carve, before anything is read, and a refused run of several partitions reruns as one partition holding the whole partition share, that run's refusal the one reported.

Two operator tests pin the mechanism: a two-partition overwrite charges the same consumers the same bytes on two runs and nothing outside the shared consumer set charges between the carve and the drain's end, and a share under the admission floor refuses with the smallest admitting limit before any partition is polled; the admission .gqt case pins that refusal and the load a wider limit admits, and load_csv_keyed_overwrite_admits_at_the_smallest_limit pins the contract at the edge, a one-partition keyed overwrite refused at 72,524 with actual 72525 and loaded at 72,525, on the local engine and two DST seeds, where the same load admitted at 59,462 before this change with a figure that moved between runs. At default settings a keyed one-column overwrite loads to 256,000 rows and refuses at 512,000, where the base loads 512,000; the smallest admitting limit for 256,000 rows is 97.4 MB against the base's 70.8 MB. The SF30 overwrite suite, under a limit no share fills, loads in 107.85 s on this branch against 108.96 s on the base over two interleaved rounds whose run walls overlap, so the share charge, one atomic per charge, is off the hot path.

For review: the decision to challenge is the accepted cost, that a keyed overwrite near the limit needs more query_memory_bytes than before: a one-partition share is 60 of 64 parts of the room by design, and the measured excess is larger, 1.22× at 30 rows and 1.29× per row at 128,000 to 256,000 rows, the rest being what the share keeps charged at the decode peak (the slice's decoded batches and their copies under the decode transient) where the pool's total at that instant was lower; the release note says the threshold moved and the settings guide says why. The input that exposes the fixed fractions is a many-partition overwrite with one partition's rows far wider than the rest, which cannot borrow another's unused room. Least sure is the uncharged part: the fragment sink's open chunks, a share's lease metadata and the registry's growth are counted on the pool but never refused, so a load can end past the limit by those bytes, within the four uncharged 64ths on the corpus's cases and past them at a limit of tens of KiB with many batches. The storage-cost golden is unchanged, since the shares decide when a load refuses and not what it reads or writes; the execution guide states the carve, the settings and mutations guides the contract, and changelog.d/load-memory-admitted-once.changed.md the behaviour change: a refusal now comes before anything is read, actual is the smallest admitting limit where it was the pool's total at the refused grow, and a keyed overwrite near the limit may need a higher one.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-10-27T23:23:18.447391Z
We couldn’t complete this review. Comment @codex review to try again.
7939dc2 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@azimafroozeh
azimafroozeh added this pull request to the merge queue Oct 28, 2026
Merged via the queue into ModernRelay:main with commit 8700601 Oct 28, 2026
32 checks passed
@azimafroozeh
azimafroozeh deleted the load-admission-backpressure branch October 28, 2026 05:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: DST replay mismatch, a load's memory refusal reports a timing-dependent actual

1 participant