From 84e39007d29544457605870d9b7c2a4cd61ffc1c Mon Sep 17 00:00:00 2001 From: Ling Jin <7138436+3AceShowHand@users.noreply.github.com> Date: Mon, 20 Jul 2026 20:14:11 +0800 Subject: [PATCH 1/6] This is an automated cherry-pick of #5617 Signed-off-by: ti-chi-bot --- cmd/kafka-consumer/consumer.go | 18 ++++++ downstreamadapter/sink/kafka/helper.go | 9 ++- downstreamadapter/sink/kafka/sink.go | 77 +++++++++++++++++++++++++- pkg/sink/codec/builder.go | 3 +- pkg/sink/kafka/factory.go | 4 -- 5 files changed, 99 insertions(+), 12 deletions(-) diff --git a/cmd/kafka-consumer/consumer.go b/cmd/kafka-consumer/consumer.go index 4e78582f9e..eaf954a99a 100644 --- a/cmd/kafka-consumer/consumer.go +++ b/cmd/kafka-consumer/consumer.go @@ -51,7 +51,25 @@ func getPartitionNum(o *option) (int32, error) { timeout += 100 continue } +<<<<<<< HEAD return 0, errors.Trace(err) +======= + + topicDetail, ok := resp.Topics[topic] + if ok && topicDetail.Error.Code() == kafka.ErrNoError { + numPartitions := int32(len(topicDetail.Partitions)) + log.Info("get partition number of topic", + zap.String("topic", topic), + zap.Int32("partitionNum", numPartitions)) + if numPartitions > maxPartitionNum { + maxPartitionNum = numPartitions + } + found = true + break + } + log.Info("retry get partition number", zap.String("topic", topic)) + time.Sleep(1 * time.Second) +>>>>>>> 2667ed8c2 (kafka: make the verify lightweight (#5617)) } if topicDetail, ok := resp.Topics[o.topic]; ok { numPartitions := int32(len(topicDetail.Partitions)) diff --git a/downstreamadapter/sink/kafka/helper.go b/downstreamadapter/sink/kafka/helper.go index c45b7eb10f..4a71000ac0 100644 --- a/downstreamadapter/sink/kafka/helper.go +++ b/downstreamadapter/sink/kafka/helper.go @@ -49,11 +49,11 @@ func (c components) close() { } } -func newKafkaSinkComponentWithFactory(ctx context.Context, +func newKafkaSinkComponent( + ctx context.Context, changefeedID commonType.ChangeFeedID, sinkURI *url.URL, sinkConfig *config.SinkConfig, - factoryCreator kafka.FactoryCreator, ) (components, config.Protocol, error) { kafkaComponent := components{} protocol, err := helper.GetProtocol(utils.GetOrZero(sinkConfig.Protocol)) @@ -72,7 +72,7 @@ func newKafkaSinkComponentWithFactory(ctx context.Context, } options.Topic = topic - kafkaComponent.factory, err = factoryCreator(ctx, options, changefeedID) + kafkaComponent.factory, err = kafka.NewSaramaFactory(ctx, options, changefeedID) if err != nil { return kafkaComponent, protocol, errors.WrapError(errors.ErrKafkaNewProducer, err) } @@ -128,6 +128,7 @@ func newKafkaSinkComponentWithFactory(ctx context.Context, } return kafkaComponent, protocol, nil } +<<<<<<< HEAD func newKafkaSinkComponent( ctx context.Context, @@ -146,3 +147,5 @@ func newKafkaSinkComponentForTest( ) (components, config.Protocol, error) { return newKafkaSinkComponentWithFactory(ctx, changefeedID, sinkURI, sinkConfig, kafka.NewMockFactory) } +======= +>>>>>>> 2667ed8c2 (kafka: make the verify lightweight (#5617)) diff --git a/downstreamadapter/sink/kafka/sink.go b/downstreamadapter/sink/kafka/sink.go index e6d1038b26..ab70c076e0 100644 --- a/downstreamadapter/sink/kafka/sink.go +++ b/downstreamadapter/sink/kafka/sink.go @@ -19,12 +19,15 @@ import ( "time" "github.com/pingcap/log" + "github.com/pingcap/ticdc/downstreamadapter/sink/columnselector" + "github.com/pingcap/ticdc/downstreamadapter/sink/eventrouter" "github.com/pingcap/ticdc/downstreamadapter/sink/helper" commonType "github.com/pingcap/ticdc/pkg/common" commonEvent "github.com/pingcap/ticdc/pkg/common/event" "github.com/pingcap/ticdc/pkg/config" "github.com/pingcap/ticdc/pkg/errors" "github.com/pingcap/ticdc/pkg/metrics" + "github.com/pingcap/ticdc/pkg/sink/codec" "github.com/pingcap/ticdc/pkg/sink/codec/common" "github.com/pingcap/ticdc/pkg/sink/kafka" "github.com/pingcap/ticdc/pkg/util" @@ -68,9 +71,77 @@ func (s *sink) SinkType() commonType.SinkType { } func Verify(ctx context.Context, changefeedID commonType.ChangeFeedID, uri *url.URL, sinkConfig *config.SinkConfig) error { - comp, _, err := newKafkaSinkComponent(ctx, changefeedID, uri, sinkConfig) - defer comp.close() - return err + protocol, err := helper.GetProtocol(util.GetOrZero(sinkConfig.Protocol)) + if err != nil { + return errors.Trace(err) + } + + topic, err := helper.GetTopic(uri) + if err != nil { + return errors.Trace(err) + } + + options := kafka.NewOptions() + if err = options.Apply(changefeedID, uri, sinkConfig); err != nil { + return errors.WrapError(errors.ErrKafkaInvalidConfig, err) + } + options.Topic = topic + + encoderConfig, err := helper.GetEncoderConfig(changefeedID, uri, protocol, sinkConfig, options.MaxMessageBytes) + if err != nil { + return errors.Trace(err) + } + + isAvroLike := protocol == config.ProtocolAvro || protocol == config.ProtocolDebeziumAvro + if _, err = eventrouter.NewEventRouter(sinkConfig, topic, false, isAvroLike); err != nil { + return errors.Trace(err) + } + + if _, err = columnselector.New(sinkConfig); err != nil { + return errors.Trace(err) + } + + factory, err := kafka.NewSaramaFactory(ctx, options, changefeedID) + if err != nil { + return errors.WrapError(errors.ErrKafkaNewProducer, err) + } + + adminClient, err := factory.AdminClient(ctx) + if err != nil { + return errors.WrapError(errors.ErrKafkaNewProducer, err) + } + defer adminClient.Close() + + topics, err := adminClient.GetTopicsMeta([]string{topic}, false) + if err != nil { + return errors.Trace(err) + } + if _, exists := topics[topic]; exists { + return nil + } + + topicConfig := options.DeriveTopicConfig() + if !topicConfig.AutoCreate { + return errors.ErrKafkaInvalidConfig.GenWithStack("`auto-create-topic` is false, and %s not found", topic) + } + + // the topic is not created, only validate. + err = adminClient.CreateTopic(&kafka.TopicDetail{ + Name: topic, + NumPartitions: topicConfig.PartitionNum, + ReplicationFactor: topicConfig.ReplicationFactor, + }, true) + if err != nil { + return errors.WrapError(errors.ErrKafkaCreateTopic, err) + } + + encoder, err := codec.NewEventEncoder(ctx, encoderConfig) + if err != nil { + return errors.Trace(err) + } + encoder.Clean() + + return nil } func New( diff --git a/pkg/sink/codec/builder.go b/pkg/sink/codec/builder.go index 8a17146921..d0f44b5342 100644 --- a/pkg/sink/codec/builder.go +++ b/pkg/sink/codec/builder.go @@ -20,7 +20,6 @@ import ( "github.com/pingcap/log" "github.com/pingcap/ticdc/pkg/config" "github.com/pingcap/ticdc/pkg/errors" - cerror "github.com/pingcap/ticdc/pkg/errors" "github.com/pingcap/ticdc/pkg/sink/codec/avro" "github.com/pingcap/ticdc/pkg/sink/codec/canal" "github.com/pingcap/ticdc/pkg/sink/codec/common" @@ -60,7 +59,7 @@ func NewEventDecoder( case config.ProtocolAvro: schemaM, err := avro.NewConfluentSchemaManager(ctx, codecConfig.AvroConfluentSchemaRegistry, nil) if err != nil { - return nil, cerror.Trace(err) + return nil, errors.Trace(err) } return avro.NewDecoder(codecConfig, idx, schemaM, topic, upstreamTiDB), nil case config.ProtocolSimple: diff --git a/pkg/sink/kafka/factory.go b/pkg/sink/kafka/factory.go index 72a458a508..14d83b390c 100644 --- a/pkg/sink/kafka/factory.go +++ b/pkg/sink/kafka/factory.go @@ -16,7 +16,6 @@ package kafka import ( "context" - commonType "github.com/pingcap/ticdc/pkg/common" "github.com/pingcap/ticdc/pkg/sink/codec/common" ) @@ -32,9 +31,6 @@ type Factory interface { MetricsCollector(adminClient ClusterAdminClient) MetricsCollector } -// FactoryCreator defines the type of factory creator. -type FactoryCreator func(context.Context, *options, commonType.ChangeFeedID) (Factory, error) - // SyncProducer is the kafka sync producer type SyncProducer interface { // SendMessage produces a given message, and returns only when it either has From 45dfe1607fe4b8ffc65648d5731de136edd86a70 Mon Sep 17 00:00:00 2001 From: Ling Jin <7138436+3AceShowHand@users.noreply.github.com> Date: Fri, 24 Jul 2026 23:17:26 +0800 Subject: [PATCH 2/6] This is an automated cherry-pick of #5718 Signed-off-by: ti-chi-bot --- downstreamadapter/sink/kafka/helper.go | 73 +++++++----- downstreamadapter/sink/kafka/sink.go | 104 +++++++++++++++-- downstreamadapter/sink/kafka/sink_test.go | 75 ++++++++++++ downstreamadapter/sink/pulsar/helper.go | 4 +- pkg/sink/codec/avro/arvo.go | 2 - pkg/sink/codec/bootstraper.go | 1 - pkg/sink/codec/builder.go | 9 +- pkg/sink/codec/canal/canal_json_encoder.go | 12 +- .../codec/canal/canal_json_encoder_test.go | 50 ++++---- pkg/sink/codec/canal/canal_json_test.go | 44 +++---- pkg/sink/codec/common/encoder.go | 2 - pkg/sink/codec/debezium/encoder.go | 2 - pkg/sink/codec/encoder_group.go | 11 +- pkg/sink/codec/open/encoder.go | 12 +- pkg/sink/codec/open/encoder_test.go | 108 +++++++++++++----- pkg/sink/codec/simple/encoder.go | 13 +-- pkg/sink/codec/simple/encoder_test.go | 53 ++++++--- pkg/sink/kafka/claimcheck/claim_check.go | 22 ++-- pkg/sink/kafka/claimcheck/claim_check_test.go | 96 ++++++++++++++++ 19 files changed, 500 insertions(+), 193 deletions(-) create mode 100644 pkg/sink/kafka/claimcheck/claim_check_test.go diff --git a/downstreamadapter/sink/kafka/helper.go b/downstreamadapter/sink/kafka/helper.go index c45b7eb10f..df2d61cf1d 100644 --- a/downstreamadapter/sink/kafka/helper.go +++ b/downstreamadapter/sink/kafka/helper.go @@ -27,6 +27,7 @@ import ( "github.com/pingcap/ticdc/pkg/sink/codec" "github.com/pingcap/ticdc/pkg/sink/codec/common" "github.com/pingcap/ticdc/pkg/sink/kafka" + "github.com/pingcap/ticdc/pkg/sink/kafka/claimcheck" "github.com/pingcap/tidb/br/pkg/utils" ) @@ -38,6 +39,7 @@ type components struct { topicManager topicmanager.TopicManager adminClient kafka.ClusterAdminClient factory kafka.Factory + claimCheck *claimcheck.ClaimCheck } func (c components) close() { @@ -47,6 +49,9 @@ func (c components) close() { if c.topicManager != nil { c.topicManager.Close() } + if c.claimCheck != nil { + c.claimCheck.Close() + } } func newKafkaSinkComponentWithFactory(ctx context.Context, @@ -55,78 +60,94 @@ func newKafkaSinkComponentWithFactory(ctx context.Context, sinkConfig *config.SinkConfig, factoryCreator kafka.FactoryCreator, ) (components, config.Protocol, error) { - kafkaComponent := components{} + var ( + comp components + err error + ) + // must release resources when error occurs. + defer func() { + if err != nil { + comp.close() + } + }() protocol, err := helper.GetProtocol(utils.GetOrZero(sinkConfig.Protocol)) if err != nil { - return kafkaComponent, config.ProtocolUnknown, errors.Trace(err) + return comp, config.ProtocolUnknown, errors.Trace(err) } topic, err := helper.GetTopic(sinkURI) if err != nil { - return kafkaComponent, protocol, errors.Trace(err) + return comp, protocol, errors.Trace(err) } options := kafka.NewOptions() if err = options.Apply(changefeedID, sinkURI, sinkConfig); err != nil { - return kafkaComponent, protocol, errors.WrapError(errors.ErrKafkaInvalidConfig, err) + return comp, protocol, errors.WrapError(errors.ErrKafkaInvalidConfig, err) } options.Topic = topic +<<<<<<< HEAD kafkaComponent.factory, err = factoryCreator(ctx, options, changefeedID) +======= + comp.factory, err = kafka.NewSaramaFactory(ctx, options, changefeedID) +>>>>>>> bc474b549 (kafka: share one claimcheck instance across encoders (#5718)) if err != nil { - return kafkaComponent, protocol, errors.WrapError(errors.ErrKafkaNewProducer, err) + return comp, protocol, errors.WrapError(errors.ErrKafkaNewProducer, err) } +<<<<<<< HEAD kafkaComponent.eventRouter, err = eventrouter.NewEventRouter( sinkConfig, topic, false, protocol == config.ProtocolAvro) +======= + isAvroLike := protocol == config.ProtocolAvro || protocol == config.ProtocolDebeziumAvro + comp.eventRouter, err = eventrouter.NewEventRouter( + sinkConfig, topic, false, isAvroLike) +>>>>>>> bc474b549 (kafka: share one claimcheck instance across encoders (#5718)) if err != nil { - return kafkaComponent, protocol, errors.Trace(err) + return comp, protocol, errors.Trace(err) } - kafkaComponent.columnSelector, err = columnselector.New(sinkConfig) + comp.columnSelector, err = columnselector.New(sinkConfig) if err != nil { - return kafkaComponent, protocol, errors.Trace(err) + return comp, protocol, errors.Trace(err) } encoderConfig, err := helper.GetEncoderConfig(changefeedID, sinkURI, protocol, sinkConfig, options.MaxMessageBytes) if err != nil { - return kafkaComponent, protocol, errors.Trace(err) + return comp, protocol, errors.Trace(err) } - kafkaComponent.encoderGroup, err = codec.NewEncoderGroup(ctx, sinkConfig, encoderConfig, changefeedID) + comp.claimCheck, err = claimcheck.New(ctx, encoderConfig.LargeMessageHandle, changefeedID) if err != nil { - return kafkaComponent, protocol, errors.Trace(err) + return comp, protocol, errors.Trace(err) } - kafkaComponent.encoder, err = codec.NewEventEncoder(ctx, encoderConfig) + comp.encoderGroup, err = codec.NewEncoderGroup(ctx, sinkConfig, encoderConfig, comp.claimCheck, changefeedID) if err != nil { - return kafkaComponent, protocol, errors.Trace(err) + return comp, protocol, errors.Trace(err) } - kafkaComponent.adminClient, err = kafkaComponent.factory.AdminClient(ctx) + comp.encoder, err = codec.NewEventEncoder(ctx, encoderConfig, comp.claimCheck) if err != nil { - return kafkaComponent, protocol, errors.WrapError(errors.ErrKafkaNewProducer, err) + return comp, protocol, errors.Trace(err) } - // We must close adminClient when this func return cause by an error - // otherwise the adminClient will never be closed and lead to a goroutine leak. - defer func() { - if err != nil && kafkaComponent.adminClient != nil { - kafkaComponent.adminClient.Close() - } - }() + comp.adminClient, err = comp.factory.AdminClient(ctx) + if err != nil { + return comp, protocol, errors.WrapError(errors.ErrKafkaNewProducer, err) + } - kafkaComponent.topicManager, err = topicmanager.GetTopicManagerAndTryCreateTopic( + comp.topicManager, err = topicmanager.GetTopicManagerAndTryCreateTopic( ctx, changefeedID, topic, options.DeriveTopicConfig(), - kafkaComponent.adminClient, + comp.adminClient, ) if err != nil { - return kafkaComponent, protocol, errors.Trace(err) + return comp, protocol, errors.Trace(err) } - return kafkaComponent, protocol, nil + return comp, protocol, nil } func newKafkaSinkComponent( diff --git a/downstreamadapter/sink/kafka/sink.go b/downstreamadapter/sink/kafka/sink.go index e6d1038b26..c68bba0f30 100644 --- a/downstreamadapter/sink/kafka/sink.go +++ b/downstreamadapter/sink/kafka/sink.go @@ -27,6 +27,7 @@ import ( "github.com/pingcap/ticdc/pkg/metrics" "github.com/pingcap/ticdc/pkg/sink/codec/common" "github.com/pingcap/ticdc/pkg/sink/kafka" + "github.com/pingcap/ticdc/pkg/sink/kafka/claimcheck" "github.com/pingcap/ticdc/pkg/util" "github.com/pingcap/ticdc/utils/chann" "go.uber.org/atomic" @@ -68,9 +69,90 @@ func (s *sink) SinkType() commonType.SinkType { } func Verify(ctx context.Context, changefeedID commonType.ChangeFeedID, uri *url.URL, sinkConfig *config.SinkConfig) error { +<<<<<<< HEAD comp, _, err := newKafkaSinkComponent(ctx, changefeedID, uri, sinkConfig) defer comp.close() return err +======= + protocol, err := helper.GetProtocol(util.GetOrZero(sinkConfig.Protocol)) + if err != nil { + return errors.Trace(err) + } + + topic, err := helper.GetTopic(uri) + if err != nil { + return errors.Trace(err) + } + + options := kafka.NewOptions() + if err = options.Apply(changefeedID, uri, sinkConfig); err != nil { + return errors.WrapError(errors.ErrKafkaInvalidConfig, err) + } + options.Topic = topic + + encoderConfig, err := helper.GetEncoderConfig( + changefeedID, uri, protocol, sinkConfig, + options.MaxMessageBytes, options.MaxBatchedBytes, + ) + if err != nil { + return errors.Trace(err) + } + + claimCheck, err := claimcheck.New(ctx, encoderConfig.LargeMessageHandle, changefeedID) + if err != nil { + return err + } + defer claimCheck.Close() + + isAvroLike := protocol == config.ProtocolAvro || protocol == config.ProtocolDebeziumAvro + if _, err = eventrouter.NewEventRouter(sinkConfig, topic, false, isAvroLike); err != nil { + return errors.Trace(err) + } + + if _, err = columnselector.New(sinkConfig); err != nil { + return errors.Trace(err) + } + + factory, err := kafka.NewSaramaFactory(ctx, options, changefeedID) + if err != nil { + return errors.WrapError(errors.ErrKafkaNewProducer, err) + } + + adminClient, err := factory.AdminClient(ctx) + if err != nil { + return errors.WrapError(errors.ErrKafkaNewProducer, err) + } + defer adminClient.Close() + + topics, err := adminClient.GetTopicsMeta([]string{topic}, false) + if err != nil { + return errors.Trace(err) + } + if _, exists := topics[topic]; exists { + return nil + } + + topicConfig := options.DeriveTopicConfig() + if !topicConfig.AutoCreate { + return errors.ErrKafkaInvalidConfig.GenWithStack("`auto-create-topic` is false, and %s not found", topic) + } + + // the topic is not created, only validate. + err = adminClient.CreateTopic(&kafka.TopicDetail{ + Name: topic, + NumPartitions: topicConfig.PartitionNum, + ReplicationFactor: topicConfig.ReplicationFactor, + }, true) + if err != nil { + return errors.WrapError(errors.ErrKafkaCreateTopic, err) + } + + _, err = codec.NewEventEncoder(ctx, encoderConfig, claimCheck) + if err != nil { + return errors.Trace(err) + } + return nil +>>>>>>> bc474b549 (kafka: share one claimcheck instance across encoders (#5718)) } func New( @@ -89,24 +171,30 @@ func newWithComponents( protocol config.Protocol, comp components, ) (*sink, error) { + statistics := metrics.NewStatistics(changefeedID, keyspaceID, "sink") var ( err error asyncProducer kafka.AsyncProducer syncProducer kafka.SyncProducer ) defer func() { - if err != nil { - if syncProducer != nil { - syncProducer.Close() - } - if asyncProducer != nil { - asyncProducer.Close() - } - comp.close() + if err == nil { + return + } + if syncProducer != nil { + syncProducer.Close() + } + if asyncProducer != nil { + asyncProducer.Close() } + comp.close() + statistics.Close() }() +<<<<<<< HEAD statistics := metrics.NewStatistics(changefeedID, "sink") +======= +>>>>>>> bc474b549 (kafka: share one claimcheck instance across encoders (#5718)) asyncProducer, err = comp.factory.AsyncProducer(ctx) if err != nil { return nil, err diff --git a/downstreamadapter/sink/kafka/sink_test.go b/downstreamadapter/sink/kafka/sink_test.go index 0bb4708f58..5799845d2d 100644 --- a/downstreamadapter/sink/kafka/sink_test.go +++ b/downstreamadapter/sink/kafka/sink_test.go @@ -51,7 +51,82 @@ func newKafkaSinkForTestWithProducers(ctx context.Context, statistics := metrics.NewStatistics(changefeedID, "sink") comp, protocol, err := newKafkaSinkComponentForTest(ctx, changefeedID, sinkURI, sinkConfig) if err != nil { +<<<<<<< HEAD return nil, errors.Trace(err) +======= + return nil, err + } + topic, err := helper.GetTopic(sinkURI) + if err != nil { + return nil, err + } + options := kafka.NewOptions() + if err = options.Apply(changefeedID, sinkURI, sinkConfig); err != nil { + return nil, err + } + options.Topic = topic + + adminClient := kafka.NewMockClusterAdminClient(ctrl) + adminClient.EXPECT().GetTopicsMeta([]string{kafkaSinkTestTopic}, true).Return( + map[string]kafka.TopicDetail{ + kafkaSinkTestTopic: { + Name: kafkaSinkTestTopic, + NumPartitions: 1, + }, + }, nil) + adminClient.EXPECT().Close().AnyTimes() + + metricsCollector := kafka.NewMockMetricsCollector(ctrl) + metricsCollector.EXPECT().Run(gomock.Any()).AnyTimes() + + factory := kafka.NewMockFactory(ctrl) + factory.EXPECT().AsyncProducer(gomock.Any()).Return(asyncProducer, nil) + factory.EXPECT().SyncProducer(gomock.Any()).Return(syncProducer, nil) + factory.EXPECT().MetricsCollector(adminClient).Return(metricsCollector) + + eventRouter, err := eventrouter.NewEventRouter(sinkConfig, topic, false, false) + if err != nil { + return nil, err + } + columnSelector, err := columnselector.New(sinkConfig) + if err != nil { + return nil, err + } + encoderConfig, err := helper.GetEncoderConfig( + changefeedID, sinkURI, protocol, sinkConfig, + options.MaxMessageBytes, options.MaxBatchedBytes, + ) + if err != nil { + return nil, err + } + encoderGroup, err := codec.NewEncoderGroup(ctx, sinkConfig, encoderConfig, nil, changefeedID) + if err != nil { + return nil, err + } + encoder, err := codec.NewEventEncoder(ctx, encoderConfig, nil) + if err != nil { + return nil, err + } + topicManager, err := topicmanager.GetTopicManagerAndTryCreateTopic( + ctx, + changefeedID, + topic, + options.DeriveTopicConfig(), + adminClient, + ) + if err != nil { + return nil, err + } + + comp := components{ + encoderGroup: encoderGroup, + encoder: encoder, + columnSelector: columnSelector, + eventRouter: eventRouter, + topicManager: topicManager, + adminClient: adminClient, + factory: factory, +>>>>>>> bc474b549 (kafka: share one claimcheck instance across encoders (#5718)) } // We must close adminClient when this func return cause by an error diff --git a/downstreamadapter/sink/pulsar/helper.go b/downstreamadapter/sink/pulsar/helper.go index acc1cddd38..fb7e5ca642 100644 --- a/downstreamadapter/sink/pulsar/helper.go +++ b/downstreamadapter/sink/pulsar/helper.go @@ -127,12 +127,12 @@ func newPulsarSinkComponentWithFactory(ctx context.Context, return pulsarComponent, protocol, errors.Trace(err) } - pulsarComponent.encoderGroup, err = codec.NewEncoderGroup(ctx, sinkConfig, encoderConfig, changefeedID) + pulsarComponent.encoderGroup, err = codec.NewEncoderGroup(ctx, sinkConfig, encoderConfig, nil, changefeedID) if err != nil { return pulsarComponent, protocol, errors.Trace(err) } - pulsarComponent.encoder, err = codec.NewEventEncoder(ctx, encoderConfig) + pulsarComponent.encoder, err = codec.NewEventEncoder(ctx, encoderConfig, nil) if err != nil { return pulsarComponent, protocol, errors.Trace(err) } diff --git a/pkg/sink/codec/avro/arvo.go b/pkg/sink/codec/avro/arvo.go index 515de064d9..cd15233bc8 100644 --- a/pkg/sink/codec/avro/arvo.go +++ b/pkg/sink/codec/avro/arvo.go @@ -698,8 +698,6 @@ func (a *BatchEncoder) columnToAvroData( } } -func (a *BatchEncoder) Clean() {} - type avroEncodeResult struct { data []byte // header is the message header, it will be encoder into the head diff --git a/pkg/sink/codec/bootstraper.go b/pkg/sink/codec/bootstraper.go index a57da8360f..9bff4368a8 100644 --- a/pkg/sink/codec/bootstraper.go +++ b/pkg/sink/codec/bootstraper.go @@ -79,7 +79,6 @@ func (b *bootstrapWorker) run(ctx context.Context) error { sendTicker := time.NewTicker(bootstrapWorkerTickerInterval) gcTicker := time.NewTicker(bootstrapWorkerGCInterval) defer func() { - b.rowEventEncoder.Clean() gcTicker.Stop() sendTicker.Stop() }() diff --git a/pkg/sink/codec/builder.go b/pkg/sink/codec/builder.go index 8a17146921..5bcb591232 100644 --- a/pkg/sink/codec/builder.go +++ b/pkg/sink/codec/builder.go @@ -28,21 +28,22 @@ import ( "github.com/pingcap/ticdc/pkg/sink/codec/debezium" "github.com/pingcap/ticdc/pkg/sink/codec/open" "github.com/pingcap/ticdc/pkg/sink/codec/simple" + "github.com/pingcap/ticdc/pkg/sink/kafka/claimcheck" "go.uber.org/zap" ) -func NewEventEncoder(ctx context.Context, cfg *common.Config) (common.EventEncoder, error) { +func NewEventEncoder(ctx context.Context, cfg *common.Config, claimCheck *claimcheck.ClaimCheck) (common.EventEncoder, error) { switch cfg.Protocol { case config.ProtocolDefault, config.ProtocolOpen: - return open.NewBatchEncoder(ctx, cfg) + return open.NewBatchEncoder(cfg, claimCheck) case config.ProtocolAvro: return avro.NewAvroEncoder(ctx, cfg) case config.ProtocolCanalJSON: - return canal.NewJSONRowEventEncoder(ctx, cfg) + return canal.NewJSONRowEventEncoder(cfg, claimCheck) case config.ProtocolDebezium: return debezium.NewBatchEncoder(cfg, config.GetGlobalServerConfig().ClusterID), nil case config.ProtocolSimple: - return simple.NewEncoder(ctx, cfg) + return simple.NewEncoder(cfg, claimCheck) default: return nil, errors.ErrSinkUnknownProtocol.GenWithStackByArgs(cfg.Protocol) } diff --git a/pkg/sink/codec/canal/canal_json_encoder.go b/pkg/sink/codec/canal/canal_json_encoder.go index 7425a666ef..dc076f3ee0 100644 --- a/pkg/sink/codec/canal/canal_json_encoder.go +++ b/pkg/sink/codec/canal/canal_json_encoder.go @@ -373,11 +373,7 @@ type JSONRowEventEncoder struct { } // NewJSONRowEventEncoder creates a new JSONRowEventEncoder -func NewJSONRowEventEncoder(ctx context.Context, config *common.Config) (common.EventEncoder, error) { - claimCheck, err := claimcheck.New(ctx, config.LargeMessageHandle, config.ChangefeedID) - if err != nil { - return nil, err - } +func NewJSONRowEventEncoder(config *common.Config, claimCheck *claimcheck.ClaimCheck) (common.EventEncoder, error) { return &JSONRowEventEncoder{ messages: make([]*common.Message, 0, 1), config: config, @@ -582,9 +578,3 @@ func (c *JSONRowEventEncoder) EncodeDDLEvent(e *commonEvent.DDLEvent) (*common.M return common.NewMsg(nil, value), nil } - -func (c *JSONRowEventEncoder) Clean() { - if c.claimCheck != nil { - c.claimCheck.CleanMetrics() - } -} diff --git a/pkg/sink/codec/canal/canal_json_encoder_test.go b/pkg/sink/codec/canal/canal_json_encoder_test.go index 2953642f89..91e9f297f7 100644 --- a/pkg/sink/codec/canal/canal_json_encoder_test.go +++ b/pkg/sink/codec/canal/canal_json_encoder_test.go @@ -25,6 +25,7 @@ import ( "github.com/pingcap/ticdc/pkg/config" "github.com/pingcap/ticdc/pkg/errors" "github.com/pingcap/ticdc/pkg/sink/codec/common" + "github.com/pingcap/ticdc/pkg/sink/kafka/claimcheck" "github.com/stretchr/testify/require" ) @@ -47,7 +48,7 @@ func TestDMLE2E(t *testing.T) { codecConfig := common.NewConfig(config.ProtocolCanalJSON) for _, enableTiDBExtension := range []bool{false, true} { codecConfig.EnableTiDBExtension = enableTiDBExtension - encIface, err := NewJSONRowEventEncoder(ctx, codecConfig) + encIface, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) encoder := encIface.(*JSONRowEventEncoder) @@ -131,7 +132,7 @@ func TestCanalJSONCompressionE2E(t *testing.T) { codecConfig.LargeMessageHandle.LargeMessageHandleCompression = compression.LZ4 ctx := context.Background() - encIface, err := NewJSONRowEventEncoder(ctx, codecConfig) + encIface, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) encoder := encIface.(*JSONRowEventEncoder) @@ -208,7 +209,7 @@ func TestEncodeRoutedDMLEventUsesTargetNames(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolCanalJSON) - encIface, err := NewJSONRowEventEncoder(ctx, codecConfig) + encIface, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) encoder := encIface.(*JSONRowEventEncoder) @@ -237,7 +238,7 @@ func TestEncodeRoutedDMLEventUsesTargetNames(t *testing.T) { func TestEncodeRoutedDDLEventUsesTargetNames(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolCanalJSON) - encIface, err := NewJSONRowEventEncoder(ctx, codecConfig) + encIface, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) encoder := encIface.(*JSONRowEventEncoder) @@ -269,8 +270,11 @@ func TestCanalJSONClaimCheckE2E(t *testing.T) { for _, rawValue := range []bool{false, true} { codecConfig.LargeMessageHandle.ClaimCheckRawValue = rawValue + claimCheck, err := claimcheck.New(ctx, codecConfig.LargeMessageHandle, codecConfig.ChangefeedID) + require.NoError(t, err) + t.Cleanup(claimCheck.Close) - encIface, err := NewJSONRowEventEncoder(ctx, codecConfig) + encIface, err := NewJSONRowEventEncoder(codecConfig, claimCheck) require.NoError(t, err) encoder := encIface.(*JSONRowEventEncoder) @@ -315,9 +319,7 @@ func TestNewCanalJSONMessageHandleKeyOnly4LargeMessage(t *testing.T) { codecConfig.LargeMessageHandle.LargeMessageHandleCompression = compression.LZ4 codecConfig.MaxMessageBytes = 500 - ctx := context.Background() - - encIface, err := NewJSONRowEventEncoder(ctx, codecConfig) + encIface, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) encoder := encIface.(*JSONRowEventEncoder) @@ -360,9 +362,8 @@ func TestNewCanalJSONMessageFromDDL(t *testing.T) { defer helper.Close() codecConfig := common.NewConfig(config.ProtocolCanalJSON) - ctx := context.Background() - encIface, err := NewJSONRowEventEncoder(ctx, codecConfig) + encIface, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) encoder := encIface.(*JSONRowEventEncoder) @@ -382,7 +383,7 @@ func TestNewCanalJSONMessageFromDDL(t *testing.T) { require.Equal(t, "CREATE", msg.EventType) codecConfig.EnableTiDBExtension = true - encIface, err = NewJSONRowEventEncoder(ctx, codecConfig) + encIface, err = NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) encoder = encIface.(*JSONRowEventEncoder) @@ -397,9 +398,8 @@ func TestNewCanalJSONMessageFromDDL(t *testing.T) { } func TestBatching(t *testing.T) { - ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolCanalJSON) - encIface, err := NewJSONRowEventEncoder(ctx, codecConfig) + encIface, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) encoder := encIface.(*JSONRowEventEncoder) require.NotNil(t, encoder) @@ -434,13 +434,12 @@ func TestBatching(t *testing.T) { func TestEncodeCheckpointEvent(t *testing.T) { t.Parallel() - ctx := context.Background() var watermark uint64 = 2333 for _, enable := range []bool{false, true} { codecConfig := common.NewConfig(config.ProtocolCanalJSON) codecConfig.EnableTiDBExtension = enable - encoder, err := NewJSONRowEventEncoder(ctx, codecConfig) + encoder, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) msg, err := encoder.EncodeCheckpointEvent(watermark) @@ -482,9 +481,7 @@ func TestCheckpointEventValueMarshal(t *testing.T) { codecConfig := common.NewConfig(config.ProtocolCanalJSON) codecConfig.EnableTiDBExtension = true - ctx := context.Background() - - encoder, err := NewJSONRowEventEncoder(ctx, codecConfig) + encoder, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) var watermark uint64 = 1024 @@ -519,7 +516,7 @@ func TestDDLEventWithExtension(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolCanalJSON) codecConfig.EnableTiDBExtension = true - encoder, err := NewJSONRowEventEncoder(ctx, codecConfig) + encoder, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) require.NotNil(t, encoder) @@ -561,9 +558,8 @@ func TestCanalJSONAppendRowChangedEventWithCallback(t *testing.T) { codecConfig := common.NewConfig(config.ProtocolCanalJSON) codecConfig.EnableTiDBExtension = true - ctx := context.Background() - encoder, err := NewJSONRowEventEncoder(ctx, codecConfig) + encoder, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) count := 0 @@ -654,7 +650,7 @@ func TestMaxMessageBytes(t *testing.T) { maxMessageBytes := 300 codecConfig := common.NewConfig(config.ProtocolCanalJSON).WithMaxMessageBytes(maxMessageBytes) - encIface, err := NewJSONRowEventEncoder(ctx, codecConfig) + encIface, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) encoder := encIface.(*JSONRowEventEncoder) @@ -669,7 +665,7 @@ func TestMaxMessageBytes(t *testing.T) { // the test message length is larger than max-message-bytes codecConfig = codecConfig.WithMaxMessageBytes(100) - encIface, err = NewJSONRowEventEncoder(ctx, codecConfig) + encIface, err = NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) encoder = encIface.(*JSONRowEventEncoder) @@ -689,7 +685,7 @@ func TestCanalJSONContentCompatibleE2E(t *testing.T) { codecConfig.ContentCompatible = true codecConfig.OnlyOutputUpdatedColumns = true - encoder, err := NewJSONRowEventEncoder(ctx, codecConfig) + encoder, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) decoder, err := NewDecoder(ctx, codecConfig, nil) @@ -737,7 +733,7 @@ func TestE2EPartitionTableByHash(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolCanalJSON) - encoder, err := NewJSONRowEventEncoder(ctx, codecConfig) + encoder, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) decoder, err := NewDecoder(ctx, codecConfig, nil) @@ -794,7 +790,7 @@ func TestE2EPartitionTableByRange(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolCanalJSON) - encoder, err := NewJSONRowEventEncoder(ctx, codecConfig) + encoder, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) decoder, err := NewDecoder(ctx, codecConfig, nil) @@ -858,7 +854,7 @@ func TestE2EPartitionTable(t *testing.T) { for _, enableTiDBExtension := range []bool{false, true} { codecConfig.EnableTiDBExtension = enableTiDBExtension - encoder, err := NewJSONRowEventEncoder(ctx, codecConfig) + encoder, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) decoder, err := NewDecoder(ctx, codecConfig, nil) diff --git a/pkg/sink/codec/canal/canal_json_test.go b/pkg/sink/codec/canal/canal_json_test.go index 4fa008a27b..4c115bbfc9 100644 --- a/pkg/sink/codec/canal/canal_json_test.go +++ b/pkg/sink/codec/canal/canal_json_test.go @@ -24,6 +24,7 @@ import ( "github.com/pingcap/ticdc/pkg/config" "github.com/pingcap/ticdc/pkg/errors" "github.com/pingcap/ticdc/pkg/sink/codec/common" + "github.com/pingcap/ticdc/pkg/sink/kafka/claimcheck" "github.com/pingcap/tidb/pkg/util/chunk" "github.com/stretchr/testify/require" ) @@ -67,7 +68,7 @@ func TestIntegerContentCompatible(t *testing.T) { codecConfig.ContentCompatible = true codecConfig.OnlyOutputUpdatedColumns = true - encoder, err := NewJSONRowEventEncoder(ctx, codecConfig) + encoder, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) err = encoder.AppendRowChangedEvent(ctx, "", insertRowEvent) @@ -151,7 +152,7 @@ func TestIntegerTypes(t *testing.T) { for _, enableTiDBExtension := range []bool{true, false} { for _, event := range []*commonEvent.RowEvent{minValueEvent, maxValueEvent} { codecConfig.EnableTiDBExtension = enableTiDBExtension - encoder, err := NewJSONRowEventEncoder(ctx, codecConfig) + encoder, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) err = encoder.AppendRowChangedEvent(ctx, "", event) @@ -213,7 +214,7 @@ func TestFloatTypes(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolCanalJSON) - encoder, err := NewJSONRowEventEncoder(ctx, codecConfig) + encoder, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) err = encoder.AppendRowChangedEvent(ctx, "", rowEvent) @@ -262,7 +263,7 @@ func TestTimeTypes(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolCanalJSON) - encoder, err := NewJSONRowEventEncoder(ctx, codecConfig) + encoder, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) err = encoder.AppendRowChangedEvent(ctx, "", rowEvent) @@ -312,7 +313,7 @@ func TestStringTypes(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolCanalJSON) - encoder, err := NewJSONRowEventEncoder(ctx, codecConfig) + encoder, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) err = encoder.AppendRowChangedEvent(ctx, "", rowEvent) @@ -362,7 +363,7 @@ func TestBlobTypes(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolCanalJSON) - encoder, err := NewJSONRowEventEncoder(ctx, codecConfig) + encoder, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) err = encoder.AppendRowChangedEvent(ctx, "", rowEvent) @@ -412,7 +413,7 @@ func TestTextTypes(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolCanalJSON) - encoder, err := NewJSONRowEventEncoder(ctx, codecConfig) + encoder, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) err = encoder.AppendRowChangedEvent(ctx, "", rowEvent) @@ -471,7 +472,7 @@ func TestOtherTypes(t *testing.T) { codecConfig := common.NewConfig(config.ProtocolCanalJSON) codecConfig.ContentCompatible = true - encoder, err := NewJSONRowEventEncoder(ctx, codecConfig) + encoder, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) err = encoder.AppendRowChangedEvent(ctx, "", rowEvent) @@ -529,7 +530,7 @@ func TestDMLEventWithColumnSelector(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolCanalJSON) - encoder, err := NewJSONRowEventEncoder(ctx, codecConfig) + encoder, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) err = encoder.AppendRowChangedEvent(ctx, "", rowEvent) @@ -588,7 +589,7 @@ func TestDMLMultiplePK(t *testing.T) { codecConfig := common.NewConfig(config.ProtocolCanalJSON) codecConfig.ContentCompatible = true - encoder, err := NewJSONRowEventEncoder(ctx, codecConfig) + encoder, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) err = encoder.AppendRowChangedEvent(ctx, "", rowEvent) @@ -674,7 +675,7 @@ func TestDMLMessageTooLarge(t *testing.T) { codecConfig := common.NewConfig(config.ProtocolCanalJSON) codecConfig = codecConfig.WithMaxMessageBytes(300) codecConfig.EnableTiDBExtension = true - encoder, err := NewJSONRowEventEncoder(context.Background(), codecConfig) + encoder, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) err = encoder.AppendRowChangedEvent(context.Background(), "", rowEvent) require.ErrorIs(t, err, errors.ErrMessageTooLarge) @@ -772,7 +773,10 @@ func TestLargeMessageClaimCheck(t *testing.T) { codecConfig.LargeMessageHandle.LargeMessageHandleCompression = "snappy" codecConfig.LargeMessageHandle.ClaimCheckStorageURI = "file:///tmp/canal-json-claim-check" - encoder, err := NewJSONRowEventEncoder(ctx, codecConfig) + claimCheck, err := claimcheck.New(ctx, codecConfig.LargeMessageHandle, codecConfig.ChangefeedID) + require.NoError(t, err) + t.Cleanup(claimCheck.Close) + encoder, err := NewJSONRowEventEncoder(codecConfig, claimCheck) require.NoError(t, err) err = encoder.AppendRowChangedEvent(ctx, "", insertEvent) @@ -863,7 +867,7 @@ func TestMessageLargeHandleKeyOnly(t *testing.T) { codecConfig.LargeMessageHandle.LargeMessageHandleOption = config.LargeMessageHandleOptionHandleKeyOnly codecConfig.EnableTiDBExtension = true - encoder, err := NewJSONRowEventEncoder(ctx, codecConfig) + encoder, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) err = encoder.AppendRowChangedEvent(ctx, "", rowEvent) @@ -949,7 +953,7 @@ func TestDMLTypeEvent(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolCanalJSON) - encoder, err := NewJSONRowEventEncoder(ctx, codecConfig) + encoder, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) decoder, err := NewDecoder(ctx, codecConfig, nil) @@ -981,7 +985,7 @@ func TestDMLTypeEvent(t *testing.T) { // update with only updated columns codecConfig.OnlyOutputUpdatedColumns = true - encoder, err = NewJSONRowEventEncoder(ctx, codecConfig) + encoder, err = NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) err = encoder.AppendRowChangedEvent(ctx, "", updateEvent) @@ -1012,7 +1016,7 @@ func TestDDLSequence(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolCanalJSON) - encoder, err := NewJSONRowEventEncoder(ctx, codecConfig) + encoder, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) dec, err := NewDecoder(ctx, codecConfig, nil) @@ -1142,7 +1146,7 @@ func TestCreateTableDDL(t *testing.T) { for _, enableTiDBExtension := range []bool{false, true} { codecConfig.EnableTiDBExtension = enableTiDBExtension - encoder, err := NewJSONRowEventEncoder(ctx, codecConfig) + encoder, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) message, err := encoder.EncodeDDLEvent(ddlEvent) @@ -1173,7 +1177,7 @@ func TestCreateTableDDL(t *testing.T) { func TestCheckpointTs(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolCanalJSON) - encoder, err := NewJSONRowEventEncoder(ctx, codecConfig) + encoder, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) watermark := uint64(179394) @@ -1183,7 +1187,7 @@ func TestCheckpointTs(t *testing.T) { // with extension codecConfig.EnableTiDBExtension = true - encoder, err = NewJSONRowEventEncoder(ctx, codecConfig) + encoder, err = NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) message, err = encoder.EncodeCheckpointEvent(watermark) require.NoError(t, err) @@ -1243,7 +1247,7 @@ func TestRowKey(t *testing.T) { codecConfig.OnlyOutputUpdatedColumns = true codecConfig.EnableTiDBExtension = true codecConfig.OutputRowKey = true - encoder, err := NewJSONRowEventEncoder(ctx, codecConfig) + encoder, err := NewJSONRowEventEncoder(codecConfig, nil) require.NoError(t, err) err = encoder.AppendRowChangedEvent(ctx, "", event) require.NoError(t, err) diff --git a/pkg/sink/codec/common/encoder.go b/pkg/sink/codec/common/encoder.go index bcb9afd365..95bf191e59 100644 --- a/pkg/sink/codec/common/encoder.go +++ b/pkg/sink/codec/common/encoder.go @@ -31,8 +31,6 @@ type EventEncoder interface { AppendRowChangedEvent(context.Context, string, *commonEvent.RowEvent) error // Build builds the batch messages from AppendRowChangedEvent and returns the messages. Build() []*Message - // clean the resources - Clean() } // TxnEventEncoder is an abstraction for events encoder diff --git a/pkg/sink/codec/debezium/encoder.go b/pkg/sink/codec/debezium/encoder.go index c0f8c3d07a..8a78c6e9f4 100644 --- a/pkg/sink/codec/debezium/encoder.go +++ b/pkg/sink/codec/debezium/encoder.go @@ -165,8 +165,6 @@ func (d *BatchEncoder) Build() []*common.Message { return result } -func (d *BatchEncoder) Clean() {} - // newBatchEncoder creates a new Debezium BatchEncoder. func NewBatchEncoder(c *common.Config, clusterID string) common.EventEncoder { batch := &BatchEncoder{ diff --git a/pkg/sink/codec/encoder_group.go b/pkg/sink/codec/encoder_group.go index 7ae503c985..eeeca21a20 100644 --- a/pkg/sink/codec/encoder_group.go +++ b/pkg/sink/codec/encoder_group.go @@ -25,6 +25,7 @@ import ( "github.com/pingcap/ticdc/pkg/config" "github.com/pingcap/ticdc/pkg/errors" "github.com/pingcap/ticdc/pkg/sink/codec/common" + "github.com/pingcap/ticdc/pkg/sink/kafka/claimcheck" "github.com/pingcap/ticdc/pkg/util" "go.uber.org/zap" "golang.org/x/sync/errgroup" @@ -67,18 +68,21 @@ func NewEncoderGroup( ctx context.Context, cfg *config.SinkConfig, encoderConfig *common.Config, + claimCheck *claimcheck.ClaimCheck, changefeedID commonType.ChangeFeedID, ) (*encoderGroup, error) { concurrency := util.GetOrZero(cfg.EncoderConcurrency) if concurrency <= 0 { concurrency = config.DefaultEncoderGroupConcurrency } + inputCh := make([]chan *future, concurrency) rowEventEncoders := make([]common.EventEncoder, concurrency) + var err error for i := 0; i < concurrency; i++ { inputCh[i] = make(chan *future, defaultInputChanSize) - rowEventEncoders[i], err = NewEventEncoder(ctx, encoderConfig) + rowEventEncoders[i], err = NewEventEncoder(ctx, encoderConfig, claimCheck) if err != nil { log.Error("failed to create row event encoder", zap.Error(err)) return nil, errors.Trace(err) @@ -88,7 +92,7 @@ func NewEncoderGroup( var bw *bootstrapWorker if cfg.ShouldSendBootstrapMsg() { - encoder, err := NewEventEncoder(ctx, encoderConfig) + encoder, err := NewEventEncoder(ctx, encoderConfig, claimCheck) if err != nil { log.Error("failed to create row event encoder", zap.Error(err)) return nil, errors.Trace(err) @@ -206,9 +210,6 @@ func (g *encoderGroup) Output() <-chan *future { func (g *encoderGroup) cleanMetrics() { encoderGroupInputChanSizeGauge.DeleteLabelValues(g.changefeedID.Keyspace(), g.changefeedID.Name()) - for _, encoder := range g.rowEventEncoders { - encoder.Clean() - } common.CleanMetrics(g.changefeedID) } diff --git a/pkg/sink/codec/open/encoder.go b/pkg/sink/codec/open/encoder.go index 75f82e9d03..567ce8b60f 100644 --- a/pkg/sink/codec/open/encoder.go +++ b/pkg/sink/codec/open/encoder.go @@ -50,11 +50,7 @@ type batchEncoder struct { } // NewBatchEncoder creates a new batchEncoder. -func NewBatchEncoder(ctx context.Context, config *common.Config) (common.EventEncoder, error) { - claimCheck, err := claimcheck.New(ctx, config.LargeMessageHandle, config.ChangefeedID) - if err != nil { - return nil, errors.Trace(err) - } +func NewBatchEncoder(config *common.Config, claimCheck *claimcheck.ClaimCheck) (common.EventEncoder, error) { lock.Lock() clear(columnFlagsCache) lock.Unlock() @@ -64,12 +60,6 @@ func NewBatchEncoder(ctx context.Context, config *common.Config) (common.EventEn }, nil } -func (d *batchEncoder) Clean() { - if d.claimCheck != nil { - d.claimCheck.CleanMetrics() - } -} - func (d *batchEncoder) fetchColumnFlags(e *commonEvent.RowEvent) map[string]uint64 { lock.RLock() result, ok := columnFlagsCache[e.GetTableID()] diff --git a/pkg/sink/codec/open/encoder_test.go b/pkg/sink/codec/open/encoder_test.go index 9b02366709..45e48d7cbf 100644 --- a/pkg/sink/codec/open/encoder_test.go +++ b/pkg/sink/codec/open/encoder_test.go @@ -64,7 +64,7 @@ func TestEncodeFlag(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolOpen) - enc, err := NewBatchEncoder(ctx, codecConfig) + enc, err := NewBatchEncoder(codecConfig, nil) require.NoError(t, err) err = enc.AppendRowChangedEvent(ctx, "", insertEvent) @@ -153,7 +153,7 @@ func TestIntegerTypes(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolOpen) for _, event := range []*commonEvent.RowEvent{minValueEvent, maxValueEvent} { - encoder, err := NewBatchEncoder(ctx, codecConfig) + encoder, err := NewBatchEncoder(codecConfig, nil) require.NoError(t, err) err = encoder.AppendRowChangedEvent(ctx, "", event) @@ -209,7 +209,7 @@ func TestFloatTypes(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolOpen) - encoder, err := NewBatchEncoder(ctx, codecConfig) + encoder, err := NewBatchEncoder(codecConfig, nil) require.NoError(t, err) err = encoder.AppendRowChangedEvent(ctx, "", rowEvent) @@ -258,7 +258,7 @@ func TestTimeTypes(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolOpen) - encoder, err := NewBatchEncoder(ctx, codecConfig) + encoder, err := NewBatchEncoder(codecConfig, nil) require.NoError(t, err) err = encoder.AppendRowChangedEvent(ctx, "", rowEvent) @@ -307,7 +307,7 @@ func TestStringTypes(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolOpen) - encoder, err := NewBatchEncoder(ctx, codecConfig) + encoder, err := NewBatchEncoder(codecConfig, nil) require.NoError(t, err) err = encoder.AppendRowChangedEvent(ctx, "", rowEvent) @@ -357,7 +357,7 @@ func TestBlobTypes(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolOpen) - encoder, err := NewBatchEncoder(ctx, codecConfig) + encoder, err := NewBatchEncoder(codecConfig, nil) require.NoError(t, err) err = encoder.AppendRowChangedEvent(ctx, "", rowEvent) @@ -407,7 +407,7 @@ func TestTextTypes(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolOpen) - encoder, err := NewBatchEncoder(ctx, codecConfig) + encoder, err := NewBatchEncoder(codecConfig, nil) require.NoError(t, err) err = encoder.AppendRowChangedEvent(ctx, "", rowEvent) @@ -454,7 +454,7 @@ func TestVectorType(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolOpen) - encoder, err := NewBatchEncoder(ctx, codecConfig) + encoder, err := NewBatchEncoder(codecConfig, nil) require.NoError(t, err) err = encoder.AppendRowChangedEvent(ctx, "", insertRowEvent) @@ -503,7 +503,7 @@ func TestCollation(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolOpen) - encoder, err := NewBatchEncoder(ctx, codecConfig) + encoder, err := NewBatchEncoder(codecConfig, nil) require.NoError(t, err) err = encoder.AppendRowChangedEvent(ctx, "", rowEvent) @@ -561,7 +561,7 @@ func TestOtherTypes(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolOpen) - encoder, err := NewBatchEncoder(ctx, codecConfig) + encoder, err := NewBatchEncoder(codecConfig, nil) require.NoError(t, err) err = encoder.AppendRowChangedEvent(ctx, "", rowEvent) @@ -588,7 +588,7 @@ func TestOtherTypes(t *testing.T) { func TestEncodeCheckpoint(t *testing.T) { codecConfig := common.NewConfig(config.ProtocolOpen) ctx := context.Background() - encoder, err := NewBatchEncoder(ctx, codecConfig) + encoder, err := NewBatchEncoder(codecConfig, nil) require.NoError(t, err) checkpoint := uint64(12345678) @@ -629,7 +629,7 @@ func TestCreateTableDDL(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolOpen) - encoder, err := NewBatchEncoder(ctx, codecConfig) + encoder, err := NewBatchEncoder(codecConfig, nil) require.NoError(t, err) message, err := encoder.EncodeDDLEvent(ddlEvent) @@ -658,7 +658,7 @@ func TestEncodeRoutedDMLEventUsesTargetNames(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolOpen) - encoder, err := NewBatchEncoder(ctx, codecConfig) + encoder, err := NewBatchEncoder(codecConfig, nil) require.NoError(t, err) require.NoError(t, encoder.AppendRowChangedEvent(ctx, "", rowEvent)) @@ -688,7 +688,7 @@ func TestEncodeRoutedDDLEventUsesTargetNames(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolOpen) - encoder, err := NewBatchEncoder(ctx, codecConfig) + encoder, err := NewBatchEncoder(codecConfig, nil) require.NoError(t, err) message, err := encoder.EncodeDDLEvent(routedDDL) @@ -711,7 +711,7 @@ func TestEncodeRoutedDDLEventUsesTargetNames(t *testing.T) { func TestEncoderOneMessage(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolOpen) - encoder, err := NewBatchEncoder(ctx, codecConfig) + encoder, err := NewBatchEncoder(codecConfig, nil) require.NoError(t, err) helper := commonEvent.NewEventTestHelper(t) @@ -778,8 +778,15 @@ func TestEncoderMultipleMessage(t *testing.T) { `insert into test.t values (3, 333)`) ctx := context.Background() +<<<<<<< HEAD codecConfig := common.NewConfig(config.ProtocolOpen).WithMaxMessageBytes(400) encoder, err := NewBatchEncoder(ctx, codecConfig) +======= + codecConfig := common.NewConfig(config.ProtocolOpen). + WithMaxMessageBytes(1000). + WithMaxBatchedBytes(400) + encoder, err := NewBatchEncoder(codecConfig, nil) +>>>>>>> bc474b549 (kafka: share one claimcheck instance across encoders (#5718)) require.NoError(t, err) insertEvents := make([]*commonEvent.RowEvent, 0, 3) @@ -856,7 +863,7 @@ func TestEncoderMultipleMessage(t *testing.T) { func TestMessageTooLarge(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolOpen).WithMaxMessageBytes(100) - encoder, err := NewBatchEncoder(ctx, codecConfig) + encoder, err := NewBatchEncoder(codecConfig, nil) require.NoError(t, err) helper := commonEvent.NewEventTestHelper(t) @@ -885,6 +892,51 @@ func TestMessageTooLarge(t *testing.T) { require.Equal(t, count, 0) } +<<<<<<< HEAD +======= +func TestMessageLargerThanBatchLimit(t *testing.T) { + ctx := context.Background() + codecConfig := common.NewConfig(config.ProtocolOpen). + WithMaxMessageBytes(400). + WithMaxBatchedBytes(100) + encoder, err := NewBatchEncoder(codecConfig, nil) + require.NoError(t, err) + + helper := commonEvent.NewEventTestHelper(t) + defer helper.Close() + helper.Tk().MustExec("use test") + + job := helper.DDL2Job(`create table test.t(a tinyint primary key, b int)`) + tableInfo := helper.GetTableInfo(job) + dmlEvent := helper.DML2Event("test", "t", `insert into test.t values (1, 123)`) + require.NotNil(t, dmlEvent) + insertRow, ok := dmlEvent.GetNextRow() + require.True(t, ok) + + count := 0 + insertRowEvent := &commonEvent.RowEvent{ + TableInfo: tableInfo, + CommitTs: dmlEvent.GetCommitTs(), + Event: insertRow, + ColumnSelector: columnselector.NewDefaultColumnSelector(), + Callback: func() { count += 1 }, + } + + err = encoder.AppendRowChangedEvent(ctx, "", insertRowEvent) + require.NoError(t, err) + + messages := encoder.Build() + require.Len(t, messages, 1) + require.Equal(t, 1, messages[0].GetRowsCount()) + require.Greater(t, messages[0].Length(), codecConfig.MaxBatchedBytes) + require.LessOrEqual(t, messages[0].Length(), codecConfig.MaxMessageBytes) + require.Equal(t, 0, count) + + messages[0].Callback() + require.Equal(t, 1, count) +} + +>>>>>>> bc474b549 (kafka: share one claimcheck instance across encoders (#5718)) func TestLargeMessageWithHandleEnableHandleKeyOnly(t *testing.T) { helper := commonEvent.NewEventTestHelper(t) defer helper.Close() @@ -909,7 +961,7 @@ func TestLargeMessageWithHandleEnableHandleKeyOnly(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolOpen).WithMaxMessageBytes(168) codecConfig.LargeMessageHandle.LargeMessageHandleOption = config.LargeMessageHandleOptionHandleKeyOnly - encoder, err := NewBatchEncoder(ctx, codecConfig) + encoder, err := NewBatchEncoder(codecConfig, nil) require.NoError(t, err) err = encoder.AppendRowChangedEvent(ctx, "", insertRowEvent) @@ -949,7 +1001,7 @@ func TestLargeMessageWithoutHandle(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolOpen).WithMaxMessageBytes(150) codecConfig.LargeMessageHandle.LargeMessageHandleOption = config.LargeMessageHandleOptionHandleKeyOnly - encoder, err := NewBatchEncoder(ctx, codecConfig) + encoder, err := NewBatchEncoder(codecConfig, nil) require.NoError(t, err) helper := commonEvent.NewEventTestHelper(t) @@ -1010,7 +1062,7 @@ func TestDMLEventWithColumnSelector(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolOpen) - encoder, err := NewBatchEncoder(ctx, codecConfig) + encoder, err := NewBatchEncoder(codecConfig, nil) require.NoError(t, err) err = encoder.AppendRowChangedEvent(ctx, "", rowEvent) @@ -1077,7 +1129,7 @@ func TestE2EPartitionTable(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolOpen) - enc, err := NewBatchEncoder(ctx, codecConfig) + enc, err := NewBatchEncoder(codecConfig, nil) require.NoError(t, err) dec, err := NewDecoder(ctx, 0, codecConfig, nil) @@ -1212,7 +1264,7 @@ func TestGenerateColumn(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolOpen) - encoder, err := NewBatchEncoder(ctx, codecConfig) + encoder, err := NewBatchEncoder(codecConfig, nil) require.NoError(t, err) dec, err := NewDecoder(ctx, 0, codecConfig, nil) @@ -1347,7 +1399,7 @@ func TestDMLEvent(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolOpen) - encoder, err := NewBatchEncoder(ctx, codecConfig) + encoder, err := NewBatchEncoder(codecConfig, nil) require.NoError(t, err) decoder, err := NewDecoder(ctx, 0, codecConfig, nil) @@ -1403,7 +1455,7 @@ func TestOnlyOutputUpdatedEvent(t *testing.T) { codecConfig := common.NewConfig(config.ProtocolOpen) codecConfig.OnlyOutputUpdatedColumns = true - encoder, err := NewBatchEncoder(ctx, codecConfig) + encoder, err := NewBatchEncoder(codecConfig, nil) require.NoError(t, err) decoder, err := NewDecoder(ctx, 0, codecConfig, nil) @@ -1448,7 +1500,7 @@ func TestPKWithUK(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolOpen) - encoder, err := NewBatchEncoder(ctx, codecConfig) + encoder, err := NewBatchEncoder(codecConfig, nil) require.NoError(t, err) err = encoder.AppendRowChangedEvent(ctx, "", insertRowEvent) @@ -1497,7 +1549,7 @@ func TestUniqueKeyWithoutPKDMLEvent(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolOpen) - encoder, err := NewBatchEncoder(ctx, codecConfig) + encoder, err := NewBatchEncoder(codecConfig, nil) require.NoError(t, err) err = encoder.AppendRowChangedEvent(ctx, "", insertRowEvent) @@ -1547,7 +1599,7 @@ func TestHandleOnlyEvent(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolOpen) - encoder, err := NewBatchEncoder(ctx, codecConfig) + encoder, err := NewBatchEncoder(codecConfig, nil) require.NoError(t, err) decoder, err := NewDecoder(ctx, 0, codecConfig, nil) @@ -1597,7 +1649,7 @@ func TestRenameTable(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolOpen) - encoder, err := NewBatchEncoder(ctx, codecConfig) + encoder, err := NewBatchEncoder(codecConfig, nil) require.NoError(t, err) dec, err := NewDecoder(ctx, 0, codecConfig, nil) @@ -1655,7 +1707,7 @@ func TestDDLSequence(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolOpen) - encoder, err := NewBatchEncoder(ctx, codecConfig) + encoder, err := NewBatchEncoder(codecConfig, nil) require.NoError(t, err) decoder, err := NewDecoder(ctx, 0, codecConfig, nil) diff --git a/pkg/sink/codec/simple/encoder.go b/pkg/sink/codec/simple/encoder.go index b8ef228561..c8a4208f59 100644 --- a/pkg/sink/codec/simple/encoder.go +++ b/pkg/sink/codec/simple/encoder.go @@ -31,11 +31,7 @@ type Encoder struct { marshaller marshaller } -func NewEncoder(ctx context.Context, config *common.Config) (common.EventEncoder, error) { - claimCheck, err := claimcheck.New(ctx, config.LargeMessageHandle, config.ChangefeedID) - if err != nil { - return nil, errors.Trace(err) - } +func NewEncoder(config *common.Config, claimCheck *claimcheck.ClaimCheck) (common.EventEncoder, error) { marshaller, err := newMarshaller(config) if err != nil { return nil, errors.Trace(err) @@ -161,10 +157,3 @@ func (e *Encoder) EncodeDDLEvent(event *commonEvent.DDLEvent) (*common.Message, } return result, nil } - -// CleanMetrics implement the RowEventEncoderBuilder interface -func (e *Encoder) Clean() { - if e.claimCheck != nil { - e.claimCheck.CleanMetrics() - } -} diff --git a/pkg/sink/codec/simple/encoder_test.go b/pkg/sink/codec/simple/encoder_test.go index 33997736af..4c5c9f5cfa 100644 --- a/pkg/sink/codec/simple/encoder_test.go +++ b/pkg/sink/codec/simple/encoder_test.go @@ -23,12 +23,14 @@ import ( "github.com/DATA-DOG/go-sqlmock" "github.com/golang/mock/gomock" "github.com/pingcap/ticdc/downstreamadapter/sink/columnselector" + commonType "github.com/pingcap/ticdc/pkg/common" commonEvent "github.com/pingcap/ticdc/pkg/common/event" "github.com/pingcap/ticdc/pkg/compression" "github.com/pingcap/ticdc/pkg/config" "github.com/pingcap/ticdc/pkg/errors" "github.com/pingcap/ticdc/pkg/sink/codec/common" mock_simple "github.com/pingcap/ticdc/pkg/sink/codec/simple/mock" + "github.com/pingcap/ticdc/pkg/sink/kafka/claimcheck" "github.com/pingcap/ticdc/pkg/util" timodel "github.com/pingcap/tidb/pkg/meta/model" "github.com/pingcap/tidb/pkg/parser/mysql" @@ -52,7 +54,7 @@ func TestEncodeCheckpoint(t *testing.T) { compression.LZ4, } { codecConfig.LargeMessageHandle.LargeMessageHandleCompression = compressionType - enc, err := NewEncoder(ctx, codecConfig) + enc, err := NewEncoder(codecConfig, nil) require.NoError(t, err) checkpoint := 446266400629063682 @@ -97,7 +99,7 @@ func TestEncodeDMLEnableChecksum(t *testing.T) { } { codecConfig.LargeMessageHandle.LargeMessageHandleCompression = compressionType - enc, err := NewEncoder(ctx, codecConfig) + enc, err := NewEncoder(codecConfig, nil) require.NoError(t, err) rowEventDecoder, err := NewDecoder(ctx, codecConfig, nil) @@ -147,7 +149,7 @@ func TestEncodeDMLEnableChecksum(t *testing.T) { // updateEvent.Checksum.Current = 1 // updateEvent.Checksum.Previous = 2 - enc, err := NewEncoder(ctx, codecConfig) + enc, err := NewEncoder(codecConfig, nil) require.NoError(t, err) rowEventDecoder, err := NewDecoder(ctx, codecConfig, nil) @@ -192,7 +194,7 @@ func TestEncodeRoutedEventsUsesTargetNames(t *testing.T) { codecConfig := common.NewConfig(config.ProtocolSimple) codecConfig.EncodingFormat = format - encIface, err := NewEncoder(ctx, codecConfig) + encIface, err := NewEncoder(codecConfig, nil) require.NoError(t, err) encoder := encIface.(*Encoder) @@ -268,7 +270,7 @@ func TestE2EPartitionTable(t *testing.T) { common.EncodingFormatAvro, } { codecConfig.EncodingFormat = format - enc, err := NewEncoder(ctx, codecConfig) + enc, err := NewEncoder(codecConfig, nil) require.NoError(t, err) dec, err := NewDecoder(ctx, codecConfig, nil) require.NoError(t, err) @@ -415,7 +417,7 @@ func TestEncodeDDLSequence(t *testing.T) { } { codecConfig.LargeMessageHandle.LargeMessageHandleCompression = compressionType - enc, err := NewEncoder(ctx, codecConfig) + enc, err := NewEncoder(codecConfig, nil) require.NoError(t, err) rowEventDecoder, err := NewDecoder(ctx, codecConfig, nil) @@ -851,7 +853,7 @@ func TestEncodeDDLEvent(t *testing.T) { insertEvent.Rewind() insertEvent2.Rewind() codecConfig.LargeMessageHandle.LargeMessageHandleCompression = compressionType - enc, err := NewEncoder(ctx, codecConfig) + enc, err := NewEncoder(codecConfig, nil) require.NoError(t, err) rowEventDecoder, err := NewDecoder(ctx, codecConfig, nil) @@ -996,7 +998,7 @@ func TestColumnFlags(t *testing.T) { common.EncodingFormatJSON, } { codecConfig.EncodingFormat = format - enc, err := NewEncoder(ctx, codecConfig) + enc, err := NewEncoder(codecConfig, nil) require.NoError(t, err) m, err := enc.EncodeDDLEvent(createTableDDLEvent) @@ -1077,7 +1079,7 @@ func TestEncodeIntegerTypes(t *testing.T) { minValues.Rewind() maxValues.Rewind() codecConfig.EncodingFormat = format - enc, err := NewEncoder(ctx, codecConfig) + enc, err := NewEncoder(codecConfig, nil) require.NoError(t, err) m, err := enc.EncodeDDLEvent(ddlEvent) @@ -1156,7 +1158,7 @@ func TestEncoderOtherTypes(t *testing.T) { } { event.Rewind() codecConfig.EncodingFormat = format - enc, err := NewEncoder(ctx, codecConfig) + enc, err := NewEncoder(codecConfig, nil) require.NoError(t, err) m, err := enc.EncodeDDLEvent(ddlEvent) @@ -1233,7 +1235,7 @@ func TestE2EPartitionTableDMLBeforeDDL(t *testing.T) { common.EncodingFormatAvro, } { codecConfig.EncodingFormat = format - enc, err := NewEncoder(ctx, codecConfig) + enc, err := NewEncoder(codecConfig, nil) require.NoError(t, err) dec, err := NewDecoder(ctx, codecConfig, nil) @@ -1301,7 +1303,7 @@ func TestEncodeDMLBeforeDDL(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolSimple) - enc, err := NewEncoder(ctx, codecConfig) + enc, err := NewEncoder(codecConfig, nil) require.NoError(t, err) row, ok := event.GetNextRow() @@ -1384,7 +1386,7 @@ func TestEncodeBootstrapEvent(t *testing.T) { } { dmlEvent.Rewind() codecConfig.LargeMessageHandle.LargeMessageHandleCompression = compressionType - enc, err := NewEncoder(ctx, codecConfig) + enc, err := NewEncoder(codecConfig, nil) require.NoError(t, err) m, err := enc.EncodeDDLEvent(ddlEvent) @@ -1461,7 +1463,7 @@ func TestEncodeLargeEventsNormal(t *testing.T) { } { codecConfig.LargeMessageHandle.LargeMessageHandleCompression = compressionType - enc, err := NewEncoder(ctx, codecConfig) + enc, err := NewEncoder(codecConfig, nil) require.NoError(t, err) rowEventDecoder, err := NewDecoder(ctx, codecConfig, nil) @@ -1543,7 +1545,7 @@ func TestDDLMessageTooLarge(t *testing.T) { common.EncodingFormatJSON, } { codecConfig.EncodingFormat = format - enc, err := NewEncoder(context.Background(), codecConfig) + enc, err := NewEncoder(codecConfig, nil) require.NoError(t, err) _, err = enc.EncodeDDLEvent(ddlEvent) @@ -1554,6 +1556,9 @@ func TestDDLMessageTooLarge(t *testing.T) { func TestDMLMessageTooLarge(t *testing.T) { _, insertEvent, _, _ := common.NewLargeEvent4Test(t) + ctx := context.Background() + changefeedID := commonType.NewChangeFeedIDWithName("test", "") + codecConfig := common.NewConfig(config.ProtocolSimple) codecConfig.MaxMessageBytes = 50 @@ -1568,11 +1573,18 @@ func TestDMLMessageTooLarge(t *testing.T) { config.LargeMessageHandleOptionHandleKeyOnly, config.LargeMessageHandleOptionClaimCheck, } { + var ( + claimCheck *claimcheck.ClaimCheck + err error + ) codecConfig.LargeMessageHandle.LargeMessageHandleOption = handle if handle == config.LargeMessageHandleOptionClaimCheck { codecConfig.LargeMessageHandle.ClaimCheckStorageURI = "file:///tmp/simple-claim-check" + claimCheck, err = claimcheck.New(ctx, codecConfig.LargeMessageHandle, changefeedID) + require.NoError(t, err) + t.Cleanup(claimCheck.Close) } - enc, err := NewEncoder(context.Background(), codecConfig) + enc, err := NewEncoder(codecConfig, claimCheck) require.NoError(t, err) err = enc.AppendRowChangedEvent(context.Background(), "", insertEvent) @@ -1597,6 +1609,9 @@ func TestLargerMessageHandleClaimCheck(t *testing.T) { codecConfig.LargeMessageHandle.ClaimCheckStorageURI = "file:///tmp/simple-claim-check" for _, rawValue := range []bool{false, true} { codecConfig.LargeMessageHandle.ClaimCheckRawValue = rawValue + claimCheck, err := claimcheck.New(ctx, codecConfig.LargeMessageHandle, codecConfig.ChangefeedID) + require.NoError(t, err) + t.Cleanup(claimCheck.Close) for _, format := range []common.EncodingFormatType{ common.EncodingFormatAvro, common.EncodingFormatJSON, @@ -1610,7 +1625,7 @@ func TestLargerMessageHandleClaimCheck(t *testing.T) { codecConfig.MaxMessageBytes = config.DefaultMaxMessageBytes codecConfig.LargeMessageHandle.LargeMessageHandleCompression = compressionType - enc, err := NewEncoder(ctx, codecConfig) + enc, err := NewEncoder(codecConfig, claimCheck) require.NoError(t, err) m, err := enc.EncodeDDLEvent(ddlEvent) @@ -1690,7 +1705,7 @@ func TestLargeMessageHandleKeyOnly(t *testing.T) { codecConfig.MaxMessageBytes = config.DefaultMaxMessageBytes codecConfig.LargeMessageHandle.LargeMessageHandleCompression = compressionType - enc, err := NewEncoder(ctx, codecConfig) + enc, err := NewEncoder(codecConfig, nil) require.NoError(t, err) rowEventDecoder, err := NewDecoder(ctx, codecConfig, db) @@ -1770,7 +1785,7 @@ func TestMarshallerError(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolSimple) - enc, err := NewEncoder(ctx, codecConfig) + enc, err := NewEncoder(codecConfig, nil) require.NoError(t, err) mockMarshaller := mock_simple.NewMockmarshaller(gomock.NewController(t)) diff --git a/pkg/sink/kafka/claimcheck/claim_check.go b/pkg/sink/kafka/claimcheck/claim_check.go index 052785e2fa..952b0a3d48 100644 --- a/pkg/sink/kafka/claimcheck/claim_check.go +++ b/pkg/sink/kafka/claimcheck/claim_check.go @@ -53,11 +53,6 @@ func New(ctx context.Context, config *config.LargeMessageHandleConfig, changefee return nil, nil } - log.Info("claim check enabled, start create the external storage", - zap.String("keyspace", changefeedID.Keyspace()), - zap.String("changefeed", changefeedID.Name()), - zap.String("storageURI", util.MaskSensitiveDataInURI(config.ClaimCheckStorageURI))) - start := time.Now() externalStorage, err := util.GetExternalStorageWithDefaultTimeout(ctx, config.ClaimCheckStorageURI) if err != nil { @@ -70,12 +65,6 @@ func New(ctx context.Context, config *config.LargeMessageHandleConfig, changefee return nil, errors.Trace(err) } - log.Info("claim-check create the external storage success", - zap.String("keyspace", changefeedID.Keyspace()), - zap.String("changefeed", changefeedID.Name()), - zap.String("storageURI", util.MaskSensitiveDataInURI(config.ClaimCheckStorageURI)), - zap.Duration("duration", time.Since(start))) - return &ClaimCheck{ changefeedID: changefeedID, storage: externalStorage, @@ -112,8 +101,15 @@ func (c *ClaimCheck) FileNameWithPrefix(fileName string) string { return strings.TrimSuffix(c.storage.URI(), "/") + "/" + fileName } -// CleanMetrics the claim check by clean up the metrics. -func (c *ClaimCheck) CleanMetrics() { +// Close closes the claim-check storage. +func (c *ClaimCheck) Close() { + if c == nil { + return + } + + if c.storage != nil { + c.storage.Close() + } claimCheckSendMessageDuration.DeleteLabelValues(c.changefeedID.Keyspace(), c.changefeedID.Name()) claimCheckSendMessageCount.DeleteLabelValues(c.changefeedID.Keyspace(), c.changefeedID.Name()) } diff --git a/pkg/sink/kafka/claimcheck/claim_check_test.go b/pkg/sink/kafka/claimcheck/claim_check_test.go new file mode 100644 index 0000000000..e51cfc5626 --- /dev/null +++ b/pkg/sink/kafka/claimcheck/claim_check_test.go @@ -0,0 +1,96 @@ +// Copyright 2023 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// See the License for the specific language governing permissions and +// limitations under the License. + +package claimcheck + +import ( + "context" + "fmt" + "testing" + + commonType "github.com/pingcap/ticdc/pkg/common" + "github.com/pingcap/ticdc/pkg/config" + "github.com/pingcap/tidb/pkg/objstore" + "github.com/pingcap/tidb/pkg/objstore/mockobjstore" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + "golang.org/x/sync/errgroup" +) + +func TestClaimCheck(t *testing.T) { + t.Parallel() + + ctx := context.Background() + + changefeedID := commonType.NewChangeFeedIDWithName("test", "") + largeHandleConfig := config.NewDefaultLargeMessageHandleConfig() + + claimCheck, err := New(ctx, largeHandleConfig, changefeedID) + require.NoError(t, err) + require.Nil(t, claimCheck) + + largeHandleConfig.LargeMessageHandleOption = config.LargeMessageHandleOptionClaimCheck + largeHandleConfig.ClaimCheckStorageURI = "file:///tmp/abc/" + claimCheck, err = New(ctx, largeHandleConfig, changefeedID) + require.NoError(t, err) + t.Cleanup(claimCheck.Close) + + fileName := claimCheck.FileNameWithPrefix("file.json") + require.Equal(t, "file:///tmp/abc/file.json", fileName) +} + +func TestClaimCheckCloseClosesStorage(t *testing.T) { + var nilClaimCheck *ClaimCheck + require.NotPanics(t, nilClaimCheck.Close) + + ctrl := gomock.NewController(t) + storage := mockobjstore.NewMockStorage(ctrl) + storage.EXPECT().Close().Times(1) + claimCheck := &ClaimCheck{ + storage: storage, + changefeedID: commonType.NewChangeFeedIDWithName("test", "default"), + } + + claimCheck.Close() +} + +func TestClaimCheckConcurrentWrites(t *testing.T) { + ctx := context.Background() + storage := objstore.NewMemStorage() + changefeedID := commonType.NewChangeFeedIDWithName("test", "default") + claimCheck := &ClaimCheck{ + storage: storage, + rawValue: true, + changefeedID: changefeedID, + metricSendMessageDuration: claimCheckSendMessageDuration.WithLabelValues(changefeedID.Keyspace(), changefeedID.Name()), + metricSendMessageCount: claimCheckSendMessageCount.WithLabelValues(changefeedID.Keyspace(), changefeedID.Name()), + } + t.Cleanup(claimCheck.Close) + + const concurrency = 32 + group := new(errgroup.Group) + for i := range concurrency { + fileName := fmt.Sprintf("%d.json", i) + group.Go(func() error { + return claimCheck.WriteMessage(ctx, nil, []byte(fileName), fileName) + }) + } + require.NoError(t, group.Wait()) + + for i := range concurrency { + fileName := fmt.Sprintf("%d.json", i) + data, err := storage.ReadFile(ctx, fileName) + require.NoError(t, err) + require.Equal(t, fileName, string(data)) + } +} From f7a1a67a371d1d87c680ac5f3fd98464516749d8 Mon Sep 17 00:00:00 2001 From: Ling Jin <7138436+3AceShowHand@users.noreply.github.com> Date: Mon, 27 Jul 2026 20:01:27 +0800 Subject: [PATCH 3/6] This is an automated cherry-pick of #5715 Signed-off-by: ti-chi-bot --- downstreamadapter/sink/kafka/sink.go | 82 +++++++++++++ downstreamadapter/sink/kafka/sink_test.go | 51 ++++++++ .../sink/topicmanager/kafka_topic_manager.go | 9 +- .../topicmanager/kafka_topic_manager_test.go | 46 ++++++- pkg/sink/kafka/options.go | 107 ++++++++++++++++- pkg/sink/kafka/options_test.go | 113 +++++++++++++++++- pkg/sink/kafka/sarama_factory.go | 4 + 7 files changed, 405 insertions(+), 7 deletions(-) diff --git a/downstreamadapter/sink/kafka/sink.go b/downstreamadapter/sink/kafka/sink.go index e6d1038b26..90653aec87 100644 --- a/downstreamadapter/sink/kafka/sink.go +++ b/downstreamadapter/sink/kafka/sink.go @@ -68,9 +68,91 @@ func (s *sink) SinkType() commonType.SinkType { } func Verify(ctx context.Context, changefeedID commonType.ChangeFeedID, uri *url.URL, sinkConfig *config.SinkConfig) error { +<<<<<<< HEAD comp, _, err := newKafkaSinkComponent(ctx, changefeedID, uri, sinkConfig) defer comp.close() return err +======= + protocol, err := helper.GetProtocol(util.GetOrZero(sinkConfig.Protocol)) + if err != nil { + return errors.Trace(err) + } + + topic, err := helper.GetTopic(uri) + if err != nil { + return errors.Trace(err) + } + + options := kafka.NewOptions() + if err = options.Apply(changefeedID, uri, sinkConfig); err != nil { + return errors.WrapError(errors.ErrKafkaInvalidConfig, err) + } + options.Topic = topic + + encoderConfig, err := helper.GetEncoderConfig( + changefeedID, uri, protocol, sinkConfig, + options.MaxMessageBytes, options.MaxBatchedBytes, + ) + if err != nil { + return errors.Trace(err) + } + + claimCheck, err := claimcheck.New(ctx, encoderConfig.LargeMessageHandle, changefeedID) + if err != nil { + return err + } + defer claimCheck.Close() + + isAvroLike := protocol == config.ProtocolAvro || protocol == config.ProtocolDebeziumAvro + if _, err = eventrouter.NewEventRouter(sinkConfig, topic, false, isAvroLike); err != nil { + return errors.Trace(err) + } + + if _, err = columnselector.New(sinkConfig); err != nil { + return errors.Trace(err) + } + + factory, err := kafka.NewSaramaFactory(ctx, options, changefeedID) + if err != nil { + return errors.WrapError(errors.ErrKafkaNewProducer, err) + } + + adminClient, err := factory.AdminClient(ctx) + if err != nil { + return errors.WrapError(errors.ErrKafkaNewProducer, err) + } + defer adminClient.Close() + + topics, err := adminClient.GetTopicsMeta([]string{topic}, false) + if err != nil { + return errors.Trace(err) + } + if _, exists := topics[topic]; !exists { + topicConfig := options.DeriveTopicConfig() + if !topicConfig.AutoCreate { + return errors.ErrKafkaInvalidConfig.GenWithStack("`auto-create-topic` is false, and %s not found", topic) + } + if err = topicConfig.ValidateReplicationFactor(adminClient); err != nil { + return err + } + + // the topic is not created, only validate. + err = adminClient.CreateTopic(&kafka.TopicDetail{ + Name: topic, + NumPartitions: topicConfig.PartitionNum, + ReplicationFactor: topicConfig.ReplicationFactor, + }, true) + if err != nil { + return errors.WrapError(errors.ErrKafkaCreateTopic, err) + } + } + + _, err = codec.NewEventEncoder(ctx, encoderConfig, claimCheck) + if err != nil { + return errors.Trace(err) + } + return nil +>>>>>>> 0d4929739 (kafka: verify replication-factor when need to create the topic (#5715)) } func New( diff --git a/downstreamadapter/sink/kafka/sink_test.go b/downstreamadapter/sink/kafka/sink_test.go index 0bb4708f58..8888e1e54b 100644 --- a/downstreamadapter/sink/kafka/sink_test.go +++ b/downstreamadapter/sink/kafka/sink_test.go @@ -16,11 +16,20 @@ package kafka import ( "context" "fmt" + "net/http" + "net/http/httptest" "net/url" "testing" "time" +<<<<<<< HEAD "github.com/pingcap/errors" +======= + "github.com/IBM/sarama" + "github.com/golang/mock/gomock" + "github.com/pingcap/ticdc/downstreamadapter/sink/columnselector" + "github.com/pingcap/ticdc/downstreamadapter/sink/eventrouter" +>>>>>>> 0d4929739 (kafka: verify replication-factor when need to create the topic (#5715)) "github.com/pingcap/ticdc/downstreamadapter/sink/helper" "github.com/pingcap/ticdc/pkg/common" commonEvent "github.com/pingcap/ticdc/pkg/common/event" @@ -32,6 +41,48 @@ import ( "go.uber.org/atomic" ) +<<<<<<< HEAD +======= +const kafkaSinkTestTopic = "mock_topic" + +func TestVerifyInvalidConfig(t *testing.T) { + broker := sarama.NewMockBroker(t, 1) + defer broker.Close() + broker.SetHandlerByMap(map[string]sarama.MockResponse{ + "ApiVersionsRequest": sarama.NewMockApiVersionsResponse(t).SetApiKeys( + []sarama.ApiVersionsResponseKey{ + {ApiKey: 0}, + {ApiKey: 1}, + {ApiKey: 2}, + {ApiKey: 3, MaxVersion: 9}, + }), + "MetadataRequest": sarama.NewMockMetadataResponse(t). + SetController(broker.BrokerID()). + SetBroker(broker.Addr(), broker.BrokerID()). + SetLeader(kafkaSinkTestTopic, 0, broker.BrokerID()), + "DescribeConfigsRequest": sarama.NewMockDescribeConfigsResponse(t), + }) + + schemaRegistry := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + http.Error(w, "invalid response", http.StatusInternalServerError) + })) + defer schemaRegistry.Close() + + avroProtocol := config.ProtocolAvro.String() + sinkConfig := &config.SinkConfig{ + Protocol: &avroProtocol, + SchemaRegistry: &schemaRegistry.URL, + } + sinkURI, err := url.Parse("kafka://" + broker.Addr() + "/" + kafkaSinkTestTopic + + "?required-acks=1&kafka-version=2.4.0") + require.NoError(t, err) + + changefeedID := common.NewChangefeedID4Test("test", "verify-invalid-config") + err = Verify(context.Background(), changefeedID, sinkURI, sinkConfig) + require.ErrorContains(t, err, "ErrAvroSchemaAPIError") +} + +>>>>>>> 0d4929739 (kafka: verify replication-factor when need to create the topic (#5715)) func newKafkaSinkForTestWithProducers(ctx context.Context, asyncProducer kafka.AsyncProducer, syncProducer kafka.SyncProducer, diff --git a/downstreamadapter/sink/topicmanager/kafka_topic_manager.go b/downstreamadapter/sink/topicmanager/kafka_topic_manager.go index 8e92167327..168a532be6 100644 --- a/downstreamadapter/sink/topicmanager/kafka_topic_manager.go +++ b/downstreamadapter/sink/topicmanager/kafka_topic_manager.go @@ -15,7 +15,6 @@ package topicmanager import ( "context" - "fmt" "sync" "time" @@ -242,9 +241,17 @@ func (m *kafkaTopicManager) createTopic( topicName string, ) (int32, error) { if !m.cfg.AutoCreate { +<<<<<<< HEAD return 0, cerror.ErrKafkaInvalidConfig.GenWithStack( fmt.Sprintf("`auto-create-topic` is false, "+ "and %s not found", topicName)) +======= + return 0, errors.ErrKafkaInvalidConfig.GenWithStack("`auto-create-topic` is false, and %s not found", topicName) + } + + if err := m.cfg.ValidateReplicationFactor(m.admin); err != nil { + return 0, err +>>>>>>> 0d4929739 (kafka: verify replication-factor when need to create the topic (#5715)) } start := time.Now() diff --git a/downstreamadapter/sink/topicmanager/kafka_topic_manager_test.go b/downstreamadapter/sink/topicmanager/kafka_topic_manager_test.go index bf02658b24..b77777cfe9 100644 --- a/downstreamadapter/sink/topicmanager/kafka_topic_manager_test.go +++ b/downstreamadapter/sink/topicmanager/kafka_topic_manager_test.go @@ -31,6 +31,7 @@ func TestCreateTopic(t *testing.T) { AutoCreate: true, PartitionNum: 2, ReplicationFactor: 1, + RequiredAcks: kafka.WaitForAll, } changefeedID := common.NewChangefeedID4Test("test", "test") @@ -41,6 +42,7 @@ func TestCreateTopic(t *testing.T) { require.NoError(t, err) require.Equal(t, int32(2), partitionNum) + cfg.RequiredAcks = kafka.WaitForLocal partitionNum, err = manager.CreateTopicAndWaitUntilVisible(ctx, "new-topic") require.NoError(t, err) require.Equal(t, int32(2), partitionNum) @@ -49,7 +51,12 @@ func TestCreateTopic(t *testing.T) { require.Equal(t, int32(2), partitionsNum) // Try to create a topic without auto create. - cfg.AutoCreate = false + cfg = &kafka.AutoCreateTopicConfig{ + AutoCreate: false, + PartitionNum: 2, + ReplicationFactor: 1, + RequiredAcks: kafka.WaitForAll, + } manager = newKafkaTopicManager(ctx, "new-topic2", changefeedID, adminClient, cfg) defer manager.Close() _, err = manager.CreateTopicAndWaitUntilVisible(ctx, "new-topic2") @@ -77,7 +84,44 @@ func TestCreateTopic(t *testing.T) { ) } +<<<<<<< HEAD func TestCreateTopicWithDelay(t *testing.T) { +======= +func TestCreateTopicValidatesReplicationFactor(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + adminClient := kafka.NewMockClusterAdminClient(ctrl) + topic := "new-topic" + gomock.InOrder( + adminClient.EXPECT().GetTopicsMeta([]string{topic}, true). + Return(map[string]kafka.TopicDetail{}, nil), + adminClient.EXPECT().GetTopicsMeta([]string{topic}, false). + Return(map[string]kafka.TopicDetail{}, nil), + adminClient.EXPECT().GetBrokerConfig(kafka.MinInsyncReplicasConfigName). + Return("2", nil), + ) + + manager := newKafkaTopicManager( + context.Background(), + topic, + common.NewChangefeedID4Test("test", "test"), + adminClient, + &kafka.AutoCreateTopicConfig{ + AutoCreate: true, + PartitionNum: 2, + ReplicationFactor: 1, + RequiredAcks: kafka.WaitForAll, + }, + ) + defer manager.Close() + + _, err := manager.CreateTopicAndWaitUntilVisible(context.Background(), topic) + require.ErrorContains(t, err, "`replication-factor` 1 is smaller than the `min.insync.replicas` 2 of broker") +} + +func TestCreateTopicWaitsUntilVisible(t *testing.T) { +>>>>>>> 0d4929739 (kafka: verify replication-factor when need to create the topic (#5715)) t.Parallel() adminClient := kafka.NewClusterAdminClientMockImpl() diff --git a/pkg/sink/kafka/options.go b/pkg/sink/kafka/options.go index c9b992814e..48ed5c5677 100644 --- a/pkg/sink/kafka/options.go +++ b/pkg/sink/kafka/options.go @@ -14,7 +14,6 @@ package kafka import ( - "context" "encoding/base64" "fmt" "net/http" @@ -249,6 +248,9 @@ func (o *options) Apply(changefeedID common.ChangeFeedID, } if urlParameter.ReplicationFactor != nil { + if *urlParameter.ReplicationFactor <= 0 { + return errors.ErrKafkaInvalidConfig.GenWithStack("invalid replication-factor %d", *urlParameter.ReplicationFactor) + } o.ReplicationFactor = *urlParameter.ReplicationFactor } @@ -537,11 +539,12 @@ func (o *options) applySASL(urlParameter *urlConfig, sinkConfig *config.SinkConf return nil } -// AutoCreateTopicConfig is used to create topic configuration. +// AutoCreateTopicConfig contains settings used to create and validate a topic. type AutoCreateTopicConfig struct { AutoCreate bool PartitionNum int32 ReplicationFactor int16 + RequiredAcks RequiredAcks } func (o *options) DeriveTopicConfig() *AutoCreateTopicConfig { @@ -549,7 +552,40 @@ func (o *options) DeriveTopicConfig() *AutoCreateTopicConfig { AutoCreate: o.AutoCreate, PartitionNum: o.PartitionNum, ReplicationFactor: o.ReplicationFactor, + RequiredAcks: o.RequiredAcks, + } +} + +// ValidateReplicationFactor checks whether a topic created with this config +// can satisfy the configured acknowledgment requirement. +func (c *AutoCreateTopicConfig) ValidateReplicationFactor(admin ClusterAdminClient) error { + if c.RequiredAcks != WaitForAll { + return nil + } + + raw, err := admin.GetBrokerConfig(MinInsyncReplicasConfigName) + if err != nil { + log.Warn("cannot get Kafka broker configuration, assume replication factor is valid", + zap.String("configName", MinInsyncReplicasConfigName), + zap.Int16("replicationFactor", c.ReplicationFactor), + zap.Error(err)) + return nil + } + minInsyncReplicas, err := strconv.Atoi(raw) + if err != nil { + return err } + + if int(c.ReplicationFactor) < minInsyncReplicas { + return errors.ErrKafkaInvalidConfig.GenWithStack( + "TiCDC Kafka sink's `request.required.acks` defaults to -1, "+ + "TiCDC cannot deliver messages when the `replication-factor` %d "+ + "is smaller than the `min.insync.replicas` %d of broker", + c.ReplicationFactor, minInsyncReplicas, + ) + } + + return nil } var ( @@ -577,7 +613,11 @@ func NewKafkaClientID(captureAddr string, // adjustOptions adjust the `options` and `sarama.Config` by condition. func adjustOptions( +<<<<<<< HEAD ctx context.Context, +======= + changefeedID common.ChangeFeedID, +>>>>>>> 0d4929739 (kafka: verify replication-factor when need to create the topic (#5715)) admin ClusterAdminClient, options *options, topic string, @@ -587,6 +627,7 @@ func adjustOptions( return errors.Trace(err) } +<<<<<<< HEAD // Only check replicationFactor >= minInsyncReplicas when producer's required acks is -1. // If we don't check it, the producer probably can not send message to the topic. // Because it will wait for the ack from all replicas. But we do not have enough replicas. @@ -597,10 +638,13 @@ func adjustOptions( } } +======= +>>>>>>> 0d4929739 (kafka: verify replication-factor when need to create the topic (#5715)) info, exists := topics[topic] // once we have found the topic, no matter `auto-create-topic`, // make sure user input parameters are valid. if exists { +<<<<<<< HEAD // make sure that producer's `MaxMessageBytes` smaller than topic's `max.message.bytes` topicMaxMessageBytesStr, err := getTopicConfig( ctx, admin, info.Name, @@ -649,6 +693,28 @@ func adjustOptions( return errors.Trace(err) } brokerMessageMaxBytes, err := strconv.Atoi(brokerMessageMaxBytesStr) +======= + err = adjustExistingTopicOption(changefeedID, admin, options, topic, info) + } else { + adjustNewTopicOptions(admin, changefeedID, options, topic) + } + if err != nil { + return err + } + + options.MaxBatchedBytes = min(options.MaxBatchedBytes, options.MaxMessageBytes) + return nil +} + +func adjustExistingTopicOption( + changefeedID common.ChangeFeedID, + admin ClusterAdminClient, + options *options, + topic string, + info TopicDetail, +) error { + maxMessageBytes, err := getTopicMaxMessageBytes(admin, info.Name) +>>>>>>> 0d4929739 (kafka: verify replication-factor when need to create the topic (#5715)) if err != nil { return errors.Trace(err) } @@ -677,6 +743,7 @@ func adjustOptions( log.Warn("partition-num is not set, use the default partition count", zap.String("topic", topic), zap.Int32("partitions", options.PartitionNum)) } +<<<<<<< HEAD return nil } @@ -747,12 +814,46 @@ func validateMinInsyncReplicas( return nil } +======= +} + +func getTopicMaxMessageBytes( + admin ClusterAdminClient, + topic string, +) (int, error) { + raw, err := getTopicConfig( + admin, topic, + TopicMaxMessageBytesConfigName, + BrokerMessageMaxBytesConfigName, + ) + if err != nil { + return 0, errors.Trace(err) + } + maxMessageBytes, err := strconv.Atoi(raw) + if err != nil { + return 0, errors.Trace(err) + } + return maxMessageBytes, nil +} + +func getBrokerMaxMessageBytes(admin ClusterAdminClient) (int, error) { + raw, err := admin.GetBrokerConfig(BrokerMessageMaxBytesConfigName) + if err != nil { + return 0, errors.Trace(err) + } + messageMaxBytes, err := strconv.Atoi(raw) + if err != nil { + return 0, errors.Trace(err) + } + return messageMaxBytes, nil +} + +>>>>>>> 0d4929739 (kafka: verify replication-factor when need to create the topic (#5715)) // getTopicConfig gets topic config by name. // If the topic does not have this configuration, // we will try to get it from the broker's configuration. // NOTICE: The configuration names of topic and broker may be different for the same configuration. func getTopicConfig( - ctx context.Context, admin ClusterAdminClient, topicName string, topicConfigName string, diff --git a/pkg/sink/kafka/options_test.go b/pkg/sink/kafka/options_test.go index 8f64d49762..48cce4d8f6 100644 --- a/pkg/sink/kafka/options_test.go +++ b/pkg/sink/kafka/options_test.go @@ -69,6 +69,18 @@ func TestCompleteOptions(t *testing.T) { options = NewOptions() err = options.Apply(commonType.NewChangefeedID4Test(commonType.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink) require.Regexp(t, ".*invalid syntax.*", errors.Cause(err)) + for _, replicationFactor := range []string{"0", "-1"} { + uri = "kafka://127.0.0.1:9092/abc?replication-factor=" + replicationFactor + sinkURI, err = url.Parse(uri) + require.NoError(t, err) + options = NewOptions() + err = options.Apply( + commonType.NewChangefeedID4Test(commonType.DefaultKeyspaceName, "test"), + sinkURI, + config.GetDefaultReplicaConfig().Sink, + ) + require.ErrorContains(t, err, "invalid replication-factor "+replicationFactor) + } // Illegal max-message-bytes. uri = "kafka://127.0.0.1:9092/abc?kafka-version=2.6.0&max-message-bytes=a" @@ -321,9 +333,55 @@ func TestAdjustConfigTopicExist(t *testing.T) { // When the topic exists, but the topic does not have `max.message.bytes` // create a topic without `max.message.bytes` topicName := "test-topic" +<<<<<<< HEAD detail := &TopicDetail{ Name: topicName, NumPartitions: 3, +======= + changefeedID := commonType.NewChangefeedID4Test(commonType.DefaultKeyspaceName, "test") + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + adminFixture := newKafkaAdminFixture(t) + adminClient := adminFixture.admin + + detail := &TopicDetail{ + Name: topicName, + NumPartitions: 3, + } + err := adminClient.CreateTopic(detail, false) + require.NoError(t, err) + + configuredMaxMessageBytes := test.configuredMaxMessageBytes(adminFixture) + sinkURI, err := url.Parse(fmt.Sprintf( + "kafka://127.0.0.1:9092/%s?max-message-bytes=%d", + topicName, configuredMaxMessageBytes, + )) + require.NoError(t, err) + + options := NewOptions() + err = options.Apply(changefeedID, sinkURI, config.GetDefaultReplicaConfig().Sink) + require.NoError(t, err) + require.Equal(t, configuredMaxMessageBytes, options.MaxMessageBytes) + require.Equal(t, configuredMaxMessageBytes, options.MaxBatchedBytes) + expectedProducerLimit := adminFixture.brokerMessageMaxBytes() + + ctx := context.Background() + err = adjustOptions(changefeedID, adminClient, options, topicName) + require.NoError(t, err) + + saramaConfig, err := newSaramaConfig(ctx, options) + require.NoError(t, err) + + require.NotEqual(t, configuredMaxMessageBytes, options.MaxMessageBytes) + require.Equal(t, expectedProducerLimit, options.MaxMessageBytes) + require.Equal( + t, + min(configuredMaxMessageBytes, expectedProducerLimit), + options.MaxBatchedBytes, + ) + require.Equal(t, expectedProducerLimit, saramaConfig.Producer.MaxMessageBytes) + }) +>>>>>>> 0d4929739 (kafka: verify replication-factor when need to create the topic (#5715)) } err = adminClient.CreateTopic(detail, false) require.NoError(t, err) @@ -353,6 +411,7 @@ func TestAdjustConfigTopicExist(t *testing.T) { require.Equal(t, maxMessageBytes, saramaConfig.Producer.MaxMessageBytes) } +<<<<<<< HEAD func TestAdjustConfigMinInsyncReplicas(t *testing.T) { adminClient := NewClusterAdminClientMockImpl() defer adminClient.Close() @@ -371,12 +430,26 @@ func TestAdjustConfigMinInsyncReplicas(t *testing.T) { options, "create-new-fail-invalid-min-insync-replicas", ) +======= +func TestValidateReplicationFactor(t *testing.T) { + adminFixture := newKafkaAdminFixture(t) + adminClient := adminFixture.admin + adminFixture.setMinInsyncReplicas("2") + + topicConfig := &AutoCreateTopicConfig{ + AutoCreate: true, + ReplicationFactor: 1, + RequiredAcks: WaitForAll, + } + err := topicConfig.ValidateReplicationFactor(adminClient) +>>>>>>> 0d4929739 (kafka: verify replication-factor when need to create the topic (#5715)) require.Regexp( t, ".*`replication-factor` 1 is smaller than the `min.insync.replicas` 2 of broker.*", errors.Cause(err), ) +<<<<<<< HEAD // topic not exist, and `min.insync.replicas` not found in broker's configuration adminClient.DropBrokerConfig(MinInsyncReplicasConfigName) topicName := "no-topic-no-min-insync-replicas" @@ -384,10 +457,17 @@ func TestAdjustConfigMinInsyncReplicas(t *testing.T) { require.Nil(t, err) err = adminClient.CreateTopic(&TopicDetail{ Name: topicName, +======= + localAcksConfig := &AutoCreateTopicConfig{ + AutoCreate: true, +>>>>>>> 0d4929739 (kafka: verify replication-factor when need to create the topic (#5715)) ReplicationFactor: 1, - }, false) - require.ErrorIs(t, err, sarama.ErrPolicyViolation) + RequiredAcks: WaitForLocal, + } + err = localAcksConfig.ValidateReplicationFactor(adminClient) + require.NoError(t, err) +<<<<<<< HEAD // Report an error if the replication-factor is less than min.insync.replicas // when the topic does exist. @@ -428,6 +508,16 @@ func TestSkipAdjustConfigMinInsyncReplicasWhenRequiredAcksIsNotWailAll(t *testin "skip-check-min-insync-replicas", ) require.Nil(t, err, "Should not report an error when `required-acks` is not `all`") +======= + adminFixture.dropBrokerConfig(MinInsyncReplicasConfigName) + missingBrokerConfig := &AutoCreateTopicConfig{ + AutoCreate: true, + ReplicationFactor: 1, + RequiredAcks: WaitForAll, + } + err = missingBrokerConfig.ValidateReplicationFactor(adminClient) + require.NoError(t, err) +>>>>>>> 0d4929739 (kafka: verify replication-factor when need to create the topic (#5715)) } func TestCreateProducerFailed(t *testing.T) { @@ -640,10 +730,17 @@ func TestConfigurationCombinations(t *testing.T) { sinkURI, err := url.Parse(uri) require.Nil(t, err) +<<<<<<< HEAD ctx := context.Background() options := NewOptions() err = options.Apply(commonType.NewChangefeedID4Test(commonType.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink) require.Nil(t, err) +======= + options := NewOptions() + err = options.Apply(commonType.NewChangefeedID4Test(commonType.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink) + require.Nil(t, err) + configuredMaxMessageBytes := options.MaxMessageBytes +>>>>>>> 0d4929739 (kafka: verify replication-factor when need to create the topic (#5715)) changefeed := commonType.NewChangefeedID4Test(commonType.DefaultKeyspaceName, "changefeed-test") factory, err := NewMockFactory(ctx, options, changefeed) @@ -652,11 +749,23 @@ func TestConfigurationCombinations(t *testing.T) { adminClient, err := factory.AdminClient(ctx) require.NoError(t, err) +<<<<<<< HEAD topic, ok := a.uriParams[0].(string) require.True(t, ok) require.NotEqual(t, "", topic) err = adjustOptions(ctx, adminClient, options, topic) require.Nil(t, err) +======= + changefeedID := commonType.NewChangefeedID4Test(commonType.DefaultKeyspaceName, "test") + err = adjustOptions(changefeedID, adminClient, options, topic) + require.Nil(t, err) + require.Equal(t, sourceMaxMessageBytes, options.MaxMessageBytes) + require.Equal( + t, + min(configuredMaxMessageBytes, sourceMaxMessageBytes), + options.MaxBatchedBytes, + ) +>>>>>>> 0d4929739 (kafka: verify replication-factor when need to create the topic (#5715)) encoderConfig := common.NewConfig(config.ProtocolOpen) err = encoderConfig.Apply(sinkURI, &config.SinkConfig{ diff --git a/pkg/sink/kafka/sarama_factory.go b/pkg/sink/kafka/sarama_factory.go index 650f346b4c..5d9e6882c2 100644 --- a/pkg/sink/kafka/sarama_factory.go +++ b/pkg/sink/kafka/sarama_factory.go @@ -57,7 +57,11 @@ func NewSaramaFactory( admin.Close() }() +<<<<<<< HEAD if err = adjustOptions(ctx, admin, o, o.Topic); err != nil { +======= + if err = adjustOptions(changefeedID, admin, o, o.Topic); err != nil { +>>>>>>> 0d4929739 (kafka: verify replication-factor when need to create the topic (#5715)) return nil, errors.Trace(err) } From 28605d0dd0a56d6fca2e663c1fbf5c2aa767a978 Mon Sep 17 00:00:00 2001 From: Ling Jin <7138436+3AceShowHand@users.noreply.github.com> Date: Tue, 28 Jul 2026 17:13:10 +0800 Subject: [PATCH 4/6] This is an automated cherry-pick of #5786 Signed-off-by: ti-chi-bot --- .../sink/eventrouter/topic/expression.go | 7 +- .../sink/eventrouter/topic/expression_test.go | 4 +- downstreamadapter/sink/kafka/helper.go | 94 +++++- downstreamadapter/sink/kafka/sink.go | 130 +++++++- downstreamadapter/sink/kafka/sink_test.go | 142 ++++++++- downstreamadapter/sink/pulsar/helper.go | 14 +- .../sink/topicmanager/kafka_topic_manager.go | 26 +- .../topicmanager/kafka_topic_manager_test.go | 96 ++++++ pkg/errors/error.go | 47 +-- pkg/errors/error_test.go | 20 ++ pkg/sink/kafka/admin.go | 28 +- pkg/sink/kafka/admin_test.go | 86 ++++++ pkg/sink/kafka/claimcheck/claim_check.go | 19 +- pkg/sink/kafka/claimcheck/claim_check_test.go | 110 +++++++ pkg/sink/kafka/cluster_admin_client.go | 8 +- pkg/sink/kafka/cluster_admin_client_mock.go | 136 +++++++++ pkg/sink/kafka/logutil.go | 24 +- pkg/sink/kafka/logutil_test.go | 46 ++- pkg/sink/kafka/oauth2_token_provider.go | 4 +- pkg/sink/kafka/oauth2_token_provider_test.go | 6 +- pkg/sink/kafka/options.go | 180 ++++++++++- pkg/sink/kafka/options_test.go | 285 ++++++++++++++++-- pkg/sink/kafka/sarama_async_producer.go | 59 ++-- pkg/sink/kafka/sarama_config.go | 10 +- pkg/sink/kafka/sarama_config_test.go | 21 +- pkg/sink/kafka/sarama_factory.go | 30 +- pkg/sink/kafka/sarama_sync_producer.go | 51 ++-- pkg/sink/kafka/sarama_sync_producer_test.go | 129 +++++++- pkg/util/external_storage.go | 6 +- .../http_api/util/test_case.py | 5 +- .../kafka_big_messages/run.sh | 183 +++++++++++ tests/integration_tests/kafka_log_info/run.sh | 134 -------- .../mq_sink_error_resume/run.sh | 21 +- tests/integration_tests/run_heavy_it_in_ci.sh | 2 +- tests/integration_tests/run_light_it_in_ci.sh | 2 +- 35 files changed, 1760 insertions(+), 405 deletions(-) create mode 100644 pkg/sink/kafka/claimcheck/claim_check_test.go create mode 100644 pkg/sink/kafka/cluster_admin_client_mock.go delete mode 100755 tests/integration_tests/kafka_log_info/run.sh diff --git a/downstreamadapter/sink/eventrouter/topic/expression.go b/downstreamadapter/sink/eventrouter/topic/expression.go index bc60085974..453ba423a3 100644 --- a/downstreamadapter/sink/eventrouter/topic/expression.go +++ b/downstreamadapter/sink/eventrouter/topic/expression.go @@ -68,15 +68,14 @@ func (e Expression) validate() error { return nil } - return errors.ErrKafkaInvalidTopicExpression.GenWithStackByArgs(e) + return errors.ErrKafkaInvalidConfig.GenWithStack("invalid topic expression: %s", e) } // ValidateForAvro checks whether topic pattern is {schema}_{table}, the only allowed func (e Expression) validateForAvro() error { if ok := avroTopicNameRE.MatchString(string(e)); !ok { - return errors.ErrKafkaInvalidTopicExpression.GenWithStackByArgs(e, - "topic rule for Avro must contain {schema} and {table}", - ) + return errors.ErrKafkaInvalidConfig.GenWithStack( + "invalid topic expression %s: topic rule for Avro must contain {schema} and {table}", e) } return nil diff --git a/downstreamadapter/sink/eventrouter/topic/expression_test.go b/downstreamadapter/sink/eventrouter/topic/expression_test.go index 82c75f265a..78151ad23d 100644 --- a/downstreamadapter/sink/eventrouter/topic/expression_test.go +++ b/downstreamadapter/sink/eventrouter/topic/expression_test.go @@ -265,11 +265,11 @@ func TestInvalidExpression(t *testing.T) { topicExpr := Expression(invalidExpr) err := topicExpr.validate() - require.ErrorIs(t, err, errors.ErrKafkaInvalidTopicExpression) + require.ErrorIs(t, err, errors.ErrKafkaInvalidConfig) require.ErrorContains(t, err, invalidExpr) err = topicExpr.validateForAvro() - require.ErrorIs(t, err, errors.ErrKafkaInvalidTopicExpression) + require.ErrorIs(t, err, errors.ErrKafkaInvalidConfig) require.ErrorContains(t, err, "Avro") require.ErrorContains(t, err, invalidExpr) } diff --git a/downstreamadapter/sink/kafka/helper.go b/downstreamadapter/sink/kafka/helper.go index c45b7eb10f..c66b62f96e 100644 --- a/downstreamadapter/sink/kafka/helper.go +++ b/downstreamadapter/sink/kafka/helper.go @@ -21,18 +21,17 @@ import ( "github.com/pingcap/ticdc/downstreamadapter/sink/eventrouter" "github.com/pingcap/ticdc/downstreamadapter/sink/helper" "github.com/pingcap/ticdc/downstreamadapter/sink/topicmanager" - commonType "github.com/pingcap/ticdc/pkg/common" + "github.com/pingcap/ticdc/pkg/common" "github.com/pingcap/ticdc/pkg/config" - "github.com/pingcap/ticdc/pkg/errors" "github.com/pingcap/ticdc/pkg/sink/codec" - "github.com/pingcap/ticdc/pkg/sink/codec/common" + codecCommon "github.com/pingcap/ticdc/pkg/sink/codec/common" "github.com/pingcap/ticdc/pkg/sink/kafka" "github.com/pingcap/tidb/br/pkg/utils" ) type components struct { encoderGroup codec.EncoderGroup - encoder common.EventEncoder + encoder codecCommon.EventEncoder columnSelector *columnselector.ColumnSelectors eventRouter *eventrouter.EventRouter topicManager topicmanager.TopicManager @@ -131,10 +130,11 @@ func newKafkaSinkComponentWithFactory(ctx context.Context, func newKafkaSinkComponent( ctx context.Context, - changefeedID commonType.ChangeFeedID, + changefeedID common.ChangeFeedID, sinkURI *url.URL, sinkConfig *config.SinkConfig, ) (components, config.Protocol, error) { +<<<<<<< HEAD return newKafkaSinkComponentWithFactory(ctx, changefeedID, sinkURI, sinkConfig, kafka.NewSaramaFactory) } @@ -145,4 +145,88 @@ func newKafkaSinkComponentForTest( sinkConfig *config.SinkConfig, ) (components, config.Protocol, error) { return newKafkaSinkComponentWithFactory(ctx, changefeedID, sinkURI, sinkConfig, kafka.NewMockFactory) +======= + var ( + comp components + err error + ) + // must release resources when error occurs. + defer func() { + if err != nil { + comp.close() + } + }() + protocol, err := helper.GetProtocol(utils.GetOrZero(sinkConfig.Protocol)) + if err != nil { + return comp, config.ProtocolUnknown, err + } + + topic, err := helper.GetTopic(sinkURI) + if err != nil { + return comp, protocol, err + } + + options := kafka.NewOptions() + if err = options.Apply(changefeedID, sinkURI, sinkConfig); err != nil { + return comp, protocol, err + } + options.Topic = topic + + comp.factory, err = kafka.NewSaramaFactory(ctx, options, changefeedID) + if err != nil { + return comp, protocol, err + } + + isAvroLike := protocol == config.ProtocolAvro || protocol == config.ProtocolDebeziumAvro + comp.eventRouter, err = eventrouter.NewEventRouter( + sinkConfig, topic, false, isAvroLike) + if err != nil { + return comp, protocol, err + } + + comp.columnSelector, err = columnselector.New(sinkConfig) + if err != nil { + return comp, protocol, err + } + + encoderConfig, err := helper.GetEncoderConfig( + changefeedID, sinkURI, protocol, sinkConfig, + options.MaxMessageBytes, options.MaxBatchedBytes, + ) + if err != nil { + return comp, protocol, err + } + + comp.claimCheck, err = claimcheck.New(ctx, encoderConfig.LargeMessageHandle, changefeedID) + if err != nil { + return comp, protocol, err + } + + comp.encoderGroup, err = codec.NewEncoderGroup(ctx, sinkConfig, encoderConfig, comp.claimCheck, changefeedID) + if err != nil { + return comp, protocol, err + } + + comp.encoder, err = codec.NewEventEncoder(ctx, encoderConfig, comp.claimCheck) + if err != nil { + return comp, protocol, err + } + + comp.adminClient, err = comp.factory.AdminClient(ctx) + if err != nil { + return comp, protocol, err + } + + comp.topicManager, err = topicmanager.GetTopicManagerAndTryCreateTopic( + ctx, + changefeedID, + topic, + options.DeriveTopicConfig(), + comp.adminClient, + ) + if err != nil { + return comp, protocol, err + } + return comp, protocol, nil +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) } diff --git a/downstreamadapter/sink/kafka/sink.go b/downstreamadapter/sink/kafka/sink.go index e6d1038b26..e7ee8cd08a 100644 --- a/downstreamadapter/sink/kafka/sink.go +++ b/downstreamadapter/sink/kafka/sink.go @@ -20,12 +20,17 @@ import ( "github.com/pingcap/log" "github.com/pingcap/ticdc/downstreamadapter/sink/helper" - commonType "github.com/pingcap/ticdc/pkg/common" + "github.com/pingcap/ticdc/pkg/common" commonEvent "github.com/pingcap/ticdc/pkg/common/event" "github.com/pingcap/ticdc/pkg/config" "github.com/pingcap/ticdc/pkg/errors" "github.com/pingcap/ticdc/pkg/metrics" +<<<<<<< HEAD "github.com/pingcap/ticdc/pkg/sink/codec/common" +======= + "github.com/pingcap/ticdc/pkg/sink/codec" + codecCommon "github.com/pingcap/ticdc/pkg/sink/codec/common" +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) "github.com/pingcap/ticdc/pkg/sink/kafka" "github.com/pingcap/ticdc/pkg/util" "github.com/pingcap/ticdc/utils/chann" @@ -40,7 +45,7 @@ const ( ) type sink struct { - changefeedID commonType.ChangeFeedID + changefeedID common.ChangeFeedID dmlProducer kafka.AsyncProducer ddlProducer kafka.SyncProducer @@ -63,10 +68,11 @@ type sink struct { ctx context.Context } -func (s *sink) SinkType() commonType.SinkType { - return commonType.KafkaSinkType +func (s *sink) SinkType() common.SinkType { + return common.KafkaSinkType } +<<<<<<< HEAD func Verify(ctx context.Context, changefeedID commonType.ChangeFeedID, uri *url.URL, sinkConfig *config.SinkConfig) error { comp, _, err := newKafkaSinkComponent(ctx, changefeedID, uri, sinkConfig) defer comp.close() @@ -75,17 +81,108 @@ func Verify(ctx context.Context, changefeedID commonType.ChangeFeedID, uri *url. func New( ctx context.Context, changefeedID commonType.ChangeFeedID, sinkURI *url.URL, sinkConfig *config.SinkConfig, +======= +func Verify(ctx context.Context, changefeedID common.ChangeFeedID, uri *url.URL, sinkConfig *config.SinkConfig) error { + protocol, err := helper.GetProtocol(util.GetOrZero(sinkConfig.Protocol)) + if err != nil { + return err + } + + topic, err := helper.GetTopic(uri) + if err != nil { + return err + } + + options := kafka.NewOptions() + if err = options.Apply(changefeedID, uri, sinkConfig); err != nil { + return err + } + options.Topic = topic + + encoderConfig, err := helper.GetEncoderConfig( + changefeedID, uri, protocol, sinkConfig, + options.MaxMessageBytes, options.MaxBatchedBytes, + ) + if err != nil { + return err + } + + claimCheck, err := claimcheck.New(ctx, encoderConfig.LargeMessageHandle, changefeedID) + if err != nil { + return err + } + defer claimCheck.Close() + + isAvroLike := protocol == config.ProtocolAvro || protocol == config.ProtocolDebeziumAvro + if _, err = eventrouter.NewEventRouter(sinkConfig, topic, false, isAvroLike); err != nil { + return err + } + + if _, err = columnselector.New(sinkConfig); err != nil { + return err + } + + factory, err := kafka.NewSaramaFactory(ctx, options, changefeedID) + if err != nil { + return err + } + + adminClient, err := factory.AdminClient(ctx) + if err != nil { + return err + } + defer adminClient.Close() + + topics, err := adminClient.GetTopicsMeta([]string{topic}, false) + if err != nil { + return err + } + if _, exists := topics[topic]; !exists { + topicConfig := options.DeriveTopicConfig() + if !topicConfig.AutoCreate { + return errors.ErrKafkaInvalidConfig.GenWithStack("`auto-create-topic` is false, and %s not found", topic) + } + if err = topicConfig.ValidateReplicationFactor(adminClient); err != nil { + return err + } + + // the topic is not created, only validate. + err = adminClient.CreateTopic(&kafka.TopicDetail{ + Name: topic, + NumPartitions: topicConfig.PartitionNum, + ReplicationFactor: topicConfig.ReplicationFactor, + }, true) + if err != nil { + return err + } + } + + _, err = codec.NewEventEncoder(ctx, encoderConfig, claimCheck) + if err != nil { + return err + } + return nil +} + +func New( + ctx context.Context, changefeedID common.ChangeFeedID, sinkURI *url.URL, sinkConfig *config.SinkConfig, keyspaceID uint32, +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) ) (*sink, error) { comp, protocol, err := newKafkaSinkComponent(ctx, changefeedID, sinkURI, sinkConfig) if err != nil { - return nil, errors.Trace(err) + return nil, err } return newWithComponents(ctx, changefeedID, protocol, comp) } func newWithComponents( ctx context.Context, +<<<<<<< HEAD changefeedID commonType.ChangeFeedID, +======= + changefeedID common.ChangeFeedID, + keyspaceID uint32, +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) protocol config.Protocol, comp components, ) (*sink, error) { @@ -153,7 +250,7 @@ func (s *sink) Run(ctx context.Context) error { }) err := g.Wait() s.isNormal.Store(false) - return errors.Trace(err) + return err } func (s *sink) IsNormal() bool { @@ -224,7 +321,7 @@ func (s *sink) calculateKeyPartitions(ctx context.Context) error { for { select { case <-ctx.Done(): - return errors.Trace(ctx.Err()) + return context.Cause(ctx) default: event, ok := s.eventChan.Get() if !ok { @@ -242,6 +339,7 @@ func (s *sink) calculateKeyPartitions(ctx context.Context) error { } partitionGenerator := s.comp.eventRouter.GetPartitionGenerator(schema, table) +<<<<<<< HEAD selector := s.comp.columnSelector.Get(schema, table) rowsCount := uint64(event.Len()) events := make([]*commonEvent.MQRowEvent, 0, rowsCount) @@ -277,6 +375,12 @@ func (s *sink) calculateKeyPartitions(ctx context.Context) error { Checksum: row.Checksum, }, }) +======= + selector := s.comp.columnSelector.GetForTableInfo(event.TableInfo) + events, err := helper.NewMQRowEvents(event, topic, partitionNum, partitionGenerator, selector) + if err != nil { + return err +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) } s.rowChan.Push(events...) } @@ -287,7 +391,7 @@ func (s *sink) nonBatchEncodeRun(ctx context.Context) error { for { select { case <-ctx.Done(): - return errors.Trace(ctx.Err()) + return context.Cause(ctx) default: event, ok := s.rowChan.Get() if !ok { @@ -380,7 +484,7 @@ func (s *sink) sendMessages(ctx context.Context) error { for { select { case <-ctx.Done(): - return errors.Trace(ctx.Err()) + return context.Cause(ctx) case future, ok := <-outCh: if !ok { log.Info("kafka sink encoder's output channel closed", @@ -429,7 +533,7 @@ func (s *sink) sendDDLEvent(event *commonEvent.DDLEvent) error { zap.Stringer("changefeed", s.changefeedID)) continue } - common.SetDDLMessageLogInfo(message, e) + codecCommon.SetDDLMessageLogInfo(message, e) topic := s.comp.eventRouter.GetTopicForDDL(e) // Notice: We must call GetPartitionNum here, // which will be responsible for automatically creating topics when they don't exist. @@ -479,14 +583,14 @@ func (s *sink) sendCheckpoint(ctx context.Context) error { }() var ( - msg *common.Message + msg *codecCommon.Message partitionNum int32 err error ) for { select { case <-ctx.Done(): - return errors.Trace(ctx.Err()) + return context.Cause(ctx) case ts, ok := <-s.checkpointChan: if !ok { log.Warn("kafka sink checkpoint channel closed", @@ -503,7 +607,7 @@ func (s *sink) sendCheckpoint(ctx context.Context) error { if msg == nil { continue } - common.SetCheckpointMessageLogInfo(msg, ts) + codecCommon.SetCheckpointMessageLogInfo(msg, ts) tableNames := s.getAllTableNames(ts) // NOTICE: When there are no tables to replicate, diff --git a/downstreamadapter/sink/kafka/sink_test.go b/downstreamadapter/sink/kafka/sink_test.go index 0bb4708f58..319008d618 100644 --- a/downstreamadapter/sink/kafka/sink_test.go +++ b/downstreamadapter/sink/kafka/sink_test.go @@ -25,13 +25,107 @@ import ( "github.com/pingcap/ticdc/pkg/common" commonEvent "github.com/pingcap/ticdc/pkg/common/event" "github.com/pingcap/ticdc/pkg/config" +<<<<<<< HEAD "github.com/pingcap/ticdc/pkg/metrics" +======= + "github.com/pingcap/ticdc/pkg/errors" + "github.com/pingcap/ticdc/pkg/sink/codec" + codecCommon "github.com/pingcap/ticdc/pkg/sink/codec/common" +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) "github.com/pingcap/ticdc/pkg/sink/kafka" "github.com/pingcap/ticdc/utils/chann" "github.com/stretchr/testify/require" "go.uber.org/atomic" ) +<<<<<<< HEAD +======= +const kafkaSinkTestTopic = "mock_topic" + +func TestSinkWorkersReturnContextError(t *testing.T) { + contexts := []struct { + name string + newContext func() (context.Context, context.CancelFunc) + cause error + }{ + { + name: "canceled", + newContext: func() (context.Context, context.CancelFunc) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + return ctx, cancel + }, + cause: context.Canceled, + }, + { + name: "deadline exceeded", + newContext: func() (context.Context, context.CancelFunc) { + return context.WithTimeout(context.Background(), 0) + }, + cause: context.DeadlineExceeded, + }, + } + workers := []struct { + name string + run func(*sink, context.Context) error + }{ + {name: "calculate key partitions", run: (*sink).calculateKeyPartitions}, + {name: "non batch encode", run: (*sink).nonBatchEncodeRun}, + {name: "checkpoint", run: (*sink).sendCheckpoint}, + } + + for _, worker := range workers { + for _, contextCase := range contexts { + t.Run(worker.name+"/"+contextCase.name, func(t *testing.T) { + ctx, cancel := contextCase.newContext() + defer cancel() + + err := worker.run(&sink{}, ctx) + + require.ErrorIs(t, err, contextCase.cause) + }) + } + } +} + +func TestVerifyInvalidConfig(t *testing.T) { + broker := sarama.NewMockBroker(t, 1) + defer broker.Close() + broker.SetHandlerByMap(map[string]sarama.MockResponse{ + "ApiVersionsRequest": sarama.NewMockApiVersionsResponse(t).SetApiKeys( + []sarama.ApiVersionsResponseKey{ + {ApiKey: 0}, + {ApiKey: 1}, + {ApiKey: 2}, + {ApiKey: 3, MaxVersion: 9}, + }), + "MetadataRequest": sarama.NewMockMetadataResponse(t). + SetController(broker.BrokerID()). + SetBroker(broker.Addr(), broker.BrokerID()). + SetLeader(kafkaSinkTestTopic, 0, broker.BrokerID()), + "DescribeConfigsRequest": sarama.NewMockDescribeConfigsResponse(t), + }) + + schemaRegistry := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + http.Error(w, "invalid response", http.StatusInternalServerError) + })) + defer schemaRegistry.Close() + + avroProtocol := config.ProtocolAvro.String() + sinkConfig := &config.SinkConfig{ + Protocol: &avroProtocol, + SchemaRegistry: &schemaRegistry.URL, + } + sinkURI, err := url.Parse("kafka://" + broker.Addr() + "/" + kafkaSinkTestTopic + + "?required-acks=1&kafka-version=2.4.0") + require.NoError(t, err) + + changefeedID := common.NewChangefeedID4Test("test", "verify-invalid-config") + err = Verify(context.Background(), changefeedID, sinkURI, sinkConfig) + require.ErrorContains(t, err, "ErrAvroSchemaAPIError") +} + +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) func newKafkaSinkForTestWithProducers(ctx context.Context, asyncProducer kafka.AsyncProducer, syncProducer kafka.SyncProducer, @@ -94,12 +188,33 @@ func newKafkaSinkForTestWithProducers(ctx context.Context, isNormal: atomic.NewBool(true), ctx: ctx, } - go s.Run(ctx) return s, nil } +<<<<<<< HEAD func newKafkaSinkForTest(ctx context.Context) (*sink, error) { return newKafkaSinkForTestWithProducers(ctx, nil, nil) +======= +func TestKafkaSinkRunReturnsAsyncProducerError(t *testing.T) { + ctx := t.Context() + + ctrl := gomock.NewController(t) + producerErr := errors.ErrKafkaSendMessage.GenWithStackByArgs() + asyncProducer := kafka.NewMockAsyncProducer(ctrl) + syncProducer := kafka.NewMockSyncProducer(ctrl) + asyncProducer.EXPECT().AsyncRunCallback(gomock.Any()).Return(producerErr) + asyncProducer.EXPECT().Close().AnyTimes() + syncProducer.EXPECT().Close().AnyTimes() + + kafkaSink, err := newKafkaSinkForTestWithProducers(ctx, t, ctrl, asyncProducer, syncProducer) + require.NoError(t, err) + defer kafkaSink.Close() + + err = kafkaSink.Run(ctx) + + require.ErrorIs(t, err, errors.ErrKafkaSendMessage) + require.False(t, kafkaSink.IsNormal()) +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) } func TestKafkaSinkBasicFunctionality(t *testing.T) { @@ -153,9 +268,34 @@ func TestKafkaSinkBasicFunctionality(t *testing.T) { dmlEvent.CommitTs = 2 ctx, cancel := context.WithCancel(context.Background()) +<<<<<<< HEAD kafkaSink, err := newKafkaSinkForTest(ctx) +======= + ctrl := gomock.NewController(t) + asyncProducer := kafka.NewMockAsyncProducer(ctrl) + syncProducer := kafka.NewMockSyncProducer(ctrl) + asyncProducer.EXPECT().AsyncRunCallback(gomock.Any()).Return(nil).AnyTimes() + asyncProducer.EXPECT().AsyncSend(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + DoAndReturn(func( + _ context.Context, + _ string, + _ int32, + message *codecCommon.Message, + ) error { + if message.Callback != nil { + message.Callback() + } + return nil + }).Times(2) + asyncProducer.EXPECT().Close().AnyTimes() + syncProducer.EXPECT().SendMessages(gomock.Any(), int32(1), gomock.Any()).Return(nil) + syncProducer.EXPECT().Close().AnyTimes() + + kafkaSink, err := newKafkaSinkForTestWithProducers(ctx, t, ctrl, asyncProducer, syncProducer) +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) require.NoError(t, err) defer cancel() + go kafkaSink.Run(ctx) kafkaSink.ddlProducer.(*kafka.MockSaramaSyncProducer).SyncProducer.ExpectSendMessageAndSucceed() err = kafkaSink.WriteBlockEvent(ddlEvent) diff --git a/downstreamadapter/sink/pulsar/helper.go b/downstreamadapter/sink/pulsar/helper.go index acc1cddd38..cb80f8b0ff 100644 --- a/downstreamadapter/sink/pulsar/helper.go +++ b/downstreamadapter/sink/pulsar/helper.go @@ -23,11 +23,11 @@ import ( "github.com/pingcap/ticdc/downstreamadapter/sink/eventrouter" "github.com/pingcap/ticdc/downstreamadapter/sink/helper" "github.com/pingcap/ticdc/downstreamadapter/sink/topicmanager" - commonType "github.com/pingcap/ticdc/pkg/common" + "github.com/pingcap/ticdc/pkg/common" "github.com/pingcap/ticdc/pkg/config" "github.com/pingcap/ticdc/pkg/errors" "github.com/pingcap/ticdc/pkg/sink/codec" - "github.com/pingcap/ticdc/pkg/sink/codec/common" + codecCommon "github.com/pingcap/ticdc/pkg/sink/codec/common" "github.com/pingcap/ticdc/pkg/sink/pulsar" putil "github.com/pingcap/ticdc/pkg/util" "go.uber.org/zap" @@ -36,7 +36,7 @@ import ( type component struct { config *config.PulsarConfig encoderGroup codec.EncoderGroup - encoder common.EventEncoder + encoder codecCommon.EventEncoder columnSelector *columnselector.ColumnSelectors eventRouter *eventrouter.EventRouter topicManager topicmanager.TopicManager @@ -54,7 +54,7 @@ func (c component) close() { func newPulsarSinkComponent( ctx context.Context, - changefeedID commonType.ChangeFeedID, + changefeedID common.ChangeFeedID, sinkURI *url.URL, sinkConfig *config.SinkConfig, ) (component, config.Protocol, error) { @@ -63,7 +63,7 @@ func newPulsarSinkComponent( func newPulsarSinkComponentForTest( ctx context.Context, - changefeedID commonType.ChangeFeedID, + changefeedID common.ChangeFeedID, sinkURI *url.URL, sinkConfig *config.SinkConfig, ) (component, config.Protocol, error) { @@ -71,7 +71,7 @@ func newPulsarSinkComponentForTest( } func newPulsarSinkComponentWithFactory(ctx context.Context, - changefeedID commonType.ChangeFeedID, + changefeedID common.ChangeFeedID, sinkURI *url.URL, sinkConfig *config.SinkConfig, factoryCreator pulsar.FactoryCreator, @@ -98,7 +98,7 @@ func newPulsarSinkComponentWithFactory(ctx context.Context, pulsarComponent.client, err = factoryCreator(pulsarComponent.config, changefeedID, sinkConfig) if err != nil { - return pulsarComponent, protocol, errors.WrapError(errors.ErrKafkaNewProducer, err) + return pulsarComponent, protocol, errors.WrapError(errors.ErrPulsarNewProducer, err) } topic, err := helper.GetTopic(sinkURI) diff --git a/downstreamadapter/sink/topicmanager/kafka_topic_manager.go b/downstreamadapter/sink/topicmanager/kafka_topic_manager.go index 8e92167327..c44b5c853e 100644 --- a/downstreamadapter/sink/topicmanager/kafka_topic_manager.go +++ b/downstreamadapter/sink/topicmanager/kafka_topic_manager.go @@ -63,7 +63,11 @@ func GetTopicManagerAndTryCreateTopic( ) if _, err := topicManager.CreateTopicAndWaitUntilVisible(ctx, topic); err != nil { +<<<<<<< HEAD return nil, cerror.WrapError(cerror.ErrKafkaCreateTopic, err) +======= + return nil, err +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) } return topicManager, nil @@ -104,7 +108,7 @@ func (m *kafkaTopicManager) GetPartitionNum( // If the topic is not in the metadata, we try to create the topic. partitionNum, err := m.CreateTopicAndWaitUntilVisible(ctx, topic) if err != nil { - return 0, errors.Trace(err) + return 0, err } return partitionNum, nil @@ -264,7 +268,11 @@ func (m *kafkaTopicManager) createTopic( zap.Error(err), zap.Duration("duration", time.Since(start)), ) +<<<<<<< HEAD return 0, cerror.WrapError(cerror.ErrKafkaCreateTopic, err) +======= + return 0, err +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) } log.Info( @@ -290,7 +298,14 @@ func (m *kafkaTopicManager) CreateTopicAndWaitUntilVisible( // which means we should create the topic later. topicDetails, err := m.admin.GetTopicsMeta([]string{topicName}, true) if err != nil { +<<<<<<< HEAD return 0, errors.Trace(err) +======= + if kafka.IsAdminAuthorizationFailed(err) { + return m.useConfiguredPartitionNum(topicName, err), nil + } + return 0, err +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) } if detail, ok := topicDetails[topicName]; ok { numPartition := detail.NumPartitions @@ -303,12 +318,19 @@ func (m *kafkaTopicManager) CreateTopicAndWaitUntilVisible( partitionNum, err := m.createTopic(ctx, topicName) if err != nil { +<<<<<<< HEAD return 0, errors.Trace(err) +======= + if kafka.IsAdminAuthorizationFailed(err) { + return m.useConfiguredPartitionNum(topicName, err), nil + } + return 0, err +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) } err = m.waitUntilTopicVisible(ctx, topicName) if err != nil { - return 0, errors.Trace(err) + return 0, err } return partitionNum, nil diff --git a/downstreamadapter/sink/topicmanager/kafka_topic_manager_test.go b/downstreamadapter/sink/topicmanager/kafka_topic_manager_test.go index bf02658b24..1a2bd7eba1 100644 --- a/downstreamadapter/sink/topicmanager/kafka_topic_manager_test.go +++ b/downstreamadapter/sink/topicmanager/kafka_topic_manager_test.go @@ -18,6 +18,7 @@ import ( "testing" "github.com/pingcap/ticdc/pkg/common" + "github.com/pingcap/ticdc/pkg/errors" "github.com/pingcap/ticdc/pkg/sink/kafka" "github.com/stretchr/testify/require" ) @@ -35,7 +36,56 @@ func TestCreateTopic(t *testing.T) { changefeedID := common.NewChangefeedID4Test("test", "test") ctx := context.Background() +<<<<<<< HEAD manager := newKafkaTopicManager(ctx, kafka.DefaultMockTopicName, changefeedID, adminClient, cfg) +======= + var gotNewTopicDetail *kafka.TopicDetail + var gotNewTopicValidateOnly bool + var gotFailedTopicDetail *kafka.TopicDetail + var gotFailedTopicValidateOnly bool + gomock.InOrder( + adminClient.EXPECT().GetTopicsMeta([]string{kafkaTopicManagerTestTopic}, true).Return( + map[string]kafka.TopicDetail{ + kafkaTopicManagerTestTopic: { + Name: kafkaTopicManagerTestTopic, + NumPartitions: 2, + }, + }, nil), + adminClient.EXPECT().GetTopicsMeta([]string{"new-topic"}, true).Return( + map[string]kafka.TopicDetail{}, nil), + adminClient.EXPECT().GetTopicsMeta([]string{"new-topic"}, false).Return( + map[string]kafka.TopicDetail{}, nil), + adminClient.EXPECT().CreateTopic(gomock.Any(), false).DoAndReturn( + func(detail *kafka.TopicDetail, validateOnly bool) error { + gotNewTopicDetail = detail + gotNewTopicValidateOnly = validateOnly + return nil + }), + adminClient.EXPECT().GetTopicsMeta([]string{"new-topic"}, false).Return( + map[string]kafka.TopicDetail{ + "new-topic": { + Name: "new-topic", + NumPartitions: 2, + }, + }, nil), + adminClient.EXPECT().GetTopicsMeta([]string{"new-topic2"}, true).Return( + map[string]kafka.TopicDetail{}, nil), + adminClient.EXPECT().GetTopicsMeta([]string{"new-topic2"}, false).Return( + map[string]kafka.TopicDetail{}, nil), + adminClient.EXPECT().GetTopicsMeta([]string{"new-topic-failed"}, true).Return( + map[string]kafka.TopicDetail{}, nil), + adminClient.EXPECT().GetTopicsMeta([]string{"new-topic-failed"}, false).Return( + map[string]kafka.TopicDetail{}, nil), + adminClient.EXPECT().CreateTopic(gomock.Any(), false).DoAndReturn( + func(detail *kafka.TopicDetail, validateOnly bool) error { + gotFailedTopicDetail = detail + gotFailedTopicValidateOnly = validateOnly + return errors.WrapError(errors.ErrKafkaAdminAPI, sarama.ErrInvalidReplicationFactor, "create-topic", detail.Name) + }), + ) + + manager := newKafkaTopicManager(ctx, kafkaTopicManagerTestTopic, changefeedID, adminClient, cfg) +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) defer manager.Close() partitionNum, err := manager.CreateTopicAndWaitUntilVisible(ctx, kafka.DefaultMockTopicName) require.NoError(t, err) @@ -70,18 +120,64 @@ func TestCreateTopic(t *testing.T) { manager = newKafkaTopicManager(ctx, topic, changefeedID, adminClient, cfg) defer manager.Close() _, err = manager.CreateTopicAndWaitUntilVisible(ctx, topic) +<<<<<<< HEAD require.Regexp( t, "kafka create topic failed: kafka server: Replication-factor is invalid", err, ) +======= + require.ErrorIs(t, err, errors.ErrKafkaAdminAPI) + require.ErrorIs(t, err, sarama.ErrInvalidReplicationFactor) + require.NotNil(t, gotFailedTopicDetail) + require.Equal(t, "new-topic-failed", gotFailedTopicDetail.Name) + require.False(t, gotFailedTopicValidateOnly) +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) } func TestCreateTopicWithDelay(t *testing.T) { t.Parallel() +<<<<<<< HEAD adminClient := kafka.NewClusterAdminClientMockImpl() defer adminClient.Close() +======= + ctrl := gomock.NewController(t) + adminClient := kafka.NewMockClusterAdminClient(ctrl) + topic := "new-topic" + gomock.InOrder( + adminClient.EXPECT().GetTopicsMeta([]string{topic}, true). + Return(map[string]kafka.TopicDetail{}, nil), + adminClient.EXPECT().GetTopicsMeta([]string{topic}, false). + Return(map[string]kafka.TopicDetail{}, nil), + adminClient.EXPECT().GetBrokerConfig(kafka.MinInsyncReplicasConfigName). + Return("2", true, nil), + ) + + manager := newKafkaTopicManager( + context.Background(), + topic, + common.NewChangefeedID4Test("test", "test"), + adminClient, + &kafka.AutoCreateTopicConfig{ + AutoCreate: true, + PartitionNum: 2, + ReplicationFactor: 1, + RequiredAcks: kafka.WaitForAll, + }, + ) + defer manager.Close() + + _, err := manager.CreateTopicAndWaitUntilVisible(context.Background(), topic) + require.ErrorContains(t, err, "`replication-factor` 1 is smaller than the `min.insync.replicas` 2 of broker") +} + +func TestCreateTopicWaitsUntilVisible(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + adminClient := kafka.NewMockClusterAdminClient(ctrl) +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) cfg := &kafka.AutoCreateTopicConfig{ AutoCreate: true, PartitionNum: 2, diff --git a/pkg/errors/error.go b/pkg/errors/error.go index e15d05e818..e75864ce9f 100644 --- a/pkg/errors/error.go +++ b/pkg/errors/error.go @@ -123,50 +123,21 @@ var ( "kafka send message failed", errors.RFCCodeText("CDC:ErrKafkaSendMessage"), ) - ErrKafkaProducerClosed = errors.Normalize( - "kafka producer closed", - errors.RFCCodeText("CDC:ErrKafkaProducerClosed"), + ErrKafkaSinkClosed = errors.Normalize( + "kafka sink closed", + errors.RFCCodeText("CDC:ErrKafkaSinkClosed"), ) - ErrKafkaAsyncSendMessage = errors.Normalize( - "kafka async send message failed", - errors.RFCCodeText("CDC:ErrKafkaAsyncSendMessage"), - ) - ErrKafkaInvalidPartitionNum = errors.Normalize( - "invalid partition num %d", - errors.RFCCodeText("CDC:ErrKafkaInvalidPartitionNum"), - ) - ErrKafkaInvalidRequiredAcks = errors.Normalize( - "invalid required acks %d, "+ - "only support these values: 0(NoResponse),1(WaitForLocal) and -1(WaitForAll)", - errors.RFCCodeText("CDC:ErrKafkaInvalidRequiredAcks"), - ) - ErrKafkaNewProducer = errors.Normalize( - "new kafka producer", - errors.RFCCodeText("CDC:ErrKafkaNewProducer"), - ) - ErrKafkaInvalidClientID = errors.Normalize( - "invalid kafka client ID '%s'", - errors.RFCCodeText("CDC:ErrKafkaInvalidClientID"), - ) - ErrKafkaInvalidVersion = errors.Normalize( - "invalid kafka version", - errors.RFCCodeText("CDC:ErrKafkaInvalidVersion"), + ErrNewKafkaSink = errors.Normalize( + "new kafka sink", + errors.RFCCodeText("CDC:ErrNewKafkaSink"), ) ErrKafkaInvalidConfig = errors.Normalize( "kafka config invalid", errors.RFCCodeText("CDC:ErrKafkaInvalidConfig"), ) - ErrKafkaCreateTopic = errors.Normalize( - "kafka create topic failed", - errors.RFCCodeText("CDC:ErrKafkaCreateTopic"), - ) - ErrKafkaInvalidTopicExpression = errors.Normalize( - "invalid topic expression: %s ", - errors.RFCCodeText("CDC:ErrKafkaTopicExprInvalid"), - ) - ErrKafkaConfigNotFound = errors.Normalize( - "kafka config item not found", - errors.RFCCodeText("CDC:ErrKafkaConfigNotFound"), + ErrKafkaAdminAPI = errors.Normalize( + "kafka admin API %s failed: %s", + errors.RFCCodeText("CDC:ErrKafkaAdminAPI"), ) ErrPulsarInvalidTopicExpression = errors.Normalize( "invalid topic expression", diff --git a/pkg/errors/error_test.go b/pkg/errors/error_test.go index e040eedc67..2c76ca8978 100644 --- a/pkg/errors/error_test.go +++ b/pkg/errors/error_test.go @@ -98,6 +98,26 @@ func TestShouldFailChangefeed(t *testing.T) { err: ErrKafkaInvalidConfig.GenWithStackByArgs("invalid config"), expected: true, }, + { + name: "ErrNewKafkaSink should return false", + err: ErrNewKafkaSink.GenWithStackByArgs(), + expected: false, + }, + { + name: "ErrKafkaAdminAPI should return false", + err: ErrKafkaAdminAPI.GenWithStackByArgs("describe-topic", "test-topic"), + expected: false, + }, + { + name: "ErrKafkaSendMessage should return false", + err: ErrKafkaSendMessage.GenWithStackByArgs(), + expected: false, + }, + { + name: "ErrKafkaSinkClosed should return false", + err: ErrKafkaSinkClosed.GenWithStackByArgs(), + expected: false, + }, { name: "ErrMySQLInvalidConfig should return true", err: ErrMySQLInvalidConfig.GenWithStackByArgs("invalid config"), diff --git a/pkg/sink/kafka/admin.go b/pkg/sink/kafka/admin.go index b8cfd1cfc3..b3d9029511 100644 --- a/pkg/sink/kafka/admin.go +++ b/pkg/sink/kafka/admin.go @@ -58,10 +58,10 @@ func (a *saramaAdminClient) GetAllBrokers() []Broker { return result } -func (a *saramaAdminClient) GetBrokerConfig(configName string) (string, error) { +func (a *saramaAdminClient) GetBrokerConfig(configName string) (string, bool, error) { _, controller, err := a.admin.DescribeCluster() if err != nil { - return "", errors.Trace(err) + return "", false, errors.WrapError(errors.ErrKafkaAdminAPI, err, "describe-cluster", "cluster") } configEntries, err := a.admin.DescribeConfig(sarama.ConfigResource{ @@ -70,7 +70,7 @@ func (a *saramaAdminClient) GetBrokerConfig(configName string) (string, error) { ConfigNames: []string{configName}, }) if err != nil { - return "", errors.Trace(err) + return "", false, errors.WrapError(errors.ErrKafkaAdminAPI, err, "describe-config", configName) } // For compatibility with KOP, we checked all return values. @@ -78,7 +78,7 @@ func (a *saramaAdminClient) GetBrokerConfig(configName string) (string, error) { // 2. Kop returns all configs. for _, entry := range configEntries { if entry.Name == configName { - return entry.Value, nil + return entry.Value, true, nil } } @@ -86,18 +86,17 @@ func (a *saramaAdminClient) GetBrokerConfig(configName string) (string, error) { zap.String("keyspace", a.changefeed.Keyspace()), zap.String("changefeed", a.changefeed.Name()), zap.String("configName", configName)) - return "", errors.ErrKafkaConfigNotFound.GenWithStack( - "cannot find the `%s` from the broker's configuration", configName) + return "", false, nil } -func (a *saramaAdminClient) GetTopicConfig(topicName string, configName string) (string, error) { +func (a *saramaAdminClient) GetTopicConfig(topicName string, configName string) (string, bool, error) { configEntries, err := a.admin.DescribeConfig(sarama.ConfigResource{ Type: sarama.TopicResource, Name: topicName, ConfigNames: []string{configName}, }) if err != nil { - return "", errors.Trace(err) + return "", false, errors.WrapError(errors.ErrKafkaAdminAPI, err, "describe-config", topicName) } // For compatibility with KOP, we checked all return values. @@ -110,7 +109,7 @@ func (a *saramaAdminClient) GetTopicConfig(topicName string, configName string) zap.String("changefeed", a.changefeed.Name()), zap.String("configName", configName), zap.String("configValue", entry.Value)) - return entry.Value, nil + return entry.Value, true, nil } } @@ -118,8 +117,7 @@ func (a *saramaAdminClient) GetTopicConfig(topicName string, configName string) zap.String("keyspace", a.changefeed.Keyspace()), zap.String("changefeed", a.changefeed.Name()), zap.String("configName", configName)) - return "", errors.ErrKafkaConfigNotFound.GenWithStack( - "cannot find the `%s` from the topic's configuration", configName) + return "", false, nil } func (a *saramaAdminClient) GetTopicsMeta(topics []string, ignoreTopicError bool) (map[string]TopicDetail, error) { @@ -127,7 +125,7 @@ func (a *saramaAdminClient) GetTopicsMeta(topics []string, ignoreTopicError bool metaList, err := a.admin.DescribeTopics(topics) if err != nil { - return nil, errors.Trace(err) + return nil, errors.WrapError(errors.ErrKafkaAdminAPI, err, "describe-topics", strings.Join(topics, ",")) } for _, meta := range metaList { @@ -136,7 +134,7 @@ func (a *saramaAdminClient) GetTopicsMeta(topics []string, ignoreTopicError bool continue } if !ignoreTopicError { - return nil, meta.Err + return nil, errors.WrapError(errors.ErrKafkaAdminAPI, meta.Err, "describe-topic", meta.Name) } log.Warn("fetch topic meta failed", zap.String("keyspace", a.changefeed.Keyspace()), @@ -158,7 +156,7 @@ func (a *saramaAdminClient) GetTopicsPartitionsNum(topics []string) (map[string] for _, topic := range topics { partition, err := a.client.Partitions(topic) if err != nil { - return nil, errors.Trace(err) + return nil, errors.WrapError(errors.ErrKafkaAdminAPI, err, "list-partitions", topic) } result[topic] = int32(len(partition)) } @@ -175,7 +173,7 @@ func (a *saramaAdminClient) CreateTopic(detail *TopicDetail, validateOnly bool) err := a.admin.CreateTopic(detail.Name, request, validateOnly) // Ignore the already exists error because it's not harmful. if err != nil && !strings.Contains(err.Error(), sarama.ErrTopicAlreadyExists.Error()) { - return err + return errors.WrapError(errors.ErrKafkaAdminAPI, err, "create-topic", detail.Name) } return nil } diff --git a/pkg/sink/kafka/admin_test.go b/pkg/sink/kafka/admin_test.go index ecab2e122d..4873f7ef45 100644 --- a/pkg/sink/kafka/admin_test.go +++ b/pkg/sink/kafka/admin_test.go @@ -14,13 +14,20 @@ package kafka import ( + "io" "testing" "github.com/IBM/sarama" +<<<<<<< HEAD +======= + "github.com/golang/mock/gomock" +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) "github.com/pingcap/ticdc/pkg/common" + "github.com/pingcap/ticdc/pkg/errors" "github.com/stretchr/testify/require" ) +<<<<<<< HEAD type testSaramaClient struct { closed bool } @@ -88,4 +95,83 @@ func TestSaramaAdminClientCloseFallsBackToClientWhenAdminIsNil(t *testing.T) { } require.NotPanics(t, func() { a.Close() }) require.True(t, client.closed) +======= +func TestGetBrokerConfig(t *testing.T) { + t.Parallel() + + t.Run("not found", func(t *testing.T) { + ctrl := gomock.NewController(t) + admin := NewMocksaramaClusterAdmin(ctrl) + admin.EXPECT().DescribeCluster().Return(nil, int32(1), nil) + admin.EXPECT().DescribeConfig(gomock.Any()).Return([]sarama.ConfigEntry{}, nil) + + client := &saramaAdminClient{ + changefeed: common.NewChangeFeedIDWithName("test", "default"), + admin: admin, + } + value, found, err := client.GetBrokerConfig("missing") + + require.NoError(t, err) + require.False(t, found) + require.Empty(t, value) + }) + + t.Run("admin error", func(t *testing.T) { + ctrl := gomock.NewController(t) + admin := NewMocksaramaClusterAdmin(ctrl) + cause := io.ErrUnexpectedEOF + admin.EXPECT().DescribeCluster().Return(nil, int32(0), cause) + + client := &saramaAdminClient{ + changefeed: common.NewChangeFeedIDWithName("test", "default"), + admin: admin, + } + _, _, err := client.GetBrokerConfig("missing") + + require.ErrorIs(t, err, errors.ErrKafkaAdminAPI) + require.ErrorIs(t, err, cause) + }) +} + +func TestAdminClientClose(t *testing.T) { + tests := []struct { + name string + setup func(*gomock.Controller) *saramaAdminClient + }{ + { + name: "uses admin close", + setup: func(ctrl *gomock.Controller) *saramaAdminClient { + client := NewMocksaramaClient(ctrl) + admin := NewMocksaramaClusterAdmin(ctrl) + admin.EXPECT().Close().Return(nil) + client.EXPECT().Close().Times(0) + return &saramaAdminClient{ + changefeed: common.NewChangeFeedIDWithName("test", "default"), + client: client, + admin: admin, + } + }, + }, + { + name: "falls back to client when admin is nil", + setup: func(ctrl *gomock.Controller) *saramaAdminClient { + client := NewMocksaramaClient(ctrl) + client.EXPECT().Close().Return(nil) + return &saramaAdminClient{ + changefeed: common.NewChangeFeedIDWithName("test", "default"), + client: client, + } + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + adminClient := test.setup(ctrl) + + require.NotPanics(t, func() { adminClient.Close() }) + }) + } +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) } diff --git a/pkg/sink/kafka/claimcheck/claim_check.go b/pkg/sink/kafka/claimcheck/claim_check.go index 052785e2fa..50f1ec8356 100644 --- a/pkg/sink/kafka/claimcheck/claim_check.go +++ b/pkg/sink/kafka/claimcheck/claim_check.go @@ -22,9 +22,14 @@ import ( "github.com/google/uuid" "github.com/pingcap/errors" "github.com/pingcap/log" - commonType "github.com/pingcap/ticdc/pkg/common" + "github.com/pingcap/ticdc/pkg/common" "github.com/pingcap/ticdc/pkg/config" +<<<<<<< HEAD "github.com/pingcap/ticdc/pkg/sink/codec/common" +======= + "github.com/pingcap/ticdc/pkg/errors" + codecCommon "github.com/pingcap/ticdc/pkg/sink/codec/common" +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) "github.com/pingcap/ticdc/pkg/util" "github.com/pingcap/tidb/br/pkg/storage" "github.com/prometheus/client_golang/prometheus" @@ -40,7 +45,7 @@ type ClaimCheck struct { storage storage.ExternalStorage rawValue bool - changefeedID commonType.ChangeFeedID + changefeedID common.ChangeFeedID // metricSendMessageDuration tracks the time duration // cost on send messages to the claim check external storage. metricSendMessageDuration prometheus.Observer @@ -48,7 +53,7 @@ type ClaimCheck struct { } // New return a new ClaimCheck. -func New(ctx context.Context, config *config.LargeMessageHandleConfig, changefeedID commonType.ChangeFeedID) (*ClaimCheck, error) { +func New(ctx context.Context, config *config.LargeMessageHandleConfig, changefeedID common.ChangeFeedID) (*ClaimCheck, error) { if !config.EnableClaimCheck() { return nil, nil } @@ -67,7 +72,7 @@ func New(ctx context.Context, config *config.LargeMessageHandleConfig, changefee zap.String("storageURI", util.MaskSensitiveDataInURI(config.ClaimCheckStorageURI)), zap.Duration("duration", time.Since(start)), zap.Error(err)) - return nil, errors.Trace(err) + return nil, err } log.Info("claim-check create the external storage success", @@ -88,19 +93,19 @@ func New(ctx context.Context, config *config.LargeMessageHandleConfig, changefee // WriteMessage write message to the claim check external storage. func (c *ClaimCheck) WriteMessage(ctx context.Context, key, value []byte, fileName string) (err error) { if !c.rawValue { - m := common.ClaimCheckMessage{ + m := codecCommon.ClaimCheckMessage{ Key: key, Value: value, } value, err = json.Marshal(m) if err != nil { - return errors.Trace(err) + return errors.WrapError(errors.ErrMarshalFailed, err) } } start := time.Now() err = c.storage.WriteFile(ctx, fileName, value) if err != nil { - return errors.Trace(err) + return err } c.metricSendMessageDuration.Observe(time.Since(start).Seconds()) c.metricSendMessageCount.Inc() diff --git a/pkg/sink/kafka/claimcheck/claim_check_test.go b/pkg/sink/kafka/claimcheck/claim_check_test.go new file mode 100644 index 0000000000..080e62de0f --- /dev/null +++ b/pkg/sink/kafka/claimcheck/claim_check_test.go @@ -0,0 +1,110 @@ +// Copyright 2023 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// See the License for the specific language governing permissions and +// limitations under the License. + +package claimcheck + +import ( + "context" + "fmt" + "strings" + "testing" + + "github.com/pingcap/ticdc/pkg/common" + "github.com/pingcap/ticdc/pkg/config" + "github.com/pingcap/ticdc/pkg/errors" + "github.com/pingcap/tidb/pkg/objstore" + "github.com/pingcap/tidb/pkg/objstore/mockobjstore" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + "golang.org/x/sync/errgroup" +) + +func TestClaimCheck(t *testing.T) { + t.Parallel() + + ctx := context.Background() + + changefeedID := common.NewChangeFeedIDWithName("test", "") + largeHandleConfig := config.NewDefaultLargeMessageHandleConfig() + + claimCheck, err := New(ctx, largeHandleConfig, changefeedID) + require.NoError(t, err) + require.Nil(t, claimCheck) + + largeHandleConfig.LargeMessageHandleOption = config.LargeMessageHandleOptionClaimCheck + largeHandleConfig.ClaimCheckStorageURI = "file:///tmp/abc/" + claimCheck, err = New(ctx, largeHandleConfig, changefeedID) + require.NoError(t, err) + t.Cleanup(claimCheck.Close) + + fileName := claimCheck.FileNameWithPrefix("file.json") + require.Equal(t, "file:///tmp/abc/file.json", fileName) +} + +func TestClaimCheckStorageErrorWrappedOnce(t *testing.T) { + largeHandleConfig := config.NewDefaultLargeMessageHandleConfig() + largeHandleConfig.LargeMessageHandleOption = config.LargeMessageHandleOptionClaimCheck + largeHandleConfig.ClaimCheckStorageURI = "invalid://bucket" + + claimCheck, err := New(context.Background(), largeHandleConfig, common.NewChangeFeedIDWithName("test", "default")) + + require.Nil(t, claimCheck) + require.ErrorIs(t, err, errors.ErrExternalStorageAPI) + require.Equal(t, 1, strings.Count(err.Error(), string(errors.ErrExternalStorageAPI.RFCCode()))) +} + +func TestClaimCheckCloseClosesStorage(t *testing.T) { + var nilClaimCheck *ClaimCheck + require.NotPanics(t, nilClaimCheck.Close) + + ctrl := gomock.NewController(t) + storage := mockobjstore.NewMockStorage(ctrl) + storage.EXPECT().Close().Times(1) + claimCheck := &ClaimCheck{ + storage: storage, + changefeedID: common.NewChangeFeedIDWithName("test", "default"), + } + + claimCheck.Close() +} + +func TestClaimCheckConcurrentWrites(t *testing.T) { + ctx := context.Background() + storage := objstore.NewMemStorage() + changefeedID := common.NewChangeFeedIDWithName("test", "default") + claimCheck := &ClaimCheck{ + storage: storage, + rawValue: true, + changefeedID: changefeedID, + metricSendMessageDuration: claimCheckSendMessageDuration.WithLabelValues(changefeedID.Keyspace(), changefeedID.Name()), + metricSendMessageCount: claimCheckSendMessageCount.WithLabelValues(changefeedID.Keyspace(), changefeedID.Name()), + } + t.Cleanup(claimCheck.Close) + + const concurrency = 32 + group := new(errgroup.Group) + for i := range concurrency { + fileName := fmt.Sprintf("%d.json", i) + group.Go(func() error { + return claimCheck.WriteMessage(ctx, nil, []byte(fileName), fileName) + }) + } + require.NoError(t, group.Wait()) + + for i := range concurrency { + fileName := fmt.Sprintf("%d.json", i) + data, err := storage.ReadFile(ctx, fileName) + require.NoError(t, err) + require.Equal(t, fileName, string(data)) + } +} diff --git a/pkg/sink/kafka/cluster_admin_client.go b/pkg/sink/kafka/cluster_admin_client.go index 3c6c331c88..4f1ff36996 100644 --- a/pkg/sink/kafka/cluster_admin_client.go +++ b/pkg/sink/kafka/cluster_admin_client.go @@ -31,11 +31,11 @@ type ClusterAdminClient interface { // GetAllBrokers return all brokers among the cluster GetAllBrokers() []Broker - // GetBrokerConfig return the broker level configuration with the `configName` - GetBrokerConfig(configName string) (string, error) + // GetBrokerConfig returns the broker-level configuration and whether it exists. + GetBrokerConfig(configName string) (value string, found bool, err error) - // GetTopicConfig return the topic level configuration with the `configName` - GetTopicConfig(topicName string, configName string) (string, error) + // GetTopicConfig returns the topic-level configuration and whether it exists. + GetTopicConfig(topicName string, configName string) (value string, found bool, err error) // GetTopicsMeta return all target topics' metadata // if `ignoreTopicError` is true, ignore the topic error and return the metadata of valid topics diff --git a/pkg/sink/kafka/cluster_admin_client_mock.go b/pkg/sink/kafka/cluster_admin_client_mock.go new file mode 100644 index 0000000000..dfeebbd773 --- /dev/null +++ b/pkg/sink/kafka/cluster_admin_client_mock.go @@ -0,0 +1,136 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: pkg/sink/kafka/cluster_admin_client.go + +// Package kafka is a generated GoMock package. +package kafka + +import ( + reflect "reflect" + + gomock "github.com/golang/mock/gomock" +) + +// MockClusterAdminClient is a mock of ClusterAdminClient interface. +type MockClusterAdminClient struct { + ctrl *gomock.Controller + recorder *MockClusterAdminClientMockRecorder +} + +// MockClusterAdminClientMockRecorder is the mock recorder for MockClusterAdminClient. +type MockClusterAdminClientMockRecorder struct { + mock *MockClusterAdminClient +} + +// NewMockClusterAdminClient creates a new mock instance. +func NewMockClusterAdminClient(ctrl *gomock.Controller) *MockClusterAdminClient { + mock := &MockClusterAdminClient{ctrl: ctrl} + mock.recorder = &MockClusterAdminClientMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockClusterAdminClient) EXPECT() *MockClusterAdminClientMockRecorder { + return m.recorder +} + +// Close mocks base method. +func (m *MockClusterAdminClient) Close() { + m.ctrl.T.Helper() + m.ctrl.Call(m, "Close") +} + +// Close indicates an expected call of Close. +func (mr *MockClusterAdminClientMockRecorder) Close() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Close", reflect.TypeOf((*MockClusterAdminClient)(nil).Close)) +} + +// CreateTopic mocks base method. +func (m *MockClusterAdminClient) CreateTopic(detail *TopicDetail, validateOnly bool) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CreateTopic", detail, validateOnly) + ret0, _ := ret[0].(error) + return ret0 +} + +// CreateTopic indicates an expected call of CreateTopic. +func (mr *MockClusterAdminClientMockRecorder) CreateTopic(detail, validateOnly interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateTopic", reflect.TypeOf((*MockClusterAdminClient)(nil).CreateTopic), detail, validateOnly) +} + +// GetAllBrokers mocks base method. +func (m *MockClusterAdminClient) GetAllBrokers() []Broker { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetAllBrokers") + ret0, _ := ret[0].([]Broker) + return ret0 +} + +// GetAllBrokers indicates an expected call of GetAllBrokers. +func (mr *MockClusterAdminClientMockRecorder) GetAllBrokers() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAllBrokers", reflect.TypeOf((*MockClusterAdminClient)(nil).GetAllBrokers)) +} + +// GetBrokerConfig mocks base method. +func (m *MockClusterAdminClient) GetBrokerConfig(configName string) (string, bool, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetBrokerConfig", configName) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(bool) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 +} + +// GetBrokerConfig indicates an expected call of GetBrokerConfig. +func (mr *MockClusterAdminClientMockRecorder) GetBrokerConfig(configName interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetBrokerConfig", reflect.TypeOf((*MockClusterAdminClient)(nil).GetBrokerConfig), configName) +} + +// GetTopicConfig mocks base method. +func (m *MockClusterAdminClient) GetTopicConfig(topicName, configName string) (string, bool, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetTopicConfig", topicName, configName) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(bool) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 +} + +// GetTopicConfig indicates an expected call of GetTopicConfig. +func (mr *MockClusterAdminClientMockRecorder) GetTopicConfig(topicName, configName interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetTopicConfig", reflect.TypeOf((*MockClusterAdminClient)(nil).GetTopicConfig), topicName, configName) +} + +// GetTopicsMeta mocks base method. +func (m *MockClusterAdminClient) GetTopicsMeta(topics []string, ignoreTopicError bool) (map[string]TopicDetail, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetTopicsMeta", topics, ignoreTopicError) + ret0, _ := ret[0].(map[string]TopicDetail) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetTopicsMeta indicates an expected call of GetTopicsMeta. +func (mr *MockClusterAdminClientMockRecorder) GetTopicsMeta(topics, ignoreTopicError interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetTopicsMeta", reflect.TypeOf((*MockClusterAdminClient)(nil).GetTopicsMeta), topics, ignoreTopicError) +} + +// GetTopicsPartitionsNum mocks base method. +func (m *MockClusterAdminClient) GetTopicsPartitionsNum(topics []string) (map[string]int32, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetTopicsPartitionsNum", topics) + ret0, _ := ret[0].(map[string]int32) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetTopicsPartitionsNum indicates an expected call of GetTopicsPartitionsNum. +func (mr *MockClusterAdminClientMockRecorder) GetTopicsPartitionsNum(topics interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetTopicsPartitionsNum", reflect.TypeOf((*MockClusterAdminClient)(nil).GetTopicsPartitionsNum), topics) +} diff --git a/pkg/sink/kafka/logutil.go b/pkg/sink/kafka/logutil.go index 8a90e7e3e9..cb5ae54de2 100644 --- a/pkg/sink/kafka/logutil.go +++ b/pkg/sink/kafka/logutil.go @@ -18,12 +18,11 @@ import ( "strconv" "strings" - "github.com/pingcap/errors" - "github.com/pingcap/ticdc/pkg/sink/codec/common" + codecCommon "github.com/pingcap/ticdc/pkg/sink/codec/common" ) // DetermineEventType infers the event type based on MessageLogInfo content. -func DetermineEventType(info *common.MessageLogInfo) string { +func DetermineEventType(info *codecCommon.MessageLogInfo) string { if info == nil { return "unknown" } @@ -40,7 +39,7 @@ func DetermineEventType(info *common.MessageLogInfo) string { } // BuildEventLogContext builds a textual representation of event info. -func BuildEventLogContext(keyspace, changefeed string, info *common.MessageLogInfo) string { +func BuildEventLogContext(keyspace, changefeed string, info *codecCommon.MessageLogInfo) string { var sb strings.Builder sb.WriteString("keyspace=") sb.WriteString(keyspace) @@ -83,22 +82,7 @@ func BuildEventLogContext(keyspace, changefeed string, info *common.MessageLogIn return sb.String() } -// AnnotateEventError logs the event context and annotates the error with that context. -func AnnotateEventError( - keyspace, changefeed string, - info *common.MessageLogInfo, - err error, -) error { - if err == nil { - return nil - } - if contextStr := BuildEventLogContext(keyspace, changefeed, info); contextStr != "" { - return errors.Annotate(err, contextStr+"; ErrorInfo:"+err.Error()) - } - return err -} - -func formatDMLInfo(rows []common.RowLogInfo) string { +func formatDMLInfo(rows []codecCommon.RowLogInfo) string { data, err := json.Marshal(rows) if err != nil { return "" diff --git a/pkg/sink/kafka/logutil_test.go b/pkg/sink/kafka/logutil_test.go index eddaa41f32..2af2a0d618 100644 --- a/pkg/sink/kafka/logutil_test.go +++ b/pkg/sink/kafka/logutil_test.go @@ -16,26 +16,26 @@ import ( "strings" "testing" - "github.com/pingcap/ticdc/pkg/sink/codec/common" + codecCommon "github.com/pingcap/ticdc/pkg/sink/codec/common" "github.com/stretchr/testify/require" ) func TestDetermineEventType(t *testing.T) { require.Equal(t, "unknown", DetermineEventType(nil)) - require.Equal(t, "dml", DetermineEventType(&common.MessageLogInfo{Rows: []common.RowLogInfo{{}}})) - require.Equal(t, "ddl", DetermineEventType(&common.MessageLogInfo{DDL: &common.DDLLogInfo{}})) - require.Equal(t, "checkpoint", DetermineEventType(&common.MessageLogInfo{Checkpoint: &common.CheckpointLogInfo{CommitTs: 1}})) - require.Equal(t, "unknown", DetermineEventType(&common.MessageLogInfo{})) + require.Equal(t, "dml", DetermineEventType(&codecCommon.MessageLogInfo{Rows: []codecCommon.RowLogInfo{{}}})) + require.Equal(t, "ddl", DetermineEventType(&codecCommon.MessageLogInfo{DDL: &codecCommon.DDLLogInfo{}})) + require.Equal(t, "checkpoint", DetermineEventType(&codecCommon.MessageLogInfo{Checkpoint: &codecCommon.CheckpointLogInfo{CommitTs: 1}})) + require.Equal(t, "unknown", DetermineEventType(&codecCommon.MessageLogInfo{})) } func TestBuildEventLogContextRowsIncluded(t *testing.T) { - rows := []common.RowLogInfo{ + rows := []codecCommon.RowLogInfo{ { Type: "insert", Database: "db1", Table: "t1", CommitTs: 1, - PrimaryKeys: []common.ColumnLogInfo{ + PrimaryKeys: []codecCommon.ColumnLogInfo{ {Name: "id", Value: 1}, }, }, @@ -46,7 +46,7 @@ func TestBuildEventLogContextRowsIncluded(t *testing.T) { CommitTs: 2, }, } - info := &common.MessageLogInfo{Rows: rows} + info := &codecCommon.MessageLogInfo{Rows: rows} ctx := BuildEventLogContext("ks", "cf", info) expected := formatDMLInfo(rows) require.Contains(t, ctx, "dmlInfo="+expected) @@ -56,8 +56,8 @@ func TestBuildEventLogContextRowsIncluded(t *testing.T) { func TestBuildEventLogContextLargeData(t *testing.T) { largeValue := strings.Repeat("a", 12*1024) - info := &common.MessageLogInfo{ - Rows: []common.RowLogInfo{ + info := &codecCommon.MessageLogInfo{ + Rows: []codecCommon.RowLogInfo{ {Type: "insert", Table: largeValue}, }, } @@ -65,3 +65,29 @@ func TestBuildEventLogContextLargeData(t *testing.T) { require.Contains(t, ctx, largeValue) require.NotContains(t, ctx, "...(truncated)") } + +func TestBuildEventLogContextBlockEvents(t *testing.T) { + t.Run("ddl", func(t *testing.T) { + ctx := BuildEventLogContext("ks", "cf", &codecCommon.MessageLogInfo{ + DDL: &codecCommon.DDLLogInfo{ + Query: "CREATE TABLE t(id INT PRIMARY KEY)", + StartTs: 1, + CommitTs: 2, + }, + }) + + require.Contains(t, ctx, "eventType=ddl") + require.Contains(t, ctx, "ddlQuery=\"CREATE TABLE t(id INT PRIMARY KEY)\"") + require.Contains(t, ctx, "ddlStartTs=1") + require.Contains(t, ctx, "ddlCommitTs=2") + }) + + t.Run("checkpoint", func(t *testing.T) { + ctx := BuildEventLogContext("ks", "cf", &codecCommon.MessageLogInfo{ + Checkpoint: &codecCommon.CheckpointLogInfo{CommitTs: 3}, + }) + + require.Contains(t, ctx, "eventType=checkpoint") + require.Contains(t, ctx, "checkpointTs=3") + }) +} diff --git a/pkg/sink/kafka/oauth2_token_provider.go b/pkg/sink/kafka/oauth2_token_provider.go index dd25b3ff31..b38e150d93 100644 --- a/pkg/sink/kafka/oauth2_token_provider.go +++ b/pkg/sink/kafka/oauth2_token_provider.go @@ -18,7 +18,7 @@ import ( "net/url" "github.com/IBM/sarama" - "github.com/pingcap/errors" + "github.com/pingcap/ticdc/pkg/errors" "golang.org/x/oauth2" "golang.org/x/oauth2/clientcredentials" ) @@ -68,7 +68,7 @@ func newTokenProvider(ctx context.Context, o *options) (sarama.AccessTokenProvid tokenURL, err := url.Parse(o.SASL.OAuth2.TokenURL) if err != nil { - return nil, errors.Trace(err) + return nil, errors.WrapError(errors.ErrKafkaInvalidConfig, err) } cfg := clientcredentials.Config{ diff --git a/pkg/sink/kafka/oauth2_token_provider_test.go b/pkg/sink/kafka/oauth2_token_provider_test.go index 4438377824..0ed4d7c044 100644 --- a/pkg/sink/kafka/oauth2_token_provider_test.go +++ b/pkg/sink/kafka/oauth2_token_provider_test.go @@ -15,8 +15,10 @@ package kafka import ( "context" + "net/url" "testing" + "github.com/pingcap/ticdc/pkg/errors" "github.com/pingcap/ticdc/pkg/security" "github.com/stretchr/testify/require" ) @@ -66,7 +68,9 @@ func TestNewTokenProvider(t *testing.T) { if ts.expectedErr == "" { require.NoError(t, err) } else { - require.Error(t, err) + require.ErrorIs(t, err, errors.ErrKafkaInvalidConfig) + var escapeErr url.EscapeError + require.ErrorAs(t, err, &escapeErr) require.Contains(t, err.Error(), ts.expectedErr) } }) diff --git a/pkg/sink/kafka/options.go b/pkg/sink/kafka/options.go index c9b992814e..a19a6ddb44 100644 --- a/pkg/sink/kafka/options.go +++ b/pkg/sink/kafka/options.go @@ -108,7 +108,13 @@ func requireAcksFromString(acks int) (RequiredAcks, error) { case int(NoResponse): return NoResponse, nil default: +<<<<<<< HEAD return Unknown, cerror.ErrKafkaInvalidRequiredAcks.GenWithStackByArgs(acks) +======= + return Unknown, errors.ErrKafkaInvalidConfig.GenWithStack( + "invalid required acks %d, only support these values: "+ + "0(NoResponse), 1(WaitForLocal) and -1(WaitForAll)", acks) +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) } } @@ -219,7 +225,11 @@ func (o *options) setPartitionNum(realPartitionCount int32) error { // the real partition count, since messages would be dispatched to different // partitions, this could prevent potential correctness problems. if o.PartitionNum > realPartitionCount { +<<<<<<< HEAD return cerror.ErrKafkaInvalidPartitionNum.GenWithStack( +======= + return errors.ErrKafkaInvalidConfig.GenWithStack( +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) "the number of partition (%d) specified in sink-uri is more than that of actual topic (%d)", o.PartitionNum, realPartitionCount) } @@ -236,15 +246,23 @@ func (o *options) Apply(changefeedID common.ChangeFeedID, req := &http.Request{URL: sinkURI} urlParameter := &urlConfig{} if err = binding.Query.Bind(req, urlParameter); err != nil { +<<<<<<< HEAD return cerror.WrapError(cerror.ErrMySQLInvalidConfig, err) +======= + return errors.WrapError(errors.ErrKafkaInvalidConfig, err) +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) } if urlParameter, err = mergeConfig(sinkConfig, urlParameter); err != nil { - return err + return errors.WrapError(errors.ErrKafkaInvalidConfig, err) } if urlParameter.PartitionNum != nil { o.PartitionNum = *urlParameter.PartitionNum if o.PartitionNum <= 0 { +<<<<<<< HEAD return cerror.ErrKafkaInvalidPartitionNum.GenWithStackByArgs(o.PartitionNum) +======= + return errors.ErrKafkaInvalidConfig.GenWithStack("invalid partition num %d", o.PartitionNum) +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) } } @@ -289,7 +307,7 @@ func (o *options) Apply(changefeedID common.ChangeFeedID, if urlParameter.DialTimeout != nil && *urlParameter.DialTimeout != "" { a, err := time.ParseDuration(*urlParameter.DialTimeout) if err != nil { - return err + return errors.WrapError(errors.ErrKafkaInvalidConfig, err) } o.DialTimeout = a } @@ -297,7 +315,7 @@ func (o *options) Apply(changefeedID common.ChangeFeedID, if urlParameter.WriteTimeout != nil && *urlParameter.WriteTimeout != "" { a, err := time.ParseDuration(*urlParameter.WriteTimeout) if err != nil { - return err + return errors.WrapError(errors.ErrKafkaInvalidConfig, err) } o.WriteTimeout = a } @@ -305,7 +323,7 @@ func (o *options) Apply(changefeedID common.ChangeFeedID, if urlParameter.ReadTimeout != nil && *urlParameter.ReadTimeout != "" { a, err := time.ParseDuration(*urlParameter.ReadTimeout) if err != nil { - return err + return errors.WrapError(errors.ErrKafkaInvalidConfig, err) } o.ReadTimeout = a } @@ -387,8 +405,12 @@ func (o *options) applyTLS(params *urlConfig) error { if o.Credential != nil && !o.Credential.IsEmpty() && !o.Credential.IsTLSEnabled() { +<<<<<<< HEAD return cerror.WrapError(cerror.ErrKafkaInvalidConfig, errors.New("ca, cert and key files should all be supplied")) +======= + return errors.ErrKafkaInvalidConfig.GenWithStack("ca, cert and key files should all be supplied") +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) } // if enable-tls is not set, but credential files are set, @@ -401,8 +423,12 @@ func (o *options) applyTLS(params *urlConfig) error { enableTLS := *params.EnableTLS if o.Credential != nil && o.Credential.IsTLSEnabled() && !enableTLS { +<<<<<<< HEAD return cerror.WrapError(cerror.ErrKafkaInvalidConfig, errors.New("credential files are supplied, but 'enable-tls' is set to false")) +======= + return errors.ErrKafkaInvalidConfig.GenWithStack("credential files are supplied, but 'enable-tls' is set to false") +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) } o.EnableTLS = enableTLS } else { @@ -493,8 +519,12 @@ func (o *options) applySASL(urlParameter *urlConfig, sinkConfig *config.SinkConf decodedClientSecret, err := base64.StdEncoding.DecodeString(clientSecret) if err != nil { log.Error("OAuth2 client secret is not base64 encoded", zap.Error(err)) +<<<<<<< HEAD return cerror.ErrKafkaInvalidConfig.GenWithStack( "OAuth2 client secret is not base64 encoded") +======= + return errors.ErrKafkaInvalidConfig.GenWithStack("OAuth2 client secret is not base64 encoded") +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) } o.SASL.OAuth2.ClientSecret = string(decodedClientSecret) } @@ -516,7 +546,11 @@ func (o *options) applySASL(urlParameter *urlConfig, sinkConfig *config.SinkConf } if err := o.SASL.OAuth2.Validate(); err != nil { +<<<<<<< HEAD return cerror.ErrKafkaInvalidConfig.Wrap(err) +======= + return errors.WrapError(errors.ErrKafkaInvalidConfig, err) +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) } o.SASL.OAuth2.SetDefault() } @@ -552,6 +586,47 @@ func (o *options) DeriveTopicConfig() *AutoCreateTopicConfig { } } +<<<<<<< HEAD +======= +// ValidateReplicationFactor checks whether a topic created with this config +// can satisfy the configured acknowledgment requirement. +func (c *AutoCreateTopicConfig) ValidateReplicationFactor(admin ClusterAdminClient) error { + if c.RequiredAcks != WaitForAll { + return nil + } + + raw, found, err := admin.GetBrokerConfig(MinInsyncReplicasConfigName) + if err != nil { + log.Warn("cannot get Kafka broker configuration, assume replication factor is valid", + zap.String("configName", MinInsyncReplicasConfigName), + zap.Int16("replicationFactor", c.ReplicationFactor), + zap.Error(err)) + return nil + } + if !found { + log.Warn("Kafka broker configuration not found, assume replication factor is valid", + zap.String("configName", MinInsyncReplicasConfigName), + zap.Int16("replicationFactor", c.ReplicationFactor)) + return nil + } + minInsyncReplicas, err := strconv.Atoi(raw) + if err != nil { + return errors.WrapError(errors.ErrKafkaAdminAPI, err, "parse-config", MinInsyncReplicasConfigName) + } + + if int(c.ReplicationFactor) < minInsyncReplicas { + return errors.ErrKafkaInvalidConfig.GenWithStack( + "TiCDC Kafka sink's `request.required.acks` defaults to -1, "+ + "TiCDC cannot deliver messages when the `replication-factor` %d "+ + "is smaller than the `min.insync.replicas` %d of broker", + c.ReplicationFactor, minInsyncReplicas, + ) + } + + return nil +} + +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) var ( validClientID = regexp.MustCompile(`\A[A-Za-z0-9._-]+\z`) commonInvalidChar = regexp.MustCompile(`[\?:,"]`) @@ -570,7 +645,11 @@ func NewKafkaClientID(captureAddr string, clientID = commonInvalidChar.ReplaceAllString(clientID, "_") } if !validClientID.MatchString(clientID) { +<<<<<<< HEAD return "", cerror.ErrKafkaInvalidClientID.GenWithStackByArgs(clientID) +======= + return "", errors.ErrKafkaInvalidConfig.GenWithStack("invalid kafka client ID %q", clientID) +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) } return } @@ -584,7 +663,7 @@ func adjustOptions( ) error { topics, err := admin.GetTopicsMeta([]string{topic}, true) if err != nil { - return errors.Trace(err) + return err } // Only check replicationFactor >= minInsyncReplicas when producer's required acks is -1. @@ -650,11 +729,48 @@ func adjustOptions( } brokerMessageMaxBytes, err := strconv.Atoi(brokerMessageMaxBytesStr) if err != nil { +<<<<<<< HEAD return errors.Trace(err) +======= + return err + } + + options.MaxBatchedBytes = min(options.MaxBatchedBytes, options.MaxMessageBytes) + return nil +} + +func adjustExistingTopicOption( + changefeedID common.ChangeFeedID, + admin ClusterAdminClient, + options *options, + topic string, + info TopicDetail, +) error { + maxMessageBytes, found, err := getTopicMaxMessageBytes(admin, info.Name) + if err != nil || !found { + log.Warn("`max.message.bytes` not found from topic's configuration, use the option `MaxMessageBytes` as default", + zap.String("namespace", changefeedID.Keyspace()), zap.String("changefeed", changefeedID.Name()), + zap.Int("maxMessageBytes", options.MaxMessageBytes), zap.Error(err)) + maxMessageBytes = options.MaxMessageBytes + } + options.MaxMessageBytes = maxMessageBytes + + // no need to create the topic, + // but we would have to log user if they found enter wrong topic name later + if options.AutoCreate { + log.Warn("topic already exist, TiCDC will not create the topic", + zap.String("namespace", changefeedID.Keyspace()), zap.String("changefeed", changefeedID.Name()), + zap.String("topic", topic), zap.Any("detail", info)) + } + + if err = options.setPartitionNum(changefeedID, info.NumPartitions); err != nil { + return err +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) } // when create the topic, `max.message.bytes` is decided by the broker, // it would use broker's `message.max.bytes` to set topic's `max.message.bytes`. +<<<<<<< HEAD // TiCDC need to make sure that the producer's `MaxMessageBytes` won't larger than // broker's `message.max.bytes`. maxMessageBytes := brokerMessageMaxBytes - maxMessageBytesOverhead @@ -669,6 +785,14 @@ func adjustOptions( if maxMessageBytes < options.MaxMessageBytes { options.MaxMessageBytes = maxMessageBytes } +======= + messageMaxBytes, found, err := getBrokerMaxMessageBytes(admin) + if err != nil || !found { + log.Warn("`message.max.bytes` not found from broker's configuration, use the option `MaxMessageBytes` as default", + zap.String("namespace", changefeedID.Keyspace()), zap.String("changefeed", changefeedID.Name()), + zap.Int("maxMessageBytes", options.MaxMessageBytes), zap.Error(err)) + messageMaxBytes = options.MaxMessageBytes +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) } // topic not exists yet, and user does not specify the `partition-num` in the sink uri. @@ -685,6 +809,7 @@ func validateMinInsyncReplicas( admin ClusterAdminClient, topics map[string]TopicDetail, topic string, +<<<<<<< HEAD replicationFactor int, ) error { minInsyncReplicasConfigGetter := func() (string, bool, error) { @@ -706,10 +831,39 @@ func validateMinInsyncReplicas( } return minInsyncReplicasStr, false, nil +======= +) (int, bool, error) { + raw, found, err := getTopicConfig( + admin, topic, + TopicMaxMessageBytesConfigName, + BrokerMessageMaxBytesConfigName, + ) + if err != nil { + return 0, false, err + } + if !found { + return 0, false, nil + } + maxMessageBytes, err := strconv.Atoi(raw) + if err != nil { + return 0, false, errors.WrapError(errors.ErrKafkaAdminAPI, err, "parse-config", TopicMaxMessageBytesConfigName) + } + return maxMessageBytes, true, nil +} + +func getBrokerMaxMessageBytes(admin ClusterAdminClient) (int, bool, error) { + raw, found, err := admin.GetBrokerConfig(BrokerMessageMaxBytesConfigName) + if err != nil { + return 0, false, err + } + if !found { + return 0, false, nil +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) } minInsyncReplicasStr, exists, err := minInsyncReplicasConfigGetter() if err != nil { +<<<<<<< HEAD // 'min.insync.replica' is invisible to us in Confluent Cloud Kafka. if cerror.ErrKafkaConfigNotFound.Equal(err) { log.Warn("TiCDC cannot find `min.insync.replicas` from broker's configuration, " + @@ -745,6 +899,11 @@ func validateMinInsyncReplicas( } return nil +======= + return 0, false, errors.WrapError(errors.ErrKafkaAdminAPI, err, "parse-config", BrokerMessageMaxBytesConfigName) + } + return messageMaxBytes, true, nil +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) } // getTopicConfig gets topic config by name. @@ -757,12 +916,13 @@ func getTopicConfig( topicName string, topicConfigName string, brokerConfigName string, -) (string, error) { - if c, err := admin.GetTopicConfig(topicName, topicConfigName); err == nil { - return c, nil +) (string, bool, error) { + c, found, err := admin.GetTopicConfig(topicName, topicConfigName) + if err == nil && found { + return c, true, nil } - log.Info("kafka sink cannot find the configuration from topic, try to get it from broker", - zap.String("topic", topicName), zap.String("config", topicConfigName)) + log.Info("kafka sink cannot get the configuration from topic, try to get it from broker", + zap.String("topic", topicName), zap.String("config", topicConfigName), zap.Error(err)) return admin.GetBrokerConfig(brokerConfigName) } diff --git a/pkg/sink/kafka/options_test.go b/pkg/sink/kafka/options_test.go index 8f64d49762..e7a20d51f1 100644 --- a/pkg/sink/kafka/options_test.go +++ b/pkg/sink/kafka/options_test.go @@ -23,14 +23,155 @@ import ( "time" "github.com/IBM/sarama" +<<<<<<< HEAD "github.com/aws/aws-sdk-go/aws" commonType "github.com/pingcap/ticdc/pkg/common" +======= + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/golang/mock/gomock" + "github.com/pingcap/ticdc/pkg/common" +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) "github.com/pingcap/ticdc/pkg/config" "github.com/pingcap/ticdc/pkg/errors" "github.com/pingcap/ticdc/pkg/sink/codec/common" "github.com/stretchr/testify/require" ) +<<<<<<< HEAD +======= +const ( + defaultMockTopicName = "mock_topic" + + // These values model Kafka admin responses, not TiCDC option defaults. + mockClusterReplicationFactor int16 = 3 + mockBrokerMessageMaxBytes = "1048588" + mockTopicMessageMaxBytes = "1048588" + mockMinInsyncReplicas = "1" +) + +type kafkaAdminFixture struct { + admin *MockClusterAdminClient + topics map[string]TopicDetail + brokerConfig map[string]string + topicConfig map[string]map[string]string +} + +func newKafkaAdminFixture(t *testing.T) *kafkaAdminFixture { + t.Helper() + + ctrl := gomock.NewController(t) + fixture := &kafkaAdminFixture{ + admin: NewMockClusterAdminClient(ctrl), + topics: make(map[string]TopicDetail), + brokerConfig: map[string]string{ + BrokerMessageMaxBytesConfigName: mockBrokerMessageMaxBytes, + MinInsyncReplicasConfigName: mockMinInsyncReplicas, + }, + topicConfig: make(map[string]map[string]string), + } + fixture.addTopic(defaultMockTopicName, defaultPartitionNum) + fixture.topicConfig[defaultMockTopicName] = map[string]string{ + TopicMaxMessageBytesConfigName: mockTopicMessageMaxBytes, + MinInsyncReplicasConfigName: mockMinInsyncReplicas, + } + + fixture.admin.EXPECT().Close().AnyTimes() + fixture.admin.EXPECT().GetTopicsMeta(gomock.Any(), gomock.Any()). + DoAndReturn(fixture.getTopicsMeta).AnyTimes() + fixture.admin.EXPECT().GetTopicsPartitionsNum(gomock.Any()). + DoAndReturn(fixture.getTopicsPartitionsNum).AnyTimes() + fixture.admin.EXPECT().GetBrokerConfig(gomock.Any()). + DoAndReturn(fixture.getBrokerConfig).AnyTimes() + fixture.admin.EXPECT().GetTopicConfig(gomock.Any(), gomock.Any()). + DoAndReturn(fixture.getTopicConfig).AnyTimes() + fixture.admin.EXPECT().CreateTopic(gomock.Any(), gomock.Any()). + DoAndReturn(fixture.createTopic).AnyTimes() + + return fixture +} + +func (f *kafkaAdminFixture) addTopic(name string, partitionNum int32) { + f.topics[name] = TopicDetail{Name: name, NumPartitions: partitionNum} +} + +func (f *kafkaAdminFixture) getTopicsMeta( + topics []string, _ bool, +) (map[string]TopicDetail, error) { + result := make(map[string]TopicDetail, len(topics)) + for _, topic := range topics { + if detail, ok := f.topics[topic]; ok { + result[topic] = detail + } + } + return result, nil +} + +func (f *kafkaAdminFixture) getTopicsPartitionsNum( + topics []string, +) (map[string]int32, error) { + result := make(map[string]int32, len(topics)) + for _, topic := range topics { + if detail, ok := f.topics[topic]; ok { + result[topic] = detail.NumPartitions + } + } + return result, nil +} + +func (f *kafkaAdminFixture) getBrokerConfig(configName string) (string, bool, error) { + if value, ok := f.brokerConfig[configName]; ok { + return value, true, nil + } + return "", false, nil +} + +func (f *kafkaAdminFixture) getTopicConfig(topicName string, configName string) (string, bool, error) { + if _, ok := f.topics[topicName]; !ok { + return "", false, nil + } + if value, ok := f.topicConfig[topicName][configName]; ok { + return value, true, nil + } + return "", false, nil +} + +func (f *kafkaAdminFixture) createTopic(detail *TopicDetail, _ bool) error { + if detail.ReplicationFactor > mockClusterReplicationFactor { + return sarama.ErrInvalidReplicationFactor + } + if _, ok := f.brokerConfig[MinInsyncReplicasConfigName]; !ok && + detail.ReplicationFactor != mockClusterReplicationFactor { + return sarama.ErrPolicyViolation + } + f.topics[detail.Name] = *detail + return nil +} + +func (f *kafkaAdminFixture) brokerMessageMaxBytes() int { + value, _ := strconv.Atoi(f.brokerConfig[BrokerMessageMaxBytesConfigName]) + return value +} + +func (f *kafkaAdminFixture) topicMaxMessageBytes(topicName string) int { + value, _ := strconv.Atoi(f.topicConfig[topicName][TopicMaxMessageBytesConfigName]) + return value +} + +func (f *kafkaAdminFixture) setMessageMaxBytes(brokerValue, topicValue string) { + f.brokerConfig[BrokerMessageMaxBytesConfigName] = brokerValue + f.topicConfig[defaultMockTopicName][TopicMaxMessageBytesConfigName] = topicValue +} + +func (f *kafkaAdminFixture) setMinInsyncReplicas(minInsyncReplicas string) { + f.topicConfig[defaultMockTopicName][MinInsyncReplicasConfigName] = minInsyncReplicas + f.brokerConfig[MinInsyncReplicasConfigName] = minInsyncReplicas +} + +func (f *kafkaAdminFixture) dropBrokerConfig(configName string) { + delete(f.brokerConfig, configName) +} + +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) func TestCompleteOptions(t *testing.T) { options := NewOptions() @@ -43,7 +184,7 @@ func TestCompleteOptions(t *testing.T) { sinkURI, err := url.Parse(uri) require.NoError(t, err) - err = options.Apply(commonType.NewChangefeedID4Test(commonType.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink) + err = options.Apply(common.NewChangefeedID4Test(common.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink) require.NoError(t, err) require.Equal(t, int32(1), options.PartitionNum) require.Equal(t, int16(3), options.ReplicationFactor) @@ -57,7 +198,7 @@ func TestCompleteOptions(t *testing.T) { sinkURI, err = url.Parse(uri) require.NoError(t, err) options = NewOptions() - err = options.Apply(commonType.NewChangefeedID4Test(commonType.DefaultKeyspaceName, "test"), + err = options.Apply(common.NewChangefeedID4Test(common.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink) require.NoError(t, err) require.Len(t, options.BrokerEndpoints, 3) @@ -67,15 +208,30 @@ func TestCompleteOptions(t *testing.T) { sinkURI, err = url.Parse(uri) require.NoError(t, err) options = NewOptions() - err = options.Apply(commonType.NewChangefeedID4Test(commonType.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink) + err = options.Apply(common.NewChangefeedID4Test(common.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink) require.Regexp(t, ".*invalid syntax.*", errors.Cause(err)) +<<<<<<< HEAD +======= + for _, replicationFactor := range []string{"0", "-1"} { + uri = "kafka://127.0.0.1:9092/abc?replication-factor=" + replicationFactor + sinkURI, err = url.Parse(uri) + require.NoError(t, err) + options = NewOptions() + err = options.Apply( + common.NewChangefeedID4Test(common.DefaultKeyspaceName, "test"), + sinkURI, + config.GetDefaultReplicaConfig().Sink, + ) + require.ErrorContains(t, err, "invalid replication-factor "+replicationFactor) + } +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) // Illegal max-message-bytes. uri = "kafka://127.0.0.1:9092/abc?kafka-version=2.6.0&max-message-bytes=a" sinkURI, err = url.Parse(uri) require.NoError(t, err) options = NewOptions() - err = options.Apply(commonType.NewChangefeedID4Test(commonType.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink) + err = options.Apply(common.NewChangefeedID4Test(common.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink) require.Regexp(t, ".*invalid syntax.*", errors.Cause(err)) // Illegal max-retry. @@ -83,7 +239,7 @@ func TestCompleteOptions(t *testing.T) { sinkURI, err = url.Parse(uri) require.NoError(t, err) options = NewOptions() - err = options.Apply(commonType.NewChangefeedID4Test(commonType.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink) + err = options.Apply(common.NewChangefeedID4Test(common.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink) require.Regexp(t, ".*invalid syntax.*", errors.Cause(err)) // Illegal partition-num. @@ -91,7 +247,7 @@ func TestCompleteOptions(t *testing.T) { sinkURI, err = url.Parse(uri) require.NoError(t, err) options = NewOptions() - err = options.Apply(commonType.NewChangefeedID4Test(commonType.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink) + err = options.Apply(common.NewChangefeedID4Test(common.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink) require.Regexp(t, ".*invalid syntax.*", errors.Cause(err)) // Out of range partition-num. @@ -99,7 +255,7 @@ func TestCompleteOptions(t *testing.T) { sinkURI, err = url.Parse(uri) require.NoError(t, err) options = NewOptions() - err = options.Apply(commonType.NewChangefeedID4Test(commonType.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink) + err = options.Apply(common.NewChangefeedID4Test(common.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink) require.Regexp(t, ".*invalid partition num.*", errors.Cause(err)) // Unknown required-acks. @@ -107,7 +263,7 @@ func TestCompleteOptions(t *testing.T) { sinkURI, err = url.Parse(uri) require.NoError(t, err) options = NewOptions() - err = options.Apply(commonType.NewChangefeedID4Test(commonType.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink) + err = options.Apply(common.NewChangefeedID4Test(common.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink) require.Regexp(t, ".*invalid required acks 3.*", errors.Cause(err)) // invalid kafka client id @@ -115,15 +271,15 @@ func TestCompleteOptions(t *testing.T) { sinkURI, err = url.Parse(uri) require.NoError(t, err) options = NewOptions() - err = options.Apply(commonType.NewChangefeedID4Test(commonType.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink) - require.True(t, errors.ErrKafkaInvalidClientID.Equal(err)) + err = options.Apply(common.NewChangefeedID4Test(common.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink) + require.True(t, errors.ErrKafkaInvalidConfig.Equal(err)) // max-retry accepts non-negative sink-uri values. uri = "kafka://127.0.0.1:9092/abc?max-retry=7" sinkURI, err = url.Parse(uri) require.NoError(t, err) options = NewOptions() - err = options.Apply(commonType.NewChangefeedID4Test(commonType.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink) + err = options.Apply(common.NewChangefeedID4Test(common.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink) require.NoError(t, err) require.Equal(t, 7, options.MaxRetry) @@ -131,7 +287,7 @@ func TestCompleteOptions(t *testing.T) { sinkURI, err = url.Parse(uri) require.NoError(t, err) options = NewOptions() - err = options.Apply(commonType.NewChangefeedID4Test(commonType.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink) + err = options.Apply(common.NewChangefeedID4Test(common.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink) require.NoError(t, err) require.Equal(t, 0, options.MaxRetry) @@ -140,14 +296,76 @@ func TestCompleteOptions(t *testing.T) { sinkURI, err = url.Parse(uri) require.NoError(t, err) options = NewOptions() - err = options.Apply(commonType.NewChangefeedID4Test(commonType.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink) + err = options.Apply(common.NewChangefeedID4Test(common.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink) require.NoError(t, err) require.Equal(t, defaultMaxRetry, options.MaxRetry) } +<<<<<<< HEAD func TestSetPartitionNum(t *testing.T) { options := NewOptions() err := options.setPartitionNum(2) +======= +func TestApplyRejectsNonPositiveMaxMessageBytes(t *testing.T) { + tests := []struct { + name string + uri string + configValue *int + expected int + }{ + { + name: "zero from URI", + uri: "kafka://127.0.0.1:9092/test-topic?max-message-bytes=0", + expected: 0, + }, + { + name: "negative from URI", + uri: "kafka://127.0.0.1:9092/test-topic?max-message-bytes=-1", + expected: -1, + }, + { + name: "zero from sink config", + uri: "kafka://127.0.0.1:9092/test-topic", + configValue: aws.Int(0), + expected: 0, + }, + { + name: "negative from sink config", + uri: "kafka://127.0.0.1:9092/test-topic", + configValue: aws.Int(-1), + expected: -1, + }, + } + + changefeedID := common.NewChangefeedID4Test(common.DefaultKeyspaceName, "test") + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + sinkURI, err := url.Parse(test.uri) + require.NoError(t, err) + + sinkConfig := config.GetDefaultReplicaConfig().Sink + if test.configValue != nil { + sinkConfig.KafkaConfig = &config.KafkaConfig{ + MaxMessageBytes: test.configValue, + } + } + + options := NewOptions() + err = options.Apply(changefeedID, sinkURI, sinkConfig) + require.ErrorContains( + t, err, fmt.Sprintf("invalid max-message-bytes %d", test.expected)) + errCode, ok := errors.RFCCode(err) + require.True(t, ok) + require.Equal(t, errors.ErrKafkaInvalidConfig.RFCCode(), errCode) + }) + } +} + +func TestSetPartitionNum(t *testing.T) { + options := NewOptions() + changefeedID := common.NewChangefeedID4Test(common.DefaultKeyspaceName, "test") + err := options.setPartitionNum(changefeedID, 2) +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) require.NoError(t, err) require.Equal(t, int32(2), options.PartitionNum) @@ -157,8 +375,13 @@ func TestSetPartitionNum(t *testing.T) { require.Equal(t, int32(1), options.PartitionNum) options.PartitionNum = 3 +<<<<<<< HEAD err = options.setPartitionNum(2) require.True(t, errors.ErrKafkaInvalidPartitionNum.Equal(err)) +======= + err = options.setPartitionNum(changefeedID, 2) + require.True(t, errors.ErrKafkaInvalidConfig.Equal(err)) +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) } func TestClientID(t *testing.T) { @@ -196,7 +419,7 @@ func TestClientID(t *testing.T) { } for _, tc := range testCases { id, err := NewKafkaClientID(tc.addr, - commonType.NewChangefeedID4Test(commonType.DefaultKeyspaceName, tc.changefeedID), tc.configuredID) + common.NewChangefeedID4Test(common.DefaultKeyspaceName, tc.changefeedID), tc.configuredID) if tc.hasError { require.Error(t, err) } else { @@ -217,7 +440,7 @@ func TestTimeout(t *testing.T) { sinkURI, err := url.Parse(uri) require.NoError(t, err) - err = options.Apply(commonType.NewChangefeedID4Test(commonType.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink) + err = options.Apply(common.NewChangefeedID4Test(common.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink) require.NoError(t, err) require.Equal(t, 5*time.Second, options.DialTimeout) @@ -230,8 +453,17 @@ func TestAdjustConfigTopicNotExist(t *testing.T) { adminClient := NewClusterAdminClientMockImpl() defer adminClient.Close() +<<<<<<< HEAD options := NewOptions() options.BrokerEndpoints = []string{"127.0.0.1:9092"} +======= + topicName := "test-topic" + changefeedID := common.NewChangefeedID4Test(common.DefaultKeyspaceName, "test") + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + adminFixture := newKafkaAdminFixture(t) + adminClient := adminFixture.admin +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) // topic not exist, `max-message-bytes` = `message.max.bytes` options.MaxMessageBytes = adminClient.GetBrokerMessageMaxBytes() @@ -640,10 +872,17 @@ func TestConfigurationCombinations(t *testing.T) { sinkURI, err := url.Parse(uri) require.Nil(t, err) +<<<<<<< HEAD ctx := context.Background() options := NewOptions() err = options.Apply(commonType.NewChangefeedID4Test(commonType.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink) require.Nil(t, err) +======= + options := NewOptions() + err = options.Apply(common.NewChangefeedID4Test(common.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink) + require.Nil(t, err) + configuredMaxMessageBytes := options.MaxMessageBytes +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) changefeed := commonType.NewChangefeedID4Test(commonType.DefaultKeyspaceName, "changefeed-test") factory, err := NewMockFactory(ctx, options, changefeed) @@ -652,11 +891,23 @@ func TestConfigurationCombinations(t *testing.T) { adminClient, err := factory.AdminClient(ctx) require.NoError(t, err) +<<<<<<< HEAD topic, ok := a.uriParams[0].(string) require.True(t, ok) require.NotEqual(t, "", topic) err = adjustOptions(ctx, adminClient, options, topic) require.Nil(t, err) +======= + changefeedID := common.NewChangefeedID4Test(common.DefaultKeyspaceName, "test") + err = adjustOptions(changefeedID, adminClient, options, topic) + require.Nil(t, err) + require.Equal(t, sourceMaxMessageBytes, options.MaxMessageBytes) + require.Equal( + t, + min(configuredMaxMessageBytes, sourceMaxMessageBytes), + options.MaxBatchedBytes, + ) +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) encoderConfig := common.NewConfig(config.ProtocolOpen) err = encoderConfig.Apply(sinkURI, &config.SinkConfig{ @@ -709,7 +960,7 @@ func TestMerge(t *testing.T) { Key: aws.String("key.pem"), } c := NewOptions() - err = c.Apply(commonType.NewChangefeedID4Test(commonType.DefaultKeyspaceName, "test"), sinkURI, replicaConfig.Sink) + err = c.Apply(common.NewChangefeedID4Test(common.DefaultKeyspaceName, "test"), sinkURI, replicaConfig.Sink) require.NoError(t, err) require.Equal(t, int32(12), c.PartitionNum) require.Equal(t, int16(5), c.ReplicationFactor) @@ -790,7 +1041,7 @@ func TestMerge(t *testing.T) { Key: aws.String("key2.pem"), } c = NewOptions() - err = c.Apply(commonType.NewChangefeedID4Test(commonType.DefaultKeyspaceName, "test"), sinkURI, replicaConfig.Sink) + err = c.Apply(common.NewChangefeedID4Test(common.DefaultKeyspaceName, "test"), sinkURI, replicaConfig.Sink) require.NoError(t, err) require.Equal(t, int32(12), c.PartitionNum) require.Equal(t, int16(5), c.ReplicationFactor) diff --git a/pkg/sink/kafka/sarama_async_producer.go b/pkg/sink/kafka/sarama_async_producer.go index 9c0bd82104..f0c0f6b5d1 100644 --- a/pkg/sink/kafka/sarama_async_producer.go +++ b/pkg/sink/kafka/sarama_async_producer.go @@ -18,12 +18,10 @@ import ( "time" "github.com/IBM/sarama" - "github.com/pingcap/errors" - "github.com/pingcap/failpoint" "github.com/pingcap/log" - commonType "github.com/pingcap/ticdc/pkg/common" - cerror "github.com/pingcap/ticdc/pkg/errors" - "github.com/pingcap/ticdc/pkg/sink/codec/common" + "github.com/pingcap/ticdc/pkg/common" + "github.com/pingcap/ticdc/pkg/errors" + codecCommon "github.com/pingcap/ticdc/pkg/sink/codec/common" "go.uber.org/atomic" "go.uber.org/zap" ) @@ -31,15 +29,14 @@ import ( type saramaAsyncProducer struct { client sarama.Client producer sarama.AsyncProducer - changefeedID commonType.ChangeFeedID + changefeedID common.ChangeFeedID - closed *atomic.Bool - failpointCh chan *sarama.ProducerError + closed *atomic.Bool } type messageMetadata struct { callback func() - logInfo *common.MessageLogInfo + logInfo *codecCommon.MessageLogInfo } func (p *saramaAsyncProducer) Close() { @@ -103,13 +100,7 @@ func (p *saramaAsyncProducer) AsyncRunCallback( log.Info("async producer exit since context is done", zap.String("keyspace", p.changefeedID.Keyspace()), zap.String("changefeed", p.changefeedID.Name())) - return errors.Trace(ctx.Err()) - case err := <-p.failpointCh: - log.Warn("Receive from failpoint chan in kafka DML producer", - zap.String("keyspace", p.changefeedID.Keyspace()), - zap.String("changefeed", p.changefeedID.Name()), - zap.Error(err)) - return p.handleProducerError(err) + return context.Cause(ctx) case ack := <-p.producer.Successes(): if ack != nil { switch meta := ack.Metadata.(type) { @@ -137,37 +128,23 @@ func (p *saramaAsyncProducer) AsyncRunCallback( } func (p *saramaAsyncProducer) handleProducerError(err *sarama.ProducerError) error { - errWithInfo := AnnotateEventError( - p.changefeedID.Keyspace(), - p.changefeedID.Name(), - extractLogInfo(err.Msg), - err.Err, - ) - return cerror.WrapError(cerror.ErrKafkaAsyncSendMessage, errWithInfo) + log.Error("send message to kafka failed", + zap.String("keyspace", p.changefeedID.Keyspace()), + zap.String("changefeed", p.changefeedID.Name()), + zap.String("eventContext", BuildEventLogContext( + p.changefeedID.Keyspace(), p.changefeedID.Name(), extractLogInfo(err.Msg))), + zap.Error(err.Err)) + return errors.WrapError(errors.ErrKafkaSendMessage, err.Err) } // AsyncSend is the input channel for the user to write messages to that they // wish to send. func (p *saramaAsyncProducer) AsyncSend( - ctx context.Context, topic string, partition int32, message *common.Message, + ctx context.Context, topic string, partition int32, message *codecCommon.Message, ) error { if p.closed.Load() { - return cerror.ErrKafkaProducerClosed.GenWithStackByArgs() + return errors.ErrKafkaSinkClosed.GenWithStackByArgs() } - failpoint.Inject("KafkaSinkAsyncSendError", func() { - // simulate sending message to input channel successfully but flushing - // message to Kafka meets error - log.Info("KafkaSinkAsyncSendError error injected", zap.String("keyspace", p.changefeedID.Keyspace()), - zap.String("changefeed", p.changefeedID.Name())) - p.failpointCh <- &sarama.ProducerError{ - Err: errors.New("kafka sink injected error"), - Msg: &sarama.ProducerMessage{Metadata: &messageMetadata{ - callback: message.Callback, - logInfo: message.LogInfo, - }}, - } - failpoint.Return(nil) - }) meta := &messageMetadata{ callback: message.Callback, logInfo: message.LogInfo, @@ -181,13 +158,13 @@ func (p *saramaAsyncProducer) AsyncSend( } select { case <-ctx.Done(): - return errors.Trace(ctx.Err()) + return context.Cause(ctx) case p.producer.Input() <- msg: } return nil } -func extractLogInfo(msg *sarama.ProducerMessage) *common.MessageLogInfo { +func extractLogInfo(msg *sarama.ProducerMessage) *codecCommon.MessageLogInfo { if msg == nil { return nil } diff --git a/pkg/sink/kafka/sarama_config.go b/pkg/sink/kafka/sarama_config.go index b53dc47b37..4988c79c52 100644 --- a/pkg/sink/kafka/sarama_config.go +++ b/pkg/sink/kafka/sarama_config.go @@ -108,7 +108,7 @@ func newSaramaConfig(ctx context.Context, o *options) (*sarama.Config, error) { if o.Credential != nil && o.Credential.IsTLSEnabled() { config.Net.TLS.Config, err = o.Credential.ToTLSConfig() if err != nil { - return nil, errors.Trace(err) + return nil, errors.WrapError(errors.ErrKafkaInvalidConfig, err) } } @@ -117,7 +117,7 @@ func newSaramaConfig(ctx context.Context, o *options) (*sarama.Config, error) { err = completeSaramaSASLConfig(ctx, config, o) if err != nil { - return nil, errors.WrapError(errors.ErrKafkaInvalidConfig, err) + return nil, err } kafkaVersion, err := getKafkaVersion(config, o) @@ -130,7 +130,7 @@ func newSaramaConfig(ctx context.Context, o *options) (*sarama.Config, error) { if o.IsAssignedVersion { version, err := sarama.ParseKafkaVersion(o.Version) if err != nil { - return nil, errors.WrapError(errors.ErrKafkaInvalidVersion, err) + return nil, errors.WrapError(errors.ErrKafkaInvalidConfig, err) } config.Version = version if !version.IsAtLeast(maxKafkaVersion) && version.String() != kafkaVersion.String() { @@ -177,7 +177,7 @@ func completeSaramaSASLConfig(ctx context.Context, config *sarama.Config, o *opt case SASLTypeOAuth: p, err := newTokenProvider(ctx, o) if err != nil { - return errors.Trace(err) + return err } config.Net.SASL.TokenProvider = p } @@ -216,7 +216,7 @@ func getKafkaVersion(config *sarama.Config, o *options) (sarama.KafkaVersion, er if o.IsAssignedVersion { assignedVersion, err := sarama.ParseKafkaVersion(o.Version) if err != nil { - return assignedVersion, errors.WrapError(errors.ErrKafkaInvalidVersion, err) + return assignedVersion, errors.WrapError(errors.ErrKafkaInvalidConfig, err) } if !assignedVersion.IsAtLeast(maxKafkaVersion) && assignedVersion.String() != targetVersion.String() { log.Warn("The Kafka version you assigned may not be correct. "+ diff --git a/pkg/sink/kafka/sarama_config_test.go b/pkg/sink/kafka/sarama_config_test.go index bfb0147a1c..ca3adbbdcb 100644 --- a/pkg/sink/kafka/sarama_config_test.go +++ b/pkg/sink/kafka/sarama_config_test.go @@ -21,9 +21,9 @@ import ( "github.com/IBM/sarama" "github.com/gin-gonic/gin/binding" - "github.com/pingcap/errors" - commonType "github.com/pingcap/ticdc/pkg/common" + "github.com/pingcap/ticdc/pkg/common" "github.com/pingcap/ticdc/pkg/config" + "github.com/pingcap/ticdc/pkg/errors" "github.com/pingcap/ticdc/pkg/security" "github.com/stretchr/testify/require" ) @@ -85,6 +85,21 @@ func TestNewSaramaConfig(t *testing.T) { require.Equal(t, sarama.SASLMechanism("SCRAM-SHA-256"), cfg.Net.SASL.Mechanism) } +func TestNewSaramaConfigInvalidOAuthTokenURL(t *testing.T) { + options := NewOptions() + options.SASL = &security.SASL{ + SASLMechanism: security.OAuthMechanism, + OAuth2: security.OAuth2{ + TokenURL: "http://test.com/Segment%%2815197306101420000%29", + }, + } + + _, err := newSaramaConfig(t.Context(), options) + require.ErrorIs(t, err, errors.ErrKafkaInvalidConfig) + var escapeErr url.EscapeError + require.ErrorAs(t, err, &escapeErr) +} + func TestNewSaramaConfigMaxRetryFromSinkURI(t *testing.T) { t.Parallel() @@ -126,7 +141,7 @@ func TestNewSaramaConfigMaxRetryFromSinkURI(t *testing.T) { sinkURI, err := url.Parse(test.sinkURI) require.NoError(t, err) err = options.Apply( - commonType.NewChangefeedID4Test(commonType.DefaultKeyspaceName, "test"), + common.NewChangefeedID4Test(common.DefaultKeyspaceName, "test"), sinkURI, config.GetDefaultReplicaConfig().Sink, ) diff --git a/pkg/sink/kafka/sarama_factory.go b/pkg/sink/kafka/sarama_factory.go index 650f346b4c..04a195cffb 100644 --- a/pkg/sink/kafka/sarama_factory.go +++ b/pkg/sink/kafka/sarama_factory.go @@ -46,19 +46,24 @@ func NewSaramaFactory( zap.Stringer("changefeedID", changefeedID), zap.Any("duration", duration)) } if err != nil { - return nil, errors.Trace(err) + return nil, err } admin, err := newAdminClient(changefeedID, o.BrokerEndpoints, config) if err != nil { - return nil, errors.Trace(err) + return nil, err } defer func() { admin.Close() }() +<<<<<<< HEAD if err = adjustOptions(ctx, admin, o, o.Topic); err != nil { return nil, errors.Trace(err) +======= + if err = adjustOptions(changefeedID, admin, o, o.Topic); err != nil { + return nil, err +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) } return &saramaFactory{ @@ -77,7 +82,7 @@ func newAdminClient(changefeedID common.ChangeFeedID, endpoints []string, config zap.Any("duration", duration), zap.Stringer("changefeedID", changefeedID)) } if err != nil { - return nil, errors.Trace(err) + return nil, errors.WrapError(errors.ErrNewKafkaSink, err) } start = time.Now() @@ -91,7 +96,7 @@ func newAdminClient(changefeedID common.ChangeFeedID, endpoints []string, config // `sarama.NewClusterAdminFromClient` does not take ownership of the client, // so we need to close it on failures to avoid leaking background goroutines. _ = client.Close() - return nil, errors.Trace(err) + return nil, errors.WrapError(errors.ErrNewKafkaSink, err) } return &saramaAdminClient{ client: client, @@ -103,7 +108,7 @@ func newAdminClient(changefeedID common.ChangeFeedID, endpoints []string, config func (f *saramaFactory) AdminClient(ctx context.Context) (ClusterAdminClient, error) { config, err := newSaramaConfig(ctx, f.option) if err != nil { - return nil, errors.WrapError(errors.ErrKafkaNewProducer, err) + return nil, err } return newAdminClient(f.changefeedID, f.option.BrokerEndpoints, config) } @@ -113,18 +118,19 @@ func (f *saramaFactory) AdminClient(ctx context.Context) (ClusterAdminClient, er func (f *saramaFactory) SyncProducer(ctx context.Context) (SyncProducer, error) { config, err := newSaramaConfig(ctx, f.option) if err != nil { - return nil, errors.WrapError(errors.ErrKafkaNewProducer, err) + return nil, err } config.MetricRegistry = f.metricRegistry client, err := sarama.NewClient(f.option.BrokerEndpoints, config) if err != nil { - return nil, errors.WrapError(errors.ErrKafkaNewProducer, err) + return nil, errors.WrapError(errors.ErrNewKafkaSink, err) } p, err := sarama.NewSyncProducerFromClient(client) if err != nil { - return nil, errors.WrapError(errors.ErrKafkaNewProducer, err) + _ = client.Close() + return nil, errors.WrapError(errors.ErrNewKafkaSink, err) } return &saramaSyncProducer{ @@ -140,25 +146,25 @@ func (f *saramaFactory) SyncProducer(ctx context.Context) (SyncProducer, error) func (f *saramaFactory) AsyncProducer(ctx context.Context) (AsyncProducer, error) { config, err := newSaramaConfig(ctx, f.option) if err != nil { - return nil, errors.WrapError(errors.ErrKafkaNewProducer, err) + return nil, err } config.MetricRegistry = f.metricRegistry client, err := sarama.NewClient(f.option.BrokerEndpoints, config) if err != nil { - return nil, errors.WrapError(errors.ErrKafkaNewProducer, err) + return nil, errors.WrapError(errors.ErrNewKafkaSink, err) } p, err := sarama.NewAsyncProducerFromClient(client) if err != nil { - return nil, errors.WrapError(errors.ErrKafkaNewProducer, err) + _ = client.Close() + return nil, errors.WrapError(errors.ErrNewKafkaSink, err) } return &saramaAsyncProducer{ client: client, producer: p, changefeedID: f.changefeedID, closed: atomic.NewBool(false), - failpointCh: make(chan *sarama.ProducerError, 1), }, nil } diff --git a/pkg/sink/kafka/sarama_sync_producer.go b/pkg/sink/kafka/sarama_sync_producer.go index 9d5efdfb0b..754d1db9e1 100644 --- a/pkg/sink/kafka/sarama_sync_producer.go +++ b/pkg/sink/kafka/sarama_sync_producer.go @@ -17,11 +17,10 @@ import ( "time" "github.com/IBM/sarama" - "github.com/pingcap/failpoint" "github.com/pingcap/log" - commonType "github.com/pingcap/ticdc/pkg/common" + "github.com/pingcap/ticdc/pkg/common" "github.com/pingcap/ticdc/pkg/errors" - "github.com/pingcap/ticdc/pkg/sink/codec/common" + codecCommon "github.com/pingcap/ticdc/pkg/sink/codec/common" "go.uber.org/atomic" "go.uber.org/zap" ) @@ -38,15 +37,15 @@ type saramaSyncProducerClient interface { } type saramaSyncProducer struct { - id commonType.ChangeFeedID + id common.ChangeFeedID client saramaSyncClient producer saramaSyncProducerClient closed *atomic.Bool } -func (p *saramaSyncProducer) SendMessage(topic string, partitionNum int32, message *common.Message) error { +func (p *saramaSyncProducer) SendMessage(topic string, partitionNum int32, message *codecCommon.Message) error { if p.closed.Load() { - return errors.ErrKafkaProducerClosed.GenWithStackByArgs() + return errors.ErrKafkaSinkClosed.GenWithStackByArgs() } msg := &sarama.ProducerMessage{ @@ -56,24 +55,20 @@ func (p *saramaSyncProducer) SendMessage(topic string, partitionNum int32, messa Partition: partitionNum, } _, _, err := p.producer.SendMessage(msg) - - failpoint.Inject("KafkaSinkSyncSendMessageError", func() { - err = errors.WrapError(errors.ErrKafkaSendMessage, errors.New("kafka sink sync send message injected error")) - }) - if err != nil { - err = AnnotateEventError( - p.id.Keyspace(), - p.id.Name(), - message.LogInfo, - err, - ) + if err == nil { + return nil } + log.Error("send message to kafka failed", + zap.String("keyspace", p.id.Keyspace()), + zap.String("changefeed", p.id.Name()), + zap.String("eventContext", BuildEventLogContext(p.id.Keyspace(), p.id.Name(), message.LogInfo)), + zap.Error(err)) return errors.WrapError(errors.ErrKafkaSendMessage, err) } -func (p *saramaSyncProducer) SendMessages(topic string, partitionNum int32, message *common.Message) error { +func (p *saramaSyncProducer) SendMessages(topic string, partitionNum int32, message *codecCommon.Message) error { if p.closed.Load() { - return errors.ErrKafkaProducerClosed.GenWithStackByArgs() + return errors.ErrKafkaSinkClosed.GenWithStackByArgs() } msgs := make([]*sarama.ProducerMessage, partitionNum) @@ -86,18 +81,14 @@ func (p *saramaSyncProducer) SendMessages(topic string, partitionNum int32, mess } } err := p.producer.SendMessages(msgs) - - failpoint.Inject("KafkaSinkSyncSendMessagesError", func() { - err = errors.WrapError(errors.ErrKafkaSendMessage, errors.New("kafka sink sync send messages injected error")) - }) - if err != nil { - err = AnnotateEventError( - p.id.Keyspace(), - p.id.Name(), - message.LogInfo, - err, - ) + if err == nil { + return nil } + log.Error("send message to kafka failed", + zap.String("keyspace", p.id.Keyspace()), + zap.String("changefeed", p.id.Name()), + zap.String("eventContext", BuildEventLogContext(p.id.Keyspace(), p.id.Name(), message.LogInfo)), + zap.Error(err)) return errors.WrapError(errors.ErrKafkaSendMessage, err) } diff --git a/pkg/sink/kafka/sarama_sync_producer_test.go b/pkg/sink/kafka/sarama_sync_producer_test.go index 281af9c634..cd1b0078fa 100644 --- a/pkg/sink/kafka/sarama_sync_producer_test.go +++ b/pkg/sink/kafka/sarama_sync_producer_test.go @@ -14,15 +14,25 @@ package kafka import ( - "errors" + "context" + "io" + "strings" "testing" "github.com/IBM/sarama" +<<<<<<< HEAD "github.com/pingcap/ticdc/pkg/common" +======= + "github.com/golang/mock/gomock" + "github.com/pingcap/ticdc/pkg/common" + "github.com/pingcap/ticdc/pkg/errors" + codecCommon "github.com/pingcap/ticdc/pkg/sink/codec/common" +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) "github.com/stretchr/testify/require" "go.uber.org/atomic" ) +<<<<<<< HEAD type testSyncProducerClient struct { closeCalls int closeErr error @@ -84,4 +94,121 @@ func TestSaramaSyncProducerCloseStillClosesProducerWhenClientCloseFails(t *testi require.Equal(t, 1, client.closeCalls) require.Equal(t, 1, producer.closeCalls) +======= +func TestProducerRejectsSendAfterClose(t *testing.T) { + t.Parallel() + + message := &codecCommon.Message{} + syncProducer := &saramaSyncProducer{closed: atomic.NewBool(true)} + require.ErrorIs(t, syncProducer.SendMessage("topic", 1, message), errors.ErrKafkaSinkClosed) + require.ErrorIs(t, syncProducer.SendMessages("topic", 1, message), errors.ErrKafkaSinkClosed) + + asyncProducer := &saramaAsyncProducer{closed: atomic.NewBool(true)} + require.ErrorIs(t, asyncProducer.AsyncSend(context.Background(), "topic", 0, message), errors.ErrKafkaSinkClosed) +} + +func TestSyncProducerClose(t *testing.T) { + tests := []struct { + name string + clientCloseErr error + }{ + { + name: "closes client and producer", + }, + { + name: "still closes producer when client close fails", + clientCloseErr: io.ErrClosedPipe, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + client := NewMocksaramaSyncClient(ctrl) + producer := NewMocksaramaSyncProducerClient(ctrl) + gomock.InOrder( + client.EXPECT().Close().Return(test.clientCloseErr), + producer.EXPECT().Close().Return(nil), + ) + + p := &saramaSyncProducer{ + id: common.NewChangeFeedIDWithName("test", "default"), + client: client, + producer: producer, + closed: atomic.NewBool(false), + } + + p.Close() + }) + } +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) +} + +func TestSyncProducerErrorWrappedOnce(t *testing.T) { + cause := io.ErrClosedPipe + tests := []struct { + name string + expectSend func(*MocksaramaSyncProducerClient) + send func(*saramaSyncProducer, *codecCommon.Message) error + }{ + { + name: "single message", + expectSend: func(producer *MocksaramaSyncProducerClient) { + producer.EXPECT().SendMessage(gomock.Any()).Return(int32(0), int64(0), cause) + }, + send: func(producer *saramaSyncProducer, message *codecCommon.Message) error { + return producer.SendMessage("topic", 0, message) + }, + }, + { + name: "message batch", + expectSend: func(producer *MocksaramaSyncProducerClient) { + producer.EXPECT().SendMessages(gomock.Any()).Return(cause) + }, + send: func(producer *saramaSyncProducer, message *codecCommon.Message) error { + return producer.SendMessages("topic", 1, message) + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + producer := NewMocksaramaSyncProducerClient(ctrl) + test.expectSend(producer) + p := &saramaSyncProducer{ + id: common.NewChangeFeedIDWithName("test", "default"), + producer: producer, + closed: atomic.NewBool(false), + } + message := &codecCommon.Message{LogInfo: &codecCommon.MessageLogInfo{}} + + err := test.send(p, message) + + requireKafkaSendError(t, err, cause) + }) + } +} + +func TestAsyncProducerErrorWrappedOnce(t *testing.T) { + cause := io.ErrClosedPipe + producer := &saramaAsyncProducer{ + changefeedID: common.NewChangeFeedIDWithName("test", "default"), + } + err := producer.handleProducerError(&sarama.ProducerError{ + Err: cause, + Msg: &sarama.ProducerMessage{Metadata: &messageMetadata{ + logInfo: &codecCommon.MessageLogInfo{}, + }}, + }) + + requireKafkaSendError(t, err, cause) +} + +func requireKafkaSendError(t *testing.T, err, cause error) { + t.Helper() + require.ErrorIs(t, err, errors.ErrKafkaSendMessage) + require.ErrorIs(t, err, cause) + require.Equal(t, 1, strings.Count(err.Error(), string(errors.ErrKafkaSendMessage.RFCCode()))) + require.NotContains(t, err.Error(), "keyspace=test") } diff --git a/pkg/util/external_storage.go b/pkg/util/external_storage.go index f276c69638..43e5fe3010 100644 --- a/pkg/util/external_storage.go +++ b/pkg/util/external_storage.go @@ -64,7 +64,7 @@ func getExternalStorage( ) (storage.ExternalStorage, error) { backEnd, err := storage.ParseBackend(uri, opts) if err != nil { - return nil, errors.Trace(err) + return nil, errors.WrapError(errors.ErrExternalStorageAPI, err) } ret, err := storage.New(ctx, backEnd, &storage.ExternalStorageOptions{ @@ -72,7 +72,7 @@ func getExternalStorage( S3Retryer: retryer, }) if err != nil { - return nil, errors.WrapError(errors.ErrFailToCreateExternalStorage, err) + return nil, errors.WrapError(errors.ErrExternalStorageAPI, err) } defer func() { if err != nil { @@ -83,7 +83,7 @@ func getExternalStorage( // Check the connection and ignore the returned bool value, since we don't care if the file exists. _, err = ret.FileExists(ctx, "test") if err != nil { - return nil, errors.WrapError(errors.ErrFailToCreateExternalStorage, err) + return nil, errors.WrapError(errors.ErrExternalStorageAPI, err) } return ret, nil } diff --git a/tests/integration_tests/http_api/util/test_case.py b/tests/integration_tests/http_api/util/test_case.py index bd1f27296c..b7120e6200 100644 --- a/tests/integration_tests/http_api/util/test_case.py +++ b/tests/integration_tests/http_api/util/test_case.py @@ -1,4 +1,5 @@ import sys +import os import requests as rq from requests.exceptions import RequestException import time @@ -175,7 +176,9 @@ def create_changefeed(sink_uri): }) headers = {"Content-Type": "application/json"} resp = rq.post(url, data=data, headers=headers) - assert "CDC:ErrKafkaNewProducer" in resp.text, f"{resp.text}" + expected_error = "CDC:ErrKafkaNewProducer" if os.getenv( + "TICDC_NEWARCH") == "false" else "CDC:ErrNewKafkaSink" + assert expected_error in resp.text, f"{resp.text}" assert "not found, ResolveEndpointV2" not in resp.text, f"{resp.text}" print("pass test: create changefeed") diff --git a/tests/integration_tests/kafka_big_messages/run.sh b/tests/integration_tests/kafka_big_messages/run.sh index 0628eaa92e..c36f7068e9 100755 --- a/tests/integration_tests/kafka_big_messages/run.sh +++ b/tests/integration_tests/kafka_big_messages/run.sh @@ -8,6 +8,178 @@ WORK_DIR=$OUT_DIR/$TEST_NAME CDC_BINARY=cdc.test SINK_TYPE=$1 +<<<<<<< HEAD +======= +STATE_WAIT_TIMEOUT_SECONDS=30 +STATE_CHECK_INTERVAL_SECONDS=1 +TABLE_CHECK_RETRIES=15 +BATCH_LIMIT=262144 +SMALL_TOPIC_LIMIT=524288 +LARGE_TOPIC_LIMIT=2097152 +ROW_BYTES=1048576 +SCHEMA_REGISTRY_URI=http://127.0.0.1:8088 +GENERATOR_DIR=$CUR/../../utils/gen_kafka_big_messages +consumer_pid="" + +function start_schema_registry() { + if curl -o /dev/null -s "$SCHEMA_REGISTRY_URI"; then + return + fi + + echo "Starting schema registry..." + ./bin/bin/schema-registry-start -daemon ./bin/etc/schema-registry/schema-registry.properties + local i=0 + while ! curl -o /dev/null -s "$SCHEMA_REGISTRY_URI"; do + i=$((i + 1)) + if [ "$i" -gt 30 ]; then + echo "Failed to start schema registry" + exit 1 + fi + sleep 2 + done + curl -X PUT -H "Content-Type: application/vnd.schemaregistry.v1+json" --data '{"compatibility": "NONE"}' "$SCHEMA_REGISTRY_URI/config" +} + +function build_message_generator() { + if [ ! -f "$GENERATOR_DIR/gen_kafka_big_messages" ]; then + (cd "$GENERATOR_DIR" && GO111MODULE=on go build) + fi +} + +function kafka_sink_uri() { + local topic_name=$1 + local protocol=$2 + local extra_params=$3 + local sink_uri="kafka://127.0.0.1:9092/${topic_name}?protocol=${protocol}&partition-num=1&kafka-version=${KAFKA_VERSION}&max-message-bytes=${BATCH_LIMIT}" + if [ "$extra_params" != "" ]; then + sink_uri="${sink_uri}&${extra_params}" + fi + echo "$sink_uri" +} + +function start_kafka_consumer() { + local work_dir=$1 + local sink_uri=$2 + local schema_registry_uri=$3 + local protocol_case=$4 + local downstream_uri="mysql://root@${DOWN_TIDB_HOST}:${DOWN_TIDB_PORT}/?safe-mode=true&batch-dml-enable=false&enable-ddl-ts=false" + local args=( + --log-file "$work_dir/cdc_kafka_consumer.log" + --log-level debug + --upstream-uri "$sink_uri" + --downstream-uri "$downstream_uri" + ) + if [ "$schema_registry_uri" != "" ]; then + args+=(--schema-registry-uri "$schema_registry_uri") + fi + if [[ "$protocol_case" == simple_* ]]; then + args+=(--upstream-tidb-dsn "root@tcp(${UP_TIDB_HOST}:${UP_TIDB_PORT})/?") + fi + + cdc_kafka_consumer "${args[@]}" >>"$work_dir/cdc_kafka_consumer_stdout.log" 2>&1 & + consumer_pid=$! +} + +function stop_kafka_consumer() { + if [ "$consumer_pid" != "" ]; then + kill -9 "$consumer_pid" 2>/dev/null || true + wait "$consumer_pid" 2>/dev/null || true + consumer_pid="" + fi +} + +function wait_changefeed_state() { + local pd_addr=$1 + local changefeed_id=$2 + local expected_state=$3 + local expected_error=$4 + local deadline=$((SECONDS + STATE_WAIT_TIMEOUT_SECONDS)) + + while true; do + if check_changefeed_state "$pd_addr" "$changefeed_id" "$expected_state" "$expected_error" ""; then + return + fi + if [ "$SECONDS" -ge "$deadline" ]; then + echo "changefeed $changefeed_id did not reach state $expected_state within ${STATE_WAIT_TIMEOUT_SECONDS}s" + return 1 + fi + sleep "$STATE_CHECK_INTERVAL_SECONDS" + done +} + +function render_diff_config() { + local work_dir=$1 + local database_name=$2 + local diff_config=$3 + + sed -e "s/database_name/${database_name}/g" \ + -e "s|/tmp/tidb_cdc_test/kafka_big_messages/sync_diff/output|${work_dir}/sync_diff/output|g" \ + "$CUR/conf/diff_config.toml" >"$diff_config" +} + +function run_protocol_case() { + local protocol_case=$1 + local protocol=$2 + local schema_registry_uri=$3 + local extra_params=$4 + local topic_case=${protocol_case//_/-} + local topic_name="big-message-${topic_case}-${RANDOM}" + local changefeed_id="kafka-big-messages-${topic_case}" + local database_name="kafka_big_messages_${protocol_case}" + local work_dir="$WORK_DIR/$protocol_case" + local sql_file="$work_dir/test.sql" + local diff_config="$work_dir/diff_config.toml" + local pd_addr="http://${UP_PD_HOST_1}:${UP_PD_PORT_1}" + local sink_uri + local initial_topic_limit=$SMALL_TOPIC_LIMIT + local expected_error=ErrMessageTooLarge + if [ "$protocol_case" = "async_error" ]; then + initial_topic_limit=$LARGE_TOPIC_LIMIT + expected_error=ErrKafkaSendMessage + fi + + mkdir -p "$work_dir" + render_diff_config "$work_dir" "$database_name" "$diff_config" + kafka_topic --topic "$topic_name" --max-message-bytes "$initial_topic_limit" + local start_ts + start_ts=$(run_cdc_cli_tso_query "$UP_PD_HOST_1" "$UP_PD_PORT_1") + sink_uri=$(kafka_sink_uri "$topic_name" "$protocol" "$extra_params") + + if [ "$schema_registry_uri" != "" ]; then + cdc_cli_changefeed create --start-ts="$start_ts" --sink-uri="$sink_uri" --schema-registry="$schema_registry_uri" -c "$changefeed_id" + else + cdc_cli_changefeed create --start-ts="$start_ts" --sink-uri="$sink_uri" -c "$changefeed_id" + fi + start_kafka_consumer "$work_dir" "$sink_uri" "$schema_registry_uri" "$protocol_case" + wait_changefeed_state "$pd_addr" "$changefeed_id" "normal" "null" + + # Lower the topic limit after the producer has started. The encoder and + # producer still accept the message, then Kafka rejects it asynchronously. + if [ "$protocol_case" = "async_error" ]; then + local ready_database="${database_name}_ready" + run_sql "CREATE DATABASE ${ready_database}; CREATE TABLE ${ready_database}.ready(id INT PRIMARY KEY); INSERT INTO ${ready_database}.ready VALUES (1)" "$UP_TIDB_HOST" "$UP_TIDB_PORT" + ensure "$TABLE_CHECK_RETRIES" "run_sql 'SELECT id FROM ${ready_database}.ready' '$DOWN_TIDB_HOST' '$DOWN_TIDB_PORT' && check_contains 'id: 1'" + kafka_topic --topic "$topic_name" --max-message-bytes "$SMALL_TOPIC_LIMIT" --alter + fi + + "$GENERATOR_DIR/gen_kafka_big_messages" --row-bytes="$ROW_BYTES" --row-count=1 --database-name="$database_name" --table-name=test --sql-file-path="$sql_file" + run_sql_file "$sql_file" "$UP_TIDB_HOST" "$UP_TIDB_PORT" + run_sql "CREATE TABLE ${database_name}.finish_mark(id INT PRIMARY KEY)" "$UP_TIDB_HOST" "$UP_TIDB_PORT" + + wait_changefeed_state "$pd_addr" "$changefeed_id" "warning" "$expected_error" + + # Only increase Kafka's topic limit. TiCDC must recreate the sink, read the + # new limit, and resume without updating, pausing, or resuming the changefeed. + kafka_topic --topic "$topic_name" --max-message-bytes "$LARGE_TOPIC_LIMIT" --alter + wait_changefeed_state "$pd_addr" "$changefeed_id" "normal" "null" + check_table_exists "${database_name}.finish_mark" "$DOWN_TIDB_HOST" "$DOWN_TIDB_PORT" "$TABLE_CHECK_RETRIES" + check_sync_diff "$work_dir" "$diff_config" + + cdc_cli_changefeed remove -c "$changefeed_id" + stop_kafka_consumer +} + +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) function run() { # test kafka sink only in this case if [ "$SINK_TYPE" != "kafka" ]; then @@ -15,7 +187,18 @@ function run() { fi rm -rf $WORK_DIR && mkdir -p $WORK_DIR +<<<<<<< HEAD start_tidb_cluster --workdir $WORK_DIR +======= + local cases=( + "canal_json|canal-json||enable-tidb-extension=true" + "open_protocol|open-protocol||" + "async_error|open-protocol||max-retry=0" + "simple_json|simple||" + "simple_avro|simple||encoding-format=avro" + "avro|avro|$SCHEMA_REGISTRY_URI|enable-tidb-extension=true&avro-enable-watermark=true&avro-decimal-handling-mode=string&avro-bigint-unsigned-handling-mode=string" + ) +>>>>>>> fa340f118 (kafka: unify sink errors and replace failpoint tests (#5786)) TOPIC_NAME="big-message-test-$RANDOM" diff --git a/tests/integration_tests/kafka_log_info/run.sh b/tests/integration_tests/kafka_log_info/run.sh deleted file mode 100755 index de4ae60465..0000000000 --- a/tests/integration_tests/kafka_log_info/run.sh +++ /dev/null @@ -1,134 +0,0 @@ -#!/bin/bash - -set -eu - -CUR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) -source $CUR/../_utils/test_prepare -WORK_DIR=$OUT_DIR/$TEST_NAME -CDC_BINARY=cdc.test -SINK_TYPE=$1 - -MAX_RETRIES=20 -pd_addr="http://$UP_PD_HOST_1:$UP_PD_PORT_1" -protocols=("open-protocol" "canal-json" "simple") - -declare -r DB_NAME="kafka_log_info" - -function build_sink_uri() { - local protocol=$1 - local topic=$2 - echo "kafka://127.0.0.1:9092/$topic?protocol=$protocol&partition-num=1&kafka-version=${KAFKA_VERSION}&max-message-bytes=10485760" -} - -function cleanup_changefeed() { - local id=$1 - cdc_cli_changefeed remove --pd="${pd_addr}" --changefeed-id="$id" >/dev/null 2>&1 || true - # Wait for the changefeed removal to be fully persisted and visible to all components. - # Otherwise, the next TiCDC process may resume the leftover changefeed and consume failpoints unexpectedly. - sleep 5 -} - -function assert_no_changefeeds() { - local feed_count - feed_count=$(cdc_cli_changefeed list --pd="$pd_addr" | grep -v "Command to ticdc" | jq '.|length') - if [[ "$feed_count" != "0" ]]; then - echo "[$(date)] <<<<< existing changefeeds detected before create, count: ${feed_count} >>>>>" - cdc_cli_changefeed list --pd="$pd_addr" | grep -v "Command to ticdc" | jq . - exit 1 - fi -} - -function stop_cdc() { - cleanup_process $CDC_BINARY - export GO_FAILPOINTS="" -} - -function start_cdc_with_failpoint() { - local failpoints=$1 - export GO_FAILPOINTS="$failpoints" - run_cdc_server --workdir $WORK_DIR --binary $CDC_BINARY --addr "127.0.0.1:8300" --pd $pd_addr -} - -function test_dml_log_info() { - local protocol=$1 - local topic="kafka-log-info-dml-${protocol}-${RANDOM}" - local changefeed_id="kafka-log-info-${protocol}-dml" - local sink_uri=$(build_sink_uri $protocol $topic) - - run_sql "DROP TABLE IF EXISTS ${DB_NAME}.dml_table" ${UP_TIDB_HOST} ${UP_TIDB_PORT} - run_sql "CREATE TABLE ${DB_NAME}.dml_table(id INT PRIMARY KEY AUTO_INCREMENT, val INT);" ${UP_TIDB_HOST} ${UP_TIDB_PORT} - - start_cdc_with_failpoint 'github.com/pingcap/ticdc/pkg/sink/kafka/KafkaSinkAsyncSendError=1*return(true)' - assert_no_changefeeds - cdc_cli_changefeed create --pd=$pd_addr --sink-uri="$sink_uri" --changefeed-id="$changefeed_id" - - run_sql "INSERT INTO ${DB_NAME}.dml_table(val) VALUES (1);" ${UP_TIDB_HOST} ${UP_TIDB_PORT} - - local pattern='eventType=dml.*\\"Table\\":\\"dml_table\\".*\\"StartTs\\":.*\\"CommitTs\\":' - ensure $MAX_RETRIES "check_logs_contains $WORK_DIR '$pattern' ''" - - cleanup_changefeed $changefeed_id - stop_cdc -} - -function test_ddl_log_info() { - local protocol=$1 - local topic="kafka-log-info-ddl-${protocol}-${RANDOM}" - local changefeed_id="kafka-log-info-${protocol}-ddl" - local sink_uri=$(build_sink_uri $protocol $topic) - - run_sql "DROP TABLE IF EXISTS ${DB_NAME}.ddl_table;" ${UP_TIDB_HOST} ${UP_TIDB_PORT} - - start_cdc_with_failpoint 'github.com/pingcap/ticdc/pkg/sink/kafka/KafkaSinkSyncSendMessageError=1*return(true);github.com/pingcap/ticdc/pkg/sink/kafka/KafkaSinkSyncSendMessagesError=1*return(true)' - assert_no_changefeeds - cdc_cli_changefeed create --pd=$pd_addr --sink-uri="$sink_uri" --changefeed-id="$changefeed_id" - - run_sql "CREATE TABLE ${DB_NAME}.ddl_table(id INT PRIMARY KEY);" ${UP_TIDB_HOST} ${UP_TIDB_PORT} - - local ddl_pattern="eventType=ddl.*ddlQuery=.*CREATE TABLE*" - ensure $MAX_RETRIES "check_logs_contains $WORK_DIR '$ddl_pattern' ''" - - cleanup_changefeed $changefeed_id - stop_cdc -} - -function test_checkpoint_log_info() { - local protocol=$1 - local topic="kafka-log-info-checkpoint-${protocol}-${RANDOM}" - local changefeed_id="kafka-log-info-${protocol}-checkpoint" - local sink_uri=$(build_sink_uri $protocol $topic) - - start_cdc_with_failpoint 'github.com/pingcap/ticdc/pkg/sink/kafka/KafkaSinkSyncSendMessagesError=1*return(true)' - assert_no_changefeeds - cdc_cli_changefeed create --pd=$pd_addr --sink-uri="$sink_uri" --changefeed-id="$changefeed_id" - - ensure $MAX_RETRIES "check_logs_contains $WORK_DIR 'eventType=checkpoint.*checkpointTs=' ''" - - cleanup_changefeed $changefeed_id - stop_cdc -} - -function run() { - if [ "$SINK_TYPE" != "kafka" ]; then - echo "skip kafka_log_info for sink type $SINK_TYPE" - return - fi - - rm -rf $WORK_DIR && mkdir -p $WORK_DIR - start_tidb_cluster --workdir $WORK_DIR - run_sql "DROP DATABASE IF EXISTS ${DB_NAME};" ${UP_TIDB_HOST} ${UP_TIDB_PORT} - run_sql "CREATE DATABASE ${DB_NAME};" ${UP_TIDB_HOST} ${UP_TIDB_PORT} - - for protocol in "${protocols[@]}"; do - test_dml_log_info $protocol - test_ddl_log_info $protocol - test_checkpoint_log_info $protocol - done -} - -trap "stop_cdc; stop_tidb_cluster" EXIT - -run $* -check_logs $WORK_DIR - -echo "[$(date)] <<<<<< run test case $TEST_NAME success! >>>>>>" diff --git a/tests/integration_tests/mq_sink_error_resume/run.sh b/tests/integration_tests/mq_sink_error_resume/run.sh index 6fb047381f..d4f66e2f91 100755 --- a/tests/integration_tests/mq_sink_error_resume/run.sh +++ b/tests/integration_tests/mq_sink_error_resume/run.sh @@ -13,8 +13,7 @@ DB_COUNT=4 MAX_RETRIES=20 function run() { - # test MQ sink only in this case - if [ "$SINK_TYPE" != "kafka" ] && [ "$SINK_TYPE" != "pulsar" ]; then + if [ "$SINK_TYPE" != "pulsar" ]; then return fi @@ -24,22 +23,14 @@ function run() { pd_addr="http://$UP_PD_HOST_1:$UP_PD_PORT_1" TOPIC_NAME="ticdc-mq-sink-error-resume-test-$RANDOM" - case $SINK_TYPE in - kafka) SINK_URI="kafka://127.0.0.1:9092/$TOPIC_NAME?protocol=open-protocol&partition-num=4&kafka-version=${KAFKA_VERSION}&max-message-bytes=10485760" ;; - pulsar) - run_pulsar_cluster $WORK_DIR normal - SINK_URI="pulsar://127.0.0.1:6650/$TOPIC_NAME?protocol=canal-json&enable-tidb-extension=true" - ;; - esac - # Return an failpoint error to fail a kafka changefeed. + run_pulsar_cluster $WORK_DIR normal + SINK_URI="pulsar://127.0.0.1:6650/$TOPIC_NAME?protocol=canal-json&enable-tidb-extension=true" + # Return one failpoint error to fail the changefeed. # Note we return one error for the failpoint, if owner retry changefeed frequently, it may break the test. - export GO_FAILPOINTS='github.com/pingcap/ticdc/pkg/sink/kafka/KafkaSinkAsyncSendError=1*return(true);github.com/pingcap/ticdc/downstreamadapter/sink/pulsar/PulsarSinkAsyncSendError=1*return(true)' + export GO_FAILPOINTS='github.com/pingcap/ticdc/downstreamadapter/sink/pulsar/PulsarSinkAsyncSendError=1*return(true)' run_cdc_server --workdir $WORK_DIR --binary $CDC_BINARY --addr "127.0.0.1:8300" --pd $pd_addr changefeed_id=$(cdc_cli_changefeed create --pd=$pd_addr --sink-uri="$SINK_URI" | grep '^ID:' | head -n1 | awk '{print $2}') - case $SINK_TYPE in - kafka) run_kafka_consumer $WORK_DIR "kafka://127.0.0.1:9092/$TOPIC_NAME?protocol=open-protocol&partition-num=4&version=${KAFKA_VERSION}&max-message-bytes=10485760" ;; - pulsar) run_pulsar_consumer --upstream-uri $SINK_URI ;; - esac + run_pulsar_consumer --upstream-uri $SINK_URI run_sql "CREATE DATABASE mq_sink_error_resume;" ${UP_TIDB_HOST} ${UP_TIDB_PORT} run_sql "CREATE table mq_sink_error_resume.t1(id int primary key auto_increment, val int);" ${UP_TIDB_HOST} ${UP_TIDB_PORT} diff --git a/tests/integration_tests/run_heavy_it_in_ci.sh b/tests/integration_tests/run_heavy_it_in_ci.sh index 1da9b6b814..c821353511 100755 --- a/tests/integration_tests/run_heavy_it_in_ci.sh +++ b/tests/integration_tests/run_heavy_it_in_ci.sh @@ -87,7 +87,7 @@ kafka_groups=( # 'kafka_simple_claim_check kafka_simple_claim_check_avro tidb_mysql_test' 'kafka_simple_claim_check kafka_simple_claim_check_avro' # G09 - 'kafka_simple_handle_key_only kafka_simple_handle_key_only_avro mq_sink_error_resume multi_source' + 'kafka_simple_handle_key_only kafka_simple_handle_key_only_avro multi_source' # G10 'kafka_column_selector kafka_column_selector_avro ddl_with_random_move_table' # G11 diff --git a/tests/integration_tests/run_light_it_in_ci.sh b/tests/integration_tests/run_light_it_in_ci.sh index 3a3ec7488c..b3f53dd968 100755 --- a/tests/integration_tests/run_light_it_in_ci.sh +++ b/tests/integration_tests/run_light_it_in_ci.sh @@ -107,7 +107,7 @@ kafka_groups=( # G14 'kafka_simple_basic avro_basic debezium_basic fail_over_ddl_O update_changefeed_check_config' # G15 - 'kafka_simple_basic_avro split_region autorandom gc_safepoint kafka_log_info' + 'kafka_simple_basic_avro split_region autorandom gc_safepoint' ) # Resource allocation for pulsar light integration tests in CI pipelines: From fb9a32fe17dd1f8a5a271b0d325817a996e603c5 Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Thu, 30 Jul 2026 17:23:55 +0800 Subject: [PATCH 5/6] kafka: defer max message bytes backport --- pkg/sink/kafka/options.go | 196 +++++++-------- pkg/sink/kafka/options_test.go | 129 ++++------ pkg/sink/kafka/sarama_config.go | 2 +- pkg/sink/kafka/sarama_factory.go | 2 +- .../kafka_big_messages/run.sh | 223 +++--------------- 5 files changed, 172 insertions(+), 380 deletions(-) diff --git a/pkg/sink/kafka/options.go b/pkg/sink/kafka/options.go index 2e7b00a4a0..5bc933a3bf 100644 --- a/pkg/sink/kafka/options.go +++ b/pkg/sink/kafka/options.go @@ -14,6 +14,7 @@ package kafka import ( + "context" "encoding/base64" "fmt" "net/http" @@ -38,6 +39,13 @@ const ( defaultPartitionNum = 3 // defaultMaxRetry is the default retry budget for Kafka producers. defaultMaxRetry = 5 + + // the `max-message-bytes` is set equal to topic's `max.message.bytes`, and is used to check + // whether the message is larger than the max size limit. It's found some message pass the message + // size limit check at the client side and failed at the broker side since message enlarged during + // the network transmission. so we set the `max-message-bytes` to a smaller value to avoid this problem. + // maxMessageBytesOverhead is used to reduce the `max-message-bytes`. + maxMessageBytesOverhead = 128 ) const ( @@ -136,7 +144,7 @@ type urlConfig struct { InsecureSkipVerify *bool `form:"insecure-skip-verify"` } -// options stores Kafka sink configurations +// options stores user specified configurations type options struct { Topic string BrokerEndpoints []string @@ -149,14 +157,14 @@ type options struct { Version string IsAssignedVersion bool RequestVersion int16 - - // MaxMessageBytes controls the byte size limit of the producer. - MaxMessageBytes int - - MaxRetry int - Compression string - ClientID string - RequiredAcks RequiredAcks + MaxMessageBytes int + MaxRetry int + Compression string + ClientID string + RequiredAcks RequiredAcks + // Only for test. User can not set this value. + // The current prod default value is 0. + MaxMessages int // Credential is used to connect to kafka cluster. EnableTLS bool @@ -173,7 +181,8 @@ type options struct { // NewOptions returns a default Kafka configuration func NewOptions() *options { return &options{ - Version: "2.4.0", + Version: "2.4.0", + // MaxMessageBytes will be used to initialize producer MaxMessageBytes: config.DefaultMaxMessageBytes, MaxRetry: defaultMaxRetry, ReplicationFactor: 1, @@ -190,12 +199,11 @@ func NewOptions() *options { } // setPartitionNum set the partition-num by the topic's partition count. -func (o *options) setPartitionNum(changefeedID common.ChangeFeedID, realPartitionCount int32) error { +func (o *options) setPartitionNum(realPartitionCount int32) error { // user does not specify the `partition-num` in the sink-uri if o.PartitionNum == 0 { o.PartitionNum = realPartitionCount log.Info("partitionNum is not set, set by topic's partition-num", - zap.String("namespace", changefeedID.Keyspace()), zap.String("changefeed", changefeedID.Name()), zap.Int32("partitionNum", realPartitionCount)) return nil } @@ -203,8 +211,8 @@ func (o *options) setPartitionNum(changefeedID common.ChangeFeedID, realPartitio if o.PartitionNum < realPartitionCount { log.Warn("number of partition specified in sink-uri is less than that of the actual topic. "+ "Some partitions will not have messages dispatched to", - zap.String("namespace", changefeedID.Keyspace()), zap.String("changefeed", changefeedID.Name()), - zap.Int32("sinkUriPartitions", o.PartitionNum), zap.Int32("topicPartitions", realPartitionCount)) + zap.Int32("sinkUriPartitions", o.PartitionNum), + zap.Int32("topicPartitions", realPartitionCount)) return nil } @@ -254,9 +262,6 @@ func (o *options) Apply(changefeedID common.ChangeFeedID, } if urlParameter.MaxMessageBytes != nil { - if *urlParameter.MaxMessageBytes <= 0 { - return errors.ErrKafkaInvalidConfig.GenWithStack("invalid max-message-bytes %d", *urlParameter.MaxMessageBytes) - } o.MaxMessageBytes = *urlParameter.MaxMessageBytes } @@ -611,11 +616,9 @@ func NewKafkaClientID(captureAddr string, return } -// adjustOptions adjusts options with Kafka runtime metadata. -// It overwrites MaxMessageBytes with the final producer message limit derived -// from the topic or broker configuration. +// adjustOptions adjust the `options` and `sarama.Config` by condition. func adjustOptions( - changefeedID common.ChangeFeedID, + ctx context.Context, admin ClusterAdminClient, options *options, topic string, @@ -629,108 +632,90 @@ func adjustOptions( // once we have found the topic, no matter `auto-create-topic`, // make sure user input parameters are valid. if exists { - err = adjustExistingTopicOption(changefeedID, admin, options, topic, info) - } else { - adjustNewTopicOptions(admin, changefeedID, options, topic) - } - if err != nil { - return err - } + // make sure that producer's `MaxMessageBytes` smaller than topic's `max.message.bytes` + topicMaxMessageBytesStr, found, err := getTopicConfig( + ctx, admin, info.Name, + TopicMaxMessageBytesConfigName, + BrokerMessageMaxBytesConfigName, + ) + if err != nil { + return err + } + if !found { + return errors.ErrKafkaAdminAPI.GenWithStack( + "Kafka configuration %s not found in topic %s or broker", + TopicMaxMessageBytesConfigName, info.Name) + } + topicMaxMessageBytes, err := strconv.Atoi(topicMaxMessageBytesStr) + if err != nil { + return errors.WrapError(errors.ErrKafkaAdminAPI, err, + "parse-config", TopicMaxMessageBytesConfigName) + } - return nil -} + maxMessageBytes := topicMaxMessageBytes - maxMessageBytesOverhead + if topicMaxMessageBytes <= options.MaxMessageBytes { + log.Warn("topic's `max.message.bytes` less than the `max-message-bytes`,"+ + "use topic's `max.message.bytes` to initialize the Kafka producer", + zap.Int("max.message.bytes", topicMaxMessageBytes), + zap.Int("max-message-bytes", options.MaxMessageBytes), + zap.Int("real-max-message-bytes", maxMessageBytes)) + options.MaxMessageBytes = maxMessageBytes + } else if maxMessageBytes < options.MaxMessageBytes { + options.MaxMessageBytes = maxMessageBytes + } -func adjustExistingTopicOption( - changefeedID common.ChangeFeedID, - admin ClusterAdminClient, - options *options, - topic string, - info TopicDetail, -) error { - maxMessageBytes, found, err := getTopicMaxMessageBytes(admin, info.Name) - if err != nil || !found { - log.Warn("`max.message.bytes` not found from topic's configuration, use the option `MaxMessageBytes` as default", - zap.String("namespace", changefeedID.Keyspace()), zap.String("changefeed", changefeedID.Name()), - zap.Int("maxMessageBytes", options.MaxMessageBytes), zap.Error(err)) - maxMessageBytes = options.MaxMessageBytes - } - options.MaxMessageBytes = maxMessageBytes + // no need to create the topic, + // but we would have to log user if they found enter wrong topic name later + if options.AutoCreate { + log.Warn("topic already exist, TiCDC will not create the topic", + zap.String("topic", topic), zap.Any("detail", info)) + } + + if err = options.setPartitionNum(info.NumPartitions); err != nil { + return err + } - // no need to create the topic, - // but we would have to log user if they found enter wrong topic name later - if options.AutoCreate { - log.Warn("topic already exist, TiCDC will not create the topic", - zap.String("namespace", changefeedID.Keyspace()), zap.String("changefeed", changefeedID.Name()), - zap.String("topic", topic), zap.Any("detail", info)) + return nil } - if err = options.setPartitionNum(changefeedID, info.NumPartitions); err != nil { + brokerMessageMaxBytesStr, found, err := admin.GetBrokerConfig(BrokerMessageMaxBytesConfigName) + if err != nil { + log.Warn("TiCDC cannot find `message.max.bytes` from broker's configuration") return err } - return nil -} + if !found { + return errors.ErrKafkaAdminAPI.GenWithStack( + "Kafka broker configuration %s not found", BrokerMessageMaxBytesConfigName) + } + brokerMessageMaxBytes, err := strconv.Atoi(brokerMessageMaxBytesStr) + if err != nil { + return errors.WrapError(errors.ErrKafkaAdminAPI, err, + "parse-config", BrokerMessageMaxBytesConfigName) + } -func adjustNewTopicOptions( - admin ClusterAdminClient, - changefeedID common.ChangeFeedID, - options *options, - topic string, -) { // when create the topic, `max.message.bytes` is decided by the broker, // it would use broker's `message.max.bytes` to set topic's `max.message.bytes`. - messageMaxBytes, found, err := getBrokerMaxMessageBytes(admin) - if err != nil || !found { - log.Warn("`message.max.bytes` not found from broker's configuration, use the option `MaxMessageBytes` as default", - zap.String("namespace", changefeedID.Keyspace()), zap.String("changefeed", changefeedID.Name()), - zap.Int("maxMessageBytes", options.MaxMessageBytes), zap.Error(err)) - messageMaxBytes = options.MaxMessageBytes + // TiCDC need to make sure that the producer's `MaxMessageBytes` won't larger than + // broker's `message.max.bytes`. + maxMessageBytes := brokerMessageMaxBytes - maxMessageBytesOverhead + if brokerMessageMaxBytes <= options.MaxMessageBytes { + log.Warn("broker's `message.max.bytes` less than the `max-message-bytes`,"+ + "use broker's `message.max.bytes` to initialize the Kafka producer", + zap.Int("message.max.bytes", brokerMessageMaxBytes), + zap.Int("max-message-bytes", options.MaxMessageBytes), + zap.Int("real-max-message-bytes", maxMessageBytes)) + options.MaxMessageBytes = maxMessageBytes + } else if maxMessageBytes < options.MaxMessageBytes { + options.MaxMessageBytes = maxMessageBytes } - options.MaxMessageBytes = messageMaxBytes // topic not exists yet, and user does not specify the `partition-num` in the sink uri. if options.PartitionNum == 0 { options.PartitionNum = defaultPartitionNum log.Warn("partition-num is not set, use the default partition count", - zap.String("namespace", changefeedID.Keyspace()), zap.String("changefeed", changefeedID.Name()), zap.String("topic", topic), zap.Int32("partitions", options.PartitionNum)) } -} - -func getTopicMaxMessageBytes( - admin ClusterAdminClient, - topic string, -) (int, bool, error) { - raw, found, err := getTopicConfig( - admin, topic, - TopicMaxMessageBytesConfigName, - BrokerMessageMaxBytesConfigName, - ) - if err != nil { - return 0, false, err - } - if !found { - return 0, false, nil - } - maxMessageBytes, err := strconv.Atoi(raw) - if err != nil { - return 0, false, errors.WrapError(errors.ErrKafkaAdminAPI, err, "parse-config", TopicMaxMessageBytesConfigName) - } - return maxMessageBytes, true, nil -} - -func getBrokerMaxMessageBytes(admin ClusterAdminClient) (int, bool, error) { - raw, found, err := admin.GetBrokerConfig(BrokerMessageMaxBytesConfigName) - if err != nil { - return 0, false, err - } - if !found { - return 0, false, nil - } - messageMaxBytes, err := strconv.Atoi(raw) - if err != nil { - return 0, false, errors.WrapError(errors.ErrKafkaAdminAPI, err, "parse-config", BrokerMessageMaxBytesConfigName) - } - return messageMaxBytes, true, nil + return nil } // getTopicConfig gets topic config by name. @@ -738,6 +723,7 @@ func getBrokerMaxMessageBytes(admin ClusterAdminClient) (int, bool, error) { // we will try to get it from the broker's configuration. // NOTICE: The configuration names of topic and broker may be different for the same configuration. func getTopicConfig( + _ context.Context, admin ClusterAdminClient, topicName string, topicConfigName string, diff --git a/pkg/sink/kafka/options_test.go b/pkg/sink/kafka/options_test.go index befb8861b7..ab2c55d580 100644 --- a/pkg/sink/kafka/options_test.go +++ b/pkg/sink/kafka/options_test.go @@ -28,6 +28,7 @@ import ( "github.com/pingcap/ticdc/pkg/common" "github.com/pingcap/ticdc/pkg/config" "github.com/pingcap/ticdc/pkg/errors" + codecCommon "github.com/pingcap/ticdc/pkg/sink/codec/common" "github.com/stretchr/testify/require" ) @@ -154,6 +155,14 @@ func (f *kafkaAdminFixture) setMessageMaxBytes(brokerValue, topicValue string) { f.topicConfig[defaultMockTopicName][TopicMaxMessageBytesConfigName] = topicValue } +func expectedAdjustedMaxMessageBytes(configuredMaxMessageBytes, sourceMaxMessageBytes int) int { + sourceMaxMessageBytes -= maxMessageBytesOverhead + if configuredMaxMessageBytes < sourceMaxMessageBytes { + return configuredMaxMessageBytes + } + return sourceMaxMessageBytes +} + func (f *kafkaAdminFixture) setMinInsyncReplicas(minInsyncReplicas string) { f.topicConfig[defaultMockTopicName][MinInsyncReplicasConfigName] = minInsyncReplicas f.brokerConfig[MinInsyncReplicasConfigName] = minInsyncReplicas @@ -289,75 +298,19 @@ func TestCompleteOptions(t *testing.T) { require.Equal(t, defaultMaxRetry, options.MaxRetry) } -func TestApplyRejectsNonPositiveMaxMessageBytes(t *testing.T) { - tests := []struct { - name string - uri string - configValue *int - expected int - }{ - { - name: "zero from URI", - uri: "kafka://127.0.0.1:9092/test-topic?max-message-bytes=0", - expected: 0, - }, - { - name: "negative from URI", - uri: "kafka://127.0.0.1:9092/test-topic?max-message-bytes=-1", - expected: -1, - }, - { - name: "zero from sink config", - uri: "kafka://127.0.0.1:9092/test-topic", - configValue: aws.Int(0), - expected: 0, - }, - { - name: "negative from sink config", - uri: "kafka://127.0.0.1:9092/test-topic", - configValue: aws.Int(-1), - expected: -1, - }, - } - - changefeedID := common.NewChangefeedID4Test(common.DefaultKeyspaceName, "test") - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - sinkURI, err := url.Parse(test.uri) - require.NoError(t, err) - - sinkConfig := config.GetDefaultReplicaConfig().Sink - if test.configValue != nil { - sinkConfig.KafkaConfig = &config.KafkaConfig{ - MaxMessageBytes: test.configValue, - } - } - - options := NewOptions() - err = options.Apply(changefeedID, sinkURI, sinkConfig) - require.ErrorContains( - t, err, fmt.Sprintf("invalid max-message-bytes %d", test.expected)) - errCode, ok := errors.RFCCode(err) - require.True(t, ok) - require.Equal(t, errors.ErrKafkaInvalidConfig.RFCCode(), errCode) - }) - } -} - func TestSetPartitionNum(t *testing.T) { options := NewOptions() - changefeedID := common.NewChangefeedID4Test(common.DefaultKeyspaceName, "test") - err := options.setPartitionNum(changefeedID, 2) + err := options.setPartitionNum(2) require.NoError(t, err) require.Equal(t, int32(2), options.PartitionNum) options.PartitionNum = 1 - err = options.setPartitionNum(changefeedID, 2) + err = options.setPartitionNum(2) require.NoError(t, err) require.Equal(t, int32(1), options.PartitionNum) options.PartitionNum = 3 - err = options.setPartitionNum(changefeedID, 2) + err = options.setPartitionNum(2) require.True(t, errors.ErrKafkaInvalidConfig.Equal(err)) } @@ -431,13 +384,13 @@ func TestAdjustConfigFallsBackToBrokerMessageMaxBytesWhenTopicConfigMissing(t *t configuredMaxMessageBytes func(*kafkaAdminFixture) int }{ { - name: "uses broker limit when configured value is below broker", + name: "keeps configured value below broker limit", configuredMaxMessageBytes: func(*kafkaAdminFixture) int { return 1024 }, }, { - name: "uses broker limit when configured value is below broker by one byte", + name: "uses broker limit when configured value is within overhead", configuredMaxMessageBytes: func(f *kafkaAdminFixture) int { return f.brokerMessageMaxBytes() - 1 }, @@ -451,7 +404,6 @@ func TestAdjustConfigFallsBackToBrokerMessageMaxBytesWhenTopicConfigMissing(t *t } topicName := "test-topic" - changefeedID := common.NewChangefeedID4Test(common.DefaultKeyspaceName, "test") for _, test := range tests { t.Run(test.name, func(t *testing.T) { adminFixture := newKafkaAdminFixture(t) @@ -464,29 +416,23 @@ func TestAdjustConfigFallsBackToBrokerMessageMaxBytesWhenTopicConfigMissing(t *t err := adminClient.CreateTopic(detail, false) require.NoError(t, err) - configuredMaxMessageBytes := test.configuredMaxMessageBytes(adminFixture) - sinkURI, err := url.Parse(fmt.Sprintf( - "kafka://127.0.0.1:9092/%s?max-message-bytes=%d", - topicName, configuredMaxMessageBytes, - )) - require.NoError(t, err) - options := NewOptions() - err = options.Apply(changefeedID, sinkURI, config.GetDefaultReplicaConfig().Sink) - require.NoError(t, err) - require.Equal(t, configuredMaxMessageBytes, options.MaxMessageBytes) - expectedProducerLimit := adminFixture.brokerMessageMaxBytes() + options.BrokerEndpoints = []string{"127.0.0.1:9092"} + options.MaxMessageBytes = test.configuredMaxMessageBytes(adminFixture) + expectedMaxMessageBytes := expectedAdjustedMaxMessageBytes( + options.MaxMessageBytes, + adminFixture.brokerMessageMaxBytes(), + ) ctx := context.Background() - err = adjustOptions(changefeedID, adminClient, options, topicName) + err = adjustOptions(ctx, adminClient, options, topicName) require.NoError(t, err) saramaConfig, err := newSaramaConfig(ctx, options) require.NoError(t, err) - require.NotEqual(t, configuredMaxMessageBytes, options.MaxMessageBytes) - require.Equal(t, expectedProducerLimit, options.MaxMessageBytes) - require.Equal(t, expectedProducerLimit, saramaConfig.Producer.MaxMessageBytes) + require.Equal(t, expectedMaxMessageBytes, options.MaxMessageBytes) + require.Equal(t, expectedMaxMessageBytes, saramaConfig.Producer.MaxMessageBytes) }) } } @@ -579,7 +525,7 @@ func TestConfigurationCombinations(t *testing.T) { mockTopicMessageMaxBytes, }, { - "new topic broker below user", + "new topic broker overhead below user", "kafka://127.0.0.1:9092/%s?max-message-bytes=%s", []any{"not-created-topic", strconv.Itoa(1024*1024 + 1)}, mockBrokerMessageMaxBytes, @@ -645,7 +591,7 @@ func TestConfigurationCombinations(t *testing.T) { strconv.Itoa(config.DefaultMaxMessageBytes + 1), }, { - "existing topic topic below user", + "existing topic topic overhead below user", "kafka://127.0.0.1:9092/%s?max-message-bytes=%s", []any{defaultMockTopicName, strconv.Itoa(1024*1024 + 1)}, mockBrokerMessageMaxBytes, @@ -705,11 +651,30 @@ func TestConfigurationCombinations(t *testing.T) { if _, exists := adminFixture.topics[topic]; exists { sourceMaxMessageBytes = adminFixture.topicMaxMessageBytes(topic) } + expectedMaxMessageBytes := expectedAdjustedMaxMessageBytes(options.MaxMessageBytes, sourceMaxMessageBytes) + + err = adjustOptions(context.Background(), adminClient, options, topic) + require.Nil(t, err) + require.Equal(t, expectedMaxMessageBytes, options.MaxMessageBytes) - changefeedID := common.NewChangefeedID4Test(common.DefaultKeyspaceName, "test") - err = adjustOptions(changefeedID, adminClient, options, topic) + saramaConfig, err := newSaramaConfig(context.Background(), options) require.Nil(t, err) - require.Equal(t, sourceMaxMessageBytes, options.MaxMessageBytes) + require.Equal(t, expectedMaxMessageBytes, saramaConfig.Producer.MaxMessageBytes) + + encoderConfig := codecCommon.NewConfig(config.ProtocolOpen) + err = encoderConfig.Apply(sinkURI, &config.SinkConfig{ + KafkaConfig: &config.KafkaConfig{ + LargeMessageHandle: config.NewDefaultLargeMessageHandleConfig(), + }, + }) + require.Nil(t, err) + encoderConfig.WithMaxMessageBytes(options.MaxMessageBytes) + + err = encoderConfig.Validate() + require.Nil(t, err) + + // producer's `MaxMessageBytes` = encoder's `MaxMessageBytes`. + require.Equal(t, expectedMaxMessageBytes, encoderConfig.MaxMessageBytes) adminClient.Close() }) diff --git a/pkg/sink/kafka/sarama_config.go b/pkg/sink/kafka/sarama_config.go index 6f2d56456e..4988c79c52 100644 --- a/pkg/sink/kafka/sarama_config.go +++ b/pkg/sink/kafka/sarama_config.go @@ -62,7 +62,7 @@ func newSaramaConfig(ctx context.Context, o *options) (*sarama.Config, error) { config.Producer.Flush.Bytes = 0 config.Producer.Flush.Messages = 0 config.Producer.Flush.Frequency = time.Duration(0) - config.Producer.Flush.MaxMessages = 0 + config.Producer.Flush.MaxMessages = o.MaxMessages config.Net.MaxOpenRequests = 1 config.Net.DialTimeout = o.DialTimeout diff --git a/pkg/sink/kafka/sarama_factory.go b/pkg/sink/kafka/sarama_factory.go index c9678ce784..57bf5ef27c 100644 --- a/pkg/sink/kafka/sarama_factory.go +++ b/pkg/sink/kafka/sarama_factory.go @@ -57,7 +57,7 @@ func NewSaramaFactory( admin.Close() }() - if err = adjustOptions(changefeedID, admin, o, o.Topic); err != nil { + if err = adjustOptions(ctx, admin, o, o.Topic); err != nil { return nil, err } diff --git a/tests/integration_tests/kafka_big_messages/run.sh b/tests/integration_tests/kafka_big_messages/run.sh index b9a4b646fb..0628eaa92e 100755 --- a/tests/integration_tests/kafka_big_messages/run.sh +++ b/tests/integration_tests/kafka_big_messages/run.sh @@ -3,212 +3,53 @@ set -eu CUR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) -source "$CUR/../_utils/test_prepare" +source $CUR/../_utils/test_prepare WORK_DIR=$OUT_DIR/$TEST_NAME CDC_BINARY=cdc.test SINK_TYPE=$1 -STATE_WAIT_TIMEOUT_SECONDS=30 -STATE_CHECK_INTERVAL_SECONDS=1 -TABLE_CHECK_RETRIES=15 -BATCH_LIMIT=262144 -SMALL_TOPIC_LIMIT=524288 -LARGE_TOPIC_LIMIT=2097152 -ROW_BYTES=1048576 -SCHEMA_REGISTRY_URI=http://127.0.0.1:8088 -GENERATOR_DIR=$CUR/../../utils/gen_kafka_big_messages -consumer_pid="" - -function start_schema_registry() { - if curl -o /dev/null -s "$SCHEMA_REGISTRY_URI"; then +function run() { + # test kafka sink only in this case + if [ "$SINK_TYPE" != "kafka" ]; then return fi + rm -rf $WORK_DIR && mkdir -p $WORK_DIR - echo "Starting schema registry..." - ./bin/bin/schema-registry-start -daemon ./bin/etc/schema-registry/schema-registry.properties - local i=0 - while ! curl -o /dev/null -s "$SCHEMA_REGISTRY_URI"; do - i=$((i + 1)) - if [ "$i" -gt 30 ]; then - echo "Failed to start schema registry" - exit 1 - fi - sleep 2 - done - curl -X PUT -H "Content-Type: application/vnd.schemaregistry.v1+json" --data '{"compatibility": "NONE"}' "$SCHEMA_REGISTRY_URI/config" -} + start_tidb_cluster --workdir $WORK_DIR -function build_message_generator() { - if [ ! -f "$GENERATOR_DIR/gen_kafka_big_messages" ]; then - (cd "$GENERATOR_DIR" && GO111MODULE=on go build) - fi -} + TOPIC_NAME="big-message-test-$RANDOM" -function kafka_sink_uri() { - local topic_name=$1 - local protocol=$2 - local extra_params=$3 - local sink_uri="kafka://127.0.0.1:9092/${topic_name}?protocol=${protocol}&partition-num=1&kafka-version=${KAFKA_VERSION}&max-message-bytes=${BATCH_LIMIT}" - if [ "$extra_params" != "" ]; then - sink_uri="${sink_uri}&${extra_params}" - fi - echo "$sink_uri" -} + # record tso before we create tables to skip the system table DDLs + start_ts=$(run_cdc_cli_tso_query $UP_PD_HOST_1 $UP_PD_PORT_1) -function start_kafka_consumer() { - local work_dir=$1 - local sink_uri=$2 - local schema_registry_uri=$3 - local protocol_case=$4 - local downstream_uri="mysql://root@${DOWN_TIDB_HOST}:${DOWN_TIDB_PORT}/?safe-mode=true&batch-dml-enable=false&enable-ddl-ts=false" - local args=( - --log-file "$work_dir/cdc_kafka_consumer.log" - --log-level debug - --upstream-uri "$sink_uri" - --downstream-uri "$downstream_uri" - ) - if [ "$schema_registry_uri" != "" ]; then - args+=(--schema-registry-uri "$schema_registry_uri") - fi - if [[ "$protocol_case" == simple_* ]]; then - args+=(--upstream-tidb-dsn "root@tcp(${UP_TIDB_HOST}:${UP_TIDB_PORT})/?") - fi + run_cdc_server --workdir $WORK_DIR --binary $CDC_BINARY - cdc_kafka_consumer "${args[@]}" >>"$work_dir/cdc_kafka_consumer_stdout.log" 2>&1 & - consumer_pid=$! -} + # Use a max-message-bytes parameter that is larger than the kafka topic max message bytes. + # Test if TiCDC automatically uses the max-message-bytes of the topic. + # See: https://github.com/PingCAP-QE/ci/blob/ddde195ebf4364a0028d53405d1194aa37a4d853/jenkins/pipelines/ci/ticdc/cdc_ghpr_kafka_integration_test.groovy#L178 + # Use a topic that has already been created. + # See: https://github.com/PingCAP-QE/ci/blob/ddde195ebf4364a0028d53405d1194aa37a4d853/jenkins/pipelines/ci/ticdc/cdc_ghpr_kafka_integration_test.groovy#L180 + SINK_URI="kafka://127.0.0.1:9092/$TOPIC_NAME?protocol=open-protocol&partition-num=1&kafka-version=${KAFKA_VERSION}&max-message-bytes=12582912" + cdc_cli_changefeed create --start-ts=$start_ts --sink-uri="$SINK_URI" + run_kafka_consumer $WORK_DIR "kafka://127.0.0.1:9092/$TOPIC_NAME?protocol=open-protocol&partition-num=1&version=${KAFKA_VERSION}" -function stop_kafka_consumer() { - if [ "$consumer_pid" != "" ]; then - kill -9 "$consumer_pid" 2>/dev/null || true - wait "$consumer_pid" 2>/dev/null || true - consumer_pid="" + echo "Starting generate kafka big messages..." + cd $CUR/../../utils/gen_kafka_big_messages + if [ ! -f ./gen_kafka_big_messages ]; then + GO111MODULE=on go build fi -} - -function wait_changefeed_state() { - local pd_addr=$1 - local changefeed_id=$2 - local expected_state=$3 - local expected_error=$4 - local deadline=$((SECONDS + STATE_WAIT_TIMEOUT_SECONDS)) - - while true; do - if check_changefeed_state "$pd_addr" "$changefeed_id" "$expected_state" "$expected_error" ""; then - return - fi - if [ "$SECONDS" -ge "$deadline" ]; then - echo "changefeed $changefeed_id did not reach state $expected_state within ${STATE_WAIT_TIMEOUT_SECONDS}s" - return 1 - fi - sleep "$STATE_CHECK_INTERVAL_SECONDS" - done -} - -function render_diff_config() { - local work_dir=$1 - local database_name=$2 - local diff_config=$3 - - sed -e "s/database_name/${database_name}/g" \ - -e "s|/tmp/tidb_cdc_test/kafka_big_messages/sync_diff/output|${work_dir}/sync_diff/output|g" \ - "$CUR/conf/diff_config.toml" >"$diff_config" -} - -function run_protocol_case() { - local protocol_case=$1 - local protocol=$2 - local schema_registry_uri=$3 - local extra_params=$4 - local topic_case=${protocol_case//_/-} - local topic_name="big-message-${topic_case}-${RANDOM}" - local changefeed_id="kafka-big-messages-${topic_case}" - local database_name="kafka_big_messages_${protocol_case}" - local work_dir="$WORK_DIR/$protocol_case" - local sql_file="$work_dir/test.sql" - local diff_config="$work_dir/diff_config.toml" - local pd_addr="http://${UP_PD_HOST_1}:${UP_PD_PORT_1}" - local sink_uri - local initial_topic_limit=$SMALL_TOPIC_LIMIT - local expected_error=ErrMessageTooLarge - if [ "$protocol_case" = "async_error" ]; then - initial_topic_limit=$LARGE_TOPIC_LIMIT - expected_error=ErrKafkaSendMessage - fi - - mkdir -p "$work_dir" - render_diff_config "$work_dir" "$database_name" "$diff_config" - kafka_topic --topic "$topic_name" --max-message-bytes "$initial_topic_limit" - local start_ts - start_ts=$(run_cdc_cli_tso_query "$UP_PD_HOST_1" "$UP_PD_PORT_1") - sink_uri=$(kafka_sink_uri "$topic_name" "$protocol" "$extra_params") - - if [ "$schema_registry_uri" != "" ]; then - cdc_cli_changefeed create --start-ts="$start_ts" --sink-uri="$sink_uri" --schema-registry="$schema_registry_uri" -c "$changefeed_id" - else - cdc_cli_changefeed create --start-ts="$start_ts" --sink-uri="$sink_uri" -c "$changefeed_id" - fi - start_kafka_consumer "$work_dir" "$sink_uri" "$schema_registry_uri" "$protocol_case" - wait_changefeed_state "$pd_addr" "$changefeed_id" "normal" "null" - - # Lower the topic limit after the producer has started. The encoder and - # producer still accept the message, then Kafka rejects it asynchronously. - if [ "$protocol_case" = "async_error" ]; then - local ready_database="${database_name}_ready" - run_sql "CREATE DATABASE ${ready_database}; CREATE TABLE ${ready_database}.ready(id INT PRIMARY KEY); INSERT INTO ${ready_database}.ready VALUES (1)" "$UP_TIDB_HOST" "$UP_TIDB_PORT" - ensure "$TABLE_CHECK_RETRIES" "run_sql 'SELECT id FROM ${ready_database}.ready' '$DOWN_TIDB_HOST' '$DOWN_TIDB_PORT' && check_contains 'id: 1'" - kafka_topic --topic "$topic_name" --max-message-bytes "$SMALL_TOPIC_LIMIT" --alter - fi - - "$GENERATOR_DIR/gen_kafka_big_messages" --row-bytes="$ROW_BYTES" --row-count=1 --database-name="$database_name" --table-name=test --sql-file-path="$sql_file" - run_sql_file "$sql_file" "$UP_TIDB_HOST" "$UP_TIDB_PORT" - run_sql "CREATE TABLE ${database_name}.finish_mark(id INT PRIMARY KEY)" "$UP_TIDB_HOST" "$UP_TIDB_PORT" - - wait_changefeed_state "$pd_addr" "$changefeed_id" "warning" "$expected_error" - - # Only increase Kafka's topic limit. TiCDC must recreate the sink, read the - # new limit, and resume without updating, pausing, or resuming the changefeed. - kafka_topic --topic "$topic_name" --max-message-bytes "$LARGE_TOPIC_LIMIT" --alter - wait_changefeed_state "$pd_addr" "$changefeed_id" "normal" "null" - check_table_exists "${database_name}.finish_mark" "$DOWN_TIDB_HOST" "$DOWN_TIDB_PORT" "$TABLE_CHECK_RETRIES" - check_sync_diff "$work_dir" "$diff_config" - - cdc_cli_changefeed remove -c "$changefeed_id" - stop_kafka_consumer -} - -function run() { - # Test Kafka sink only in this case. - if [ "$SINK_TYPE" != "kafka" ]; then - return - fi - - local cases=( - "canal_json|canal-json||enable-tidb-extension=true" - "open_protocol|open-protocol||" - "async_error|open-protocol||max-retry=0" - "simple_json|simple||" - "simple_avro|simple||encoding-format=avro" - "avro|avro|$SCHEMA_REGISTRY_URI|enable-tidb-extension=true&avro-enable-watermark=true&avro-decimal-handling-mode=string&avro-bigint-unsigned-handling-mode=string" - ) - - rm -rf "$WORK_DIR" && mkdir -p "$WORK_DIR" - start_schema_registry - build_message_generator - start_tidb_cluster --workdir "$WORK_DIR" - run_cdc_server --workdir "$WORK_DIR" --binary "$CDC_BINARY" + # Generate data larger than kafka broker max.message.bytes. We can send this data correctly. + ./gen_kafka_big_messages --row-count=15 --sql-file-path=$CUR/test.sql - local case_entry - for case_entry in "${cases[@]}"; do - local protocol_case protocol schema_registry_uri extra_params - IFS='|' read -r protocol_case protocol schema_registry_uri extra_params <<<"$case_entry" - run_protocol_case "$protocol_case" "$protocol" "$schema_registry_uri" "$extra_params" - done + run_sql_file $CUR/test.sql ${UP_TIDB_HOST} ${UP_TIDB_PORT} + table="kafka_big_messages.test" + check_table_exists $table ${DOWN_TIDB_HOST} ${DOWN_TIDB_PORT} + check_sync_diff $WORK_DIR $CUR/conf/diff_config.toml - cleanup_process "$CDC_BINARY" + cleanup_process $CDC_BINARY } -trap 'stop_kafka_consumer; stop_test "$WORK_DIR"' EXIT -run "$@" -check_logs "$WORK_DIR" +trap 'stop_test $WORK_DIR' EXIT +run $* +check_logs $WORK_DIR echo "[$(date)] <<<<<< run test case $TEST_NAME success! >>>>>>" From f0a283073cfb9be088c1d76f455f7c695bfab500 Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Thu, 30 Jul 2026 18:29:25 +0800 Subject: [PATCH 6/6] fix consumer --- cmd/kafka-consumer/option.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/kafka-consumer/option.go b/cmd/kafka-consumer/option.go index ae7ba1f198..0113d0d3f3 100644 --- a/cmd/kafka-consumer/option.go +++ b/cmd/kafka-consumer/option.go @@ -122,11 +122,11 @@ func (o *option) Adjust(upstreamURIStr string, configFile string) { } o.partitionNum = int32(c) } - partitionNum, err := getPartitionNum(o) - if err != nil { - log.Panic("cannot get the partition number", zap.String("topic", o.topic), zap.Error(err)) - } if o.partitionNum == 0 { + partitionNum, err := getPartitionNum(o) + if err != nil { + log.Panic("cannot get the partition number", zap.String("topic", o.topic), zap.Error(err)) + } o.partitionNum = partitionNum }