Skip to content

Commit 68d1858

Browse files
committed
fix(postgres): establish logical slot for base
- Make CurrentPosition create the pgoutput logical slot if absent and anchor to its consistent point, so WAL from the base forward is retained. A logical slot only decodes WAL produced after its own creation, so creating it lazily at stream time meant the first incremental and the bounded change stream captured zero changes. - Add establishLogicalSlot helper; ensureLogicalSlot now returns the consistent point on fresh creation, "" when the slot already exists. - Fix the two Postgres integration tests to take the start/base position via CurrentPosition before the DML, matching the real ordering. - Give the cross-engine MySQL container a 180s startup wait so a slow concurrent boot is tolerated rather than failing the run.
1 parent 904763e commit 68d1858

5 files changed

Lines changed: 88 additions & 28 deletions

File tree

internal/app/cross_engine_integration_test.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,10 @@ import (
99
"testing"
1010
"time"
1111

12+
tc "github.com/testcontainers/testcontainers-go"
1213
mysqlctr "github.com/testcontainers/testcontainers-go/modules/mysql"
1314
pgctr "github.com/testcontainers/testcontainers-go/modules/postgres"
15+
"github.com/testcontainers/testcontainers-go/wait"
1416

1517
mysqlcommon "github.com/nixrajput/siphon/internal/driver/_mysqlcommon"
1618

@@ -56,10 +58,20 @@ func startIntegPG(t *testing.T) (host string, port int, cleanup func()) {
5658
func startIntegMySQL(t *testing.T) (host string, port int, cleanup func()) {
5759
t.Helper()
5860
ctx := context.Background()
61+
// Explicit, generous startup wait: this test boots Postgres + MySQL in one
62+
// run alongside the per-driver suites, so under concurrent cold-image pulls
63+
// MySQL's two-phase init (init → restart → second "ready for connections")
64+
// can exceed the module's default 60s timeout. Wait for that second ready
65+
// log with a 180s ceiling so a slow-but-healthy boot is tolerated, not failed.
5966
c, err := mysqlctr.Run(ctx, "mysql:8.0",
6067
mysqlctr.WithDatabase("test"),
6168
mysqlctr.WithUsername("root"),
6269
mysqlctr.WithPassword("rootpass"),
70+
tc.WithWaitStrategy(
71+
wait.ForLog("port: 3306 MySQL Community Server").
72+
WithOccurrence(1).
73+
WithStartupTimeout(180*time.Second),
74+
),
6375
)
6476
if err != nil {
6577
t.Fatalf("start mysql container: %v", err)

internal/driver/postgres/changestream.go

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ func (c *Conn) streamWithStop(ctx context.Context, from canonical.Position, stop
6767
}
6868
defer func() { _ = repl.Close(context.Background()) }()
6969

70-
if err := ensureLogicalSlot(ctx, repl); err != nil {
70+
if _, err := ensureLogicalSlot(ctx, repl); err != nil {
7171
return canonical.Position{}, err
7272
}
7373

@@ -375,16 +375,50 @@ func (c *Conn) ensurePublication(ctx context.Context) error {
375375
// ensureLogicalSlot creates the pgoutput logical slot on the replication
376376
// connection if absent. CreateReplicationSlot errors with "already exists" when
377377
// the slot is present; that is tolerated so repeated streams reuse the slot.
378-
func ensureLogicalSlot(ctx context.Context, repl *pgconn.PgConn) error {
379-
_, err := pglogrepl.CreateReplicationSlot(ctx, repl, siphonSlot, outputPlugin,
378+
//
379+
// It returns the slot's consistent point — the LSN from which the freshly
380+
// created slot guarantees every subsequent change is retained and decodable —
381+
// when it creates the slot, and "" when the slot already existed (the existing
382+
// slot is already retaining WAL, so the caller has no new anchor to record).
383+
func ensureLogicalSlot(ctx context.Context, repl *pgconn.PgConn) (string, error) {
384+
res, err := pglogrepl.CreateReplicationSlot(ctx, repl, siphonSlot, outputPlugin,
380385
pglogrepl.CreateReplicationSlotOptions{Temporary: false})
381386
if err != nil {
382387
if strings.Contains(err.Error(), "already exists") {
383-
return nil
388+
return "", nil
384389
}
385-
return &errs.Error{Op: "postgres.stream", Code: errs.CodeSystem, Cause: err}
390+
return "", &errs.Error{Op: "postgres.stream", Code: errs.CodeSystem, Cause: err}
386391
}
387-
return nil
392+
return res.ConsistentPoint, nil
393+
}
394+
395+
// establishLogicalSlot opens a short-lived replication connection, ensures the
396+
// publication and logical slot exist, and returns the slot's consistent point
397+
// (empty when the slot already existed).
398+
//
399+
// This is the slot-establishment primitive used to anchor WAL retention BEFORE
400+
// a workload runs: a logical slot only retains and decodes WAL produced after
401+
// its own creation, so the slot must exist before the changes a later
402+
// incremental or CDC run wants to capture. CurrentPosition calls this so a base
403+
// backup leaves the slot in place, and the changes that follow are retained.
404+
func (c *Conn) establishLogicalSlot(ctx context.Context) (string, error) {
405+
if err := c.requireLogicalWAL(ctx); err != nil {
406+
return "", err
407+
}
408+
if err := c.ensurePublication(ctx); err != nil {
409+
return "", err
410+
}
411+
repl, err := pgconn.Connect(ctx, c.replicationDSN())
412+
if err != nil {
413+
return "", &errs.Error{
414+
Op: "postgres.stream",
415+
Code: errs.CodeSystem,
416+
Cause: errs.ErrConnectionFailed,
417+
Hint: "logical replication connect: " + err.Error(),
418+
}
419+
}
420+
defer func() { _ = repl.Close(context.Background()) }()
421+
return ensureLogicalSlot(ctx, repl)
388422
}
389423

390424
// resolveStartLSN returns the LSN to start replication from. A non-empty

internal/driver/postgres/changestream_integration_test.go

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -72,14 +72,12 @@ func TestStreamChanges_Bounded(t *testing.T) {
7272

7373
conn := &Conn{db: db, p: prof}
7474

75-
// Capture the start LSN before any DML so the stream sees exactly our changes.
76-
var startLSN string
77-
if err := db.QueryRowContext(ctx, "SELECT pg_current_wal_lsn()::text").Scan(&startLSN); err != nil {
78-
t.Fatalf("capture lsn: %v", err)
79-
}
80-
// The publication+slot must exist before the DML so the slot retains the WAL.
81-
if err := conn.ensurePublication(ctx); err != nil {
82-
t.Fatalf("ensure publication: %v", err)
75+
// Capture the start position before any DML so the stream sees exactly our
76+
// changes. CurrentPosition also establishes the logical slot — a slot only
77+
// retains WAL produced after its creation, so it must exist before the DML.
78+
startPos, err := conn.CurrentPosition(ctx)
79+
if err != nil {
80+
t.Fatalf("capture start position: %v", err)
8381
}
8482

8583
// Apply the three operations.
@@ -109,7 +107,7 @@ func TestStreamChanges_Bounded(t *testing.T) {
109107
return nil
110108
}
111109

112-
if _, err := conn.StreamChanges(streamCtx, canonical.Position{LSN: startLSN}, emit); err != nil {
110+
if _, err := conn.StreamChanges(streamCtx, startPos, emit); err != nil {
113111
t.Fatalf("StreamChanges: %v", err)
114112
}
115113

internal/driver/postgres/incremental_change.go

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,28 @@ import (
1414
var _ driver.IncrementalBackuper = (*Conn)(nil)
1515
var _ driver.BasePositioner = (*Conn)(nil)
1616

17-
// CurrentPosition returns the server's current WAL position via
18-
// pg_current_wal_lsn(). app.Backup calls this right after a full backup so the
19-
// base dump's Envelope records where the first incremental should resume from.
17+
// CurrentPosition records the resume anchor for the first incremental after a
18+
// base backup. app.Backup calls this right after a full backup so the base
19+
// dump's Envelope carries where the first incremental should resume from.
20+
//
21+
// It also ESTABLISHES the logical replication slot if it does not yet exist. A
22+
// logical slot only retains and decodes WAL produced after the slot's own
23+
// creation, so the slot must exist from the recorded anchor forward or the
24+
// first incremental silently captures nothing (it would resume from an LSN the
25+
// slot never retained). When this call creates the slot, the slot's consistent
26+
// point is the correct anchor — it is the exact LSN from which the slot
27+
// guarantees decodable WAL. When the slot already exists it is already
28+
// retaining WAL, so pg_current_wal_lsn() is a safe anchor.
2029
func (c *Conn) CurrentPosition(ctx context.Context) (canonical.Position, error) {
30+
consistent, err := c.establishLogicalSlot(ctx)
31+
if err != nil {
32+
return canonical.Position{}, err
33+
}
34+
if consistent != "" {
35+
// Slot freshly created: anchor to its consistent point, the boundary
36+
// from which it retains every subsequent change.
37+
return canonical.Position{LSN: consistent}, nil
38+
}
2139
var lsn string
2240
if err := c.db.QueryRowContext(ctx, "SELECT pg_current_wal_lsn()::text").Scan(&lsn); err != nil {
2341
return canonical.Position{}, &errs.Error{Op: "postgres.current_position", Code: errs.CodeSystem, Cause: err}

internal/driver/postgres/incremental_integration_test.go

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,13 @@ func TestBackupIncremental_BoundedCaptureAndReplay(t *testing.T) {
4646

4747
conn := &Conn{db: db, p: prof}
4848

49-
// Publication + slot must exist before the DML so the slot retains the WAL.
50-
if err := conn.ensurePublication(ctx); err != nil {
51-
t.Fatalf("ensure publication: %v", err)
52-
}
53-
54-
// Capture the base-end LSN: the incremental resumes from exactly here.
55-
var baseEnd string
56-
if err := db.QueryRowContext(ctx, "SELECT pg_current_wal_lsn()::text").Scan(&baseEnd); err != nil {
57-
t.Fatalf("capture base-end lsn: %v", err)
49+
// Capture the base-end position: the incremental resumes from exactly here.
50+
// CurrentPosition also establishes the logical slot — a slot only retains
51+
// WAL produced after its creation, so it must exist before the post-base DML
52+
// or the incremental captures nothing.
53+
basePos, err := conn.CurrentPosition(ctx)
54+
if err != nil {
55+
t.Fatalf("capture base-end position: %v", err)
5856
}
5957

6058
// Post-base changes: one insert, one update, one delete.
@@ -72,7 +70,7 @@ func TestBackupIncremental_BoundedCaptureAndReplay(t *testing.T) {
7270
capCtx, cancel := context.WithTimeout(ctx, 60*time.Second)
7371
defer cancel()
7472
var buf bytes.Buffer
75-
endPos, err := conn.BackupIncremental(capCtx, canonical.Position{LSN: baseEnd}, &buf)
73+
endPos, err := conn.BackupIncremental(capCtx, basePos, &buf)
7674
if err != nil {
7775
t.Fatalf("BackupIncremental: %v", err)
7876
}

0 commit comments

Comments
 (0)