Skip to content

Replay a query with several DDL commands once - #1256

Open
joeyjooste wants to merge 1 commit into
xataio:mainfrom
joeyjooste:emit-ddl-once-per-statement
Open

joeyjooste wants to merge 1 commit into
xataio:mainfrom
joeyjooste:emit-ddl-once-per-statement

Conversation

@joeyjooste

Copy link
Copy Markdown

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 DO block 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

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • New migrations core/4_emit_ddl_statement_identity and injector/6_emit_ddl_statement_identity add xid (txid_current()) and statement_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 previous emit_ddl().
  • wal.DDLEvent reads the two fields, and StatementKey() combines them. It is empty for an event from an emit_ddl() without them.
  • BatchWriter remembers 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 in walEventToMessage, so the metadata from every command is kept.
  • A key is remembered only after all its queries ran. If a replay fails and is dropped in non-strict mode, the next event of the same query runs it again, as it does today.
  • Events without the new fields replay as they do today.
  • The same text sent again as a separate query still runs, because its 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 two ALTER TABLE commands, and a DO block that runs two. When I run it on main, both subtests fail with strict 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_PostgresToKafka now ignores the two new fields, which change on every run, as it already ignores OID, and checks that they are set.

  • make test, make integration-test, make lint and make license-check pass, apart from three tests that fail on main too on my machine: TestMapper_MapColumnValue (it expects UTC, and my machine is on BST), Test_ReplicaSourceSnapshotAndReplication and Test_BulkIngestWriter_PostGISColumns.

Additional Notes

  • The migrations need pgstream init (or --migrations-only) on the source to take effect. Until then nothing changes.
  • The replay runs the whole query text, so a query that mixes DML with DDL still runs its DML on the target once with the replay, as well as through the WAL. This change does not touch that.
  • Emit CREATE TRIGGER DDL events with the correct schema #1201 also edits emit_ddl(). If it merges first, I will rebase this PR on it. Once CREATE TRIGGER emits an event, more queries hit this bug, because each CREATE TRIGGER in a query adds one more event.

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.

A query with several DDL commands is replayed once per command, so the second replay fails

1 participant