Skip to content

Commit ae2341e

Browse files
dtunikovclaude
andcommitted
fix(clickhouse): add opt-out for staging cleanup after QRep flow
PR #4200 (StagingStore abstraction) unified the avro upload and staging cleanup paths onto the staging store's real bucket prefix. As a side effect, CleanupQRepFlow now always deletes staging avro files once a ClickHouse snapshot/QRep flow completes. Previously cleanup keyed off config.StagingPath, which is empty for ClickHouse mirrors that don't set SnapshotStagingPath (they get their bucket from PEERDB_CLICKHOUSE_AWS_S3_BUCKET_NAME), so the prefix guard `strings.HasPrefix(stagingPath, "s3://")` was false and cleanup was a no-op. Staging avro files were effectively retained. Some deployments have downstream pipelines that consume those files, and the upgrade silently broke them by deleting the files as soon as the mirror completed. Keep cleanup as the default (it's the correct behavior and avoids unbounded staging growth), but add PEERDB_CLICKHOUSE_SKIP_STAGING_CLEANUP to opt out and retain staging artifacts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 8d99a2a commit ae2341e

2 files changed

Lines changed: 25 additions & 0 deletions

File tree

flow/connectors/clickhouse/qrep.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"log/slog"
77

88
"github.com/PeerDB-io/peerdb/flow/generated/protos"
9+
"github.com/PeerDB-io/peerdb/flow/internal"
910
"github.com/PeerDB-io/peerdb/flow/model"
1011
"github.com/PeerDB-io/peerdb/flow/shared"
1112
)
@@ -44,6 +45,17 @@ func (c *ClickHouseConnector) ConsolidateQRepPartitions(_ context.Context, confi
4445
// CleanupQRepFlow function for clickhouse connector
4546
func (c *ClickHouseConnector) CleanupQRepFlow(ctx context.Context, config *protos.QRepConfig) error {
4647
flowName := config.FlowJobName
48+
49+
skipCleanup, err := internal.PeerDBClickHouseSkipStagingCleanup(ctx, config.Env)
50+
if err != nil {
51+
return fmt.Errorf("failed to read staging cleanup config: %w", err)
52+
}
53+
if skipCleanup {
54+
c.logger.Info("Skipping staging cleanup after QRepFlow (PEERDB_CLICKHOUSE_SKIP_STAGING_CLEANUP enabled)",
55+
slog.String("stagingPath", c.staging.BucketPath()), slog.String("flowName", flowName))
56+
return nil
57+
}
58+
4759
c.logger.Info("Cleaning up stage after QRepFlow",
4860
slog.String("stagingPath", c.staging.BucketPath()), slog.String("flowName", flowName))
4961

flow/internal/dynamicconf.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,15 @@ var DynamicSettings = [...]*protos.DynamicSetting{
198198
ApplyMode: protos.DynconfApplyMode_APPLY_MODE_IMMEDIATE,
199199
TargetForSetting: protos.DynconfTarget_CLICKHOUSE,
200200
},
201+
{
202+
Name: "PEERDB_CLICKHOUSE_SKIP_STAGING_CLEANUP",
203+
Description: "Skip deleting ClickHouse staging avro files after a QRep/snapshot flow completes. " +
204+
"Enable to retain staging artifacts for downstream pipelines that consume them",
205+
DefaultValue: "false",
206+
ValueType: protos.DynconfValueType_BOOL,
207+
ApplyMode: protos.DynconfApplyMode_APPLY_MODE_IMMEDIATE,
208+
TargetForSetting: protos.DynconfTarget_CLICKHOUSE,
209+
},
201210
{
202211
Name: "PEERDB_S3_UUID_PREFIX",
203212
Description: "Use random UUID as prefix instead of flow name, can help partitioning on non-AWS based s3 providers",
@@ -758,6 +767,10 @@ func PeerDBClickHouseStagingBucketName(ctx context.Context, env map[string]strin
758767
return dynLookup(ctx, env, "PEERDB_CLICKHOUSE_STAGING_BUCKET_NAME")
759768
}
760769

770+
func PeerDBClickHouseSkipStagingCleanup(ctx context.Context, env map[string]string) (bool, error) {
771+
return dynamicConfBool(ctx, env, "PEERDB_CLICKHOUSE_SKIP_STAGING_CLEANUP")
772+
}
773+
761774
func PeerDBClickHouseAWSS3BucketName(ctx context.Context, env map[string]string) (string, error) {
762775
return dynLookup(ctx, env, "PEERDB_CLICKHOUSE_AWS_S3_BUCKET_NAME")
763776
}

0 commit comments

Comments
 (0)