Skip to content

Commit b902ac2

Browse files
committed
Call WalDataToDDLEvent once per event in the processor chain
1 parent 96da911 commit b902ac2

6 files changed

Lines changed: 115 additions & 7 deletions

File tree

pkg/snapshot/generator/postgres/data/pg_snapshot_generator_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1178,6 +1178,7 @@ func TestSnapshotGenerator_CreateSnapshot(t *testing.T) {
11781178
}
11791179
diff := cmp.Diff(events, tc.wantEvents,
11801180
cmpopts.IgnoreFields(wal.Data{}, "Timestamp"),
1181+
cmpopts.IgnoreUnexported(wal.Data{}),
11811182
cmpopts.SortSlices(func(a, b *wal.Event) bool { return a.Data.Table < b.Data.Table }))
11821183
require.Empty(t, diff, fmt.Sprintf("got: \n%v, \nwant \n%v, \ndiff: \n%s", events, tc.wantEvents, diff))
11831184
})
@@ -1701,6 +1702,7 @@ func TestSnapshotGenerator_snapshotTableRange(t *testing.T) {
17011702
}
17021703
diff := cmp.Diff(events, tc.wantEvents,
17031704
cmpopts.IgnoreFields(wal.Data{}, "Timestamp"),
1705+
cmpopts.IgnoreUnexported(wal.Data{}),
17041706
cmpopts.SortSlices(func(a, b *wal.Event) bool { return a.Data.Table < b.Data.Table }))
17051707
require.Empty(t, diff, fmt.Sprintf("got: \n%v, \nwant \n%v, \ndiff: \n%s", events, tc.wantEvents, diff))
17061708
})

pkg/wal/listener/kafka/wal_kafka_reader_test.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,12 @@ func TestReader_Listen(t *testing.T) {
4444
require.Equal(t, []byte("test-value"), b)
4545
data, ok := a.(*wal.Data)
4646
require.True(t, ok)
47-
*data = *testWalEvent.Data
47+
// assign fields individually rather than copying the struct by value:
48+
// wal.Data holds a sync.Once for DDL-event memoisation and must not be
49+
// copied (this mirrors how the real deserialiser populates fields).
50+
data.Action = testWalEvent.Data.Action
51+
data.Schema = testWalEvent.Data.Schema
52+
data.Table = testWalEvent.Data.Table
4853
return nil
4954
}
5055

pkg/wal/processor/filter/wal_filter_test.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -200,11 +200,9 @@ func TestFilter_ProcessWALEvent(t *testing.T) {
200200
},
201201
processor: &mocks.Processor{
202202
ProcessWALEventFn: func(ctx context.Context, walEvent *wal.Event) error {
203-
require.Equal(t, &wal.Data{
204-
Action: wal.LogicalMessageAction,
205-
Prefix: wal.DDLPrefix,
206-
Content: string(ddlEventBytes),
207-
}, walEvent.Data)
203+
require.Equal(t, wal.LogicalMessageAction, walEvent.Data.Action)
204+
require.Equal(t, wal.DDLPrefix, walEvent.Data.Prefix)
205+
require.Equal(t, string(ddlEventBytes), walEvent.Data.Content)
208206
return nil
209207
},
210208
},

pkg/wal/wal_data.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package wal
55
import (
66
"errors"
77
"slices"
8+
"sync"
89
"time"
910

1011
"github.com/rs/xid"
@@ -33,6 +34,10 @@ type Data struct {
3334
Content string `json:"content,omitempty"`
3435
// pgstream specific metadata
3536
Metadata Metadata `json:"metadata"`
37+
38+
ddlEventOnce sync.Once
39+
ddlEvent *DDLEvent
40+
ddlEventErr error
3641
}
3742

3843
// Metadata is pgstream specific properties to help identify the id/version

pkg/wal/wal_ddl.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,16 @@ func (d *Data) IsDDLEvent() bool {
5555
return d.Action == LogicalMessageAction && d.Prefix == DDLPrefix
5656
}
5757

58-
// WalDataToDDLEvent parses the wal data content field as a DDL event
58+
// WalDataToDDLEvent parses the wal data content field as a DDL event.
59+
// The parsed result (or error) is cached on the Data value.
5960
func WalDataToDDLEvent(d *Data) (*DDLEvent, error) {
61+
d.ddlEventOnce.Do(func() {
62+
d.ddlEvent, d.ddlEventErr = parseDDLEvent(d)
63+
})
64+
return d.ddlEvent, d.ddlEventErr
65+
}
66+
67+
func parseDDLEvent(d *Data) (*DDLEvent, error) {
6068
if !d.IsDDLEvent() {
6169
return nil, fmt.Errorf("%w: action=%s, prefix=%s", ErrNotDDLEvent, d.Action, d.Prefix)
6270
}

pkg/wal/wal_ddl_test.go

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,99 @@ package wal
55
import (
66
"testing"
77

8+
jsonlib "github.com/xataio/pgstream/internal/json"
9+
810
"github.com/stretchr/testify/require"
911
)
1012

13+
const testDDLContent = `{
14+
"ddl": "CREATE TABLE users (id SERIAL PRIMARY KEY, name TEXT)",
15+
"schema_name": "public",
16+
"command_tag": "CREATE TABLE",
17+
"objects": [{
18+
"type": "table",
19+
"identity": "public.users",
20+
"schema": "public",
21+
"oid": "16384",
22+
"pgstream_id": "ck7s8u4000001"
23+
}]
24+
}`
25+
26+
func TestWalDataToDDLEvent_memoisation(t *testing.T) {
27+
t.Parallel()
28+
29+
data := &Data{
30+
Action: "M",
31+
Prefix: "pgstream.ddl",
32+
Content: testDDLContent,
33+
}
34+
35+
first, err := WalDataToDDLEvent(data)
36+
require.NoError(t, err)
37+
require.NotNil(t, first)
38+
39+
second, err := WalDataToDDLEvent(data)
40+
require.NoError(t, err)
41+
42+
// repeat calls must return the exact same cached pointer, i.e. the content
43+
// is parsed only once.
44+
require.Same(t, first, second)
45+
46+
// the error path is memoised too.
47+
notDDL := &Data{Action: "I"}
48+
_, err1 := WalDataToDDLEvent(notDDL)
49+
_, err2 := WalDataToDDLEvent(notDDL)
50+
require.ErrorIs(t, err1, ErrNotDDLEvent)
51+
require.ErrorIs(t, err2, ErrNotDDLEvent)
52+
}
53+
54+
func BenchmarkWalDataToDDLEvent_memoised(b *testing.B) {
55+
data := &Data{
56+
Action: "M",
57+
Prefix: "pgstream.ddl",
58+
Content: testDDLContent,
59+
}
60+
// prime the memo so the benchmarked calls only hit the cached fast path.
61+
if _, err := WalDataToDDLEvent(data); err != nil {
62+
b.Fatal(err)
63+
}
64+
65+
b.ReportAllocs()
66+
for i := 0; i < b.N; i++ {
67+
if _, err := WalDataToDDLEvent(data); err != nil {
68+
b.Fatal(err)
69+
}
70+
}
71+
}
72+
73+
func TestWalDataToDDLEvent_serialisationUnaffected(t *testing.T) {
74+
t.Parallel()
75+
76+
// a Data whose DDL event has been memoised must serialise byte-identically
77+
// to one that has not: the memo fields are unexported and excluded from JSON.
78+
plain := &Data{
79+
Action: "M",
80+
Prefix: "pgstream.ddl",
81+
Content: testDDLContent,
82+
Schema: "public",
83+
}
84+
memoised := &Data{
85+
Action: "M",
86+
Prefix: "pgstream.ddl",
87+
Content: testDDLContent,
88+
Schema: "public",
89+
}
90+
_, err := WalDataToDDLEvent(memoised)
91+
require.NoError(t, err)
92+
93+
plainBytes, err := jsonlib.Marshal(plain)
94+
require.NoError(t, err)
95+
memoisedBytes, err := jsonlib.Marshal(memoised)
96+
require.NoError(t, err)
97+
98+
require.Equal(t, plainBytes, memoisedBytes)
99+
}
100+
11101
func TestData_IsDDLEvent(t *testing.T) {
12102
t.Parallel()
13103

0 commit comments

Comments
 (0)