diff --git a/cmd/kafka-consumer/consumer.go b/cmd/kafka-consumer/consumer.go index 4e78582f9e..2bf26f1aee 100644 --- a/cmd/kafka-consumer/consumer.go +++ b/cmd/kafka-consumer/consumer.go @@ -42,28 +42,50 @@ func getPartitionNum(o *option) (int32, error) { } defer admin.Close() + topics := strings.Split(o.topic, ",") + maxPartitionNum := int32(0) timeout := 3000 - for i := 0; i <= 30; i++ { - resp, err := admin.GetMetadata(&o.topic, false, timeout) - if err != nil { - if err.(kafka.Error).Code() == kafka.ErrTransport { - log.Info("retry get partition number", zap.Int("retryTime", i), zap.Int("timeout", timeout)) - timeout += 100 - continue + for _, topic := range topics { + topic = strings.TrimSpace(topic) + if topic == "" { + continue + } + found := false + for i := 0; i <= 30; i++ { + resp, err := admin.GetMetadata(&topic, false, timeout) + if err != nil { + var kafkaErr kafka.Error + if errors.As(err, &kafkaErr) && kafkaErr.Code() == kafka.ErrTransport { + log.Info("retry get partition number", zap.String("topic", topic), zap.Int("retryTime", i), zap.Int("timeout", timeout)) + timeout += 100 + continue + } + return 0, errors.Trace(err) } - 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) } - if topicDetail, ok := resp.Topics[o.topic]; ok { - numPartitions := int32(len(topicDetail.Partitions)) - log.Info("get partition number of topic", - zap.String("topic", o.topic), - zap.Int32("partitionNum", numPartitions)) - return numPartitions, nil + if !found { + return 0, errors.Errorf("get partition number(%s) timeout", topic) } - log.Info("retry get partition number", zap.String("topic", o.topic)) - time.Sleep(1 * time.Second) } - return 0, errors.Errorf("get partition number(%s) timeout", o.topic) + if maxPartitionNum == 0 { + return 0, errors.Errorf("get partition number(%s) timeout", o.topic) + } + return maxPartitionNum, nil } type consumer struct { 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 } 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 de6ce2fb71..733278a59a 100644 --- a/downstreamadapter/sink/kafka/helper.go +++ b/downstreamadapter/sink/kafka/helper.go @@ -21,23 +21,24 @@ 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/ticdc/pkg/sink/kafka/claimcheck" "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 adminClient kafka.ClusterAdminClient factory kafka.Factory + claimCheck *claimcheck.ClaimCheck } func (c components) close() { @@ -47,93 +48,94 @@ func (c components) close() { if c.topicManager != nil { c.topicManager.Close() } + if c.claimCheck != nil { + c.claimCheck.Close() + } } -func newKafkaSinkComponentWithFactory(ctx context.Context, - changefeedID commonType.ChangeFeedID, +func newKafkaSinkComponent( + ctx context.Context, + changefeedID common.ChangeFeedID, sinkURI *url.URL, 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, err } topic, err := helper.GetTopic(sinkURI) if err != nil { - return kafkaComponent, protocol, errors.Trace(err) + return comp, protocol, err } options := kafka.NewOptions() if err = options.Apply(changefeedID, sinkURI, sinkConfig); err != nil { - return kafkaComponent, protocol, errors.WrapError(errors.ErrKafkaInvalidConfig, err) + return comp, protocol, err } options.Topic = topic - kafkaComponent.factory, err = factoryCreator(ctx, options, changefeedID) + comp.factory, err = kafka.NewSaramaFactory(ctx, options, changefeedID) if err != nil { - return kafkaComponent, protocol, errors.WrapError(errors.ErrKafkaNewProducer, err) + return comp, protocol, err } - kafkaComponent.eventRouter, err = eventrouter.NewEventRouter( - sinkConfig, topic, false, protocol == config.ProtocolAvro) + isAvroLike := protocol == config.ProtocolAvro + comp.eventRouter, err = eventrouter.NewEventRouter( + sinkConfig, topic, false, isAvroLike) if err != nil { - return kafkaComponent, protocol, errors.Trace(err) + return comp, protocol, 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, err } encoderConfig, err := helper.GetEncoderConfig(changefeedID, sinkURI, protocol, sinkConfig, options.MaxMessageBytes) if err != nil { - return kafkaComponent, protocol, errors.Trace(err) + return comp, protocol, 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, 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, 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, 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, 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, err } - return kafkaComponent, protocol, nil -} - -func newKafkaSinkComponent( - ctx context.Context, - changefeedID commonType.ChangeFeedID, - sinkURI *url.URL, - sinkConfig *config.SinkConfig, -) (components, config.Protocol, error) { - return newKafkaSinkComponentWithFactory(ctx, changefeedID, sinkURI, sinkConfig, kafka.NewSaramaFactory) + return comp, protocol, nil } diff --git a/downstreamadapter/sink/kafka/sink.go b/downstreamadapter/sink/kafka/sink.go index e6d1038b26..35e94534b1 100644 --- a/downstreamadapter/sink/kafka/sink.go +++ b/downstreamadapter/sink/kafka/sink.go @@ -19,14 +19,18 @@ 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" + "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/common" + "github.com/pingcap/ticdc/pkg/sink/codec" + codecCommon "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" @@ -40,7 +44,7 @@ const ( ) type sink struct { - changefeedID commonType.ChangeFeedID + changefeedID common.ChangeFeedID dmlProducer kafka.AsyncProducer ddlProducer kafka.SyncProducer @@ -63,50 +67,125 @@ type sink struct { ctx context.Context } -func (s *sink) SinkType() commonType.SinkType { - return commonType.KafkaSinkType +func (s *sink) SinkType() common.SinkType { + return common.KafkaSinkType } -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 +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) + 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 + 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 commonType.ChangeFeedID, sinkURI *url.URL, sinkConfig *config.SinkConfig, + ctx context.Context, changefeedID common.ChangeFeedID, sinkURI *url.URL, sinkConfig *config.SinkConfig, ) (*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, - changefeedID commonType.ChangeFeedID, + changefeedID common.ChangeFeedID, protocol config.Protocol, comp components, ) (*sink, error) { + statistics := metrics.NewStatistics(changefeedID, "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() }() - statistics := metrics.NewStatistics(changefeedID, "sink") asyncProducer, err = comp.factory.AsyncProducer(ctx) if err != nil { return nil, err @@ -153,7 +232,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 +303,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 { @@ -253,12 +332,11 @@ func (s *sink) calculateKeyPartitions(ctx context.Context) error { event.Rewind() break } - - index, key, err := partitionGenerator.GeneratePartitionIndexAndKey(&row, partitionNum, event.TableInfo, event.CommitTs) + index, key, err := partitionGenerator.GeneratePartitionIndexAndKey( + &row, partitionNum, event.TableInfo, event.CommitTs) if err != nil { - return errors.Trace(err) + return err } - events = append(events, &commonEvent.MQRowEvent{ Key: commonEvent.TopicPartitionKey{ Topic: topic, @@ -287,7 +365,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 +458,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 +507,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 +557,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 +581,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, @@ -559,3 +637,11 @@ func (s *sink) Close() { s.comp.close() s.statistics.Close() } + +func (s *sink) BatchCount() int { + return 4096 +} + +func (s *sink) BatchBytes() int { + return 0 +} diff --git a/downstreamadapter/sink/kafka/sink_test.go b/downstreamadapter/sink/kafka/sink_test.go index c240749ada..330b981908 100644 --- a/downstreamadapter/sink/kafka/sink_test.go +++ b/downstreamadapter/sink/kafka/sink_test.go @@ -16,10 +16,13 @@ package kafka import ( "context" "fmt" + "net/http" + "net/http/httptest" "net/url" "testing" "time" + "github.com/IBM/sarama" "github.com/golang/mock/gomock" "github.com/pingcap/ticdc/downstreamadapter/sink/columnselector" "github.com/pingcap/ticdc/downstreamadapter/sink/eventrouter" @@ -28,8 +31,9 @@ import ( "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/sink/codec" - codeccommon "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/stretchr/testify/require" "go.uber.org/atomic" @@ -37,6 +41,89 @@ import ( 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") +} + func newKafkaSinkForTestWithProducers(ctx context.Context, t *testing.T, ctrl *gomock.Controller, @@ -101,11 +188,11 @@ func newKafkaSinkForTestWithProducers(ctx context.Context, if err != nil { return nil, err } - encoderGroup, err := codec.NewEncoderGroup(ctx, sinkConfig, encoderConfig, changefeedID) + encoderGroup, err := codec.NewEncoderGroup(ctx, sinkConfig, encoderConfig, nil, changefeedID) if err != nil { return nil, err } - encoder, err := codec.NewEventEncoder(ctx, encoderConfig) + encoder, err := codec.NewEventEncoder(ctx, encoderConfig, nil) if err != nil { return nil, err } @@ -142,10 +229,30 @@ func newKafkaSinkForTestWithProducers(ctx context.Context, if err != nil { return nil, err } - go s.Run(ctx) return s, 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()) +} + func TestKafkaSinkBasicFunctionality(t *testing.T) { helper := commonEvent.NewEventTestHelper(t) defer helper.Close() @@ -206,7 +313,7 @@ func TestKafkaSinkBasicFunctionality(t *testing.T) { _ context.Context, _ string, _ int32, - message *codeccommon.Message, + message *codecCommon.Message, ) error { if message.Callback != nil { message.Callback() @@ -220,6 +327,7 @@ func TestKafkaSinkBasicFunctionality(t *testing.T) { kafkaSink, err := newKafkaSinkForTestWithProducers(ctx, t, ctrl, asyncProducer, syncProducer) require.NoError(t, err) defer cancel() + go kafkaSink.Run(ctx) err = kafkaSink.WriteBlockEvent(ddlEvent) require.NoError(t, err) @@ -238,3 +346,9 @@ func TestKafkaSinkBasicFunctionality(t *testing.T) { cancel() kafkaSink.AddCheckpointTs(12345) } + +func TestKafkaSinkBatchConfig(t *testing.T) { + sink := &sink{} + require.Equal(t, 4096, sink.BatchCount()) + require.Zero(t, sink.BatchBytes()) +} diff --git a/downstreamadapter/sink/pulsar/helper.go b/downstreamadapter/sink/pulsar/helper.go index acc1cddd38..ffb503a8f3 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) @@ -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/downstreamadapter/sink/topicmanager/kafka_topic_manager.go b/downstreamadapter/sink/topicmanager/kafka_topic_manager.go index 8e92167327..e33929d484 100644 --- a/downstreamadapter/sink/topicmanager/kafka_topic_manager.go +++ b/downstreamadapter/sink/topicmanager/kafka_topic_manager.go @@ -15,14 +15,12 @@ package topicmanager import ( "context" - "fmt" "sync" "time" - "github.com/pingcap/errors" "github.com/pingcap/log" "github.com/pingcap/ticdc/pkg/common" - cerror "github.com/pingcap/ticdc/pkg/errors" + "github.com/pingcap/ticdc/pkg/errors" "github.com/pingcap/ticdc/pkg/retry" "github.com/pingcap/ticdc/pkg/sink/kafka" "go.uber.org/zap" @@ -63,7 +61,7 @@ func GetTopicManagerAndTryCreateTopic( ) if _, err := topicManager.CreateTopicAndWaitUntilVisible(ctx, topic); err != nil { - return nil, cerror.WrapError(cerror.ErrKafkaCreateTopic, err) + return nil, err } return topicManager, nil @@ -104,7 +102,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 @@ -124,7 +122,7 @@ func (m *kafkaTopicManager) backgroundRefreshMeta(ctx context.Context) { case <-ticker.C: // We ignore the error here, because the error may be caused by the // network problem, and we can try to get the metadata next time. - topicPartitionNums, _ := m.fetchAllTopicsPartitionsNum(ctx) + topicPartitionNums, _ := m.fetchAllTopicsPartitionsNum() for topic, partitionNum := range topicPartitionNums { m.tryUpdatePartitionsAndLogging(topic, partitionNum) } @@ -163,11 +161,9 @@ func (m *kafkaTopicManager) tryUpdatePartitionsAndLogging(topic string, partitio // The error returned by this method could be a transient error that is fixable by the underlying logic. // When handling this error, please be cautious. // If you simply throw the error to the caller, it may impact the robustness of your program. -func (m *kafkaTopicManager) fetchAllTopicsPartitionsNum( - ctx context.Context, -) (map[string]int32, error) { +func (m *kafkaTopicManager) fetchAllTopicsPartitionsNum() (map[string]int32, error) { var topics []string - m.topics.Range(func(key, value any) bool { + m.topics.Range(func(key, _ any) bool { topics = append(topics, key.(string)) return true }) @@ -238,13 +234,15 @@ func (m *kafkaTopicManager) waitUntilTopicVisible( // createTopic creates a topic with the given name // and returns the number of partitions. func (m *kafkaTopicManager) createTopic( - ctx context.Context, + _ context.Context, topicName string, ) (int32, error) { if !m.cfg.AutoCreate { - 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 } start := time.Now() @@ -264,7 +262,7 @@ func (m *kafkaTopicManager) createTopic( zap.Error(err), zap.Duration("duration", time.Since(start)), ) - return 0, cerror.WrapError(cerror.ErrKafkaCreateTopic, err) + return 0, err } log.Info( @@ -290,30 +288,66 @@ func (m *kafkaTopicManager) CreateTopicAndWaitUntilVisible( // which means we should create the topic later. topicDetails, err := m.admin.GetTopicsMeta([]string{topicName}, true) if err != nil { - return 0, errors.Trace(err) + if kafka.IsAdminAuthorizationFailed(err) { + return m.useConfiguredPartitionNum(topicName, err), nil + } + return 0, err } - if detail, ok := topicDetails[topicName]; ok { - numPartition := detail.NumPartitions - if topicName == m.defaultTopic { - numPartition = m.cfg.PartitionNum + if numPartition, ok := m.tryStoreTopicMeta(topicName, topicDetails); ok { + return numPartition, nil + } + + topicDetails, err = m.admin.GetTopicsMeta([]string{topicName}, false) + if err != nil { + if kafka.IsAdminAuthorizationFailed(err) { + return m.useConfiguredPartitionNum(topicName, err), nil } - m.tryUpdatePartitionsAndLogging(topicName, numPartition) + } else if numPartition, ok := m.tryStoreTopicMeta(topicName, topicDetails); ok { return numPartition, nil } partitionNum, err := m.createTopic(ctx, topicName) if err != nil { - return 0, errors.Trace(err) + if kafka.IsAdminAuthorizationFailed(err) { + return m.useConfiguredPartitionNum(topicName, err), nil + } + return 0, err } err = m.waitUntilTopicVisible(ctx, topicName) if err != nil { - return 0, errors.Trace(err) + return 0, err } return partitionNum, nil } +func (m *kafkaTopicManager) tryStoreTopicMeta( + topicName string, topicDetails map[string]kafka.TopicDetail, +) (int32, bool) { + detail, ok := topicDetails[topicName] + if !ok { + return 0, false + } + numPartition := detail.NumPartitions + if topicName == m.defaultTopic { + numPartition = m.cfg.PartitionNum + } + m.tryUpdatePartitionsAndLogging(topicName, numPartition) + return numPartition, true +} + +func (m *kafkaTopicManager) useConfiguredPartitionNum(topicName string, cause error) int32 { + log.Warn("skip Kafka topic creation because topic authorization failed", + zap.String("keyspace", m.changefeedID.Keyspace()), + zap.String("changefeed", m.changefeedID.Name()), + zap.String("topic", topicName), + zap.Int32("partitionNumber", m.cfg.PartitionNum), + zap.Error(cause)) + m.tryUpdatePartitionsAndLogging(topicName, m.cfg.PartitionNum) + return m.cfg.PartitionNum +} + // Close exits the background goroutine. func (m *kafkaTopicManager) Close() { m.cancel() diff --git a/downstreamadapter/sink/topicmanager/kafka_topic_manager_test.go b/downstreamadapter/sink/topicmanager/kafka_topic_manager_test.go index ae3ba10694..4ee0be636a 100644 --- a/downstreamadapter/sink/topicmanager/kafka_topic_manager_test.go +++ b/downstreamadapter/sink/topicmanager/kafka_topic_manager_test.go @@ -20,12 +20,60 @@ import ( "github.com/IBM/sarama" "github.com/golang/mock/gomock" "github.com/pingcap/ticdc/pkg/common" + "github.com/pingcap/ticdc/pkg/errors" "github.com/pingcap/ticdc/pkg/sink/kafka" "github.com/stretchr/testify/require" ) const kafkaTopicManagerTestTopic = "mock_topic" +type mockAdminClientWithDeniedDescribe struct { + *kafka.MockClusterAdminClient + createTopicCalled bool + describeCount int +} + +func (m *mockAdminClientWithDeniedDescribe) GetTopicsMeta( + topics []string, + ignoreTopicError bool, +) (map[string]kafka.TopicDetail, error) { + m.describeCount++ + if ignoreTopicError { + return map[string]kafka.TopicDetail{}, nil + } + return nil, sarama.ErrTopicAuthorizationFailed +} + +func (m *mockAdminClientWithDeniedDescribe) CreateTopic( + detail *kafka.TopicDetail, + validateOnly bool, +) error { + m.createTopicCalled = true + return nil +} + +type mockAdminClientWithDeniedCreate struct { + *kafka.MockClusterAdminClient + createTopicCalled bool + describeCount int +} + +func (m *mockAdminClientWithDeniedCreate) GetTopicsMeta( + topics []string, + ignoreTopicError bool, +) (map[string]kafka.TopicDetail, error) { + m.describeCount++ + return map[string]kafka.TopicDetail{}, nil +} + +func (m *mockAdminClientWithDeniedCreate) CreateTopic( + detail *kafka.TopicDetail, + validateOnly bool, +) error { + m.createTopicCalled = true + return sarama.ErrClusterAuthorizationFailed +} + func TestCreateTopic(t *testing.T) { t.Parallel() @@ -35,6 +83,7 @@ func TestCreateTopic(t *testing.T) { AutoCreate: true, PartitionNum: 2, ReplicationFactor: 1, + RequiredAcks: kafka.WaitForAll, } changefeedID := common.NewChangefeedID4Test("test", "test") @@ -53,6 +102,8 @@ func TestCreateTopic(t *testing.T) { }, 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 @@ -68,13 +119,17 @@ func TestCreateTopic(t *testing.T) { }, 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 sarama.ErrInvalidReplicationFactor + return errors.WrapError(errors.ErrKafkaAdminAPI, sarama.ErrInvalidReplicationFactor, "create-topic", detail.Name) }), ) @@ -84,6 +139,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) @@ -98,7 +154,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") @@ -119,16 +180,46 @@ func TestCreateTopic(t *testing.T) { manager = newKafkaTopicManager(ctx, topic, changefeedID, adminClient, cfg) defer manager.Close() _, err = manager.CreateTopicAndWaitUntilVisible(ctx, topic) - 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) } +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", 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() @@ -144,6 +235,8 @@ func TestCreateTopicWaitsUntilVisible(t *testing.T) { 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().CreateTopic(gomock.Any(), false).DoAndReturn( func(detail *kafka.TopicDetail, validateOnly bool) error { require.Equal(t, &kafka.TopicDetail{ @@ -176,3 +269,61 @@ func TestCreateTopicWaitsUntilVisible(t *testing.T) { require.NoError(t, err) require.Equal(t, int32(2), partitionNum) } + +func TestCreateTopicWithTopicDescribeDenied(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + adminClient := &mockAdminClientWithDeniedDescribe{ + MockClusterAdminClient: kafka.NewMockClusterAdminClient(ctrl), + } + cfg := &kafka.AutoCreateTopicConfig{ + AutoCreate: true, + PartitionNum: 2, + ReplicationFactor: 1, + } + + changefeedID := common.NewChangefeedID4Test("test", "test") + ctx := context.Background() + manager := newKafkaTopicManager(ctx, "precreated-topic", changefeedID, adminClient, cfg) + defer manager.Close() + + partitionNum, err := manager.CreateTopicAndWaitUntilVisible(ctx, "precreated-topic") + require.NoError(t, err) + require.Equal(t, int32(2), partitionNum) + require.False(t, adminClient.createTopicCalled) + require.Equal(t, 2, adminClient.describeCount) + + partitions, ok := manager.topics.Load("precreated-topic") + require.True(t, ok) + require.Equal(t, int32(2), partitions) +} + +func TestCreateTopicWithCreateDenied(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + adminClient := &mockAdminClientWithDeniedCreate{ + MockClusterAdminClient: kafka.NewMockClusterAdminClient(ctrl), + } + cfg := &kafka.AutoCreateTopicConfig{ + AutoCreate: true, + PartitionNum: 2, + ReplicationFactor: 1, + } + + changefeedID := common.NewChangefeedID4Test("test", "test") + ctx := context.Background() + manager := newKafkaTopicManager(ctx, "precreated-topic", changefeedID, adminClient, cfg) + defer manager.Close() + + partitionNum, err := manager.CreateTopicAndWaitUntilVisible(ctx, "precreated-topic") + require.NoError(t, err) + require.Equal(t, int32(2), partitionNum) + require.True(t, adminClient.createTopicCalled) + require.Equal(t, 2, adminClient.describeCount) + + partitions, ok := manager.topics.Load("precreated-topic") + require.True(t, ok) + require.Equal(t, int32(2), partitions) +} 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/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..0422e6dc58 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" @@ -28,21 +27,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) } @@ -60,7 +60,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/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..bb374651f5 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) @@ -779,7 +779,7 @@ func TestEncoderMultipleMessage(t *testing.T) { ctx := context.Background() codecConfig := common.NewConfig(config.ProtocolOpen).WithMaxMessageBytes(400) - encoder, err := NewBatchEncoder(ctx, codecConfig) + encoder, err := NewBatchEncoder(codecConfig, nil) require.NoError(t, err) insertEvents := make([]*commonEvent.RowEvent, 0, 3) @@ -856,7 +856,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) @@ -909,7 +909,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 +949,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 +1010,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 +1077,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 +1212,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 +1347,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 +1403,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 +1448,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 +1497,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 +1547,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 +1597,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 +1655,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/admin.go b/pkg/sink/kafka/admin.go index b8cfd1cfc3..13833516ba 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()), @@ -153,12 +151,18 @@ func (a *saramaAdminClient) GetTopicsMeta(topics []string, ignoreTopicError bool return result, nil } +// IsAdminAuthorizationFailed checks whether err is an authorization failure from Kafka admin APIs. +func IsAdminAuthorizationFailed(err error) bool { + return errors.Is(err, sarama.ErrTopicAuthorizationFailed) || + errors.Is(err, sarama.ErrClusterAuthorizationFailed) +} + func (a *saramaAdminClient) GetTopicsPartitionsNum(topics []string) (map[string]int32, error) { result := make(map[string]int32, len(topics)) 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 +179,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 c2e3f90e37..3bcd3d0468 100644 --- a/pkg/sink/kafka/admin_test.go +++ b/pkg/sink/kafka/admin_test.go @@ -14,13 +14,53 @@ package kafka import ( + "io" "testing" + "github.com/IBM/sarama" "github.com/golang/mock/gomock" "github.com/pingcap/ticdc/pkg/common" + "github.com/pingcap/ticdc/pkg/errors" "github.com/stretchr/testify/require" ) +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 diff --git a/pkg/sink/kafka/claimcheck/claim_check.go b/pkg/sink/kafka/claimcheck/claim_check.go index 052785e2fa..4f8ecc2a42 100644 --- a/pkg/sink/kafka/claimcheck/claim_check.go +++ b/pkg/sink/kafka/claimcheck/claim_check.go @@ -20,27 +20,23 @@ import ( "time" "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" - "github.com/pingcap/ticdc/pkg/sink/codec/common" + "github.com/pingcap/ticdc/pkg/errors" + codecCommon "github.com/pingcap/ticdc/pkg/sink/codec/common" "github.com/pingcap/ticdc/pkg/util" "github.com/pingcap/tidb/br/pkg/storage" "github.com/prometheus/client_golang/prometheus" "go.uber.org/zap" ) -const ( - defaultTimeout = 5 * time.Minute -) - // ClaimCheck manage send message to the claim-check external storage. 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,16 +44,11 @@ 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 } - 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 { @@ -67,15 +58,9 @@ 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", - 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, @@ -88,19 +73,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() @@ -112,8 +97,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..16ad91d83e --- /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" + mockstorage "github.com/pingcap/tidb/br/pkg/mock/storage" + "github.com/pingcap/tidb/br/pkg/storage" + "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) + externalStorage := mockstorage.NewMockExternalStorage(ctrl) + externalStorage.EXPECT().Close().Times(1) + claimCheck := &ClaimCheck{ + storage: externalStorage, + changefeedID: common.NewChangeFeedIDWithName("test", "default"), + } + + claimCheck.Close() +} + +func TestClaimCheckConcurrentWrites(t *testing.T) { + ctx := context.Background() + externalStorage := storage.NewMemStorage() + changefeedID := common.NewChangeFeedIDWithName("test", "default") + claimCheck := &ClaimCheck{ + storage: externalStorage, + 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 := externalStorage.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 index 9a67b63520..dfeebbd773 100644 --- a/pkg/sink/kafka/cluster_admin_client_mock.go +++ b/pkg/sink/kafka/cluster_admin_client_mock.go @@ -74,12 +74,13 @@ func (mr *MockClusterAdminClientMockRecorder) GetAllBrokers() *gomock.Call { } // GetBrokerConfig mocks base method. -func (m *MockClusterAdminClient) GetBrokerConfig(configName string) (string, error) { +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].(error) - return ret0, ret1 + ret1, _ := ret[1].(bool) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 } // GetBrokerConfig indicates an expected call of GetBrokerConfig. @@ -89,12 +90,13 @@ func (mr *MockClusterAdminClientMockRecorder) GetBrokerConfig(configName interfa } // GetTopicConfig mocks base method. -func (m *MockClusterAdminClient) GetTopicConfig(topicName, configName string) (string, error) { +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].(error) - return ret0, ret1 + ret1, _ := ret[1].(bool) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 } // GetTopicConfig indicates an expected call of GetTopicConfig. 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 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..5bc933a3bf 100644 --- a/pkg/sink/kafka/options.go +++ b/pkg/sink/kafka/options.go @@ -26,11 +26,10 @@ import ( "github.com/gin-gonic/gin/binding" "github.com/imdario/mergo" - "github.com/pingcap/errors" "github.com/pingcap/log" "github.com/pingcap/ticdc/pkg/common" "github.com/pingcap/ticdc/pkg/config" - cerror "github.com/pingcap/ticdc/pkg/errors" + "github.com/pingcap/ticdc/pkg/errors" "github.com/pingcap/ticdc/pkg/security" "go.uber.org/zap" ) @@ -108,7 +107,9 @@ func requireAcksFromString(acks int) (RequiredAcks, error) { case int(NoResponse): return NoResponse, nil default: - 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) } } @@ -219,7 +220,7 @@ 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 { - return cerror.ErrKafkaInvalidPartitionNum.GenWithStack( + return errors.ErrKafkaInvalidConfig.GenWithStack( "the number of partition (%d) specified in sink-uri is more than that of actual topic (%d)", o.PartitionNum, realPartitionCount) } @@ -236,19 +237,22 @@ func (o *options) Apply(changefeedID common.ChangeFeedID, req := &http.Request{URL: sinkURI} urlParameter := &urlConfig{} if err = binding.Query.Bind(req, urlParameter); err != nil { - return cerror.WrapError(cerror.ErrMySQLInvalidConfig, err) + return errors.WrapError(errors.ErrKafkaInvalidConfig, err) } 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 { - return cerror.ErrKafkaInvalidPartitionNum.GenWithStackByArgs(o.PartitionNum) + return errors.ErrKafkaInvalidConfig.GenWithStack("invalid partition num %d", o.PartitionNum) } } if urlParameter.ReplicationFactor != nil { + if *urlParameter.ReplicationFactor <= 0 { + return errors.ErrKafkaInvalidConfig.GenWithStack("invalid replication-factor %d", *urlParameter.ReplicationFactor) + } o.ReplicationFactor = *urlParameter.ReplicationFactor } @@ -289,7 +293,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 +301,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 +309,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 +391,7 @@ func (o *options) applyTLS(params *urlConfig) error { if o.Credential != nil && !o.Credential.IsEmpty() && !o.Credential.IsTLSEnabled() { - 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") } // if enable-tls is not set, but credential files are set, @@ -401,8 +404,7 @@ func (o *options) applyTLS(params *urlConfig) error { enableTLS := *params.EnableTLS if o.Credential != nil && o.Credential.IsTLSEnabled() && !enableTLS { - 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") } o.EnableTLS = enableTLS } else { @@ -431,7 +433,7 @@ func (o *options) applySASL(urlParameter *urlConfig, sinkConfig *config.SinkConf if urlParameter.SASLMechanism != nil && *urlParameter.SASLMechanism != "" { mechanism, err := security.SASLMechanismFromString(*urlParameter.SASLMechanism) if err != nil { - return cerror.WrapError(cerror.ErrKafkaInvalidConfig, err) + return errors.WrapError(errors.ErrKafkaInvalidConfig, err) } o.SASL.SASLMechanism = mechanism } @@ -439,7 +441,7 @@ func (o *options) applySASL(urlParameter *urlConfig, sinkConfig *config.SinkConf if urlParameter.SASLGssAPIAuthType != nil && *urlParameter.SASLGssAPIAuthType != "" { authType, err := security.AuthTypeFromString(*urlParameter.SASLGssAPIAuthType) if err != nil { - return cerror.WrapError(cerror.ErrKafkaInvalidConfig, err) + return errors.WrapError(errors.ErrKafkaInvalidConfig, err) } o.SASL.GSSAPI.AuthType = authType } @@ -477,7 +479,7 @@ func (o *options) applySASL(urlParameter *urlConfig, sinkConfig *config.SinkConf if sinkConfig.KafkaConfig.SASLOAuthClientID != nil { clientID := *sinkConfig.KafkaConfig.SASLOAuthClientID if clientID == "" { - return cerror.ErrKafkaInvalidConfig.GenWithStack("OAuth2 client ID cannot be empty") + return errors.ErrKafkaInvalidConfig.GenWithStack("OAuth2 client ID cannot be empty") } o.SASL.OAuth2.ClientID = clientID } @@ -485,7 +487,7 @@ func (o *options) applySASL(urlParameter *urlConfig, sinkConfig *config.SinkConf if sinkConfig.KafkaConfig.SASLOAuthClientSecret != nil { clientSecret := *sinkConfig.KafkaConfig.SASLOAuthClientSecret if clientSecret == "" { - return cerror.ErrKafkaInvalidConfig.GenWithStack( + return errors.ErrKafkaInvalidConfig.GenWithStack( "OAuth2 client secret cannot be empty") } @@ -493,8 +495,7 @@ 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)) - return cerror.ErrKafkaInvalidConfig.GenWithStack( - "OAuth2 client secret is not base64 encoded") + return errors.ErrKafkaInvalidConfig.GenWithStack("OAuth2 client secret is not base64 encoded") } o.SASL.OAuth2.ClientSecret = string(decodedClientSecret) } @@ -502,7 +503,7 @@ func (o *options) applySASL(urlParameter *urlConfig, sinkConfig *config.SinkConf if sinkConfig.KafkaConfig.SASLOAuthTokenURL != nil { tokenURL := *sinkConfig.KafkaConfig.SASLOAuthTokenURL if tokenURL == "" { - return cerror.ErrKafkaInvalidConfig.GenWithStack( + return errors.ErrKafkaInvalidConfig.GenWithStack( "OAuth2 token URL cannot be empty") } o.SASL.OAuth2.TokenURL = tokenURL @@ -510,13 +511,13 @@ func (o *options) applySASL(urlParameter *urlConfig, sinkConfig *config.SinkConf if o.SASL.OAuth2.IsEnable() { if o.SASL.SASLMechanism != security.OAuthMechanism { - return cerror.ErrKafkaInvalidConfig.GenWithStack( + return errors.ErrKafkaInvalidConfig.GenWithStack( "OAuth2 is only supported with SASL mechanism type OAUTHBEARER, but got %s", o.SASL.SASLMechanism) } if err := o.SASL.OAuth2.Validate(); err != nil { - return cerror.ErrKafkaInvalidConfig.Wrap(err) + return errors.WrapError(errors.ErrKafkaInvalidConfig, err) } o.SASL.OAuth2.SetDefault() } @@ -537,11 +538,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,9 +551,48 @@ 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, 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 +} + var ( validClientID = regexp.MustCompile(`\A[A-Za-z0-9._-]+\z`) commonInvalidChar = regexp.MustCompile(`[\?:,"]`) @@ -570,7 +611,7 @@ func NewKafkaClientID(captureAddr string, clientID = commonInvalidChar.ReplaceAllString(clientID, "_") } if !validClientID.MatchString(clientID) { - return "", cerror.ErrKafkaInvalidClientID.GenWithStackByArgs(clientID) + return "", errors.ErrKafkaInvalidConfig.GenWithStack("invalid kafka client ID %q", clientID) } return } @@ -584,17 +625,7 @@ func adjustOptions( ) error { topics, err := admin.GetTopicsMeta([]string{topic}, true) if err != nil { - return errors.Trace(err) - } - - // 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. - if options.RequiredAcks == WaitForAll { - err = validateMinInsyncReplicas(ctx, admin, topics, topic, int(options.ReplicationFactor)) - if err != nil { - return errors.Trace(err) - } + return err } info, exists := topics[topic] @@ -602,17 +633,23 @@ func adjustOptions( // make sure user input parameters are valid. if exists { // make sure that producer's `MaxMessageBytes` smaller than topic's `max.message.bytes` - topicMaxMessageBytesStr, err := getTopicConfig( + topicMaxMessageBytesStr, found, err := getTopicConfig( ctx, admin, info.Name, TopicMaxMessageBytesConfigName, BrokerMessageMaxBytesConfigName, ) if err != nil { - return errors.Trace(err) + 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.Trace(err) + return errors.WrapError(errors.ErrKafkaAdminAPI, err, + "parse-config", TopicMaxMessageBytesConfigName) } maxMessageBytes := topicMaxMessageBytes - maxMessageBytesOverhead @@ -623,10 +660,8 @@ func adjustOptions( 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 - } + } else if maxMessageBytes < options.MaxMessageBytes { + options.MaxMessageBytes = maxMessageBytes } // no need to create the topic, @@ -637,20 +672,25 @@ func adjustOptions( } if err = options.setPartitionNum(info.NumPartitions); err != nil { - return errors.Trace(err) + return err } return nil } - brokerMessageMaxBytesStr, err := admin.GetBrokerConfig(BrokerMessageMaxBytesConfigName) + brokerMessageMaxBytesStr, found, err := admin.GetBrokerConfig(BrokerMessageMaxBytesConfigName) if err != nil { log.Warn("TiCDC cannot find `message.max.bytes` from broker's configuration") - return errors.Trace(err) + return err + } + if !found { + return errors.ErrKafkaAdminAPI.GenWithStack( + "Kafka broker configuration %s not found", BrokerMessageMaxBytesConfigName) } brokerMessageMaxBytes, err := strconv.Atoi(brokerMessageMaxBytesStr) if err != nil { - return errors.Trace(err) + return errors.WrapError(errors.ErrKafkaAdminAPI, err, + "parse-config", BrokerMessageMaxBytesConfigName) } // when create the topic, `max.message.bytes` is decided by the broker, @@ -665,10 +705,8 @@ func adjustOptions( 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 - } + } else if maxMessageBytes < options.MaxMessageBytes { + options.MaxMessageBytes = maxMessageBytes } // topic not exists yet, and user does not specify the `partition-num` in the sink uri. @@ -680,89 +718,23 @@ func adjustOptions( return nil } -func validateMinInsyncReplicas( - ctx context.Context, - admin ClusterAdminClient, - topics map[string]TopicDetail, - topic string, - replicationFactor int, -) error { - minInsyncReplicasConfigGetter := func() (string, bool, error) { - info, exists := topics[topic] - if exists { - minInsyncReplicasStr, err := getTopicConfig( - ctx, admin, info.Name, - MinInsyncReplicasConfigName, - MinInsyncReplicasConfigName) - if err != nil { - return "", true, err - } - return minInsyncReplicasStr, true, nil - } - - minInsyncReplicasStr, err := admin.GetBrokerConfig(MinInsyncReplicasConfigName) - if err != nil { - return "", false, err - } - - return minInsyncReplicasStr, false, nil - } - - minInsyncReplicasStr, exists, err := minInsyncReplicasConfigGetter() - if err != nil { - // '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, " + - "please make sure that the replication factor is greater than or equal " + - "to the minimum number of in-sync replicas" + - "if you want to use `required-acks` = -1." + - "Otherwise, TiCDC will not be able to send messages to the topic.") - return nil - } - return err - } - minInsyncReplicas, err := strconv.Atoi(minInsyncReplicasStr) - if err != nil { - return err - } - - configFrom := "topic" - if !exists { - configFrom = "broker" - } - - if replicationFactor < minInsyncReplicas { - msg := fmt.Sprintf("`replication-factor` cannot be smaller than the `%s` of %s", - MinInsyncReplicasConfigName, configFrom) - log.Error(msg, zap.Int("replication-factor", replicationFactor), - zap.Int("min.insync.replicas", minInsyncReplicas)) - return cerror.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 %s", - replicationFactor, minInsyncReplicas, configFrom, - ) - } - - return nil -} - // 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, + _ context.Context, admin ClusterAdminClient, 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 e068ca13b7..ab2c55d580 100644 --- a/pkg/sink/kafka/options_test.go +++ b/pkg/sink/kafka/options_test.go @@ -25,10 +25,10 @@ import ( "github.com/IBM/sarama" "github.com/aws/aws-sdk-go/aws" "github.com/golang/mock/gomock" - 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/common" + codecCommon "github.com/pingcap/ticdc/pkg/sink/codec/common" "github.com/stretchr/testify/require" ) @@ -111,24 +111,21 @@ func (f *kafkaAdminFixture) getTopicsPartitionsNum( return result, nil } -func (f *kafkaAdminFixture) getBrokerConfig(configName string) (string, error) { +func (f *kafkaAdminFixture) getBrokerConfig(configName string) (string, bool, error) { if value, ok := f.brokerConfig[configName]; ok { - return value, nil + return value, true, nil } - return "", errors.ErrKafkaConfigNotFound.GenWithStack( - "cannot find the `%s` from the broker's configuration", configName) + return "", false, nil } -func (f *kafkaAdminFixture) getTopicConfig(topicName string, configName string) (string, error) { +func (f *kafkaAdminFixture) getTopicConfig(topicName string, configName string) (string, bool, error) { if _, ok := f.topics[topicName]; !ok { - return "", errors.ErrKafkaConfigNotFound.GenWithStack( - "cannot find the `%s` from the topic's configuration", topicName) + return "", false, nil } if value, ok := f.topicConfig[topicName][configName]; ok { - return value, nil + return value, true, nil } - return "", errors.ErrKafkaConfigNotFound.GenWithStack( - "cannot find the `%s` from the topic's configuration", configName) + return "", false, nil } func (f *kafkaAdminFixture) createTopic(detail *TopicDetail, _ bool) error { @@ -187,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) @@ -201,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) @@ -211,15 +208,27 @@ 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)) + 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) + } // 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. @@ -227,7 +236,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. @@ -235,7 +244,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. @@ -243,7 +252,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. @@ -251,7 +260,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 @@ -259,15 +268,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) @@ -275,7 +284,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) @@ -284,7 +293,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, defaultMaxRetry, options.MaxRetry) } @@ -302,7 +311,7 @@ func TestSetPartitionNum(t *testing.T) { options.PartitionNum = 3 err = options.setPartitionNum(2) - require.True(t, errors.ErrKafkaInvalidPartitionNum.Equal(err)) + require.True(t, errors.ErrKafkaInvalidConfig.Equal(err)) } func TestClientID(t *testing.T) { @@ -340,7 +349,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 { @@ -361,7 +370,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) @@ -395,7 +404,6 @@ func TestAdjustConfigFallsBackToBrokerMessageMaxBytesWhenTopicConfigMissing(t *t } topicName := "test-topic" - for _, test := range tests { t.Run(test.name, func(t *testing.T) { adminFixture := newKafkaAdminFixture(t) @@ -429,81 +437,39 @@ func TestAdjustConfigFallsBackToBrokerMessageMaxBytesWhenTopicConfigMissing(t *t } } -func TestAdjustConfigMinInsyncReplicas(t *testing.T) { +func TestValidateReplicationFactor(t *testing.T) { adminFixture := newKafkaAdminFixture(t) adminClient := adminFixture.admin - - options := NewOptions() - options.BrokerEndpoints = []string{"127.0.0.1:9092"} - - // Report an error if the replication-factor is less than min.insync.replicas - // when the topic does not exist. adminFixture.setMinInsyncReplicas("2") - ctx := context.Background() - err := adjustOptions( - ctx, - adminClient, - options, - "create-new-fail-invalid-min-insync-replicas", - ) + topicConfig := &AutoCreateTopicConfig{ + AutoCreate: true, + ReplicationFactor: 1, + RequiredAcks: WaitForAll, + } + err := topicConfig.ValidateReplicationFactor(adminClient) require.Regexp( t, ".*`replication-factor` 1 is smaller than the `min.insync.replicas` 2 of broker.*", errors.Cause(err), ) - // topic not exist, and `min.insync.replicas` not found in broker's configuration - adminFixture.dropBrokerConfig(MinInsyncReplicasConfigName) - topicName := "no-topic-no-min-insync-replicas" - err = adjustOptions(ctx, adminClient, options, "no-topic-no-min-insync-replicas") - require.Nil(t, err) - err = adminClient.CreateTopic(&TopicDetail{ - Name: topicName, + localAcksConfig := &AutoCreateTopicConfig{ + AutoCreate: true, ReplicationFactor: 1, - }, false) - require.ErrorIs(t, err, sarama.ErrPolicyViolation) - - // Report an error if the replication-factor is less than min.insync.replicas - // when the topic does exist. - - // topic exist, but `min.insync.replicas` not found in topic and broker configuration - topicName = "topic-no-options-entry" - err = adminClient.CreateTopic(&TopicDetail{ - Name: topicName, - ReplicationFactor: 3, - NumPartitions: 3, - }, false) - require.Nil(t, err) - err = adjustOptions(ctx, adminClient, options, topicName) - require.Nil(t, err) - - // topic found, and have `min.insync.replicas`, but set to 2, larger than `replication-factor`. - adminFixture.setMinInsyncReplicas("2") - err = adjustOptions(ctx, adminClient, options, defaultMockTopicName) - require.Regexp(t, - ".*`replication-factor` 1 is smaller than the `min.insync.replicas` 2 of topic.*", - errors.Cause(err), - ) -} - -func TestSkipAdjustConfigMinInsyncReplicasWhenRequiredAcksIsNotWailAll(t *testing.T) { - adminFixture := newKafkaAdminFixture(t) - adminClient := adminFixture.admin - - options := NewOptions() - options.BrokerEndpoints = []string{"127.0.0.1:9092"} - options.RequiredAcks = WaitForLocal + RequiredAcks: WaitForLocal, + } + err = localAcksConfig.ValidateReplicationFactor(adminClient) + require.NoError(t, err) - // Do not report an error if the replication-factor is less than min.insync.replicas(1<2). - adminFixture.setMinInsyncReplicas("2") - err := adjustOptions( - context.Background(), - adminClient, - options, - "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) } func TestCreateProducerFailed(t *testing.T) { @@ -673,9 +639,8 @@ func TestConfigurationCombinations(t *testing.T) { sinkURI, err := url.Parse(uri) require.Nil(t, err) - ctx := context.Background() 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.Nil(t, err) topic, ok := a.uriParams[0].(string) @@ -688,15 +653,15 @@ func TestConfigurationCombinations(t *testing.T) { } expectedMaxMessageBytes := expectedAdjustedMaxMessageBytes(options.MaxMessageBytes, sourceMaxMessageBytes) - err = adjustOptions(ctx, adminClient, options, topic) + err = adjustOptions(context.Background(), adminClient, options, topic) require.Nil(t, err) require.Equal(t, expectedMaxMessageBytes, options.MaxMessageBytes) - saramaConfig, err := newSaramaConfig(ctx, options) + saramaConfig, err := newSaramaConfig(context.Background(), options) require.Nil(t, err) require.Equal(t, expectedMaxMessageBytes, saramaConfig.Producer.MaxMessageBytes) - encoderConfig := common.NewConfig(config.ProtocolOpen) + encoderConfig := codecCommon.NewConfig(config.ProtocolOpen) err = encoderConfig.Apply(sinkURI, &config.SinkConfig{ KafkaConfig: &config.KafkaConfig{ LargeMessageHandle: config.NewDefaultLargeMessageHandleConfig(), @@ -748,7 +713,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) @@ -829,7 +794,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..57bf5ef27c 100644 --- a/pkg/sink/kafka/sarama_factory.go +++ b/pkg/sink/kafka/sarama_factory.go @@ -46,19 +46,19 @@ 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() }() if err = adjustOptions(ctx, admin, o, o.Topic); err != nil { - return nil, errors.Trace(err) + return nil, err } return &saramaFactory{ @@ -77,7 +77,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 +91,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 +103,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 +113,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 +141,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 37285419c6..522dc13bcb 100644 --- a/pkg/sink/kafka/sarama_sync_producer_test.go +++ b/pkg/sink/kafka/sarama_sync_producer_test.go @@ -14,14 +14,32 @@ package kafka import ( - "errors" + "context" + "io" + "strings" "testing" + "github.com/IBM/sarama" "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" + "github.com/stretchr/testify/require" "go.uber.org/atomic" ) +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 @@ -32,7 +50,7 @@ func TestSyncProducerClose(t *testing.T) { }, { name: "still closes producer when client close fails", - clientCloseErr: errors.New("boom"), + clientCloseErr: io.ErrClosedPipe, }, } @@ -57,3 +75,72 @@ func TestSyncProducerClose(t *testing.T) { }) } } + +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_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: