Repository navigation
fix(python): ch-pg-dump detects load failures and reads one snapshot anchored to the replication slot (Spec 13.05) - #1541
Merged
Merged
Conversation
… its micro-specs Adds 13-python-toolset to the validator's expected domains, the Constitution's domain table and the specs README index. The micro-specs 13.01-13.08 follow in this branch. [spec-exempt: specification-only change, no source change] Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Behaviour-as-built specification of the non-streaming Python tools under sink-connector/python (legacy tree and the packaged tree): packaging and the two source trees, the shared connection and credential layer, MySQL snapshot dump, ClickHouse snapshot load and MySQL DDL translation, PostgreSQL snapshot dump and load, MySQL and PostgreSQL verification runners with auto_diff, and resync / ad hoc repair. Each spec carries failure modes and a defect register (S1-S4, reproduced or code-read evidence with file:line on 2.11.0) that the follow-up fixes work from. No source change; offline suite unchanged (227 passed, 5 skipped). [spec-exempt: specification-only change, no source change] Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…hored to the slot (Spec 13.05 D-13.05-1..12, D-13.05-17) D-13.05-1: build_ch_insert_cmd no longer appends "; rm -f <file>"; it returns (cmd, file) and load_table / psqlcopy_load_table remove the temp files in Python after the exit status is read, so the pipe's status is the job's status. A failed pipe fails the job, main exits 1, and the offset is written only after every table and segment loaded. D-13.05-2 (and D-13.05-17): every data-reading psql runs with -X -q -v ON_ERROR_STOP=1 and statement_timeout=0; every pipeline, including the psql-copy dump pipe, runs under set -o pipefail. D-13.05-3: a dedicated coordinator connection reads the LSN, then opens a REPEATABLE READ READ ONLY transaction and exports its snapshot (pg_export_snapshot); every COPY session imports it with SET TRANSACTION SNAPSHOT. Before any data is read, the connector's logical slot (--replication_slot / slot.name, default "debezium") must exist, belong to the database, and have restart_lsn and confirmed_flush_lsn at or before the LSN; otherwise the run stops with a message telling the operator the connector must create its slot first. D-13.05-4: --skip_existing skips a populated table only when count() FINAL equals count(*) in the run's snapshot and fails otherwise; a run that skipped a table writes no offset and exits 1. D-13.05-5: keyless tables get every column as the ReplacingMergeTree sorting key, plus allow_nullable_key = 1 when one is Nullable, mirroring ClickHouseAutoCreateTable.keylessSortingKey (also in the unused build_ch_create_table_ddl). D-13.05-6, D-13.05-7: connector table lists stay schema-qualified and anchored (new --table_include_list / --table_exclude_list, full match on schema.table), schema lists are anchored, database lists are enforced; two source tables mapping to one ClickHouse table stop the run. D-13.05-8, -9, -10: every data-reading session pins TimeZone, DateStyle, IntervalStyle, extra_float_digits, bytea_output, lc_monetary and client_encoding; zone-less timestamps are parsed in the column zone; infinity and out-of-range temporal values saturate to the bounds the connector uses; a BC timestamptz or unparseable text fails the INSERT (throwIf) instead of storing NULL or a default. D-13.05-11: load_table builds the SELECT from override-applied types. D-13.05-12: --strategy pgdump is refused at start-up (exit 2) and its corrupting pg_restore/awk converter is removed. Operator-visible: a failed load now exits 1 (it exited 0); a dump without the connector's slot exits 1; --skip_existing that skips a table exits 1 without an offset; --strategy pgdump exits 2. Tests: db_dump/tests/test_postgres_dumper_unit.py (47 tests, each fix mutation-checked). Spec 13.05 sections 1-7 updated. Destructive-Op-Check: sites=0; result=pass; removed the shell rm -f, temp files removed with os.unlink Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
End-to-end justification (PR 1544, PostgreSQL suite, observed): with the 2.11.0 ch-pg-dump, a missing replication slot and a failed ClickHouse insert both exited 0 and wrote the offset (D-13.05-3, -1, -2); skip_existing skipped a partly loaded table (D-13.05-4); two schemas merged into one table (D-13.05-7); schema and table lists were not anchored (D-13.05-6); the keyless table collapsed from 25 rows to 1 (D-13.05-5); timestamp without time zone shifted 6 hours under a non-UTC server (D-13.05-8); special dates loaded as NULL (D-13.05-9); DateStyle SQL, DMY loaded NULL dates (D-13.05-10); a column override was ignored (D-13.05-11); the pgdump strategy ran instead of refusing (D-13.05-12). 27 of 33 tests failed before, all pass after. CI job python-toolset-e2e-postgres green on PR 1544. |
…t 1 of 10) Copies files verbatim from 2.11.0 (736c6d0) so the merge that follows reconciles a small remainder. [spec-exempt: verbatim copy from the base branch, no new change] Co-Authored-By: Claude Opus 5.5 (1M context) [noreply@anthropic.com]
…t 2 of 10) Copies files verbatim from 2.11.0 (736c6d0) so the merge that follows reconciles a small remainder. [spec-exempt: verbatim copy from the base branch, no new change] Co-Authored-By: Claude Opus 5.5 (1M context) [noreply@anthropic.com]
…t 3 of 10) Copies files verbatim from 2.11.0 (736c6d0) so the merge that follows reconciles a small remainder. The two destructive sites are upstream end-to-end test fixtures (DROP DATABASE IF EXISTS on suite-owned pye2e_ scratch databases) and each already carries a DESTRUCTIVE: comment. [spec-exempt: verbatim copy from the base branch, no new change] Destructive-Op-Check: sites=2; result=pass Co-Authored-By: Claude Opus 5.5 (1M context) [noreply@anthropic.com]
…t 4 of 10) Copies files verbatim from 2.11.0 (736c6d0) so the merge that follows reconciles a small remainder. [spec-exempt: verbatim copy from the base branch, no new change] Co-Authored-By: Claude Opus 5.5 (1M context) [noreply@anthropic.com]
…t 5 of 10) Copies files verbatim from 2.11.0 (736c6d0) so the merge that follows reconciles a small remainder. [spec-exempt: verbatim copy from the base branch, no new change] Co-Authored-By: Claude Opus 5.5 (1M context) [noreply@anthropic.com]
…t 6 of 10) Copies files verbatim from 2.11.0 (736c6d0) so the merge that follows reconciles a small remainder. The three destructive sites are upstream end-to-end test fixtures (DROP DATABASE IF EXISTS on the suite-owned pyops, pyref and pyflags scratch databases) and are already covered by a DESTRUCTIVE: comment. [spec-exempt: verbatim copy from the base branch, no new change] Destructive-Op-Check: sites=3; result=pass Co-Authored-By: Claude Opus 5.5 (1M context) [noreply@anthropic.com]
…t 7 of 10) Copies files verbatim from 2.11.0 (736c6d0) so the merge that follows reconciles a small remainder. The one destructive site is an upstream end-to-end test fixture (ALTER TABLE ... DROP COLUMN on the test's own pye2e_pg_dropcol scratch clone, never the dumped database) and already carries a DESTRUCTIVE: comment. [spec-exempt: verbatim copy from the base branch, no new change] Destructive-Op-Check: sites=1; result=pass Co-Authored-By: Claude Opus 5.5 (1M context) [noreply@anthropic.com]
…t 8 of 10) Copies files verbatim from 2.11.0 (736c6d0) so the merge that follows reconciles a small remainder. The nine destructive sites are upstream end-to-end test fixtures (DROP DATABASE/TABLE IF EXISTS on suite-owned scratch databases and tables of the disposable e2e ClickHouse) and each already carries a DESTRUCTIVE: comment. [spec-exempt: verbatim copy from the base branch, no new change] Destructive-Op-Check: sites=9; result=pass Co-Authored-By: Claude Opus 5.5 (1M context) [noreply@anthropic.com]
…t 9 of 10) Copies files verbatim from 2.11.0 (736c6d0) so the merge that follows reconciles a small remainder. The files the Jinja guard flags are GitHub Actions workflow YAML (Actions expressions) and a Markdown spec documenting template variables, not Jinja templates; both workflows parse with a duplicate-key-detecting YAML loader and the spec passes a text check (non-empty, no unrendered Jinja statement or comment markers). The six destructive sites are upstream end-to-end test fixtures (DROP DATABASE IF EXISTS on suite-owned scratch databases, and docker rm of a never-started CI helper container after its binary is copied out) and are already covered by DESTRUCTIVE: comments. [spec-exempt: verbatim copy from the base branch, no new change] Jinja-Render-Check: rendered=3; formats=yaml,text; result=pass Destructive-Op-Check: sites=6; result=pass Co-Authored-By: Claude Opus 5.5 (1M context) [noreply@anthropic.com]
…t 10 of 10) Copies files verbatim from 2.11.0 (736c6d0) so the merge that follows reconciles a small remainder. The files the Jinja guard flags are a GitHub Actions workflow YAML (Actions expressions) and two Python files with format-string braces, not Jinja templates; the workflow parses with a duplicate-key-detecting YAML loader and both Python files parse with ast.parse. [spec-exempt: verbatim copy from the base branch, no new change] Jinja-Render-Check: rendered=3; formats=yaml,text; result=pass Co-Authored-By: Claude Opus 5.5 (1M context) [noreply@anthropic.com]
Resolves the conflicts with 2.11.0. #1544 was stacked on this branch and its squash-merge already brought every commit of it into 2.11.0, together with later fixes. The 2.11.0 revision is kept for every file (taken in the preceding commits, split so each commit stays reviewable), so this branch now matches 2.11.0. Co-Authored-By: Claude Opus 5.5 (1M context) [noreply@anthropic.com]
Collaborator
Author
|
Conflict with 2.11.0 resolved (merge commit 0e5c453, no force-push). #1544 contains every commit of this branch and was squash-merged into 2.11.0 together with later fixes, so this fix is already on 2.11.0. The resolution keeps the 2.11.0 revision of every file and this PR now has no remaining diff against 2.11.0. |
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.
Stacked on #1537 (domain 13 specs). Review commit bf52511 only; merge #1537 first. Packaged tree only.
Operator notes: a failed load exits 1 (was 0); a missing, invalid or ahead slot exits 1 before touching ClickHouse; the coordinator transaction stays open for the whole dump and holds back vacuum like pg_dump.
Tests: new db_dump/tests/test_postgres_dumper_unit.py (47 tests, 3 run generated SQL through clickhouse local and skip when absent). Suite 227 passed / 5 skipped before, 274 passed / 5 skipped after; 20 single-fix reverts each fail a new test. scripts/validate_specs.py PASSED. Destructive-Op-Check: sites=0.
🤖 Generated with Claude Code