fix(ingestion): stop duplicate instants reaching one INSERT - #862
Merged
Conversation
The first live materialization failed:
ON CONFLICT DO UPDATE command cannot affect row a second time
HINT: Ensure that no rows proposed for insertion within the same command
have duplicate constrained values.
Two causes, both here. Diver-HUB ranges are inclusive at both ends -- "up to
and including end time" -- while iter_windows made adjacent windows share a
boundary, so a reading logged exactly on it came back in both. And the loader
never deduplicated, so that pair reached Postgres in one statement.
Windows now leave exactly one second between them. Timestamps are
second-resolution, so nothing falls in the gap.
The loader also deduplicates within a batch, keeping the last occurrence --
which matches the upsert's own rule that a later value wins. That guard holds
whatever the source does, including a vendor logging one instant twice.
The existing idempotency test could not have caught this: it loads the same
window in two separate statements, which Postgres allows. The new test puts the
duplicates in one batch, which is what actually happened.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Coverage✅ 78.59% total — gate is 75%. Coverage for the Python files changed in this PR
|
Contributor
|
Your pull request is automatically being deployed to Dagster Cloud.
|
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.
The first live materialization failed. Both raw assets succeeded;
san_acacia_observationsfailed three times (initial + 2 retries) and took therun down.
Nothing was written — the transaction rolled back each time, so staging is
unchanged and holds only the 542,161 pre-existing AMPAPI rows.
Two causes, both mine
Overlapping windows. Diver-HUB ranges are inclusive at both ends — "from
start time up to and including end time" — but
iter_windowsmade adjacentwindows share a boundary:
[0, span]then[span, 2·span]. A reading loggedexactly on the boundary came back in both.
No deduplication in the loader. That pair then reached Postgres inside one
INSERT, which the constraint from #847 correctly refuses.Fixes
Windows now leave exactly one second between them. Timestamps are
second-resolution, so nothing falls in the gap — there is a test for that.
The loader deduplicates within a batch, keeping the last occurrence, which
matches the upsert's own rule that a later value wins. That guard holds whatever
the source does, including a vendor logging one instant twice — so this does not
depend on the window fix being perfect.
Why the existing tests missed it
test_loading_the_same_window_twice_does_not_duplicateloads the same window intwo separate statements, which Postgres allows. The failure needs duplicates
in one statement. The new test does exactly that and asserts 6 records
become 3 rows with the later value winning.
Diagnosis note
RetryPolicyobscured this: the surfaced error was "Exceeded max_retries of 2",and the real cause was two levels down under
error.cause. Worth knowing whenreading a failed run.
125 ingestion tests plus 11 loader tests pass.
🤖 Generated with Claude Code