Skip to content

Copy passthrough during snapshot mode if no transformers are configured - #1129

Open
kvch wants to merge 3 commits into
mainfrom
snapshot-copy-passthrough
Open

Copy passthrough during snapshot mode if no transformers are configured#1129
kvch wants to merge 3 commits into
mainfrom
snapshot-copy-passthrough

Conversation

@kvch

@kvch kvch commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Description

This PR adds support for snapshotting a table by piping the source's COPY ... TO STDOUT straight into the target's COPY ... FROM STDIN when the rows do not need to be parsed. When a transformer, filter, injector or sanitizer is configured, we fall back to the previous behaviour.

Previously, we read read rows, decoded every value into Go values via pgx, wrapped them in wal.Event structs, passed them through the processor chain to the bulk ingest writer, which re-encoded them into a COPY. Both ends were already COPY; only the round trip through Go in the middle is removed. I measured improvement on ~17% on a text heavy table. It is a workload-specific improvement, since the saving is largest where decoding allocates most. So it can be higher for some cases.

Screenshot 2026-08-31 at 20-43-23 pgstream benchmarks

Furthermore, #801 (cube) and #1035 (enum) exist because pgx cannot produce those types' binary encodings, which is why textOnlyCopyTypes and needsTextCopy exist. Server-to-server COPY never touches a pgx codec, so that family of bugs cannot occur on this path.

Builds on the raw COPY stream primitives from #1126.

Please review commit by commit.

  • feat: stream snapshot rows without decoding them: the new logic
  • refactor: make the page range reader a strategy: pg_snapshot_range_snapshotter.go looks like a new file but is processTableRange relocated: three receiver renames and one inlined variable

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📚 Documentation update
  • 🔧 Refactoring (no functional changes)
  • ⚡ Performance improvement
  • 🧪 Test coverage improvement
  • 🔨 Build/CI changes
  • 🧹 Code cleanup

Changes Made

Commit 1: the passthrough

  • The two COPYs run either side of an io.Pipe, so memory is bounded by the pipe rather than the page range, and each side closes its end with its error so neither can block on the other
  • Binary format throughout
  • Generated columns are named by neither end, taken from the target's catalog since the target is what rejects them. A table whose columns are all generated has nothing COPY can carry and falls back to decoding.

Commit 2: the strategy extraction

  • Reading a page range becomes an interface with two implementations
  • The reader is handed the transaction runner rather than a transaction, so one that must wait waits before the source transaction opens rather than holding a source connection idle in transaction
  • Construction goes through newSnapshotGenerator, which takes the reader as an argument, so no path can leave it unset.

Testing

  • Unit tests added/updated
  • Integration tests added/updated

Checklist

  • Code follows project style guidelines
  • Self-review completed
  • Code is well-commented
  • Documentation updated where necessary (no user-facing config surface)

@github-actions

Copy link
Copy Markdown

Coverage

Total: 60.3% (+0.1% vs main)

Coverage in packages changed by this PR:

Package Coverage Δ
internal/sync 5.0% +5.0%
pkg/snapshot/generator/postgres/data 78.3% -1.1%
pkg/stream 48.4% +0.5%
pkg/wal/processor/postgres 84.2% -0.1%

@kvch
kvch requested a review from tsg August 31, 2026 18:44
if cfg.RetryPolicy.DisableRetries {
targetConn, err = pglib.NewConnPool(ctx, cfg.TargetURL, poolOpts...)
} else {
targetConn, err = pglibretrier.NewQuerier(ctx, cfg.RetryPolicy, func(ctx context.Context) (pglib.Querier, error) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need retries in this case? From my agent:

The target pool is wrapped in the retrier, which re-runs the ExecInTx closure on a retriable error. The pipe reader is already partially consumed by then, so the retry sends a truncated COPY and fails with the row count mismatch, then retries again until the backoff gives up. Nothing corrupt lands because the target tx rolls back, but the retry loop burns time and logs misleading warnings. The passthrough should probably disable retries on the target conn and let the existing page range failure abort the table, or retry the whole range from the source.

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.

2 participants