Skip to content

Commit 88636d6

Browse files
committed
review feedbacks:
- decouple offload and restore (so restore derive based on range_offloaded flag) - always encrypt non-nil ranges regardless of type - update schema to include inserted_at timestamp - make sure metadata_qrep_partitions also does not store serialized partition in plaintext - add dynconf PEERDB_OFFLOAD_PARTITION_RANGES to allow disabling offload/encryption behind a FF - update tests
1 parent c525acc commit 88636d6

11 files changed

Lines changed: 281 additions & 210 deletions

File tree

flow/activities/flowable.go

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -591,16 +591,30 @@ func (a *FlowableActivity) GetQRepPartitions(ctx context.Context,
591591
}
592592
}
593593

594+
if last != nil && last.RangeOffloaded {
595+
if err := connmetadata.RestoreOffloadedPartitionRanges(
596+
ctx, a.CatalogPool, runUUID, []*protos.QRepPartition{last},
597+
); err != nil {
598+
return nil, a.Alerter.LogFlowError(ctx, config.FlowJobName,
599+
fmt.Errorf("failed to rehydrate last partition range: %w", err))
600+
}
601+
}
602+
594603
partitions, err := srcConn.GetQRepPartitions(ctx, config, last)
595604
if err != nil {
596605
return nil, a.Alerter.LogFlowError(ctx, config.FlowJobName, shared.WrapError("failed to get partitions from source", err))
597606
}
598607
if len(partitions) > 0 {
599-
// run before monitoring.InitializeQRepRun to avoid logging sensitive fields to peerdb_stats.qrep_partitions as well
600-
if err := connmetadata.OffloadSensitivePartitionRanges(
601-
ctx, a.CatalogPool, config.ParentMirrorName, runUUID, partitions,
602-
); err != nil {
603-
return nil, fmt.Errorf("failed to offload partition ranges: %w", err)
608+
shouldOffload, err := internal.PeerDBOffloadPartitionRanges(ctx, config.Env)
609+
if err != nil {
610+
return nil, fmt.Errorf("failed to read offload partition ranges setting: %w", err)
611+
}
612+
if shouldOffload {
613+
if err := connmetadata.OffloadPartitionRanges(
614+
ctx, a.CatalogPool, config.ParentMirrorName, runUUID, partitions,
615+
); err != nil {
616+
return nil, fmt.Errorf("failed to offload partition ranges: %w", err)
617+
}
604618
}
605619
if err := monitoring.InitializeQRepRun(
606620
ctx,
@@ -669,7 +683,7 @@ func (a *FlowableActivity) ReplicateQRepPartitions(ctx context.Context,
669683
return a.Alerter.LogFlowError(ctx, config.FlowJobName, err)
670684
}
671685

672-
if err := connmetadata.RestoreSensitivePartitionRanges(ctx, a.CatalogPool, runUUID, partitions.Partitions); err != nil {
686+
if err := connmetadata.RestoreOffloadedPartitionRanges(ctx, a.CatalogPool, runUUID, partitions.Partitions); err != nil {
673687
return fmt.Errorf("failed to rehydrate partition ranges: %w", err)
674688
}
675689

flow/connectors/external_metadata/store.go

Lines changed: 31 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66
"errors"
77
"fmt"
88
"log/slog"
9+
"maps"
10+
"slices"
911
"strconv"
1012
"time"
1113

@@ -27,7 +29,7 @@ import (
2729
const (
2830
lastSyncStateTableName = "metadata_last_sync_state"
2931
qrepTableName = "metadata_qrep_partitions"
30-
qrepPartitionRangesTableName = "metadata_qrep_partition_ranges"
32+
qrepPartitionRangesTableName = "metadata_qrep_offloaded_partition_ranges"
3133
)
3234

3335
type PostgresMetadata struct {
@@ -262,6 +264,15 @@ func (p *PostgresMetadata) FinishQRepPartition(
262264
jobName string,
263265
startTime time.Time,
264266
) error {
267+
if partition.RangeOffloaded {
268+
// The range was offloaded but has since been rehydrated, so drop it before persisting
269+
// to metadata_qrep_partitions.
270+
// Clone rather than mutate in place because this partition is shared with the pull
271+
// goroutine, cloning keeps us from relying on implicit ordering to avoid a write race.
272+
// TODO: drop the sync_partition column entirely; nothing reads it back.
273+
partition = proto.Clone(partition).(*protos.QRepPartition)
274+
partition.Range = nil
275+
}
265276
pbytes, err := protojson.Marshal(partition)
266277
if err != nil {
267278
return fmt.Errorf("failed to marshal partition to json: %w", err)
@@ -288,20 +299,16 @@ func (p *PostgresMetadata) IsQRepPartitionSynced(ctx context.Context, req *proto
288299
return exists, nil
289300
}
290301

291-
// OffloadSensitivePartitionRanges encrypts sensitive partition ranges, persists them to
292-
// the catalog, and replaces each partition's range in-place with an empty range.
293-
// Used in conjunction with RestoreSensitivePartitionRanges.
294-
func OffloadSensitivePartitionRanges(
302+
// OffloadPartitionRanges encrypts partition ranges and persists them to the catalog,
303+
// then clears each partition's range in-place and sets RangeOffloaded so the range can
304+
// be rehydrated later. Used in conjunction with RestoreOffloadedPartitionRanges.
305+
func OffloadPartitionRanges(
295306
ctx context.Context,
296307
pool shared.CatalogPool,
297308
parentMirrorName string,
298309
runUUID string,
299310
partitions []*protos.QRepPartition,
300311
) error {
301-
if !partitionsContainSensitiveRanges(partitions) {
302-
return nil
303-
}
304-
305312
key, err := internal.PeerDBCurrentEncKey(ctx)
306313
if err != nil {
307314
return fmt.Errorf("failed to load current encryption key: %w", err)
@@ -322,6 +329,9 @@ func OffloadSensitivePartitionRanges(
322329

323330
insertRows := make([][]any, 0, len(partitions))
324331
for _, partition := range partitions {
332+
if partition.Range == nil {
333+
continue
334+
}
325335
payload, err := proto.Marshal(partition.Range)
326336
if err != nil {
327337
return fmt.Errorf("failed to marshal partition range: %w", err)
@@ -332,9 +342,8 @@ func OffloadSensitivePartitionRanges(
332342
}
333343
insertRows = append(insertRows, []any{parentMirrorName, runUUID, partition.PartitionId, key.ID, encrypted})
334344

335-
partition.Range = &protos.PartitionRange{
336-
Range: &protos.PartitionRange_StringRange{StringRange: &protos.StringPartitionRange{}},
337-
}
345+
partition.Range = nil
346+
partition.RangeOffloaded = true
338347
}
339348

340349
if _, err := tx.CopyFrom(ctx,
@@ -351,29 +360,29 @@ func OffloadSensitivePartitionRanges(
351360
return nil
352361
}
353362

354-
// RestoreSensitivePartitionRanges hydrates sensitive partitions in-place with the decrypted
355-
// values fetched from the catalog. Used in conjunction with OffloadSensitivePartitionRanges
356-
func RestoreSensitivePartitionRanges(
363+
// RestoreOffloadedPartitionRanges hydrates offloaded partition ranges in-place with the decrypted
364+
// values fetched from the catalog. Used in conjunction with OffloadPartitionRanges.
365+
func RestoreOffloadedPartitionRanges(
357366
ctx context.Context,
358367
pool shared.CatalogPool,
359368
runUUID string,
360369
partitions []*protos.QRepPartition,
361370
) error {
362-
if !partitionsContainSensitiveRanges(partitions) {
363-
return nil
364-
}
365-
366-
partitionIDs := make([]string, 0, len(partitions))
367371
partitionByIDs := make(map[string]*protos.QRepPartition, len(partitions))
368372
for _, partition := range partitions {
369-
partitionIDs = append(partitionIDs, partition.PartitionId)
373+
if !partition.RangeOffloaded {
374+
continue
375+
}
370376
partitionByIDs[partition.PartitionId] = partition
371377
}
378+
if len(partitionByIDs) == 0 {
379+
return nil
380+
}
372381

373382
rows, err := pool.Query(ctx,
374383
`SELECT partition_uuid,enc_key_id,range_payload FROM `+qrepPartitionRangesTableName+` `+
375384
`WHERE run_uuid=$1 AND partition_uuid=ANY($2)`,
376-
runUUID, partitionIDs,
385+
runUUID, slices.Collect(maps.Keys(partitionByIDs)),
377386
)
378387
if err != nil {
379388
return fmt.Errorf("failed to query partition ranges: %w", err)
@@ -417,19 +426,6 @@ func RestoreSensitivePartitionRanges(
417426
return nil
418427
}
419428

420-
func partitionsContainSensitiveRanges(partitions []*protos.QRepPartition) bool {
421-
for _, partition := range partitions {
422-
if partition == nil || partition.Range == nil {
423-
continue
424-
}
425-
// String ranges may contain PII data like email, so deem it as sensitive.
426-
if _, ok := partition.Range.Range.(*protos.PartitionRange_StringRange); ok {
427-
return true
428-
}
429-
}
430-
return false
431-
}
432-
433429
func (p *PostgresMetadata) SyncFlowCleanup(ctx context.Context, jobName string) error {
434430
tx, err := p.pool.Begin(ctx)
435431
if err != nil {

flow/connectors/external_metadata/store_test.go

Lines changed: 79 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,13 @@ package connmetadata
22

33
import (
44
"testing"
5+
"time"
56

67
"github.com/google/uuid"
78
"github.com/jackc/pgx/v5"
89
"github.com/stretchr/testify/require"
10+
"google.golang.org/protobuf/proto"
11+
"google.golang.org/protobuf/types/known/timestamppb"
912

1013
"github.com/PeerDB-io/peerdb/flow/generated/protos"
1114
"github.com/PeerDB-io/peerdb/flow/internal"
@@ -20,56 +23,87 @@ func TestOffloadRestoreSensitivePartitionRanges(t *testing.T) {
2023
pool, err := internal.GetCatalogConnectionPoolFromEnv(ctx)
2124
require.NoError(t, err)
2225

23-
parentMirrorName := "test_offload_restore_sensitive_partition_ranges"
24-
runUUID := uuid.NewString()
25-
t.Cleanup(func() {
26-
_, _ = pool.Exec(ctx, `DELETE FROM `+qrepPartitionRangesTableName+` WHERE parent_mirror_name=$1`, parentMirrorName)
27-
})
28-
29-
p1Start, p1End, p2Start, p2End := "bar", "foo", "hello", "world"
30-
partitions := []*protos.QRepPartition{
26+
for _, tc := range []struct {
27+
name string
28+
ranges []*protos.PartitionRange
29+
}{
3130
{
32-
PartitionId: uuid.NewString(),
33-
Range: &protos.PartitionRange{Range: &protos.PartitionRange_StringRange{
34-
StringRange: &protos.StringPartitionRange{Start: p1Start, End: p1End},
35-
}},
31+
name: "int",
32+
ranges: []*protos.PartitionRange{
33+
{Range: &protos.PartitionRange_IntRange{IntRange: &protos.IntPartitionRange{Start: 1, End: 100}}},
34+
{Range: &protos.PartitionRange_IntRange{IntRange: &protos.IntPartitionRange{Start: 101, End: 200}}},
35+
},
3636
},
3737
{
38-
PartitionId: uuid.NewString(),
39-
Range: &protos.PartitionRange{Range: &protos.PartitionRange_StringRange{
40-
StringRange: &protos.StringPartitionRange{Start: p2Start, End: p2End, EndInclusive: true},
41-
}},
38+
name: "timestamp",
39+
ranges: []*protos.PartitionRange{
40+
{Range: &protos.PartitionRange_TimestampRange{TimestampRange: &protos.TimestampPartitionRange{
41+
Start: timestamppb.New(time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)),
42+
End: timestamppb.New(time.Date(2026, 6, 30, 23, 59, 59, 0, time.UTC)),
43+
}}},
44+
{Range: &protos.PartitionRange_TimestampRange{TimestampRange: &protos.TimestampPartitionRange{
45+
Start: timestamppb.New(time.Date(2026, 7, 1, 0, 0, 0, 0, time.UTC)),
46+
End: timestamppb.New(time.Date(2026, 12, 31, 23, 59, 59, 0, time.UTC)),
47+
}}},
48+
},
4249
},
43-
}
50+
{
51+
name: "string_uuid",
52+
ranges: []*protos.PartitionRange{
53+
{Range: &protos.PartitionRange_StringRange{
54+
StringRange: &protos.StringPartitionRange{Start: uuid.NewString(), End: uuid.NewString()},
55+
}},
56+
{Range: &protos.PartitionRange_StringRange{
57+
StringRange: &protos.StringPartitionRange{Start: uuid.NewString(), End: uuid.NewString(), EndInclusive: true},
58+
}},
59+
},
60+
},
61+
} {
62+
t.Run(tc.name, func(t *testing.T) {
63+
parentMirrorName := "test_offload_restore_partition_ranges_" + tc.name
64+
runUUID := uuid.NewString()
65+
t.Cleanup(func() {
66+
_, _ = pool.Exec(ctx, `DELETE FROM `+qrepPartitionRangesTableName+` WHERE parent_mirror_name=$1`, parentMirrorName)
67+
})
4468

45-
require.NoError(t, OffloadSensitivePartitionRanges(ctx, pool, parentMirrorName, runUUID, partitions))
46-
for _, p := range partitions {
47-
require.Empty(t, p.Range.GetStringRange().Start)
48-
require.Empty(t, p.Range.GetStringRange().End)
49-
}
69+
partitions := make([]*protos.QRepPartition, len(tc.ranges))
70+
expectedRanges := make([]*protos.PartitionRange, len(tc.ranges))
71+
for i, r := range tc.ranges {
72+
partitions[i] = &protos.QRepPartition{PartitionId: uuid.NewString(), Range: r}
73+
expectedRanges[i] = proto.Clone(r).(*protos.PartitionRange)
74+
}
5075

51-
rows, err := pool.Query(ctx,
52-
`SELECT enc_key_id, range_payload FROM `+qrepPartitionRangesTableName+` WHERE parent_mirror_name=$1`,
53-
parentMirrorName)
54-
require.NoError(t, err)
55-
results, err := pgx.CollectRows(rows, pgx.RowToStructByPos[struct {
56-
KeyID string
57-
Payload []byte
58-
}])
59-
require.NoError(t, err)
60-
require.Len(t, results, len(partitions))
61-
for _, res := range results {
62-
require.Equal(t, encKeyID, res.KeyID)
63-
for _, plaintext := range []string{p1Start, p1End, p2Start, p2End} {
64-
require.NotContains(t, string(res.Payload), plaintext)
65-
}
66-
}
76+
require.NoError(t, OffloadPartitionRanges(ctx, pool, parentMirrorName, runUUID, partitions))
77+
for _, p := range partitions {
78+
require.Nil(t, p.Range)
79+
require.True(t, p.RangeOffloaded)
80+
}
6781

68-
require.NoError(t, RestoreSensitivePartitionRanges(ctx, pool, runUUID, partitions))
69-
require.Equal(t, p1Start, partitions[0].Range.GetStringRange().Start)
70-
require.Equal(t, p1End, partitions[0].Range.GetStringRange().End)
71-
require.False(t, partitions[0].Range.GetStringRange().EndInclusive)
72-
require.Equal(t, p2Start, partitions[1].Range.GetStringRange().Start)
73-
require.Equal(t, p2End, partitions[1].Range.GetStringRange().End)
74-
require.True(t, partitions[1].Range.GetStringRange().EndInclusive)
82+
rows, err := pool.Query(ctx,
83+
`SELECT enc_key_id, range_payload FROM `+qrepPartitionRangesTableName+` WHERE parent_mirror_name=$1`,
84+
parentMirrorName)
85+
require.NoError(t, err)
86+
results, err := pgx.CollectRows(rows, pgx.RowToStructByPos[struct {
87+
KeyID string
88+
Payload []byte
89+
}])
90+
require.NoError(t, err)
91+
require.Len(t, results, len(partitions))
92+
for _, res := range results {
93+
require.Equal(t, encKeyID, res.KeyID)
94+
require.NotEmpty(t, res.Payload)
95+
for _, r := range expectedRanges {
96+
unencrypted, err := proto.Marshal(r)
97+
require.NoError(t, err)
98+
require.NotContains(t, string(res.Payload), string(unencrypted))
99+
}
100+
}
101+
102+
require.NoError(t, RestoreOffloadedPartitionRanges(ctx, pool, runUUID, partitions))
103+
for i, p := range partitions {
104+
require.Truef(t, proto.Equal(expectedRanges[i], p.Range),
105+
"partition %d range mismatch after restore: want %v, got %v", i, expectedRanges[i], p.Range)
106+
}
107+
})
108+
}
75109
}

flow/connectors/utils/monitoring/monitoring.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,11 @@ func addPartitionToQRepRun(ctx context.Context, tx pgx.Tx, flowJobName string,
332332
}
333333

334334
var rangeStart, rangeEnd *string
335-
if partition.Range != nil {
335+
if partition.RangeOffloaded {
336+
internal.LoggerFromCtx(ctx).Info(
337+
"omitting partition_start/partition_end from qrep_partitions because it's offloaded",
338+
slog.String("partitionId", partition.PartitionId))
339+
} else if partition.Range != nil {
336340
switch x := partition.Range.Range.(type) {
337341
case *protos.PartitionRange_IntRange:
338342
rangeStart, rangeEnd = new(strconv.FormatInt(x.IntRange.Start, 10)), new(strconv.FormatInt(x.IntRange.End, 10))
@@ -369,7 +373,7 @@ func addPartitionToQRepRun(ctx context.Context, tx pgx.Tx, flowJobName string,
369373
default:
370374
return fmt.Errorf("unknown range type: %v", x)
371375
}
372-
} else if !partition.FullTablePartition {
376+
} else if !partition.FullTablePartition && len(partition.ChildTableRanges) == 0 {
373377
internal.LoggerFromCtx(ctx).Warn("[monitoring]: partition "+partition.PartitionId+" has nil range",
374378
slog.String(string(shared.FlowNameKey), parentMirrorName))
375379
}

0 commit comments

Comments
 (0)