Repository navigation
Replay a query with several DDL commands once - #1256
Open
joeyjooste wants to merge 1 commit into
Open
joeyjooste wants to merge 1 commit into
joeyjooste wants to merge 1 commit into
Conversation
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.
Description
I came across an issue where a migration that pgstream replicated stopped my stream in strict mode. A query with several DDL commands emits one DDL event per command, and each event carries the whole query as its DDL. The writer ran the query once per event, so the second run failed on the first command that cannot run twice, and strict mode stopped the stream for good. A migration file sent as one query, or a
DOblock that runs several DDL commands, is enough to cause it.With this change,
emit_ddl()records which client query an event came from, and the batch writer runs each query once.Related Issue(s)
Type of Change
Changes Made
core/4_emit_ddl_statement_identityandinjector/6_emit_ddl_statement_identityaddxid(txid_current()) andstatement_timestamp(statement_timestamp()) to the DDL payload.statement_timestamp()is the same for every event from one client query and changes with the next one. The down migrations restore the previousemit_ddl().wal.DDLEventreads the two fields, andStatementKey()combines them. It is empty for an event from anemit_ddl()without them.BatchWriterremembers the key of the last DDL event whose queries all ran, and skips an event with the same key. It skips before it converts the event, and the schema observer has already seen the event inwalEventToMessage, so the metadata from every command is kept.statement_timestamp()differs.Testing
Unit tests added/updated
Integration tests added/updated
All existing tests pass
TestBatchWriter_sendBatch_ddlOncePerStatement: events of one query run it once, also across batches. The same text as a separate query runs again. Events without the new fields all run. A query whose replay was dropped is not remembered.TestDDLEvent_StatementKey: the key with and without the new fields.Test_PostgresToPostgres_MultiStatementDDL(integration, strict mode): one query with twoALTER TABLEcommands, and aDOblock that runs two. When I run it onmain, both subtests fail withstrict mode: stopping on non-internal DDL failure: relation already exists. With this change both pass, and a row inserted after each DDL reaches the target.Test_PostgresToKafkanow ignores the two new fields, which change on every run, as it already ignoresOID, and checks that they are set.make test,make integration-test,make lintandmake license-checkpass, apart from three tests that fail onmaintoo on my machine:TestMapper_MapColumnValue(it expects UTC, and my machine is on BST),Test_ReplicaSourceSnapshotAndReplicationandTest_BulkIngestWriter_PostGISColumns.Additional Notes
pgstream init(or--migrations-only) on the source to take effect. Until then nothing changes.emit_ddl(). If it merges first, I will rebase this PR on it. OnceCREATE TRIGGERemits an event, more queries hit this bug, because eachCREATE TRIGGERin a query adds one more event.