From b4783b2562c343f171b2e71f53e11a4946521c25 Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Wed, 29 Jul 2026 13:57:54 +0800 Subject: [PATCH 01/17] sink write bytes based on raw bytes --- .../sink/cloudstorage/buffer_manager.go | 9 ++++++--- downstreamadapter/sink/cloudstorage/task.go | 20 ++++++++++--------- downstreamadapter/sink/cloudstorage/writer.go | 5 +++-- .../sink/cloudstorage/writer_test.go | 6 +++++- downstreamadapter/sink/helper/mq_row_event.go | 3 +++ downstreamadapter/sink/kafka/sink.go | 7 +++++-- downstreamadapter/sink/kafka/sink_test.go | 5 +++++ downstreamadapter/sink/pulsar/sink.go | 7 +++++-- pkg/common/event/row_change.go | 1 + pkg/sink/codec/encoder_group.go | 15 +++++++++----- 10 files changed, 54 insertions(+), 24 deletions(-) diff --git a/downstreamadapter/sink/cloudstorage/buffer_manager.go b/downstreamadapter/sink/cloudstorage/buffer_manager.go index fa49c404e1..be7d8232b4 100644 --- a/downstreamadapter/sink/cloudstorage/buffer_manager.go +++ b/downstreamadapter/sink/cloudstorage/buffer_manager.go @@ -177,9 +177,10 @@ type tableBatches struct { } type tableBatch struct { - size uint64 - tableInfo *common.TableInfo - entries []*spool.Entry + size uint64 + approximateSize int64 + tableInfo *common.TableInfo + entries []*spool.Entry } func newTableBatches() tableBatches { @@ -203,6 +204,7 @@ func (t *tableBatches) addEntry(event *task, entry *spool.Entry) { tableTask := t.tables[table] tableTask.size += entry.FileBytes() + tableTask.approximateSize += event.approximateSize tableTask.entries = append(tableTask.entries, entry) t.nBytes += entry.FileBytes() } @@ -285,6 +287,7 @@ func (b *payloadBuilder) Build() *payload { tableInfo: b.batch.tableInfo, data: b.buf.Bytes(), rowsCount: b.rowsCount, + approximateSize: b.batch.approximateSize, nBytes: b.nBytes, entries: b.batch.entries, postFlushCallbacks: b.postFlushCallbacks, diff --git a/downstreamadapter/sink/cloudstorage/task.go b/downstreamadapter/sink/cloudstorage/task.go index 63ef6acd2b..218785dd70 100644 --- a/downstreamadapter/sink/cloudstorage/task.go +++ b/downstreamadapter/sink/cloudstorage/task.go @@ -38,11 +38,12 @@ type task struct { dispatcherID commonType.DispatcherID // DML-only fields. - postEnqueue func() // Transaction enqueue callback. - tableInfo *commonType.TableInfo // Table info used after event is released. - versionedTable cloudstorage.VersionedTableName // Versioned output identity for the DML event. - rowEvents []*commonEvent.RowEvent // Row events to encode and flush. - encodedMsgs []*common.Message // Encoded result built from event. + postEnqueue func() // Transaction enqueue callback. + tableInfo *commonType.TableInfo // Table info used after event is released. + versionedTable cloudstorage.VersionedTableName // Versioned output identity for the DML event. + approximateSize int64 // Approximate size of the original DML event. + rowEvents []*commonEvent.RowEvent // Row events to encode and flush. + encodedMsgs []*common.Message // Encoded result built from event. // Flush-only field. marker *flushMarker // Barrier marker used by FlushDMLBeforeBlock. @@ -55,10 +56,11 @@ func newDMLTask( ) *task { postEnqueue, postFlush := event.DetachPostCallbacks() return &task{ - kind: taskKindDML, - postEnqueue: postEnqueue, - tableInfo: event.TableInfo, - versionedTable: version, + kind: taskKindDML, + postEnqueue: postEnqueue, + tableInfo: event.TableInfo, + versionedTable: version, + approximateSize: event.GetSize(), // Storage txn encoders attach only the last row callback to the built // batch message, so the callback is triggered once per encoded txn // message. Kafka uses row-level callbacks and counts all rows before diff --git a/downstreamadapter/sink/cloudstorage/writer.go b/downstreamadapter/sink/cloudstorage/writer.go index e31a1387c6..ddd945a858 100644 --- a/downstreamadapter/sink/cloudstorage/writer.go +++ b/downstreamadapter/sink/cloudstorage/writer.go @@ -63,6 +63,7 @@ type payload struct { tableInfo *common.TableInfo data []byte rowsCount int + approximateSize int64 nBytes int64 entries []*spool.Entry postFlushCallbacks []func() @@ -229,7 +230,7 @@ func (d *writer) writeDataFile(ctx context.Context, dataFilePath, indexFilePath if err != nil { return 0, 0, err } - return payload.rowsCount, payload.nBytes, nil + return payload.rowsCount, payload.approximateSize, nil } writer, err := d.storage.Create(ctx, dataFilePath, &storeapi.WriterOption{ @@ -256,7 +257,7 @@ func (d *writer) writeDataFile(ctx context.Context, dataFilePath, indexFilePath zap.String("path", dataFilePath), zap.Error(err)) return 0, 0, err } - return payload.rowsCount, payload.nBytes, nil + return payload.rowsCount, payload.approximateSize, nil }) if err != nil { return err diff --git a/downstreamadapter/sink/cloudstorage/writer_test.go b/downstreamadapter/sink/cloudstorage/writer_test.go index 7aaf02dd82..0c0f5c679b 100644 --- a/downstreamadapter/sink/cloudstorage/writer_test.go +++ b/downstreamadapter/sink/cloudstorage/writer_test.go @@ -426,8 +426,12 @@ func TestWriterPostFlushDoesNotRunPausedPostEnqueue(t *testing.T) { defer spoolBuffer.Release(secondEntry) require.Equal(t, int64(0), secondEnqueued.Load()) - payload, err := buildPayload(spoolBuffer, &tableBatch{entries: []*spool.Entry{secondEntry}}) + payload, err := buildPayload(spoolBuffer, &tableBatch{ + approximateSize: 123, + entries: []*spool.Entry{secondEntry}, + }) require.NoError(t, err) + require.Equal(t, int64(123), payload.approximateSize) require.Len(t, payload.postFlushCallbacks, 1) for _, postFlushCallback := range payload.postFlushCallbacks { diff --git a/downstreamadapter/sink/helper/mq_row_event.go b/downstreamadapter/sink/helper/mq_row_event.go index bbd8de207c..6dcdb5a73c 100644 --- a/downstreamadapter/sink/helper/mq_row_event.go +++ b/downstreamadapter/sink/helper/mq_row_event.go @@ -28,6 +28,7 @@ func NewMQRowEvents( ) ([]*commonEvent.MQRowEvent, error) { callback := NewPostFlushRowCallback(event, uint64(event.Len())) events := make([]*commonEvent.MQRowEvent, 0, event.Len()) + approximateSize := event.GetSize() if selector == nil { selector = columnselector.NewDefaultColumnSelector() } @@ -57,12 +58,14 @@ func NewMQRowEvents( TableInfo: event.TableInfo, StartTs: event.StartTs, CommitTs: event.CommitTs, + ApproximateSize: approximateSize, Event: row, Callback: callback, ColumnSelector: selector, Checksum: row.Checksum, }, }) + approximateSize = 0 } return events, nil } diff --git a/downstreamadapter/sink/kafka/sink.go b/downstreamadapter/sink/kafka/sink.go index 13c4d539b6..67e7412419 100644 --- a/downstreamadapter/sink/kafka/sink.go +++ b/downstreamadapter/sink/kafka/sink.go @@ -443,7 +443,7 @@ func (s *sink) sendMessages(ctx context.Context) error { if err = future.Ready(ctx); err != nil { return err } - for _, message := range future.Messages { + for i, message := range future.Messages { start := time.Now() if err = s.statistics.RecordBatchExecution(func() (int, int64, error) { message.SetPartitionKey(future.Key.PartitionKey) @@ -459,7 +459,10 @@ func (s *sink) sendMessages(ctx context.Context) error { zap.Error(err)) return 0, 0, err } - return message.GetRowsCount(), int64(message.Length()), nil + if i == 0 { + return message.GetRowsCount(), future.ApproximateSize, nil + } + return message.GetRowsCount(), 0, nil }); err != nil { return err } diff --git a/downstreamadapter/sink/kafka/sink_test.go b/downstreamadapter/sink/kafka/sink_test.go index 8b205c1649..6f8aecb96e 100644 --- a/downstreamadapter/sink/kafka/sink_test.go +++ b/downstreamadapter/sink/kafka/sink_test.go @@ -32,9 +32,11 @@ import ( commonEvent "github.com/pingcap/ticdc/pkg/common/event" "github.com/pingcap/ticdc/pkg/config" "github.com/pingcap/ticdc/pkg/errors" + pmetrics "github.com/pingcap/ticdc/pkg/metrics" "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/prometheus/client_golang/prometheus/testutil" "github.com/stretchr/testify/require" "go.uber.org/atomic" ) @@ -335,6 +337,8 @@ func TestKafkaSinkBasicFunctionality(t *testing.T) { err = kafkaSink.WriteBlockEvent(ddlEvent) require.NoError(t, err) + writeBytes := pmetrics.TotalWriteBytesCounter.WithLabelValues("test", "test", "sink") + beforeWriteBytes := testutil.ToFloat64(writeBytes) kafkaSink.AddDMLEvent(dmlEvent) ddlEvent2.PostFlush() @@ -343,6 +347,7 @@ func TestKafkaSinkBasicFunctionality(t *testing.T) { func() bool { return count.Load() == int64(3) }, 5*time.Second, time.Second) + require.Equal(t, float64(dmlEvent.GetSize()), testutil.ToFloat64(writeBytes)-beforeWriteBytes) // case 2: add checkpoint ts when sink is closed and it will not block kafkaSink.Close() diff --git a/downstreamadapter/sink/pulsar/sink.go b/downstreamadapter/sink/pulsar/sink.go index 9895541233..f6b54c684d 100644 --- a/downstreamadapter/sink/pulsar/sink.go +++ b/downstreamadapter/sink/pulsar/sink.go @@ -531,14 +531,17 @@ func (s *sink) sendMessages(ctx context.Context) error { if err = future.Ready(ctx); err != nil { return errors.Trace(err) } - for _, message := range future.Messages { + for i, message := range future.Messages { start := time.Now() if err = s.statistics.RecordBatchExecution(func() (int, int64, error) { message.SetPartitionKey(future.Key.PartitionKey) if err = s.dmlProducer.asyncSendMessage(ctx, future.Key.Topic, message); err != nil { return 0, 0, err } - return message.GetRowsCount(), int64(message.Length()), nil + if i == 0 { + return message.GetRowsCount(), future.ApproximateSize, nil + } + return message.GetRowsCount(), 0, nil }); err != nil { return errors.Trace(err) } diff --git a/pkg/common/event/row_change.go b/pkg/common/event/row_change.go index 1d16eb358a..11454e27a0 100644 --- a/pkg/common/event/row_change.go +++ b/pkg/common/event/row_change.go @@ -93,6 +93,7 @@ type RowEvent struct { TableInfo *common.TableInfo StartTs uint64 CommitTs uint64 + ApproximateSize int64 Event RowChange ColumnSelector Selector Callback func() diff --git a/pkg/sink/codec/encoder_group.go b/pkg/sink/codec/encoder_group.go index eeeca21a20..fa3a506463 100644 --- a/pkg/sink/codec/encoder_group.go +++ b/pkg/sink/codec/encoder_group.go @@ -216,20 +216,25 @@ func (g *encoderGroup) cleanMetrics() { // future is a wrapper of the result of encoding events // It's used to notify the caller that the result is ready. type future struct { - Key commonEvent.TopicPartitionKey - events []*commonEvent.RowEvent - Messages []*common.Message - done chan struct{} + Key commonEvent.TopicPartitionKey + ApproximateSize int64 + events []*commonEvent.RowEvent + Messages []*common.Message + done chan struct{} } func newFuture(key commonEvent.TopicPartitionKey, events ...*commonEvent.RowEvent, ) *future { - return &future{ + future := &future{ Key: key, events: events, done: make(chan struct{}), } + for _, event := range events { + future.ApproximateSize += event.ApproximateSize + } + return future } // Ready waits until the response is ready, should be called before consuming the future. From 637b727156f02fd14991be79daa63d67cb3fdcbc Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Wed, 29 Jul 2026 14:02:26 +0800 Subject: [PATCH 02/17] remove ddl running gauge --- pkg/metrics/statistics.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/metrics/statistics.go b/pkg/metrics/statistics.go index f1815a63e5..0648a0d075 100644 --- a/pkg/metrics/statistics.go +++ b/pkg/metrics/statistics.go @@ -145,6 +145,7 @@ func (b *Statistics) Close() { keyspace := b.changefeedID.Keyspace() changefeedID := b.changefeedID.Name() ExecDDLHistogram.DeleteLabelValues(keyspace, changefeedID) + ExecDDLRunningGauge.DeleteLabelValues(keyspace, changefeedID) ExecBatchHistogram.DeleteLabelValues(keyspace, changefeedID, b.sinkType, b.keyspaceID) ExecBatchWriteBytesHistogram.DeleteLabelValues(keyspace, changefeedID, b.sinkType) EventSizeHistogram.DeleteLabelValues(keyspace, changefeedID) From 2049e49481143a0a4db1cd8b5170c0ea9ecd6280 Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Wed, 29 Jul 2026 14:25:39 +0800 Subject: [PATCH 03/17] update statistics after the data is flushed --- downstreamadapter/sink/kafka/sink.go | 46 +++++++++++-------- downstreamadapter/sink/kafka/sink_test.go | 19 ++++++-- .../sink/pulsar/mock_producer.go | 9 ++-- downstreamadapter/sink/pulsar/sink.go | 28 +++++++---- downstreamadapter/sink/pulsar/sink_test.go | 23 +++++++++- 5 files changed, 88 insertions(+), 37 deletions(-) diff --git a/downstreamadapter/sink/kafka/sink.go b/downstreamadapter/sink/kafka/sink.go index 67e7412419..c5644f0b58 100644 --- a/downstreamadapter/sink/kafka/sink.go +++ b/downstreamadapter/sink/kafka/sink.go @@ -445,26 +445,34 @@ func (s *sink) sendMessages(ctx context.Context) error { } for i, message := range future.Messages { start := time.Now() - if err = s.statistics.RecordBatchExecution(func() (int, int64, error) { - message.SetPartitionKey(future.Key.PartitionKey) - log.Debug("send message to kafka", zap.String("messageKey", util.RedactBytes(message.Key)), zap.String("messageValue", util.RedactBytes(message.Value))) - if err = s.dmlProducer.AsyncSend( - ctx, - future.Key.Topic, - future.Key.Partition, - message); err != nil { - log.Error("kafka sink send message failed", - zap.String("keyspace", s.changefeedID.Keyspace()), - zap.String("changefeed", s.changefeedID.Name()), - zap.Error(err)) - return 0, 0, err - } - if i == 0 { - return message.GetRowsCount(), future.ApproximateSize, nil + rows, writeBytes := message.GetRowsCount(), int64(0) + if i == 0 { + writeBytes = future.ApproximateSize + } + callback := message.Callback + message.Callback = func() { + _ = s.statistics.RecordBatchExecution(func() (int, int64, error) { + return rows, writeBytes, nil + }) + if callback != nil { + callback() } - return message.GetRowsCount(), 0, nil - }); err != nil { - return err + } + + message.SetPartitionKey(future.Key.PartitionKey) + log.Debug("send message to kafka", zap.String("messageKey", util.RedactBytes(message.Key)), zap.String("messageValue", util.RedactBytes(message.Value))) + if err = s.dmlProducer.AsyncSend( + ctx, + future.Key.Topic, + future.Key.Partition, + message); err != nil { + log.Error("kafka sink send message failed", + zap.String("keyspace", s.changefeedID.Keyspace()), + zap.String("changefeed", s.changefeedID.Name()), + zap.Error(err)) + return s.statistics.RecordBatchExecution(func() (int, int64, error) { + return 0, 0, err + }) } metricSendMessageDuration.Observe(time.Since(start).Seconds()) } diff --git a/downstreamadapter/sink/kafka/sink_test.go b/downstreamadapter/sink/kafka/sink_test.go index 6f8aecb96e..5dc8138c2c 100644 --- a/downstreamadapter/sink/kafka/sink_test.go +++ b/downstreamadapter/sink/kafka/sink_test.go @@ -312,6 +312,7 @@ func TestKafkaSinkBasicFunctionality(t *testing.T) { ctrl := gomock.NewController(t) asyncProducer := kafka.NewMockAsyncProducer(ctrl) syncProducer := kafka.NewMockSyncProducer(ctrl) + callbackCh := make(chan func(), 2) asyncProducer.EXPECT().AsyncRunCallback(gomock.Any()).Return(nil).AnyTimes() asyncProducer.EXPECT().AsyncSend(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). DoAndReturn(func( @@ -320,9 +321,7 @@ func TestKafkaSinkBasicFunctionality(t *testing.T) { _ int32, message *codecCommon.Message, ) error { - if message.Callback != nil { - message.Callback() - } + callbackCh <- message.Callback return nil }).Times(2) asyncProducer.EXPECT().Close().AnyTimes() @@ -340,6 +339,20 @@ func TestKafkaSinkBasicFunctionality(t *testing.T) { writeBytes := pmetrics.TotalWriteBytesCounter.WithLabelValues("test", "test", "sink") beforeWriteBytes := testutil.ToFloat64(writeBytes) kafkaSink.AddDMLEvent(dmlEvent) + callbacks := make([]func(), 0, 2) + for range 2 { + select { + case callback := <-callbackCh: + callbacks = append(callbacks, callback) + case <-time.After(5 * time.Second): + t.Fatal("timed out waiting for Kafka messages") + } + } + require.Equal(t, beforeWriteBytes, testutil.ToFloat64(writeBytes)) + for _, callback := range callbacks { + require.NotNil(t, callback) + callback() + } ddlEvent2.PostFlush() diff --git a/downstreamadapter/sink/pulsar/mock_producer.go b/downstreamadapter/sink/pulsar/mock_producer.go index a71d6a1247..b816efd91b 100644 --- a/downstreamadapter/sink/pulsar/mock_producer.go +++ b/downstreamadapter/sink/pulsar/mock_producer.go @@ -28,8 +28,9 @@ var ( // mockProducer is a mock pulsar producer type mockProducer struct { - mu sync.Mutex - events map[string][]*pulsar.ProducerMessage + mu sync.Mutex + events map[string][]*pulsar.ProducerMessage + callbackCh chan func() } func newMockDDLProducer() ddlProducer { @@ -80,7 +81,9 @@ func (p *mockProducer) asyncSendMessage(_ context.Context, topic string, message Key: message.GetPartitionKey(), } p.events[topic] = append(p.events[topic], data) - if message.Callback != nil { + if p.callbackCh != nil { + p.callbackCh <- message.Callback + } else if message.Callback != nil { message.Callback() } return nil diff --git a/downstreamadapter/sink/pulsar/sink.go b/downstreamadapter/sink/pulsar/sink.go index f6b54c684d..35b50f2087 100644 --- a/downstreamadapter/sink/pulsar/sink.go +++ b/downstreamadapter/sink/pulsar/sink.go @@ -533,17 +533,25 @@ func (s *sink) sendMessages(ctx context.Context) error { } for i, message := range future.Messages { start := time.Now() - if err = s.statistics.RecordBatchExecution(func() (int, int64, error) { - message.SetPartitionKey(future.Key.PartitionKey) - if err = s.dmlProducer.asyncSendMessage(ctx, future.Key.Topic, message); err != nil { - return 0, 0, err - } - if i == 0 { - return message.GetRowsCount(), future.ApproximateSize, nil + rows, writeBytes := message.GetRowsCount(), int64(0) + if i == 0 { + writeBytes = future.ApproximateSize + } + callback := message.Callback + message.Callback = func() { + _ = s.statistics.RecordBatchExecution(func() (int, int64, error) { + return rows, writeBytes, nil + }) + if callback != nil { + callback() } - return message.GetRowsCount(), 0, nil - }); err != nil { - return errors.Trace(err) + } + + message.SetPartitionKey(future.Key.PartitionKey) + if err = s.dmlProducer.asyncSendMessage(ctx, future.Key.Topic, message); err != nil { + return s.statistics.RecordBatchExecution(func() (int, int64, error) { + return 0, 0, errors.Trace(err) + }) } metricSendMessageDuration.Observe(time.Since(start).Seconds()) } diff --git a/downstreamadapter/sink/pulsar/sink_test.go b/downstreamadapter/sink/pulsar/sink_test.go index 973e4f15e6..4d0e22e195 100644 --- a/downstreamadapter/sink/pulsar/sink_test.go +++ b/downstreamadapter/sink/pulsar/sink_test.go @@ -27,6 +27,7 @@ import ( cerror "github.com/pingcap/ticdc/pkg/errors" "github.com/pingcap/ticdc/pkg/metrics" "github.com/pingcap/ticdc/utils/chann" + "github.com/prometheus/client_golang/prometheus/testutil" "github.com/stretchr/testify/require" "go.uber.org/atomic" ) @@ -119,19 +120,37 @@ func TestPulsarSinkBasicFunctionality(t *testing.T) { func() { count.Add(1) }, } dmlEvent.CommitTs = 2 + producer := pulsarSink.dmlProducer.(*mockProducer) + producer.callbackCh = make(chan func(), 2) err = pulsarSink.WriteBlockEvent(ddlEvent) require.NoError(t, err) + writeBytes := metrics.TotalWriteBytesCounter.WithLabelValues("test", "test", "sink") + beforeWriteBytes := testutil.ToFloat64(writeBytes) pulsarSink.AddDMLEvent(dmlEvent) - time.Sleep(1 * time.Second) + callbacks := make([]func(), 0, 2) + for range 2 { + select { + case callback := <-producer.callbackCh: + callbacks = append(callbacks, callback) + case <-time.After(5 * time.Second): + t.Fatal("timed out waiting for Pulsar messages") + } + } + require.Equal(t, beforeWriteBytes, testutil.ToFloat64(writeBytes)) + for _, callback := range callbacks { + require.NotNil(t, callback) + callback() + } ddlEvent2.PostFlush() - require.Len(t, pulsarSink.dmlProducer.(*mockProducer).GetAllEvents(), 2) + require.Len(t, producer.GetAllEvents(), 2) require.Len(t, pulsarSink.ddlProducer.(*mockProducer).GetAllEvents(), 1) require.Equal(t, count.Load(), int64(3)) + require.Equal(t, float64(dmlEvent.GetSize()), testutil.ToFloat64(writeBytes)-beforeWriteBytes) } func TestPulsarSinkBatchConfig(t *testing.T) { From d012cfcce48a3fda7e68f433b5b375f66050439e Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Wed, 29 Jul 2026 14:54:29 +0800 Subject: [PATCH 04/17] add affected rows --- pkg/sink/mysql/mysql_writer_dml_exec.go | 16 +++++++----- pkg/sink/mysql/mysql_writer_test.go | 34 +++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/pkg/sink/mysql/mysql_writer_dml_exec.go b/pkg/sink/mysql/mysql_writer_dml_exec.go index 4d0645bae7..7b949b811d 100644 --- a/pkg/sink/mysql/mysql_writer_dml_exec.go +++ b/pkg/sink/mysql/mysql_writer_dml_exec.go @@ -62,7 +62,7 @@ func (w *Writer) execDMLWithMaxRetries(dmls *preparedDMLs) error { return errors.Trace(err) } - err = w.sequenceExecute(dmls, tx, writeTimeout) + affectedRows, err := w.sequenceExecute(dmls, tx, writeTimeout) if err != nil { return err } @@ -70,6 +70,9 @@ func (w *Writer) execDMLWithMaxRetries(dmls *preparedDMLs) error { if err = tx.Commit(); err != nil { return err } + for i, rows := range affectedRows { + w.statistics.RecordRowsAffected(rows, dmls.rowTypes[i]) + } log.Debug("Exec Rows succeeded", zap.Any("rowCount", dmls.rowCount), zap.Int("writerID", w.id)) return nil @@ -128,7 +131,8 @@ func (w *Writer) execDMLWithMaxRetries(dmls *preparedDMLs) error { // sequenceExecute runs each SQL sequentially inside a transaction. func (w *Writer) sequenceExecute( dmls *preparedDMLs, tx *sql.Tx, writeTimeout time.Duration, -) error { +) (map[int]int64, error) { + affectedRows := make(map[int]int64, len(dmls.sqls)) for i, query := range dmls.sqls { args := dmls.values[i] log.Debug("exec row", zap.String("sql", query), zap.String("args", util.RedactArgs(args)), zap.Int("writerID", w.id)) @@ -167,16 +171,16 @@ func (w *Writer) sequenceExecute( } } cancelFunc() - return errors.WrapError(errors.ErrMySQLTxnError, errors.WithMessage(execError, fmt.Sprintf("Failed to execute DMLs, query info:%s, args:%v; ", query, util.RedactArgs(args)))) + return nil, errors.WrapError(errors.ErrMySQLTxnError, errors.WithMessage(execError, fmt.Sprintf("Failed to execute DMLs, query info:%s, args:%v; ", query, util.RedactArgs(args)))) } - if rowsAffected, err := res.RowsAffected(); err != nil { + if rows, err := res.RowsAffected(); err != nil { log.Warn("get rows affected rows failed", zap.Error(err)) } else { - w.statistics.RecordRowsAffected(rowsAffected, dmls.rowTypes[i]) + affectedRows[i] = rows } cancelFunc() } - return nil + return affectedRows, nil } // multiStmtExecute runs SQLs using the multi-statements protocol with an implicit transaction. diff --git a/pkg/sink/mysql/mysql_writer_test.go b/pkg/sink/mysql/mysql_writer_test.go index 00c37f9f85..e025c86163 100644 --- a/pkg/sink/mysql/mysql_writer_test.go +++ b/pkg/sink/mysql/mysql_writer_test.go @@ -38,6 +38,7 @@ import ( "github.com/pingcap/tidb/pkg/dxf/framework/handle" timodel "github.com/pingcap/tidb/pkg/meta/model" "github.com/pingcap/tidb/pkg/sessionctx/vardef" + "github.com/prometheus/client_golang/prometheus/testutil" "github.com/stretchr/testify/require" ) @@ -197,6 +198,39 @@ func TestMysqlWriter_FlushDML_DuplicateEntryRetry(t *testing.T) { require.NoError(t, err) } +func TestAffectedRowsRecordedAfterCommit(t *testing.T) { + writer, db, mock := newTestMysqlWriter(t) + defer db.Close() + writer.cfg.CachePrepStmts = false + writer.cfg.MultiStmtEnable = false + writer.statistics.Close() + + changefeedID := common.NewChangefeedID4Test("test", t.Name()) + writer.statistics = metrics.NewStatistics(changefeedID, common.DefaultKeyspaceID, "mysqlSink") + counter := metrics.ExecDMLEventRowsAffectedCounter.WithLabelValues( + changefeedID.Keyspace(), changefeedID.Name(), "actual", common.RowTypeInsert.String()) + + dmls := &preparedDMLs{ + sqls: []string{"INSERT INTO t VALUES (?)"}, + values: [][]interface{}{{1}}, + rowTypes: []common.RowType{common.RowTypeInsert}, + rowCount: 1, + } + + mock.ExpectBegin() + mock.ExpectExec("INSERT INTO t VALUES (?)").WithArgs(1).WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectCommit().WillReturnError(errors.New("commit failed")) + require.Error(t, writer.execDMLWithMaxRetries(dmls)) + require.Zero(t, testutil.ToFloat64(counter)) + + mock.ExpectBegin() + mock.ExpectExec("INSERT INTO t VALUES (?)").WithArgs(1).WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectCommit() + require.NoError(t, writer.execDMLWithMaxRetries(dmls)) + require.Equal(t, float64(1), testutil.ToFloat64(counter)) + require.NoError(t, mock.ExpectationsWereMet()) +} + func TestMysqlWriter_FlushMultiDML(t *testing.T) { writer, db, mock := newTestMysqlWriter(t) defer db.Close() From b257afc7cf5fbe39872a4e012de70aeaed82aed5 Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Wed, 29 Jul 2026 16:10:38 +0800 Subject: [PATCH 05/17] unify the statistics --- downstreamadapter/sink/blackhole/sink.go | 2 +- downstreamadapter/sink/cloudstorage/sink.go | 2 +- .../sink/cloudstorage/writer_test.go | 8 ++--- downstreamadapter/sink/kafka/sink.go | 2 +- downstreamadapter/sink/kafka/sink_test.go | 2 +- downstreamadapter/sink/mysql/sink.go | 2 +- downstreamadapter/sink/pulsar/sink.go | 2 +- downstreamadapter/sink/pulsar/sink_test.go | 4 +-- metrics/grafana/ticdc_new_arch.json | 14 ++++---- .../ticdc_new_arch_next_gen.json | 14 ++++---- .../ticdc_new_arch_with_keyspace_name.json | 14 ++++---- pkg/metrics/sink.go | 32 +------------------ pkg/metrics/statistics.go | 27 +++------------- pkg/metrics/statistics_test.go | 1 - pkg/sink/mysql/mysql_writer_ddl_ts_test.go | 4 +-- pkg/sink/mysql/mysql_writer_test.go | 6 ++-- 16 files changed, 44 insertions(+), 92 deletions(-) diff --git a/downstreamadapter/sink/blackhole/sink.go b/downstreamadapter/sink/blackhole/sink.go index 6124219ad4..76f0d1ed44 100644 --- a/downstreamadapter/sink/blackhole/sink.go +++ b/downstreamadapter/sink/blackhole/sink.go @@ -34,7 +34,7 @@ type Sink struct { func New(changefeedID common.ChangeFeedID, keyspaceID uint32) (*Sink, error) { return &Sink{ eventCh: chann.NewUnlimitedChannelDefault[*commonEvent.DMLEvent](), - statistics: metrics.NewStatistics(changefeedID, keyspaceID, "sink"), + statistics: metrics.NewStatistics(changefeedID, keyspaceID), }, nil } diff --git a/downstreamadapter/sink/cloudstorage/sink.go b/downstreamadapter/sink/cloudstorage/sink.go index 297c5f1e06..14e450445f 100644 --- a/downstreamadapter/sink/cloudstorage/sink.go +++ b/downstreamadapter/sink/cloudstorage/sink.go @@ -135,7 +135,7 @@ func New( if err != nil { return nil, err } - statistics := metrics.NewStatistics(changefeedID, keyspaceID, "cloudstorage") + statistics := metrics.NewStatistics(changefeedID, keyspaceID) defer func() { if err != nil { statistics.Close() diff --git a/downstreamadapter/sink/cloudstorage/writer_test.go b/downstreamadapter/sink/cloudstorage/writer_test.go index 0c0f5c679b..7f950331b6 100644 --- a/downstreamadapter/sink/cloudstorage/writer_test.go +++ b/downstreamadapter/sink/cloudstorage/writer_test.go @@ -59,7 +59,7 @@ func testWriter(ctx context.Context, t *testing.T, dir string) *writer { require.NoError(t, err) changefeedID := commonType.NewChangefeedID4Test("test", t.Name()) - statistics := metrics.NewStatistics(changefeedID, commonType.DefaultKeyspaceID, t.Name()) + statistics := metrics.NewStatistics(changefeedID, commonType.DefaultKeyspaceID) spoolBuffer := newTestSpool(t, changefeedID, cfg) d := newWriter(1, changefeedID, storage, cfg, ".json", statistics, spoolBuffer) @@ -483,7 +483,7 @@ func TestWriterStoresPendingMessagesInSpoolBeforeFlush(t *testing.T) { cfg.FlushInterval = time.Hour changefeedID := commonType.NewChangefeedID4Test("test", "spool-pending") - statistics := metrics.NewStatistics(changefeedID, commonType.DefaultKeyspaceID, t.Name()) + statistics := metrics.NewStatistics(changefeedID, commonType.DefaultKeyspaceID) setPDClockForTest(t, pdutil.NewClock4Test()) spoolBuffer := newTestSpool(t, changefeedID, cfg) @@ -652,7 +652,7 @@ func TestWriterIndexWriteError(t *testing.T) { cfg.FlushInterval = time.Hour changefeedID := commonType.NewChangefeedID4Test("test", "writer-error-metric") - statistics := metrics.NewStatistics(changefeedID, commonType.DefaultKeyspaceID, t.Name()) + statistics := metrics.NewStatistics(changefeedID, commonType.DefaultKeyspaceID) setPDClockForTest(t, pdutil.NewClock4Test()) spoolBuffer := newTestSpool(t, changefeedID, cfg) d := newWriter(1, changefeedID, storage, cfg, ".json", statistics, spoolBuffer) @@ -718,7 +718,7 @@ func TestWriterDataFileCloseError(t *testing.T) { cfg.FlushInterval = time.Hour changefeedID := commonType.NewChangefeedID4Test("test", "writer-close-error") - statistics := metrics.NewStatistics(changefeedID, commonType.DefaultKeyspaceID, t.Name()) + statistics := metrics.NewStatistics(changefeedID, commonType.DefaultKeyspaceID) setPDClockForTest(t, pdutil.NewClock4Test()) spoolBuffer := newTestSpool(t, changefeedID, cfg) d := newWriter(1, changefeedID, storage, cfg, ".json", statistics, spoolBuffer) diff --git a/downstreamadapter/sink/kafka/sink.go b/downstreamadapter/sink/kafka/sink.go index c5644f0b58..0d5abee2f4 100644 --- a/downstreamadapter/sink/kafka/sink.go +++ b/downstreamadapter/sink/kafka/sink.go @@ -170,7 +170,7 @@ func newWithComponents( protocol config.Protocol, comp components, ) (*sink, error) { - statistics := metrics.NewStatistics(changefeedID, keyspaceID, "sink") + statistics := metrics.NewStatistics(changefeedID, keyspaceID) var ( err error asyncProducer kafka.AsyncProducer diff --git a/downstreamadapter/sink/kafka/sink_test.go b/downstreamadapter/sink/kafka/sink_test.go index 5dc8138c2c..01032fad90 100644 --- a/downstreamadapter/sink/kafka/sink_test.go +++ b/downstreamadapter/sink/kafka/sink_test.go @@ -336,7 +336,7 @@ func TestKafkaSinkBasicFunctionality(t *testing.T) { err = kafkaSink.WriteBlockEvent(ddlEvent) require.NoError(t, err) - writeBytes := pmetrics.TotalWriteBytesCounter.WithLabelValues("test", "test", "sink") + writeBytes := pmetrics.TotalWriteBytesCounter.WithLabelValues("test", "test") beforeWriteBytes := testutil.ToFloat64(writeBytes) kafkaSink.AddDMLEvent(dmlEvent) callbacks := make([]func(), 0, 2) diff --git a/downstreamadapter/sink/mysql/sink.go b/downstreamadapter/sink/mysql/sink.go index 2856d5fce4..93da0b9d21 100644 --- a/downstreamadapter/sink/mysql/sink.go +++ b/downstreamadapter/sink/mysql/sink.go @@ -158,7 +158,7 @@ func newMySQLSinkWithDBs( progressInterval time.Duration, keyspaceID uint32, ) *Sink { - stat := metrics.NewStatistics(changefeedID, keyspaceID, "TxnSink") + stat := metrics.NewStatistics(changefeedID, keyspaceID) var activeActiveSyncStatsCollector *mysql.ActiveActiveSyncStatsCollector if enableActiveActive && cfg.IsTiDB && cfg.ActiveActiveSyncStatsInterval > 0 { diff --git a/downstreamadapter/sink/pulsar/sink.go b/downstreamadapter/sink/pulsar/sink.go index 35b50f2087..764561967e 100644 --- a/downstreamadapter/sink/pulsar/sink.go +++ b/downstreamadapter/sink/pulsar/sink.go @@ -150,7 +150,7 @@ func newWithComponent( }() failpointCh := make(chan error, 1) - statistics = metrics.NewStatistics(changefeedID, keyspaceID, "pulsar") + statistics = metrics.NewStatistics(changefeedID, keyspaceID) dmlProducer, err = newDMLProducer(changefeedID, comp, failpointCh) if err != nil { return nil, err diff --git a/downstreamadapter/sink/pulsar/sink_test.go b/downstreamadapter/sink/pulsar/sink_test.go index 4d0e22e195..00260a6a91 100644 --- a/downstreamadapter/sink/pulsar/sink_test.go +++ b/downstreamadapter/sink/pulsar/sink_test.go @@ -49,7 +49,7 @@ func newPulsarSinkForTest(t *testing.T) (*sink, error) { comp, protocol, err := newPulsarSinkComponentForTest(ctx, changefeedID, sinkURI, replicaConfig.Sink) require.NoError(t, err) - statistics := metrics.NewStatistics(changefeedID, common.DefaultKeyspaceID, "sink") + statistics := metrics.NewStatistics(changefeedID, common.DefaultKeyspaceID) pulsarSink := &sink{ changefeedID: changefeedID, dmlProducer: newMockDMLProducer(), @@ -126,7 +126,7 @@ func TestPulsarSinkBasicFunctionality(t *testing.T) { err = pulsarSink.WriteBlockEvent(ddlEvent) require.NoError(t, err) - writeBytes := metrics.TotalWriteBytesCounter.WithLabelValues("test", "test", "sink") + writeBytes := metrics.TotalWriteBytesCounter.WithLabelValues("test", "test") beforeWriteBytes := testutil.ToFloat64(writeBytes) pulsarSink.AddDMLEvent(dmlEvent) callbacks := make([]func(), 0, 2) diff --git a/metrics/grafana/ticdc_new_arch.json b/metrics/grafana/ticdc_new_arch.json index 0a3f4bbd37..9e94ea7444 100644 --- a/metrics/grafana/ticdc_new_arch.json +++ b/metrics/grafana/ticdc_new_arch.json @@ -580,7 +580,7 @@ "targets": [ { "exemplar": true, - "expr": "sum(rate(ticdc_sink_dml_event_count{k8s_cluster=\"$k8s_cluster\", tidb_cluster=\"$tidb_cluster\",namespace=~\"$namespace\", changefeed=~\"$changefeed\", instance=~\"$ticdc_instance\"}[1m])) by (namespace,changefeed, instance)", + "expr": "sum(rate(ticdc_sink_batch_row_count_sum{k8s_cluster=\"$k8s_cluster\", tidb_cluster=\"$tidb_cluster\",namespace=~\"$namespace\", changefeed=~\"$changefeed\", instance=~\"$ticdc_instance\"}[1m])) by (namespace,changefeed, instance)", "format": "time_series", "interval": "", "intervalFactor": 1, @@ -672,19 +672,19 @@ "targets": [ { "exemplar": true, - "expr": "sum(rate(ticdc_sink_write_bytes_total{k8s_cluster=\"$k8s_cluster\", tidb_cluster=\"$tidb_cluster\", namespace=~\"$namespace\", changefeed=~\"$changefeed\",instance=~\"$ticdc_instance\"}[1m])) by (instance, type, changefeed)", + "expr": "sum(rate(ticdc_sink_write_bytes_total{k8s_cluster=\"$k8s_cluster\", tidb_cluster=\"$tidb_cluster\", namespace=~\"$namespace\", changefeed=~\"$changefeed\",instance=~\"$ticdc_instance\"}[1m])) by (namespace, changefeed, instance)", "hide": false, "interval": "", - "legendFormat": "{{instance}}-{{changefeed}}-{{type}}", + "legendFormat": "{{namespace}}-{{changefeed}}-{{instance}}", "queryType": "randomWalk", "refId": "A" }, { "exemplar": true, - "expr": "avg_over_time(sum(rate(ticdc_sink_write_bytes_total{k8s_cluster=\"$k8s_cluster\", tidb_cluster=\"$tidb_cluster\", namespace=~\"$namespace\", changefeed=~\"$changefeed\",instance=~\"$ticdc_instance\"}[1m])) by (instance, type, changefeed)[1m:])", + "expr": "avg_over_time(sum(rate(ticdc_sink_write_bytes_total{k8s_cluster=\"$k8s_cluster\", tidb_cluster=\"$tidb_cluster\", namespace=~\"$namespace\", changefeed=~\"$changefeed\",instance=~\"$ticdc_instance\"}[1m])) by (namespace, changefeed, instance)[1m:])", "hide": true, "interval": "", - "legendFormat": "{{instance}}-{{changefeed}}-AVG", + "legendFormat": "{{namespace}}-{{changefeed}}-{{instance}}-AVG", "refId": "B" } ], @@ -16929,7 +16929,7 @@ "targets": [ { "exemplar": true, - "expr": "sum(delta(ticdc_sink_execution_error{k8s_cluster=\"$k8s_cluster\",tidb_cluster=\"$tidb_cluster\",namespace=~\"$namespace\",changefeed=~\"$changefeed\",instance=~\"$ticdc_instance\"}[1m])) by (namespace,changefeed,instance,event_type)", + "expr": "sum(increase(ticdc_sink_execution_error{k8s_cluster=\"$k8s_cluster\",tidb_cluster=\"$tidb_cluster\",namespace=~\"$namespace\",changefeed=~\"$changefeed\",instance=~\"$ticdc_instance\"}[1m])) by (namespace,changefeed,instance,event_type)", "interval": "", "legendFormat": "{{namespace}}-{{changefeed}}-{{instance}}-{{event_type}}", "queryType": "randomWalk", @@ -26590,7 +26590,7 @@ "targets": [ { "exemplar": true, - "expr": "sum(delta(ticdc_ddl_execution{k8s_cluster=\"$k8s_cluster\",tidb_cluster=\"$tidb_cluster\",namespace=~\"$namespace\",changefeed=~\"$changefeed\",instance=~\"$ticdc_instance\"}[1m])) by (namespace,changefeed,ddl_type)", + "expr": "sum(increase(ticdc_ddl_execution{k8s_cluster=\"$k8s_cluster\",tidb_cluster=\"$tidb_cluster\",namespace=~\"$namespace\",changefeed=~\"$changefeed\",instance=~\"$ticdc_instance\"}[1m])) by (namespace,changefeed,ddl_type)", "interval": "", "legendFormat": "{{namespace}}-{{changefeed}}-{{ddl_type}}", "queryType": "randomWalk", diff --git a/metrics/nextgengrafana/ticdc_new_arch_next_gen.json b/metrics/nextgengrafana/ticdc_new_arch_next_gen.json index f920ac2ccd..9805d306c1 100644 --- a/metrics/nextgengrafana/ticdc_new_arch_next_gen.json +++ b/metrics/nextgengrafana/ticdc_new_arch_next_gen.json @@ -580,7 +580,7 @@ "targets": [ { "exemplar": true, - "expr": "sum(rate(ticdc_sink_dml_event_count{k8s_cluster=\"$k8s_cluster\", sharedpool_id=\"$tidb_cluster\",keyspace_name=~\"$keyspace_name\", changefeed=~\"$changefeed\", instance=~\"$ticdc_instance\"}[1m])) by (keyspace_name,changefeed, instance)", + "expr": "sum(rate(ticdc_sink_batch_row_count_sum{k8s_cluster=\"$k8s_cluster\", sharedpool_id=\"$tidb_cluster\",keyspace_name=~\"$keyspace_name\", changefeed=~\"$changefeed\", instance=~\"$ticdc_instance\"}[1m])) by (keyspace_name,changefeed, instance)", "format": "time_series", "interval": "", "intervalFactor": 1, @@ -672,19 +672,19 @@ "targets": [ { "exemplar": true, - "expr": "sum(rate(ticdc_sink_write_bytes_total{k8s_cluster=\"$k8s_cluster\", sharedpool_id=\"$tidb_cluster\", keyspace_name=~\"$keyspace_name\", changefeed=~\"$changefeed\",instance=~\"$ticdc_instance\"}[1m])) by (instance, type, changefeed)", + "expr": "sum(rate(ticdc_sink_write_bytes_total{k8s_cluster=\"$k8s_cluster\", sharedpool_id=\"$tidb_cluster\", keyspace_name=~\"$keyspace_name\", changefeed=~\"$changefeed\",instance=~\"$ticdc_instance\"}[1m])) by (keyspace_name, changefeed, instance)", "hide": false, "interval": "", - "legendFormat": "{{instance}}-{{changefeed}}-{{type}}", + "legendFormat": "{{keyspace_name}}-{{changefeed}}-{{instance}}", "queryType": "randomWalk", "refId": "A" }, { "exemplar": true, - "expr": "avg_over_time(sum(rate(ticdc_sink_write_bytes_total{k8s_cluster=\"$k8s_cluster\", sharedpool_id=\"$tidb_cluster\", keyspace_name=~\"$keyspace_name\", changefeed=~\"$changefeed\",instance=~\"$ticdc_instance\"}[1m])) by (instance, type, changefeed)[1m:])", + "expr": "avg_over_time(sum(rate(ticdc_sink_write_bytes_total{k8s_cluster=\"$k8s_cluster\", sharedpool_id=\"$tidb_cluster\", keyspace_name=~\"$keyspace_name\", changefeed=~\"$changefeed\",instance=~\"$ticdc_instance\"}[1m])) by (keyspace_name, changefeed, instance)[1m:])", "hide": true, "interval": "", - "legendFormat": "{{instance}}-{{changefeed}}-AVG", + "legendFormat": "{{keyspace_name}}-{{changefeed}}-{{instance}}-AVG", "refId": "B" } ], @@ -16929,7 +16929,7 @@ "targets": [ { "exemplar": true, - "expr": "sum(delta(ticdc_sink_execution_error{k8s_cluster=\"$k8s_cluster\",sharedpool_id=\"$tidb_cluster\",keyspace_name=~\"$keyspace_name\",changefeed=~\"$changefeed\",instance=~\"$ticdc_instance\"}[1m])) by (keyspace_name,changefeed,instance,event_type)", + "expr": "sum(increase(ticdc_sink_execution_error{k8s_cluster=\"$k8s_cluster\",sharedpool_id=\"$tidb_cluster\",keyspace_name=~\"$keyspace_name\",changefeed=~\"$changefeed\",instance=~\"$ticdc_instance\"}[1m])) by (keyspace_name,changefeed,instance,event_type)", "interval": "", "legendFormat": "{{keyspace_name}}-{{changefeed}}-{{instance}}-{{event_type}}", "queryType": "randomWalk", @@ -26590,7 +26590,7 @@ "targets": [ { "exemplar": true, - "expr": "sum(delta(ticdc_ddl_execution{k8s_cluster=\"$k8s_cluster\",sharedpool_id=\"$tidb_cluster\",keyspace_name=~\"$keyspace_name\",changefeed=~\"$changefeed\",instance=~\"$ticdc_instance\"}[1m])) by (keyspace_name,changefeed,ddl_type)", + "expr": "sum(increase(ticdc_ddl_execution{k8s_cluster=\"$k8s_cluster\",sharedpool_id=\"$tidb_cluster\",keyspace_name=~\"$keyspace_name\",changefeed=~\"$changefeed\",instance=~\"$ticdc_instance\"}[1m])) by (keyspace_name,changefeed,ddl_type)", "interval": "", "legendFormat": "{{keyspace_name}}-{{changefeed}}-{{ddl_type}}", "queryType": "randomWalk", diff --git a/metrics/nextgengrafana/ticdc_new_arch_with_keyspace_name.json b/metrics/nextgengrafana/ticdc_new_arch_with_keyspace_name.json index f71e45145a..7c24188a00 100644 --- a/metrics/nextgengrafana/ticdc_new_arch_with_keyspace_name.json +++ b/metrics/nextgengrafana/ticdc_new_arch_with_keyspace_name.json @@ -391,7 +391,7 @@ "targets": [ { "exemplar": true, - "expr": "sum(rate(ticdc_sink_dml_event_count{k8s_cluster=\"$k8s_cluster\", tidb_cluster=\"$tidb_cluster\",keyspace_name=~\"$keyspace_name\", changefeed=~\"$changefeed\", instance=~\"$ticdc_instance\"}[1m])) by (keyspace_name,changefeed, instance)", + "expr": "sum(rate(ticdc_sink_batch_row_count_sum{k8s_cluster=\"$k8s_cluster\", tidb_cluster=\"$tidb_cluster\",keyspace_name=~\"$keyspace_name\", changefeed=~\"$changefeed\", instance=~\"$ticdc_instance\"}[1m])) by (keyspace_name,changefeed, instance)", "format": "time_series", "interval": "", "intervalFactor": 1, @@ -483,19 +483,19 @@ "targets": [ { "exemplar": true, - "expr": "sum(rate(ticdc_sink_write_bytes_total{k8s_cluster=\"$k8s_cluster\", tidb_cluster=\"$tidb_cluster\", keyspace_name=~\"$keyspace_name\", changefeed=~\"$changefeed\",instance=~\"$ticdc_instance\"}[1m])) by (instance, type, changefeed)", + "expr": "sum(rate(ticdc_sink_write_bytes_total{k8s_cluster=\"$k8s_cluster\", tidb_cluster=\"$tidb_cluster\", keyspace_name=~\"$keyspace_name\", changefeed=~\"$changefeed\",instance=~\"$ticdc_instance\"}[1m])) by (keyspace_name, changefeed, instance)", "hide": false, "interval": "", - "legendFormat": "{{instance}}-{{changefeed}}-{{type}}", + "legendFormat": "{{keyspace_name}}-{{changefeed}}-{{instance}}", "queryType": "randomWalk", "refId": "A" }, { "exemplar": true, - "expr": "avg_over_time(sum(rate(ticdc_sink_write_bytes_total{k8s_cluster=\"$k8s_cluster\", tidb_cluster=\"$tidb_cluster\", keyspace_name=~\"$keyspace_name\", changefeed=~\"$changefeed\",instance=~\"$ticdc_instance\"}[1m])) by (instance, type, changefeed)[1m:])", + "expr": "avg_over_time(sum(rate(ticdc_sink_write_bytes_total{k8s_cluster=\"$k8s_cluster\", tidb_cluster=\"$tidb_cluster\", keyspace_name=~\"$keyspace_name\", changefeed=~\"$changefeed\",instance=~\"$ticdc_instance\"}[1m])) by (keyspace_name, changefeed, instance)[1m:])", "hide": true, "interval": "", - "legendFormat": "{{instance}}-{{changefeed}}-AVG", + "legendFormat": "{{keyspace_name}}-{{changefeed}}-{{instance}}-AVG", "refId": "B" } ], @@ -5454,7 +5454,7 @@ "targets": [ { "exemplar": true, - "expr": "sum(delta(ticdc_sink_execution_error{k8s_cluster=\"$k8s_cluster\",tidb_cluster=\"$tidb_cluster\",keyspace_name=~\"$keyspace_name\",changefeed=~\"$changefeed\",instance=~\"$ticdc_instance\"}[1m])) by (keyspace_name,changefeed,instance,event_type)", + "expr": "sum(increase(ticdc_sink_execution_error{k8s_cluster=\"$k8s_cluster\",tidb_cluster=\"$tidb_cluster\",keyspace_name=~\"$keyspace_name\",changefeed=~\"$changefeed\",instance=~\"$ticdc_instance\"}[1m])) by (keyspace_name,changefeed,instance,event_type)", "interval": "", "legendFormat": "{{keyspace_name}}-{{changefeed}}-{{instance}}-{{event_type}}", "queryType": "randomWalk", @@ -11303,7 +11303,7 @@ "targets": [ { "exemplar": true, - "expr": "sum(delta(ticdc_ddl_execution{k8s_cluster=\"$k8s_cluster\",tidb_cluster=\"$tidb_cluster\",keyspace_name=~\"$keyspace_name\",changefeed=~\"$changefeed\",instance=~\"$ticdc_instance\"}[1m])) by (keyspace_name,changefeed,ddl_type)", + "expr": "sum(increase(ticdc_ddl_execution{k8s_cluster=\"$k8s_cluster\",tidb_cluster=\"$tidb_cluster\",keyspace_name=~\"$keyspace_name\",changefeed=~\"$changefeed\",instance=~\"$ticdc_instance\"}[1m])) by (keyspace_name,changefeed,ddl_type)", "interval": "", "legendFormat": "{{keyspace_name}}-{{changefeed}}-{{ddl_type}}", "queryType": "randomWalk", diff --git a/pkg/metrics/sink.go b/pkg/metrics/sink.go index a45582e6c2..c8235918e8 100644 --- a/pkg/metrics/sink.go +++ b/pkg/metrics/sink.go @@ -28,17 +28,7 @@ var ( Name: "batch_row_count", Help: "Row count number for a given batch.", Buckets: prometheus.ExponentialBuckets(1, 2, 18), - }, []string{getKeyspaceLabel(), "changefeed", "type", "keyspace_id"}) // type is for `sinkType` - - // ExecBatchWriteBytesHistogram records bytes written for each batch. - ExecBatchWriteBytesHistogram = prometheus.NewHistogramVec( - prometheus.HistogramOpts{ - Namespace: "ticdc", - Subsystem: "sink", - Name: "batch_write_bytes", - Help: "Bytes number for a given batch.", - Buckets: prometheus.ExponentialBuckets(1024, 2, 18), // 1KB~128MB - }, []string{getKeyspaceLabel(), "changefeed", "type"}) // type is for `sinkType` + }, []string{getKeyspaceLabel(), "changefeed", "keyspace_id"}) // ExecWriteBytesGauge records the total number of bytes written by sink. TotalWriteBytesCounter = prometheus.NewCounterVec( @@ -47,23 +37,6 @@ var ( Subsystem: "sink", Name: "write_bytes_total", Help: "Total number of bytes written by sink", - }, []string{getKeyspaceLabel(), "changefeed", "type"}) // type is for `sinkType` - - EventSizeHistogram = prometheus.NewHistogramVec( - prometheus.HistogramOpts{ - Namespace: "ticdc", - Subsystem: "sink", - Name: "event_size", - Help: "The size of changed events (in bytes).", - Buckets: prometheus.ExponentialBuckets(0.01, 2, 30), // 0~32M - }, []string{getKeyspaceLabel(), "changefeed"}) - - ExecDMLEventCounter = prometheus.NewCounterVec( - prometheus.CounterOpts{ - Namespace: "ticdc", - Subsystem: "sink", - Name: "dml_event_count", - Help: "Total count of DML events.", }, []string{getKeyspaceLabel(), "changefeed"}) ExecDMLEventRowsAffectedCounter = prometheus.NewCounterVec( @@ -235,10 +208,7 @@ var ( func initSinkMetrics(registry *prometheus.Registry) { // common sink metrics registry.MustRegister(ExecBatchHistogram) - registry.MustRegister(ExecBatchWriteBytesHistogram) registry.MustRegister(TotalWriteBytesCounter) - registry.MustRegister(EventSizeHistogram) - registry.MustRegister(ExecDMLEventCounter) registry.MustRegister(ExecDMLEventRowsAffectedCounter) registry.MustRegister(ActiveActiveConflictSkipRowsCounter) registry.MustRegister(ExecutionErrorCounter) diff --git a/pkg/metrics/statistics.go b/pkg/metrics/statistics.go index 0648a0d075..39670ccc8d 100644 --- a/pkg/metrics/statistics.go +++ b/pkg/metrics/statistics.go @@ -24,13 +24,8 @@ import ( ) // NewStatistics creates a statistics -func NewStatistics( - changefeed common.ChangeFeedID, - keyspaceID uint32, - sinkType string, -) *Statistics { +func NewStatistics(changefeed common.ChangeFeedID, keyspaceID uint32) *Statistics { statistics := &Statistics{ - sinkType: sinkType, changefeedID: changefeed, keyspaceID: FormatKeyspaceID(keyspaceID), ddlTypes: sync.Map{}, @@ -41,12 +36,10 @@ func NewStatistics( changefeedID := changefeed.Name() statistics.metricExecDDLHis = ExecDDLHistogram.WithLabelValues(keyspace, changefeedID) statistics.metricExecDDLRunningCnt = ExecDDLRunningGauge.WithLabelValues(keyspace, changefeedID) - statistics.metricExecBatchHis = ExecBatchHistogram.WithLabelValues(keyspace, changefeedID, sinkType, statistics.keyspaceID) - statistics.metricExecBatchBytesHis = ExecBatchWriteBytesHistogram.WithLabelValues(keyspace, changefeedID, sinkType) - statistics.metricTotalWriteBytesCnt = TotalWriteBytesCounter.WithLabelValues(keyspace, changefeedID, sinkType) + statistics.metricExecBatchHis = ExecBatchHistogram.WithLabelValues(keyspace, changefeedID, statistics.keyspaceID) + statistics.metricTotalWriteBytesCnt = TotalWriteBytesCounter.WithLabelValues(keyspace, changefeedID) statistics.metricExecErrCntForDDL = ExecutionErrorCounter.WithLabelValues(keyspace, changefeedID, "ddl") statistics.metricExecErrCntForDML = ExecutionErrorCounter.WithLabelValues(keyspace, changefeedID, "dml") - statistics.metricExecDMLCnt = ExecDMLEventCounter.WithLabelValues(keyspace, changefeedID) return statistics } @@ -54,7 +47,6 @@ func NewStatistics( // Statistics maintains some status and metrics of the Sink // Note: All methods of Statistics should be thread-safe. type Statistics struct { - sinkType string changefeedID common.ChangeFeedID keyspaceID string ddlTypes sync.Map @@ -67,8 +59,6 @@ type Statistics struct { // metricExecBatchHis records the executed DML batch size. // this should be only useful for the MySQL Sink, and Kafka Sink with batched protocol, such as open-protocol. metricExecBatchHis prometheus.Observer - // metricExecBatchBytesHis records the executed batch write bytes. - metricExecBatchBytesHis prometheus.Observer // metricTotalWriteBytesCnt records the executed DML event size. metricTotalWriteBytesCnt prometheus.Counter @@ -76,8 +66,6 @@ type Statistics struct { metricExecErrCntForDDL prometheus.Counter // metricExecErrCntForDML records the error count of the Sink for DML. metricExecErrCntForDML prometheus.Counter - // metricExecDMLCnt records the executed DML event count of the Sink. - metricExecDMLCnt prometheus.Counter } // RecordBatchExecution stats batch executors which return (batchRowCount, batchWriteBytes, error). @@ -88,8 +76,6 @@ func (b *Statistics) RecordBatchExecution(executor func() (int, int64, error)) e return err } b.metricExecBatchHis.Observe(float64(batchSize)) - b.metricExecBatchBytesHis.Observe(float64(batchWriteBytes)) - b.metricExecDMLCnt.Add(float64(batchSize)) b.metricTotalWriteBytesCnt.Add(float64(batchWriteBytes)) return nil } @@ -146,9 +132,7 @@ func (b *Statistics) Close() { changefeedID := b.changefeedID.Name() ExecDDLHistogram.DeleteLabelValues(keyspace, changefeedID) ExecDDLRunningGauge.DeleteLabelValues(keyspace, changefeedID) - ExecBatchHistogram.DeleteLabelValues(keyspace, changefeedID, b.sinkType, b.keyspaceID) - ExecBatchWriteBytesHistogram.DeleteLabelValues(keyspace, changefeedID, b.sinkType) - EventSizeHistogram.DeleteLabelValues(keyspace, changefeedID) + ExecBatchHistogram.DeleteLabelValues(keyspace, changefeedID, b.keyspaceID) ExecutionErrorCounter.DeleteLabelValues(keyspace, changefeedID, "ddl") ExecutionErrorCounter.DeleteLabelValues(keyspace, changefeedID, "dml") b.ddlTypes.Range(func(key, value any) bool { @@ -163,6 +147,5 @@ func (b *Statistics) Close() { ExecDMLEventRowsAffectedCounter.DeleteLabelValues(keyspace, changefeedID, countType, rowType) return true }) - TotalWriteBytesCounter.DeleteLabelValues(keyspace, changefeedID, b.sinkType) - ExecDMLEventCounter.DeleteLabelValues(keyspace, changefeedID) + TotalWriteBytesCounter.DeleteLabelValues(keyspace, changefeedID) } diff --git a/pkg/metrics/statistics_test.go b/pkg/metrics/statistics_test.go index 3afbcb2c6d..bf6e852eca 100644 --- a/pkg/metrics/statistics_test.go +++ b/pkg/metrics/statistics_test.go @@ -28,7 +28,6 @@ func TestExecBatchHistogramKeyspaceIDLabel(t *testing.T) { statistics := NewStatistics( common.NewChangefeedID4Test("test-keyspace", "batch-row-count-keyspace-id"), 123, - "sink", ) require.NoError(t, statistics.RecordBatchExecution(func() (int, int64, error) { return 2, 10, nil diff --git a/pkg/sink/mysql/mysql_writer_ddl_ts_test.go b/pkg/sink/mysql/mysql_writer_ddl_ts_test.go index f5ec56f7fe..b148ae2261 100644 --- a/pkg/sink/mysql/mysql_writer_ddl_ts_test.go +++ b/pkg/sink/mysql/mysql_writer_ddl_ts_test.go @@ -41,7 +41,7 @@ func newTestMysqlWriterForDDLTs(t *testing.T) (*Writer, *sql.DB, sqlmock.Sqlmock cfg.EnableDDLTs = true cfg.IsTiDB = false // Default to non-TiDB changefeedID := common.NewChangefeedID4Test("test", "test") - statistics := metrics.NewStatistics(changefeedID, common.DefaultKeyspaceID, "mysqlSink") + statistics := metrics.NewStatistics(changefeedID, common.DefaultKeyspaceID) writer := NewWriter(ctx, 0, db, cfg, changefeedID, statistics, nil) t.Cleanup(writer.Close) @@ -63,7 +63,7 @@ func newTestMysqlWriterForDDLTsTiDB(t *testing.T) (*Writer, *sql.DB, sqlmock.Sql cfg.EnableDDLTs = true cfg.IsTiDB = true // TiDB downstream changefeedID := common.NewChangefeedID4Test("test", "test") - statistics := metrics.NewStatistics(changefeedID, common.DefaultKeyspaceID, "mysqlSink") + statistics := metrics.NewStatistics(changefeedID, common.DefaultKeyspaceID) writer := NewWriter(ctx, 0, db, cfg, changefeedID, statistics, nil) t.Cleanup(writer.Close) diff --git a/pkg/sink/mysql/mysql_writer_test.go b/pkg/sink/mysql/mysql_writer_test.go index e025c86163..aa1aa93ccb 100644 --- a/pkg/sink/mysql/mysql_writer_test.go +++ b/pkg/sink/mysql/mysql_writer_test.go @@ -53,7 +53,7 @@ func newTestMysqlWriter(t *testing.T) (*Writer, *sql.DB, sqlmock.Sqlmock) { cfg.BatchDMLEnable = true cfg.EnableDDLTs = defaultEnableDDLTs changefeedID := common.NewChangefeedID4Test("test", "test") - statistics := metrics.NewStatistics(changefeedID, common.DefaultKeyspaceID, "mysqlSink") + statistics := metrics.NewStatistics(changefeedID, common.DefaultKeyspaceID) writer := NewWriter(ctx, 0, db, cfg, changefeedID, statistics, nil) t.Cleanup(writer.Close) // assign a no-op stmt cache to bypass actual DB operations in unit tests @@ -76,7 +76,7 @@ func newTestMysqlWriterForTiDB(t *testing.T) (*Writer, *sql.DB, sqlmock.Sqlmock) cfg.ServerInfo = version.ParseServerInfo(defaultRunningAddIndexNewSQLVersion) changefeedID := common.NewChangefeedID4Test("test", "test") - statistics := metrics.NewStatistics(changefeedID, common.DefaultKeyspaceID, "mysqlSink") + statistics := metrics.NewStatistics(changefeedID, common.DefaultKeyspaceID) writer := NewWriter(ctx, 0, db, cfg, changefeedID, statistics, nil) t.Cleanup(writer.Close) @@ -206,7 +206,7 @@ func TestAffectedRowsRecordedAfterCommit(t *testing.T) { writer.statistics.Close() changefeedID := common.NewChangefeedID4Test("test", t.Name()) - writer.statistics = metrics.NewStatistics(changefeedID, common.DefaultKeyspaceID, "mysqlSink") + writer.statistics = metrics.NewStatistics(changefeedID, common.DefaultKeyspaceID) counter := metrics.ExecDMLEventRowsAffectedCounter.WithLabelValues( changefeedID.Keyspace(), changefeedID.Name(), "actual", common.RowTypeInsert.String()) From c33c4f90d74b675e76ac1e71b61b7682999b8082 Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Wed, 29 Jul 2026 17:23:11 +0800 Subject: [PATCH 06/17] move statistics to independent pkg --- downstreamadapter/sink/blackhole/sink.go | 6 +- .../sink/cloudstorage/dml_writers.go | 6 +- downstreamadapter/sink/cloudstorage/sink.go | 5 +- downstreamadapter/sink/cloudstorage/writer.go | 6 +- .../sink/cloudstorage/writer_test.go | 10 +-- downstreamadapter/sink/kafka/sink.go | 5 +- downstreamadapter/sink/mysql/sink.go | 5 +- downstreamadapter/sink/pulsar/sink.go | 13 ++-- downstreamadapter/sink/pulsar/sink_test.go | 3 +- pkg/metrics/statistics_test.go | 41 ----------- pkg/sink/mysql/mysql_writer.go | 6 +- pkg/sink/mysql/mysql_writer_ddl_ts_test.go | 6 +- pkg/sink/mysql/mysql_writer_test.go | 7 +- pkg/{metrics => statistics}/statistics.go | 41 +++++------ pkg/statistics/statistics_test.go | 70 +++++++++++++++++++ 15 files changed, 133 insertions(+), 97 deletions(-) delete mode 100644 pkg/metrics/statistics_test.go rename pkg/{metrics => statistics}/statistics.go (71%) create mode 100644 pkg/statistics/statistics_test.go diff --git a/downstreamadapter/sink/blackhole/sink.go b/downstreamadapter/sink/blackhole/sink.go index 76f0d1ed44..2f05104f0e 100644 --- a/downstreamadapter/sink/blackhole/sink.go +++ b/downstreamadapter/sink/blackhole/sink.go @@ -19,7 +19,7 @@ import ( "github.com/pingcap/log" "github.com/pingcap/ticdc/pkg/common" commonEvent "github.com/pingcap/ticdc/pkg/common/event" - "github.com/pingcap/ticdc/pkg/metrics" + "github.com/pingcap/ticdc/pkg/statistics" "github.com/pingcap/ticdc/utils/chann" "go.uber.org/zap" ) @@ -28,13 +28,13 @@ import ( // Including DDL and DML. type Sink struct { eventCh *chann.UnlimitedChannel[*commonEvent.DMLEvent, any] - statistics *metrics.Statistics + statistics *statistics.Statistics } func New(changefeedID common.ChangeFeedID, keyspaceID uint32) (*Sink, error) { return &Sink{ eventCh: chann.NewUnlimitedChannelDefault[*commonEvent.DMLEvent](), - statistics: metrics.NewStatistics(changefeedID, keyspaceID), + statistics: statistics.New(changefeedID, keyspaceID), }, nil } diff --git a/downstreamadapter/sink/cloudstorage/dml_writers.go b/downstreamadapter/sink/cloudstorage/dml_writers.go index 6f4d8e9eb2..d4acb7da48 100644 --- a/downstreamadapter/sink/cloudstorage/dml_writers.go +++ b/downstreamadapter/sink/cloudstorage/dml_writers.go @@ -23,8 +23,8 @@ import ( "github.com/pingcap/ticdc/pkg/cloudstorage" commonType "github.com/pingcap/ticdc/pkg/common" commonEvent "github.com/pingcap/ticdc/pkg/common/event" - "github.com/pingcap/ticdc/pkg/metrics" "github.com/pingcap/ticdc/pkg/sink/codec/common" + "github.com/pingcap/ticdc/pkg/statistics" "github.com/pingcap/ticdc/utils/chann" "github.com/pingcap/tidb/pkg/objstore/storeapi" "go.uber.org/atomic" @@ -34,7 +34,7 @@ import ( // dmlWriters coordinates encoding and output shard writers. type dmlWriters struct { changefeedID commonType.ChangeFeedID - statistics *metrics.Statistics + statistics *statistics.Statistics // msgCh is the only unbounded queue in the storage sink pipeline. // External callers push tasks into it, addTasks consumes it, and @@ -55,7 +55,7 @@ func newDMLWriters( config *cloudstorage.Config, encoderConfig *common.Config, extension string, - statistics *metrics.Statistics, + statistics *statistics.Statistics, columnSelector *columnselector.ColumnSelectors, ) (*dmlWriters, error) { messageCh := chann.NewUnlimitedChannelDefault[*task]() diff --git a/downstreamadapter/sink/cloudstorage/sink.go b/downstreamadapter/sink/cloudstorage/sink.go index 14e450445f..d67af4e21e 100644 --- a/downstreamadapter/sink/cloudstorage/sink.go +++ b/downstreamadapter/sink/cloudstorage/sink.go @@ -29,6 +29,7 @@ import ( "github.com/pingcap/ticdc/pkg/config" "github.com/pingcap/ticdc/pkg/errors" "github.com/pingcap/ticdc/pkg/metrics" + "github.com/pingcap/ticdc/pkg/statistics" "github.com/pingcap/ticdc/pkg/util" "github.com/pingcap/tidb/pkg/meta/model" "github.com/pingcap/tidb/pkg/objstore/storeapi" @@ -65,7 +66,7 @@ type sink struct { lastSendCheckpointTsTime time.Time cron *cron.Cron - statistics *metrics.Statistics + statistics *statistics.Statistics isNormal *atomic.Bool cleanupJobs []func() /* only for test */ @@ -135,7 +136,7 @@ func New( if err != nil { return nil, err } - statistics := metrics.NewStatistics(changefeedID, keyspaceID) + statistics := statistics.New(changefeedID, keyspaceID) defer func() { if err != nil { statistics.Close() diff --git a/downstreamadapter/sink/cloudstorage/writer.go b/downstreamadapter/sink/cloudstorage/writer.go index ddd945a858..4a4cdf19a9 100644 --- a/downstreamadapter/sink/cloudstorage/writer.go +++ b/downstreamadapter/sink/cloudstorage/writer.go @@ -25,7 +25,7 @@ import ( "github.com/pingcap/ticdc/pkg/cloudstorage" "github.com/pingcap/ticdc/pkg/common" "github.com/pingcap/ticdc/pkg/errors" - pmetrics "github.com/pingcap/ticdc/pkg/metrics" + "github.com/pingcap/ticdc/pkg/statistics" "github.com/pingcap/tidb/pkg/objstore/storeapi" "github.com/prometheus/client_golang/prometheus" "go.uber.org/zap" @@ -45,7 +45,7 @@ type writer struct { // the channel does not need to be closed explicitly. flushCh chan flushTask - statistics *pmetrics.Statistics + statistics *statistics.Statistics filePathGenerator *cloudstorage.FilePathGenerator metricFlushBytes prometheus.Observer @@ -75,7 +75,7 @@ func newWriter( storage storeapi.Storage, config *cloudstorage.Config, extension string, - statistics *pmetrics.Statistics, + statistics *statistics.Statistics, spoolBuffer *spool.Spool, ) *writer { var ( diff --git a/downstreamadapter/sink/cloudstorage/writer_test.go b/downstreamadapter/sink/cloudstorage/writer_test.go index 7f950331b6..4d3e259c45 100644 --- a/downstreamadapter/sink/cloudstorage/writer_test.go +++ b/downstreamadapter/sink/cloudstorage/writer_test.go @@ -31,9 +31,9 @@ import ( commonType "github.com/pingcap/ticdc/pkg/common" commonEvent "github.com/pingcap/ticdc/pkg/common/event" "github.com/pingcap/ticdc/pkg/config" - "github.com/pingcap/ticdc/pkg/metrics" "github.com/pingcap/ticdc/pkg/pdutil" "github.com/pingcap/ticdc/pkg/sink/codec/common" + "github.com/pingcap/ticdc/pkg/statistics" "github.com/pingcap/ticdc/pkg/util" "github.com/pingcap/tidb/pkg/meta/model" "github.com/pingcap/tidb/pkg/objstore/objectio" @@ -59,7 +59,7 @@ func testWriter(ctx context.Context, t *testing.T, dir string) *writer { require.NoError(t, err) changefeedID := commonType.NewChangefeedID4Test("test", t.Name()) - statistics := metrics.NewStatistics(changefeedID, commonType.DefaultKeyspaceID) + statistics := statistics.New(changefeedID, commonType.DefaultKeyspaceID) spoolBuffer := newTestSpool(t, changefeedID, cfg) d := newWriter(1, changefeedID, storage, cfg, ".json", statistics, spoolBuffer) @@ -483,7 +483,7 @@ func TestWriterStoresPendingMessagesInSpoolBeforeFlush(t *testing.T) { cfg.FlushInterval = time.Hour changefeedID := commonType.NewChangefeedID4Test("test", "spool-pending") - statistics := metrics.NewStatistics(changefeedID, commonType.DefaultKeyspaceID) + statistics := statistics.New(changefeedID, commonType.DefaultKeyspaceID) setPDClockForTest(t, pdutil.NewClock4Test()) spoolBuffer := newTestSpool(t, changefeedID, cfg) @@ -652,7 +652,7 @@ func TestWriterIndexWriteError(t *testing.T) { cfg.FlushInterval = time.Hour changefeedID := commonType.NewChangefeedID4Test("test", "writer-error-metric") - statistics := metrics.NewStatistics(changefeedID, commonType.DefaultKeyspaceID) + statistics := statistics.New(changefeedID, commonType.DefaultKeyspaceID) setPDClockForTest(t, pdutil.NewClock4Test()) spoolBuffer := newTestSpool(t, changefeedID, cfg) d := newWriter(1, changefeedID, storage, cfg, ".json", statistics, spoolBuffer) @@ -718,7 +718,7 @@ func TestWriterDataFileCloseError(t *testing.T) { cfg.FlushInterval = time.Hour changefeedID := commonType.NewChangefeedID4Test("test", "writer-close-error") - statistics := metrics.NewStatistics(changefeedID, commonType.DefaultKeyspaceID) + statistics := statistics.New(changefeedID, commonType.DefaultKeyspaceID) setPDClockForTest(t, pdutil.NewClock4Test()) spoolBuffer := newTestSpool(t, changefeedID, cfg) d := newWriter(1, changefeedID, storage, cfg, ".json", statistics, spoolBuffer) diff --git a/downstreamadapter/sink/kafka/sink.go b/downstreamadapter/sink/kafka/sink.go index 0d5abee2f4..0807740d5e 100644 --- a/downstreamadapter/sink/kafka/sink.go +++ b/downstreamadapter/sink/kafka/sink.go @@ -31,6 +31,7 @@ import ( 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/statistics" "github.com/pingcap/ticdc/pkg/util" "github.com/pingcap/ticdc/utils/chann" "go.uber.org/atomic" @@ -51,7 +52,7 @@ type sink struct { metricsCollector kafka.MetricsCollector comp components - statistics *metrics.Statistics + statistics *statistics.Statistics protocol config.Protocol partitionRule helper.DDLDispatchRule @@ -170,7 +171,7 @@ func newWithComponents( protocol config.Protocol, comp components, ) (*sink, error) { - statistics := metrics.NewStatistics(changefeedID, keyspaceID) + statistics := statistics.New(changefeedID, keyspaceID) var ( err error asyncProducer kafka.AsyncProducer diff --git a/downstreamadapter/sink/mysql/sink.go b/downstreamadapter/sink/mysql/sink.go index 93da0b9d21..442cdf7091 100644 --- a/downstreamadapter/sink/mysql/sink.go +++ b/downstreamadapter/sink/mysql/sink.go @@ -28,6 +28,7 @@ import ( "github.com/pingcap/ticdc/pkg/errors" "github.com/pingcap/ticdc/pkg/metrics" "github.com/pingcap/ticdc/pkg/sink/mysql" + "github.com/pingcap/ticdc/pkg/statistics" "github.com/pingcap/tidb/pkg/parser/ast" "go.uber.org/atomic" "go.uber.org/zap" @@ -54,7 +55,7 @@ type Sink struct { // Compatibility callers built through NewMySQLSink use one shared pool. dmlDB *sql.DB controlDB *sql.DB - statistics *metrics.Statistics + statistics *statistics.Statistics conflictDetector *causality.ConflictDetector @@ -158,7 +159,7 @@ func newMySQLSinkWithDBs( progressInterval time.Duration, keyspaceID uint32, ) *Sink { - stat := metrics.NewStatistics(changefeedID, keyspaceID) + stat := statistics.New(changefeedID, keyspaceID) var activeActiveSyncStatsCollector *mysql.ActiveActiveSyncStatsCollector if enableActiveActive && cfg.IsTiDB && cfg.ActiveActiveSyncStatsInterval > 0 { diff --git a/downstreamadapter/sink/pulsar/sink.go b/downstreamadapter/sink/pulsar/sink.go index 764561967e..5c53add02a 100644 --- a/downstreamadapter/sink/pulsar/sink.go +++ b/downstreamadapter/sink/pulsar/sink.go @@ -26,6 +26,7 @@ import ( "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/statistics" "github.com/pingcap/ticdc/utils/chann" "go.uber.org/atomic" "go.uber.org/zap" @@ -51,7 +52,7 @@ type sink struct { ddlProducer ddlProducer comp component - statistics *metrics.Statistics + statistics *statistics.Statistics protocol config.Protocol partitionRule helper.DDLDispatchRule @@ -132,7 +133,7 @@ func newWithComponent( var ( dmlProducer dmlProducer ddlProducer ddlProducer - statistics *metrics.Statistics + stat *statistics.Statistics ) defer func() { if err != nil { @@ -142,15 +143,15 @@ func newWithComponent( if dmlProducer != nil { dmlProducer.close() } - if statistics != nil { - statistics.Close() + if stat != nil { + stat.Close() } comp.close() } }() failpointCh := make(chan error, 1) - statistics = metrics.NewStatistics(changefeedID, keyspaceID) + stat = statistics.New(changefeedID, keyspaceID) dmlProducer, err = newDMLProducer(changefeedID, comp, failpointCh) if err != nil { return nil, err @@ -173,7 +174,7 @@ func newWithComponent( protocol: protocol, partitionRule: helper.GetDDLDispatchRule(protocol), comp: comp, - statistics: statistics, + statistics: stat, isNormal: atomic.NewBool(true), ctx: ctx, }, nil diff --git a/downstreamadapter/sink/pulsar/sink_test.go b/downstreamadapter/sink/pulsar/sink_test.go index 00260a6a91..e472e61520 100644 --- a/downstreamadapter/sink/pulsar/sink_test.go +++ b/downstreamadapter/sink/pulsar/sink_test.go @@ -26,6 +26,7 @@ import ( "github.com/pingcap/ticdc/pkg/config" cerror "github.com/pingcap/ticdc/pkg/errors" "github.com/pingcap/ticdc/pkg/metrics" + "github.com/pingcap/ticdc/pkg/statistics" "github.com/pingcap/ticdc/utils/chann" "github.com/prometheus/client_golang/prometheus/testutil" "github.com/stretchr/testify/require" @@ -49,7 +50,7 @@ func newPulsarSinkForTest(t *testing.T) (*sink, error) { comp, protocol, err := newPulsarSinkComponentForTest(ctx, changefeedID, sinkURI, replicaConfig.Sink) require.NoError(t, err) - statistics := metrics.NewStatistics(changefeedID, common.DefaultKeyspaceID) + statistics := statistics.New(changefeedID, common.DefaultKeyspaceID) pulsarSink := &sink{ changefeedID: changefeedID, dmlProducer: newMockDMLProducer(), diff --git a/pkg/metrics/statistics_test.go b/pkg/metrics/statistics_test.go deleted file mode 100644 index bf6e852eca..0000000000 --- a/pkg/metrics/statistics_test.go +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright 2026 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 metrics - -import ( - "testing" - - "github.com/pingcap/ticdc/pkg/common" - "github.com/prometheus/client_golang/prometheus/testutil" - "github.com/stretchr/testify/require" -) - -func TestExecBatchHistogramKeyspaceIDLabel(t *testing.T) { - ExecBatchHistogram.Reset() - t.Cleanup(ExecBatchHistogram.Reset) - - statistics := NewStatistics( - common.NewChangefeedID4Test("test-keyspace", "batch-row-count-keyspace-id"), - 123, - ) - require.NoError(t, statistics.RecordBatchExecution(func() (int, int64, error) { - return 2, 10, nil - })) - - require.Equal(t, 1, testutil.CollectAndCount(ExecBatchHistogram)) - requireMetricHasLabel(t, ExecBatchHistogram, "keyspace_id", "123") - - statistics.Close() - require.Equal(t, 0, testutil.CollectAndCount(ExecBatchHistogram)) -} diff --git a/pkg/sink/mysql/mysql_writer.go b/pkg/sink/mysql/mysql_writer.go index 4c38fadaab..f2d9078a05 100644 --- a/pkg/sink/mysql/mysql_writer.go +++ b/pkg/sink/mysql/mysql_writer.go @@ -25,7 +25,7 @@ import ( "github.com/pingcap/ticdc/pkg/common" commonEvent "github.com/pingcap/ticdc/pkg/common/event" "github.com/pingcap/ticdc/pkg/errors" - "github.com/pingcap/ticdc/pkg/metrics" + "github.com/pingcap/ticdc/pkg/statistics" "go.uber.org/zap" ) @@ -64,7 +64,7 @@ type Writer struct { // implement stmtCache to improve performance, especially when the downstream is TiDB stmtCache *lru.Cache - statistics *metrics.Statistics + statistics *statistics.Statistics // activeActiveSyncStatsCollector accumulates conflict statistics from TiDB session // variable @@tidb_cdc_active_active_sync_stats. It is shared across all DML writers @@ -93,7 +93,7 @@ func NewWriter( db *sql.DB, cfg *Config, changefeedID common.ChangeFeedID, - statistics *metrics.Statistics, + statistics *statistics.Statistics, activeActiveSyncStatsCollector *ActiveActiveSyncStatsCollector, ) *Writer { writerCtx, cancel := context.WithCancel(ctx) diff --git a/pkg/sink/mysql/mysql_writer_ddl_ts_test.go b/pkg/sink/mysql/mysql_writer_ddl_ts_test.go index b148ae2261..2f63e3bbcc 100644 --- a/pkg/sink/mysql/mysql_writer_ddl_ts_test.go +++ b/pkg/sink/mysql/mysql_writer_ddl_ts_test.go @@ -25,7 +25,7 @@ import ( "github.com/pingcap/ticdc/heartbeatpb" "github.com/pingcap/ticdc/pkg/common" commonEvent "github.com/pingcap/ticdc/pkg/common/event" - "github.com/pingcap/ticdc/pkg/metrics" + "github.com/pingcap/ticdc/pkg/statistics" "github.com/stretchr/testify/require" ) @@ -41,7 +41,7 @@ func newTestMysqlWriterForDDLTs(t *testing.T) (*Writer, *sql.DB, sqlmock.Sqlmock cfg.EnableDDLTs = true cfg.IsTiDB = false // Default to non-TiDB changefeedID := common.NewChangefeedID4Test("test", "test") - statistics := metrics.NewStatistics(changefeedID, common.DefaultKeyspaceID) + statistics := statistics.New(changefeedID, common.DefaultKeyspaceID) writer := NewWriter(ctx, 0, db, cfg, changefeedID, statistics, nil) t.Cleanup(writer.Close) @@ -63,7 +63,7 @@ func newTestMysqlWriterForDDLTsTiDB(t *testing.T) (*Writer, *sql.DB, sqlmock.Sql cfg.EnableDDLTs = true cfg.IsTiDB = true // TiDB downstream changefeedID := common.NewChangefeedID4Test("test", "test") - statistics := metrics.NewStatistics(changefeedID, common.DefaultKeyspaceID) + statistics := statistics.New(changefeedID, common.DefaultKeyspaceID) writer := NewWriter(ctx, 0, db, cfg, changefeedID, statistics, nil) t.Cleanup(writer.Close) diff --git a/pkg/sink/mysql/mysql_writer_test.go b/pkg/sink/mysql/mysql_writer_test.go index aa1aa93ccb..291455356d 100644 --- a/pkg/sink/mysql/mysql_writer_test.go +++ b/pkg/sink/mysql/mysql_writer_test.go @@ -33,6 +33,7 @@ import ( cerror "github.com/pingcap/ticdc/pkg/errors" "github.com/pingcap/ticdc/pkg/metrics" "github.com/pingcap/ticdc/pkg/routing" + "github.com/pingcap/ticdc/pkg/statistics" "github.com/pingcap/tidb/br/pkg/version" ticonfig "github.com/pingcap/tidb/pkg/config" "github.com/pingcap/tidb/pkg/dxf/framework/handle" @@ -53,7 +54,7 @@ func newTestMysqlWriter(t *testing.T) (*Writer, *sql.DB, sqlmock.Sqlmock) { cfg.BatchDMLEnable = true cfg.EnableDDLTs = defaultEnableDDLTs changefeedID := common.NewChangefeedID4Test("test", "test") - statistics := metrics.NewStatistics(changefeedID, common.DefaultKeyspaceID) + statistics := statistics.New(changefeedID, common.DefaultKeyspaceID) writer := NewWriter(ctx, 0, db, cfg, changefeedID, statistics, nil) t.Cleanup(writer.Close) // assign a no-op stmt cache to bypass actual DB operations in unit tests @@ -76,7 +77,7 @@ func newTestMysqlWriterForTiDB(t *testing.T) (*Writer, *sql.DB, sqlmock.Sqlmock) cfg.ServerInfo = version.ParseServerInfo(defaultRunningAddIndexNewSQLVersion) changefeedID := common.NewChangefeedID4Test("test", "test") - statistics := metrics.NewStatistics(changefeedID, common.DefaultKeyspaceID) + statistics := statistics.New(changefeedID, common.DefaultKeyspaceID) writer := NewWriter(ctx, 0, db, cfg, changefeedID, statistics, nil) t.Cleanup(writer.Close) @@ -206,7 +207,7 @@ func TestAffectedRowsRecordedAfterCommit(t *testing.T) { writer.statistics.Close() changefeedID := common.NewChangefeedID4Test("test", t.Name()) - writer.statistics = metrics.NewStatistics(changefeedID, common.DefaultKeyspaceID) + writer.statistics = statistics.New(changefeedID, common.DefaultKeyspaceID) counter := metrics.ExecDMLEventRowsAffectedCounter.WithLabelValues( changefeedID.Keyspace(), changefeedID.Name(), "actual", common.RowTypeInsert.String()) diff --git a/pkg/metrics/statistics.go b/pkg/statistics/statistics.go similarity index 71% rename from pkg/metrics/statistics.go rename to pkg/statistics/statistics.go index 39670ccc8d..2d63067fc1 100644 --- a/pkg/metrics/statistics.go +++ b/pkg/statistics/statistics.go @@ -11,7 +11,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package metrics +package statistics import ( "fmt" @@ -20,26 +20,27 @@ import ( "time" "github.com/pingcap/ticdc/pkg/common" + "github.com/pingcap/ticdc/pkg/metrics" "github.com/prometheus/client_golang/prometheus" ) -// NewStatistics creates a statistics -func NewStatistics(changefeed common.ChangeFeedID, keyspaceID uint32) *Statistics { +// New creates a Statistics. +func New(changefeed common.ChangeFeedID, keyspaceID uint32) *Statistics { statistics := &Statistics{ changefeedID: changefeed, - keyspaceID: FormatKeyspaceID(keyspaceID), + keyspaceID: metrics.FormatKeyspaceID(keyspaceID), ddlTypes: sync.Map{}, rowsAffectedMap: sync.Map{}, } keyspace := changefeed.Keyspace() changefeedID := changefeed.Name() - statistics.metricExecDDLHis = ExecDDLHistogram.WithLabelValues(keyspace, changefeedID) - statistics.metricExecDDLRunningCnt = ExecDDLRunningGauge.WithLabelValues(keyspace, changefeedID) - statistics.metricExecBatchHis = ExecBatchHistogram.WithLabelValues(keyspace, changefeedID, statistics.keyspaceID) - statistics.metricTotalWriteBytesCnt = TotalWriteBytesCounter.WithLabelValues(keyspace, changefeedID) - statistics.metricExecErrCntForDDL = ExecutionErrorCounter.WithLabelValues(keyspace, changefeedID, "ddl") - statistics.metricExecErrCntForDML = ExecutionErrorCounter.WithLabelValues(keyspace, changefeedID, "dml") + statistics.metricExecDDLHis = metrics.ExecDDLHistogram.WithLabelValues(keyspace, changefeedID) + statistics.metricExecDDLRunningCnt = metrics.ExecDDLRunningGauge.WithLabelValues(keyspace, changefeedID) + statistics.metricExecBatchHis = metrics.ExecBatchHistogram.WithLabelValues(keyspace, changefeedID, statistics.keyspaceID) + statistics.metricTotalWriteBytesCnt = metrics.TotalWriteBytesCounter.WithLabelValues(keyspace, changefeedID) + statistics.metricExecErrCntForDDL = metrics.ExecutionErrorCounter.WithLabelValues(keyspace, changefeedID, "ddl") + statistics.metricExecErrCntForDML = metrics.ExecutionErrorCounter.WithLabelValues(keyspace, changefeedID, "dml") return statistics } @@ -94,7 +95,7 @@ func (b *Statistics) RecordDDLExecution(executor func() (string, error)) error { b.metricExecErrCntForDDL.Inc() return err } - metricExecDDLCounter := ExecDDLCounter.WithLabelValues( + metricExecDDLCounter := metrics.ExecDDLCounter.WithLabelValues( b.changefeedID.Keyspace(), b.changefeedID.Name(), ddlType) metricExecDDLCounter.Inc() b.ddlTypes.Store(ddlType, struct{}{}) @@ -119,7 +120,7 @@ func (b *Statistics) getRowsAffected(countType, rowType string) prometheus.Count if !loaded { keyspace := b.changefeedID.Keyspace() changefeedID := b.changefeedID.Name() - counter := ExecDMLEventRowsAffectedCounter.WithLabelValues(keyspace, changefeedID, countType, rowType) + counter := metrics.ExecDMLEventRowsAffectedCounter.WithLabelValues(keyspace, changefeedID, countType, rowType) b.rowsAffectedMap.Store(key, counter) return counter } @@ -130,22 +131,22 @@ func (b *Statistics) getRowsAffected(countType, rowType string) prometheus.Count func (b *Statistics) Close() { keyspace := b.changefeedID.Keyspace() changefeedID := b.changefeedID.Name() - ExecDDLHistogram.DeleteLabelValues(keyspace, changefeedID) - ExecDDLRunningGauge.DeleteLabelValues(keyspace, changefeedID) - ExecBatchHistogram.DeleteLabelValues(keyspace, changefeedID, b.keyspaceID) - ExecutionErrorCounter.DeleteLabelValues(keyspace, changefeedID, "ddl") - ExecutionErrorCounter.DeleteLabelValues(keyspace, changefeedID, "dml") + metrics.ExecDDLHistogram.DeleteLabelValues(keyspace, changefeedID) + metrics.ExecDDLRunningGauge.DeleteLabelValues(keyspace, changefeedID) + metrics.ExecBatchHistogram.DeleteLabelValues(keyspace, changefeedID, b.keyspaceID) + metrics.ExecutionErrorCounter.DeleteLabelValues(keyspace, changefeedID, "ddl") + metrics.ExecutionErrorCounter.DeleteLabelValues(keyspace, changefeedID, "dml") b.ddlTypes.Range(func(key, value any) bool { ddlType := key.(string) - ExecDDLCounter.DeleteLabelValues(keyspace, changefeedID, ddlType) + metrics.ExecDDLCounter.DeleteLabelValues(keyspace, changefeedID, ddlType) return true }) b.rowsAffectedMap.Range(func(key, value any) bool { countTypeAndRowType := key.(string) splitTypes := strings.Split(countTypeAndRowType, "-") countType, rowType := splitTypes[0], splitTypes[1] - ExecDMLEventRowsAffectedCounter.DeleteLabelValues(keyspace, changefeedID, countType, rowType) + metrics.ExecDMLEventRowsAffectedCounter.DeleteLabelValues(keyspace, changefeedID, countType, rowType) return true }) - TotalWriteBytesCounter.DeleteLabelValues(keyspace, changefeedID) + metrics.TotalWriteBytesCounter.DeleteLabelValues(keyspace, changefeedID) } diff --git a/pkg/statistics/statistics_test.go b/pkg/statistics/statistics_test.go new file mode 100644 index 0000000000..a411a8a5c1 --- /dev/null +++ b/pkg/statistics/statistics_test.go @@ -0,0 +1,70 @@ +// Copyright 2026 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, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package statistics + +import ( + "testing" + + "github.com/pingcap/ticdc/pkg/common" + "github.com/pingcap/ticdc/pkg/metrics" + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/testutil" + "github.com/stretchr/testify/require" +) + +func TestExecBatchHistogramKeyspaceIDLabel(t *testing.T) { + metrics.ExecBatchHistogram.Reset() + t.Cleanup(metrics.ExecBatchHistogram.Reset) + + statistics := New( + common.NewChangefeedID4Test("test-keyspace", "batch-row-count-keyspace-id"), + 123, + ) + require.NoError(t, statistics.RecordBatchExecution(func() (int, int64, error) { + return 2, 10, nil + })) + + require.Equal(t, 1, testutil.CollectAndCount(metrics.ExecBatchHistogram)) + requireMetricHasLabel(t, metrics.ExecBatchHistogram, "keyspace_id", "123") + + statistics.Close() + require.Equal(t, 0, testutil.CollectAndCount(metrics.ExecBatchHistogram)) +} + +func requireMetricHasLabel( + t *testing.T, + collector prometheus.Collector, + labelName string, + labelValue string, +) { + t.Helper() + + registry := prometheus.NewPedanticRegistry() + registry.MustRegister(collector) + metricFamilies, err := registry.Gather() + require.NoError(t, err) + require.NotEmpty(t, metricFamilies) + + for _, metricFamily := range metricFamilies { + for _, metric := range metricFamily.Metric { + for _, label := range metric.Label { + if label.GetName() == labelName && label.GetValue() == labelValue { + return + } + } + } + } + require.Failf(t, "metric label not found", "%s=%q", labelName, labelValue) +} From fc64e40157aadfaeb9527b6e0713e3cf627a638e Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Wed, 29 Jul 2026 17:46:59 +0800 Subject: [PATCH 07/17] fix tests --- pkg/sink/mysql/mysql_writer_test.go | 2 +- pkg/statistics/statistics_test.go | 50 +++++++++-------------------- 2 files changed, 16 insertions(+), 36 deletions(-) diff --git a/pkg/sink/mysql/mysql_writer_test.go b/pkg/sink/mysql/mysql_writer_test.go index 291455356d..46a1e6106c 100644 --- a/pkg/sink/mysql/mysql_writer_test.go +++ b/pkg/sink/mysql/mysql_writer_test.go @@ -213,7 +213,7 @@ func TestAffectedRowsRecordedAfterCommit(t *testing.T) { dmls := &preparedDMLs{ sqls: []string{"INSERT INTO t VALUES (?)"}, - values: [][]interface{}{{1}}, + values: [][]any{{1}}, rowTypes: []common.RowType{common.RowTypeInsert}, rowCount: 1, } diff --git a/pkg/statistics/statistics_test.go b/pkg/statistics/statistics_test.go index a411a8a5c1..838365bde9 100644 --- a/pkg/statistics/statistics_test.go +++ b/pkg/statistics/statistics_test.go @@ -20,51 +20,31 @@ import ( "github.com/pingcap/ticdc/pkg/common" "github.com/pingcap/ticdc/pkg/metrics" "github.com/prometheus/client_golang/prometheus" - "github.com/prometheus/client_golang/prometheus/testutil" + dto "github.com/prometheus/client_model/go" "github.com/stretchr/testify/require" ) func TestExecBatchHistogramKeyspaceIDLabel(t *testing.T) { - metrics.ExecBatchHistogram.Reset() - t.Cleanup(metrics.ExecBatchHistogram.Reset) - + const keyspaceID uint32 = 123 + changefeedID := common.NewChangefeedID4Test(t.Name()+"-keyspace", t.Name()+"-changefeed") statistics := New( - common.NewChangefeedID4Test("test-keyspace", "batch-row-count-keyspace-id"), - 123, + changefeedID, + keyspaceID, ) + t.Cleanup(statistics.Close) require.NoError(t, statistics.RecordBatchExecution(func() (int, int64, error) { return 2, 10, nil })) - require.Equal(t, 1, testutil.CollectAndCount(metrics.ExecBatchHistogram)) - requireMetricHasLabel(t, metrics.ExecBatchHistogram, "keyspace_id", "123") - - statistics.Close() - require.Equal(t, 0, testutil.CollectAndCount(metrics.ExecBatchHistogram)) -} - -func requireMetricHasLabel( - t *testing.T, - collector prometheus.Collector, - labelName string, - labelValue string, -) { - t.Helper() - - registry := prometheus.NewPedanticRegistry() - registry.MustRegister(collector) - metricFamilies, err := registry.Gather() + labelValues := []string{changefeedID.Keyspace(), changefeedID.Name(), metrics.FormatKeyspaceID(keyspaceID)} + observer, err := metrics.ExecBatchHistogram.GetMetricWithLabelValues(labelValues...) require.NoError(t, err) - require.NotEmpty(t, metricFamilies) + metric, ok := observer.(prometheus.Metric) + require.True(t, ok) + metricDTO := &dto.Metric{} + require.NoError(t, metric.Write(metricDTO)) + require.Equal(t, uint64(1), metricDTO.GetHistogram().GetSampleCount()) - for _, metricFamily := range metricFamilies { - for _, metric := range metricFamily.Metric { - for _, label := range metric.Label { - if label.GetName() == labelName && label.GetValue() == labelValue { - return - } - } - } - } - require.Failf(t, "metric label not found", "%s=%q", labelName, labelValue) + statistics.Close() + require.False(t, metrics.ExecBatchHistogram.DeleteLabelValues(labelValues...)) } From 224376670f21fbbbe04d53f2253b11e6609a3c3b Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Wed, 29 Jul 2026 18:18:42 +0800 Subject: [PATCH 08/17] move metrics to statistics --- downstreamadapter/sink/kafka/sink_test.go | 47 ++++++++-- downstreamadapter/sink/pulsar/sink_test.go | 46 +++++++-- pkg/metrics/ddl.go | 31 ------ pkg/metrics/init.go | 2 + pkg/metrics/init_test.go | 26 ++++++ pkg/metrics/sink.go | 57 ++--------- pkg/sink/mysql/mysql_writer_test.go | 49 ++++++++-- pkg/statistics/metrics.go | 104 +++++++++++++++++++++ pkg/statistics/statistics.go | 35 ++++--- pkg/statistics/statistics_test.go | 7 +- 10 files changed, 283 insertions(+), 121 deletions(-) create mode 100644 pkg/metrics/init_test.go create mode 100644 pkg/statistics/metrics.go diff --git a/downstreamadapter/sink/kafka/sink_test.go b/downstreamadapter/sink/kafka/sink_test.go index 01032fad90..84b72ae2f7 100644 --- a/downstreamadapter/sink/kafka/sink_test.go +++ b/downstreamadapter/sink/kafka/sink_test.go @@ -32,11 +32,11 @@ import ( commonEvent "github.com/pingcap/ticdc/pkg/common/event" "github.com/pingcap/ticdc/pkg/config" "github.com/pingcap/ticdc/pkg/errors" - pmetrics "github.com/pingcap/ticdc/pkg/metrics" "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/prometheus/client_golang/prometheus/testutil" + "github.com/pingcap/ticdc/pkg/statistics" + "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/require" "go.uber.org/atomic" ) @@ -259,6 +259,9 @@ func TestKafkaSinkRunReturnsAsyncProducerError(t *testing.T) { } func TestKafkaSinkBasicFunctionality(t *testing.T) { + registry := prometheus.NewRegistry() + statistics.InitMetrics(registry) + helper := commonEvent.NewEventTestHelper(t) defer helper.Close() @@ -336,8 +339,8 @@ func TestKafkaSinkBasicFunctionality(t *testing.T) { err = kafkaSink.WriteBlockEvent(ddlEvent) require.NoError(t, err) - writeBytes := pmetrics.TotalWriteBytesCounter.WithLabelValues("test", "test") - beforeWriteBytes := testutil.ToFloat64(writeBytes) + metricLabels := prometheus.Labels{"changefeed": kafkaSink.changefeedID.Name()} + beforeWriteBytes := counterValueForLabels(t, registry, "ticdc_sink_write_bytes_total", metricLabels) kafkaSink.AddDMLEvent(dmlEvent) callbacks := make([]func(), 0, 2) for range 2 { @@ -348,7 +351,7 @@ func TestKafkaSinkBasicFunctionality(t *testing.T) { t.Fatal("timed out waiting for Kafka messages") } } - require.Equal(t, beforeWriteBytes, testutil.ToFloat64(writeBytes)) + require.Equal(t, beforeWriteBytes, counterValueForLabels(t, registry, "ticdc_sink_write_bytes_total", metricLabels)) for _, callback := range callbacks { require.NotNil(t, callback) callback() @@ -360,7 +363,7 @@ func TestKafkaSinkBasicFunctionality(t *testing.T) { func() bool { return count.Load() == int64(3) }, 5*time.Second, time.Second) - require.Equal(t, float64(dmlEvent.GetSize()), testutil.ToFloat64(writeBytes)-beforeWriteBytes) + require.Equal(t, float64(dmlEvent.GetSize()), counterValueForLabels(t, registry, "ticdc_sink_write_bytes_total", metricLabels)-beforeWriteBytes) // case 2: add checkpoint ts when sink is closed and it will not block kafkaSink.Close() @@ -368,6 +371,38 @@ func TestKafkaSinkBasicFunctionality(t *testing.T) { kafkaSink.AddCheckpointTs(12345) } +func counterValueForLabels( + t *testing.T, + registry *prometheus.Registry, + metricName string, + labels prometheus.Labels, +) float64 { + t.Helper() + + metricFamilies, err := registry.Gather() + require.NoError(t, err) + for _, metricFamily := range metricFamilies { + if metricFamily.GetName() != metricName { + continue + } + for _, metric := range metricFamily.Metric { + matchedLabels := 0 + for _, label := range metric.Label { + if value, ok := labels[label.GetName()]; ok { + if value != label.GetValue() { + break + } + matchedLabels++ + } + } + if matchedLabels == len(labels) { + return metric.GetCounter().GetValue() + } + } + } + return 0 +} + func TestKafkaSinkBatchConfig(t *testing.T) { sink := &sink{} require.Equal(t, 4096, sink.BatchCount()) diff --git a/downstreamadapter/sink/pulsar/sink_test.go b/downstreamadapter/sink/pulsar/sink_test.go index e472e61520..5405446bfd 100644 --- a/downstreamadapter/sink/pulsar/sink_test.go +++ b/downstreamadapter/sink/pulsar/sink_test.go @@ -25,10 +25,9 @@ import ( commonEvent "github.com/pingcap/ticdc/pkg/common/event" "github.com/pingcap/ticdc/pkg/config" cerror "github.com/pingcap/ticdc/pkg/errors" - "github.com/pingcap/ticdc/pkg/metrics" "github.com/pingcap/ticdc/pkg/statistics" "github.com/pingcap/ticdc/utils/chann" - "github.com/prometheus/client_golang/prometheus/testutil" + "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/require" "go.uber.org/atomic" ) @@ -73,6 +72,9 @@ func newPulsarSinkForTest(t *testing.T) (*sink, error) { } func TestPulsarSinkBasicFunctionality(t *testing.T) { + registry := prometheus.NewRegistry() + statistics.InitMetrics(registry) + pulsarSink, err := newPulsarSinkForTest(t) require.NoError(t, err) @@ -127,8 +129,8 @@ func TestPulsarSinkBasicFunctionality(t *testing.T) { err = pulsarSink.WriteBlockEvent(ddlEvent) require.NoError(t, err) - writeBytes := metrics.TotalWriteBytesCounter.WithLabelValues("test", "test") - beforeWriteBytes := testutil.ToFloat64(writeBytes) + metricLabels := prometheus.Labels{"changefeed": pulsarSink.changefeedID.Name()} + beforeWriteBytes := counterValueForLabels(t, registry, "ticdc_sink_write_bytes_total", metricLabels) pulsarSink.AddDMLEvent(dmlEvent) callbacks := make([]func(), 0, 2) for range 2 { @@ -139,7 +141,7 @@ func TestPulsarSinkBasicFunctionality(t *testing.T) { t.Fatal("timed out waiting for Pulsar messages") } } - require.Equal(t, beforeWriteBytes, testutil.ToFloat64(writeBytes)) + require.Equal(t, beforeWriteBytes, counterValueForLabels(t, registry, "ticdc_sink_write_bytes_total", metricLabels)) for _, callback := range callbacks { require.NotNil(t, callback) callback() @@ -151,7 +153,39 @@ func TestPulsarSinkBasicFunctionality(t *testing.T) { require.Len(t, pulsarSink.ddlProducer.(*mockProducer).GetAllEvents(), 1) require.Equal(t, count.Load(), int64(3)) - require.Equal(t, float64(dmlEvent.GetSize()), testutil.ToFloat64(writeBytes)-beforeWriteBytes) + require.Equal(t, float64(dmlEvent.GetSize()), counterValueForLabels(t, registry, "ticdc_sink_write_bytes_total", metricLabels)-beforeWriteBytes) +} + +func counterValueForLabels( + t *testing.T, + registry *prometheus.Registry, + metricName string, + labels prometheus.Labels, +) float64 { + t.Helper() + + metricFamilies, err := registry.Gather() + require.NoError(t, err) + for _, metricFamily := range metricFamilies { + if metricFamily.GetName() != metricName { + continue + } + for _, metric := range metricFamily.Metric { + matchedLabels := 0 + for _, label := range metric.Label { + if value, ok := labels[label.GetName()]; ok { + if value != label.GetValue() { + break + } + matchedLabels++ + } + } + if matchedLabels == len(labels) { + return metric.GetCounter().GetValue() + } + } + } + return 0 } func TestPulsarSinkBatchConfig(t *testing.T) { diff --git a/pkg/metrics/ddl.go b/pkg/metrics/ddl.go index 5194e167fe..9b0c61098d 100644 --- a/pkg/metrics/ddl.go +++ b/pkg/metrics/ddl.go @@ -29,25 +29,6 @@ var ( Buckets: prometheus.ExponentialBuckets(0.01, 2, 18), }, []string{getKeyspaceLabel(), "changefeed"}) - // ExecDDLHistogram records the execution time of a DDL. - ExecDDLHistogram = prometheus.NewHistogramVec( - prometheus.HistogramOpts{ - Namespace: "ticdc", - Subsystem: "ddl", - Name: "exec_duration", - Help: "Bucketed histogram of processing time (s) of a ddl.", - Buckets: prometheus.ExponentialBuckets(0.01, 2, 18), - }, []string{getKeyspaceLabel(), "changefeed"}) - - // ExecDDLRunningGauge records the count of running DDL. - ExecDDLRunningGauge = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ - Namespace: "ticdc", - Subsystem: "ddl", - Name: "exec_running", - Help: "Total count of running ddl.", - }, []string{getKeyspaceLabel(), "changefeed"}) - // ExecDDLBlockingGauge records the count of blocking DDL. ExecDDLBlockingGauge = prometheus.NewGaugeVec( prometheus.GaugeOpts{ @@ -56,21 +37,9 @@ var ( Name: "exec_blocking", Help: "Total count of blocking ddl.", }, []string{getKeyspaceLabel(), "changefeed", "mode"}) - - // ExecDDLCounter records the execution count of different DDL types - ExecDDLCounter = prometheus.NewCounterVec( - prometheus.CounterOpts{ - Namespace: "ticdc", - Subsystem: "ddl", - Name: "execution", - Help: "Total execution count of different DDL types.", - }, []string{getKeyspaceLabel(), "changefeed", "ddl_type"}) ) func initDDLMetrics(registry *prometheus.Registry) { registry.MustRegister(HandleDDLHistogram) - registry.MustRegister(ExecDDLHistogram) - registry.MustRegister(ExecDDLRunningGauge) registry.MustRegister(ExecDDLBlockingGauge) - registry.MustRegister(ExecDDLCounter) } diff --git a/pkg/metrics/init.go b/pkg/metrics/init.go index 7583b99155..39e39725cb 100644 --- a/pkg/metrics/init.go +++ b/pkg/metrics/init.go @@ -18,6 +18,7 @@ import ( "github.com/pingcap/ticdc/pkg/common" "github.com/pingcap/ticdc/pkg/config/kerneltype" "github.com/pingcap/ticdc/pkg/sink/kafka" + "github.com/pingcap/ticdc/pkg/statistics" "github.com/pingcap/ticdc/pkg/txnutil/gc" "github.com/prometheus/client_golang/prometheus" ) @@ -31,6 +32,7 @@ func InitMetrics(registry *prometheus.Registry) { initDispatcherMetrics(registry) initMessagingMetrics(registry) initSinkMetrics(registry) + statistics.InitMetrics(registry) initEventStoreMetrics(registry) initSchemaStoreMetrics(registry) initEventServiceMetrics(registry) diff --git a/pkg/metrics/init_test.go b/pkg/metrics/init_test.go new file mode 100644 index 0000000000..938c150f12 --- /dev/null +++ b/pkg/metrics/init_test.go @@ -0,0 +1,26 @@ +// Copyright 2026 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, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package metrics + +import ( + "testing" + + "github.com/prometheus/client_golang/prometheus" +) + +func TestInitMetrics(t *testing.T) { + registry := prometheus.NewRegistry() + InitMetrics(registry) +} diff --git a/pkg/metrics/sink.go b/pkg/metrics/sink.go index c8235918e8..a6c628ac32 100644 --- a/pkg/metrics/sink.go +++ b/pkg/metrics/sink.go @@ -18,51 +18,13 @@ import "github.com/prometheus/client_golang/prometheus" // LargeRowSizeLowBound is set to 2K, only track data event with size not smaller than it. const LargeRowSizeLowBound = 2 * 1024 -// ---------- Metrics used in Statistics. ---------- // -var ( - // ExecBatchHistogram records batch size of a txn. - ExecBatchHistogram = prometheus.NewHistogramVec( - prometheus.HistogramOpts{ - Namespace: "ticdc", - Subsystem: "sink", - Name: "batch_row_count", - Help: "Row count number for a given batch.", - Buckets: prometheus.ExponentialBuckets(1, 2, 18), - }, []string{getKeyspaceLabel(), "changefeed", "keyspace_id"}) - - // ExecWriteBytesGauge records the total number of bytes written by sink. - TotalWriteBytesCounter = prometheus.NewCounterVec( - prometheus.CounterOpts{ - Namespace: "ticdc", - Subsystem: "sink", - Name: "write_bytes_total", - Help: "Total number of bytes written by sink", - }, []string{getKeyspaceLabel(), "changefeed"}) - - ExecDMLEventRowsAffectedCounter = prometheus.NewCounterVec( - prometheus.CounterOpts{ - Namespace: "ticdc", - Subsystem: "sink", - Name: "dml_event_affected_row_count", - Help: "Total count of affected rows.", - }, []string{getKeyspaceLabel(), "changefeed", "count_type", "row_type"}) - - ActiveActiveConflictSkipRowsCounter = prometheus.NewCounterVec( - prometheus.CounterOpts{ - Namespace: "ticdc", - Subsystem: "sink", - Name: "active_active_conflict_skip_rows_total", - Help: "Total number of rows skipped due to last-write-wins conflict resolution in TiDB active-active replication.", - }, []string{getKeyspaceLabel(), "changefeed"}) - // ExecutionErrorCounter is the counter of execution errors. - ExecutionErrorCounter = prometheus.NewCounterVec( - prometheus.CounterOpts{ - Namespace: "ticdc", - Subsystem: "sink", - Name: "execution_error", - Help: "Total count of execution errors.", - }, []string{getKeyspaceLabel(), "changefeed", "event_type"}) -) +var ActiveActiveConflictSkipRowsCounter = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: "ticdc", + Subsystem: "sink", + Name: "active_active_conflict_skip_rows_total", + Help: "Total number of rows skipped due to last-write-wins conflict resolution in TiDB active-active replication.", + }, []string{getKeyspaceLabel(), "changefeed"}) // ---------- Metrics for txn sink and backends. ---------- // var ( @@ -206,12 +168,7 @@ var ( // InitMetrics registers all metrics in this file. func initSinkMetrics(registry *prometheus.Registry) { - // common sink metrics - registry.MustRegister(ExecBatchHistogram) - registry.MustRegister(TotalWriteBytesCounter) - registry.MustRegister(ExecDMLEventRowsAffectedCounter) registry.MustRegister(ActiveActiveConflictSkipRowsCounter) - registry.MustRegister(ExecutionErrorCounter) // txn sink metrics registry.MustRegister(ConflictDetectDuration) diff --git a/pkg/sink/mysql/mysql_writer_test.go b/pkg/sink/mysql/mysql_writer_test.go index 46a1e6106c..5cc0b17e32 100644 --- a/pkg/sink/mysql/mysql_writer_test.go +++ b/pkg/sink/mysql/mysql_writer_test.go @@ -31,7 +31,6 @@ import ( "github.com/pingcap/ticdc/pkg/config" "github.com/pingcap/ticdc/pkg/config/kerneltype" cerror "github.com/pingcap/ticdc/pkg/errors" - "github.com/pingcap/ticdc/pkg/metrics" "github.com/pingcap/ticdc/pkg/routing" "github.com/pingcap/ticdc/pkg/statistics" "github.com/pingcap/tidb/br/pkg/version" @@ -39,7 +38,7 @@ import ( "github.com/pingcap/tidb/pkg/dxf/framework/handle" timodel "github.com/pingcap/tidb/pkg/meta/model" "github.com/pingcap/tidb/pkg/sessionctx/vardef" - "github.com/prometheus/client_golang/prometheus/testutil" + "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/require" ) @@ -200,6 +199,9 @@ func TestMysqlWriter_FlushDML_DuplicateEntryRetry(t *testing.T) { } func TestAffectedRowsRecordedAfterCommit(t *testing.T) { + registry := prometheus.NewRegistry() + statistics.InitMetrics(registry) + writer, db, mock := newTestMysqlWriter(t) defer db.Close() writer.cfg.CachePrepStmts = false @@ -208,8 +210,11 @@ func TestAffectedRowsRecordedAfterCommit(t *testing.T) { changefeedID := common.NewChangefeedID4Test("test", t.Name()) writer.statistics = statistics.New(changefeedID, common.DefaultKeyspaceID) - counter := metrics.ExecDMLEventRowsAffectedCounter.WithLabelValues( - changefeedID.Keyspace(), changefeedID.Name(), "actual", common.RowTypeInsert.String()) + metricLabels := prometheus.Labels{ + "changefeed": changefeedID.Name(), + "count_type": "actual", + "row_type": common.RowTypeInsert.String(), + } dmls := &preparedDMLs{ sqls: []string{"INSERT INTO t VALUES (?)"}, @@ -222,16 +227,48 @@ func TestAffectedRowsRecordedAfterCommit(t *testing.T) { mock.ExpectExec("INSERT INTO t VALUES (?)").WithArgs(1).WillReturnResult(sqlmock.NewResult(0, 1)) mock.ExpectCommit().WillReturnError(errors.New("commit failed")) require.Error(t, writer.execDMLWithMaxRetries(dmls)) - require.Zero(t, testutil.ToFloat64(counter)) + require.Zero(t, counterValueForLabels(t, registry, "ticdc_sink_dml_event_affected_row_count", metricLabels)) mock.ExpectBegin() mock.ExpectExec("INSERT INTO t VALUES (?)").WithArgs(1).WillReturnResult(sqlmock.NewResult(0, 1)) mock.ExpectCommit() require.NoError(t, writer.execDMLWithMaxRetries(dmls)) - require.Equal(t, float64(1), testutil.ToFloat64(counter)) + require.Equal(t, float64(1), counterValueForLabels(t, registry, "ticdc_sink_dml_event_affected_row_count", metricLabels)) require.NoError(t, mock.ExpectationsWereMet()) } +func counterValueForLabels( + t *testing.T, + registry *prometheus.Registry, + metricName string, + labels prometheus.Labels, +) float64 { + t.Helper() + + metricFamilies, err := registry.Gather() + require.NoError(t, err) + for _, metricFamily := range metricFamilies { + if metricFamily.GetName() != metricName { + continue + } + for _, metric := range metricFamily.Metric { + matchedLabels := 0 + for _, label := range metric.Label { + if value, ok := labels[label.GetName()]; ok { + if value != label.GetValue() { + break + } + matchedLabels++ + } + } + if matchedLabels == len(labels) { + return metric.GetCounter().GetValue() + } + } + } + return 0 +} + func TestMysqlWriter_FlushMultiDML(t *testing.T) { writer, db, mock := newTestMysqlWriter(t) defer db.Close() diff --git a/pkg/statistics/metrics.go b/pkg/statistics/metrics.go new file mode 100644 index 0000000000..b70a007886 --- /dev/null +++ b/pkg/statistics/metrics.go @@ -0,0 +1,104 @@ +// Copyright 2026 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, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package statistics + +import ( + "strconv" + + "github.com/pingcap/ticdc/pkg/config/kerneltype" + "github.com/prometheus/client_golang/prometheus" +) + +var ( + execDDLHistogram = prometheus.NewHistogramVec( + prometheus.HistogramOpts{ + Namespace: "ticdc", + Subsystem: "ddl", + Name: "exec_duration", + Help: "Bucketed histogram of processing time (s) of a ddl.", + Buckets: prometheus.ExponentialBuckets(0.01, 2, 18), + }, []string{getKeyspaceLabel(), "changefeed"}) + + execDDLRunningGauge = prometheus.NewGaugeVec( + prometheus.GaugeOpts{ + Namespace: "ticdc", + Subsystem: "ddl", + Name: "exec_running", + Help: "Total count of running ddl.", + }, []string{getKeyspaceLabel(), "changefeed"}) + + execDDLCounter = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: "ticdc", + Subsystem: "ddl", + Name: "execution", + Help: "Total execution count of different DDL types.", + }, []string{getKeyspaceLabel(), "changefeed", "ddl_type"}) + + execBatchHistogram = prometheus.NewHistogramVec( + prometheus.HistogramOpts{ + Namespace: "ticdc", + Subsystem: "sink", + Name: "batch_row_count", + Help: "Row count number for a given batch.", + Buckets: prometheus.ExponentialBuckets(1, 2, 18), + }, []string{getKeyspaceLabel(), "changefeed", "keyspace_id"}) + + totalWriteBytesCounter = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: "ticdc", + Subsystem: "sink", + Name: "write_bytes_total", + Help: "Total number of bytes written by sink", + }, []string{getKeyspaceLabel(), "changefeed"}) + + execDMLEventRowsAffectedCounter = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: "ticdc", + Subsystem: "sink", + Name: "dml_event_affected_row_count", + Help: "Total count of affected rows.", + }, []string{getKeyspaceLabel(), "changefeed", "count_type", "row_type"}) + + executionErrorCounter = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: "ticdc", + Subsystem: "sink", + Name: "execution_error", + Help: "Total count of execution errors.", + }, []string{getKeyspaceLabel(), "changefeed", "event_type"}) +) + +// InitMetrics registers the metrics maintained by Statistics. +func InitMetrics(registry *prometheus.Registry) { + registry.MustRegister(execDDLHistogram) + registry.MustRegister(execDDLRunningGauge) + registry.MustRegister(execDDLCounter) + registry.MustRegister(execBatchHistogram) + registry.MustRegister(totalWriteBytesCounter) + registry.MustRegister(execDMLEventRowsAffectedCounter) + registry.MustRegister(executionErrorCounter) +} + +func getKeyspaceLabel() string { + if kerneltype.IsNextGen() { + return "keyspace_name" + } + return "namespace" +} + +func formatKeyspaceID(keyspaceID uint32) string { + return strconv.FormatUint(uint64(keyspaceID), 10) +} diff --git a/pkg/statistics/statistics.go b/pkg/statistics/statistics.go index 2d63067fc1..3ea04396f1 100644 --- a/pkg/statistics/statistics.go +++ b/pkg/statistics/statistics.go @@ -20,7 +20,6 @@ import ( "time" "github.com/pingcap/ticdc/pkg/common" - "github.com/pingcap/ticdc/pkg/metrics" "github.com/prometheus/client_golang/prometheus" ) @@ -28,19 +27,19 @@ import ( func New(changefeed common.ChangeFeedID, keyspaceID uint32) *Statistics { statistics := &Statistics{ changefeedID: changefeed, - keyspaceID: metrics.FormatKeyspaceID(keyspaceID), + keyspaceID: formatKeyspaceID(keyspaceID), ddlTypes: sync.Map{}, rowsAffectedMap: sync.Map{}, } keyspace := changefeed.Keyspace() changefeedID := changefeed.Name() - statistics.metricExecDDLHis = metrics.ExecDDLHistogram.WithLabelValues(keyspace, changefeedID) - statistics.metricExecDDLRunningCnt = metrics.ExecDDLRunningGauge.WithLabelValues(keyspace, changefeedID) - statistics.metricExecBatchHis = metrics.ExecBatchHistogram.WithLabelValues(keyspace, changefeedID, statistics.keyspaceID) - statistics.metricTotalWriteBytesCnt = metrics.TotalWriteBytesCounter.WithLabelValues(keyspace, changefeedID) - statistics.metricExecErrCntForDDL = metrics.ExecutionErrorCounter.WithLabelValues(keyspace, changefeedID, "ddl") - statistics.metricExecErrCntForDML = metrics.ExecutionErrorCounter.WithLabelValues(keyspace, changefeedID, "dml") + statistics.metricExecDDLHis = execDDLHistogram.WithLabelValues(keyspace, changefeedID) + statistics.metricExecDDLRunningCnt = execDDLRunningGauge.WithLabelValues(keyspace, changefeedID) + statistics.metricExecBatchHis = execBatchHistogram.WithLabelValues(keyspace, changefeedID, statistics.keyspaceID) + statistics.metricTotalWriteBytesCnt = totalWriteBytesCounter.WithLabelValues(keyspace, changefeedID) + statistics.metricExecErrCntForDDL = executionErrorCounter.WithLabelValues(keyspace, changefeedID, "ddl") + statistics.metricExecErrCntForDML = executionErrorCounter.WithLabelValues(keyspace, changefeedID, "dml") return statistics } @@ -95,7 +94,7 @@ func (b *Statistics) RecordDDLExecution(executor func() (string, error)) error { b.metricExecErrCntForDDL.Inc() return err } - metricExecDDLCounter := metrics.ExecDDLCounter.WithLabelValues( + metricExecDDLCounter := execDDLCounter.WithLabelValues( b.changefeedID.Keyspace(), b.changefeedID.Name(), ddlType) metricExecDDLCounter.Inc() b.ddlTypes.Store(ddlType, struct{}{}) @@ -120,7 +119,7 @@ func (b *Statistics) getRowsAffected(countType, rowType string) prometheus.Count if !loaded { keyspace := b.changefeedID.Keyspace() changefeedID := b.changefeedID.Name() - counter := metrics.ExecDMLEventRowsAffectedCounter.WithLabelValues(keyspace, changefeedID, countType, rowType) + counter := execDMLEventRowsAffectedCounter.WithLabelValues(keyspace, changefeedID, countType, rowType) b.rowsAffectedMap.Store(key, counter) return counter } @@ -131,22 +130,22 @@ func (b *Statistics) getRowsAffected(countType, rowType string) prometheus.Count func (b *Statistics) Close() { keyspace := b.changefeedID.Keyspace() changefeedID := b.changefeedID.Name() - metrics.ExecDDLHistogram.DeleteLabelValues(keyspace, changefeedID) - metrics.ExecDDLRunningGauge.DeleteLabelValues(keyspace, changefeedID) - metrics.ExecBatchHistogram.DeleteLabelValues(keyspace, changefeedID, b.keyspaceID) - metrics.ExecutionErrorCounter.DeleteLabelValues(keyspace, changefeedID, "ddl") - metrics.ExecutionErrorCounter.DeleteLabelValues(keyspace, changefeedID, "dml") + execDDLHistogram.DeleteLabelValues(keyspace, changefeedID) + execDDLRunningGauge.DeleteLabelValues(keyspace, changefeedID) + execBatchHistogram.DeleteLabelValues(keyspace, changefeedID, b.keyspaceID) + executionErrorCounter.DeleteLabelValues(keyspace, changefeedID, "ddl") + executionErrorCounter.DeleteLabelValues(keyspace, changefeedID, "dml") b.ddlTypes.Range(func(key, value any) bool { ddlType := key.(string) - metrics.ExecDDLCounter.DeleteLabelValues(keyspace, changefeedID, ddlType) + execDDLCounter.DeleteLabelValues(keyspace, changefeedID, ddlType) return true }) b.rowsAffectedMap.Range(func(key, value any) bool { countTypeAndRowType := key.(string) splitTypes := strings.Split(countTypeAndRowType, "-") countType, rowType := splitTypes[0], splitTypes[1] - metrics.ExecDMLEventRowsAffectedCounter.DeleteLabelValues(keyspace, changefeedID, countType, rowType) + execDMLEventRowsAffectedCounter.DeleteLabelValues(keyspace, changefeedID, countType, rowType) return true }) - metrics.TotalWriteBytesCounter.DeleteLabelValues(keyspace, changefeedID) + totalWriteBytesCounter.DeleteLabelValues(keyspace, changefeedID) } diff --git a/pkg/statistics/statistics_test.go b/pkg/statistics/statistics_test.go index 838365bde9..e01e09d995 100644 --- a/pkg/statistics/statistics_test.go +++ b/pkg/statistics/statistics_test.go @@ -18,7 +18,6 @@ import ( "testing" "github.com/pingcap/ticdc/pkg/common" - "github.com/pingcap/ticdc/pkg/metrics" "github.com/prometheus/client_golang/prometheus" dto "github.com/prometheus/client_model/go" "github.com/stretchr/testify/require" @@ -36,8 +35,8 @@ func TestExecBatchHistogramKeyspaceIDLabel(t *testing.T) { return 2, 10, nil })) - labelValues := []string{changefeedID.Keyspace(), changefeedID.Name(), metrics.FormatKeyspaceID(keyspaceID)} - observer, err := metrics.ExecBatchHistogram.GetMetricWithLabelValues(labelValues...) + labelValues := []string{changefeedID.Keyspace(), changefeedID.Name(), formatKeyspaceID(keyspaceID)} + observer, err := execBatchHistogram.GetMetricWithLabelValues(labelValues...) require.NoError(t, err) metric, ok := observer.(prometheus.Metric) require.True(t, ok) @@ -46,5 +45,5 @@ func TestExecBatchHistogramKeyspaceIDLabel(t *testing.T) { require.Equal(t, uint64(1), metricDTO.GetHistogram().GetSampleCount()) statistics.Close() - require.False(t, metrics.ExecBatchHistogram.DeleteLabelValues(labelValues...)) + require.False(t, execBatchHistogram.DeleteLabelValues(labelValues...)) } From 46c60e55c151208ab0d038a1083c92698ce70118 Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Wed, 29 Jul 2026 19:04:05 +0800 Subject: [PATCH 09/17] ponytail review and fix the code --- downstreamadapter/sink/kafka/sink_test.go | 59 +-------------- .../sink/pulsar/mock_producer.go | 9 +-- downstreamadapter/sink/pulsar/sink_test.go | 58 +-------------- pkg/metrics/init_test.go | 26 ------- pkg/sink/mysql/mysql_writer_test.go | 72 ------------------- pkg/statistics/metrics.go | 6 -- pkg/statistics/statistics.go | 3 +- pkg/statistics/statistics_test.go | 49 ------------- 8 files changed, 10 insertions(+), 272 deletions(-) delete mode 100644 pkg/metrics/init_test.go delete mode 100644 pkg/statistics/statistics_test.go diff --git a/downstreamadapter/sink/kafka/sink_test.go b/downstreamadapter/sink/kafka/sink_test.go index 84b72ae2f7..8b205c1649 100644 --- a/downstreamadapter/sink/kafka/sink_test.go +++ b/downstreamadapter/sink/kafka/sink_test.go @@ -35,8 +35,6 @@ import ( "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/statistics" - "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/require" "go.uber.org/atomic" ) @@ -259,9 +257,6 @@ func TestKafkaSinkRunReturnsAsyncProducerError(t *testing.T) { } func TestKafkaSinkBasicFunctionality(t *testing.T) { - registry := prometheus.NewRegistry() - statistics.InitMetrics(registry) - helper := commonEvent.NewEventTestHelper(t) defer helper.Close() @@ -315,7 +310,6 @@ func TestKafkaSinkBasicFunctionality(t *testing.T) { ctrl := gomock.NewController(t) asyncProducer := kafka.NewMockAsyncProducer(ctrl) syncProducer := kafka.NewMockSyncProducer(ctrl) - callbackCh := make(chan func(), 2) asyncProducer.EXPECT().AsyncRunCallback(gomock.Any()).Return(nil).AnyTimes() asyncProducer.EXPECT().AsyncSend(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). DoAndReturn(func( @@ -324,7 +318,9 @@ func TestKafkaSinkBasicFunctionality(t *testing.T) { _ int32, message *codecCommon.Message, ) error { - callbackCh <- message.Callback + if message.Callback != nil { + message.Callback() + } return nil }).Times(2) asyncProducer.EXPECT().Close().AnyTimes() @@ -339,23 +335,7 @@ func TestKafkaSinkBasicFunctionality(t *testing.T) { err = kafkaSink.WriteBlockEvent(ddlEvent) require.NoError(t, err) - metricLabels := prometheus.Labels{"changefeed": kafkaSink.changefeedID.Name()} - beforeWriteBytes := counterValueForLabels(t, registry, "ticdc_sink_write_bytes_total", metricLabels) kafkaSink.AddDMLEvent(dmlEvent) - callbacks := make([]func(), 0, 2) - for range 2 { - select { - case callback := <-callbackCh: - callbacks = append(callbacks, callback) - case <-time.After(5 * time.Second): - t.Fatal("timed out waiting for Kafka messages") - } - } - require.Equal(t, beforeWriteBytes, counterValueForLabels(t, registry, "ticdc_sink_write_bytes_total", metricLabels)) - for _, callback := range callbacks { - require.NotNil(t, callback) - callback() - } ddlEvent2.PostFlush() @@ -363,7 +343,6 @@ func TestKafkaSinkBasicFunctionality(t *testing.T) { func() bool { return count.Load() == int64(3) }, 5*time.Second, time.Second) - require.Equal(t, float64(dmlEvent.GetSize()), counterValueForLabels(t, registry, "ticdc_sink_write_bytes_total", metricLabels)-beforeWriteBytes) // case 2: add checkpoint ts when sink is closed and it will not block kafkaSink.Close() @@ -371,38 +350,6 @@ func TestKafkaSinkBasicFunctionality(t *testing.T) { kafkaSink.AddCheckpointTs(12345) } -func counterValueForLabels( - t *testing.T, - registry *prometheus.Registry, - metricName string, - labels prometheus.Labels, -) float64 { - t.Helper() - - metricFamilies, err := registry.Gather() - require.NoError(t, err) - for _, metricFamily := range metricFamilies { - if metricFamily.GetName() != metricName { - continue - } - for _, metric := range metricFamily.Metric { - matchedLabels := 0 - for _, label := range metric.Label { - if value, ok := labels[label.GetName()]; ok { - if value != label.GetValue() { - break - } - matchedLabels++ - } - } - if matchedLabels == len(labels) { - return metric.GetCounter().GetValue() - } - } - } - return 0 -} - func TestKafkaSinkBatchConfig(t *testing.T) { sink := &sink{} require.Equal(t, 4096, sink.BatchCount()) diff --git a/downstreamadapter/sink/pulsar/mock_producer.go b/downstreamadapter/sink/pulsar/mock_producer.go index b816efd91b..a71d6a1247 100644 --- a/downstreamadapter/sink/pulsar/mock_producer.go +++ b/downstreamadapter/sink/pulsar/mock_producer.go @@ -28,9 +28,8 @@ var ( // mockProducer is a mock pulsar producer type mockProducer struct { - mu sync.Mutex - events map[string][]*pulsar.ProducerMessage - callbackCh chan func() + mu sync.Mutex + events map[string][]*pulsar.ProducerMessage } func newMockDDLProducer() ddlProducer { @@ -81,9 +80,7 @@ func (p *mockProducer) asyncSendMessage(_ context.Context, topic string, message Key: message.GetPartitionKey(), } p.events[topic] = append(p.events[topic], data) - if p.callbackCh != nil { - p.callbackCh <- message.Callback - } else if message.Callback != nil { + if message.Callback != nil { message.Callback() } return nil diff --git a/downstreamadapter/sink/pulsar/sink_test.go b/downstreamadapter/sink/pulsar/sink_test.go index 5405446bfd..543fc43846 100644 --- a/downstreamadapter/sink/pulsar/sink_test.go +++ b/downstreamadapter/sink/pulsar/sink_test.go @@ -27,7 +27,6 @@ import ( cerror "github.com/pingcap/ticdc/pkg/errors" "github.com/pingcap/ticdc/pkg/statistics" "github.com/pingcap/ticdc/utils/chann" - "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/require" "go.uber.org/atomic" ) @@ -72,9 +71,6 @@ func newPulsarSinkForTest(t *testing.T) (*sink, error) { } func TestPulsarSinkBasicFunctionality(t *testing.T) { - registry := prometheus.NewRegistry() - statistics.InitMetrics(registry) - pulsarSink, err := newPulsarSinkForTest(t) require.NoError(t, err) @@ -123,69 +119,19 @@ func TestPulsarSinkBasicFunctionality(t *testing.T) { func() { count.Add(1) }, } dmlEvent.CommitTs = 2 - producer := pulsarSink.dmlProducer.(*mockProducer) - producer.callbackCh = make(chan func(), 2) err = pulsarSink.WriteBlockEvent(ddlEvent) require.NoError(t, err) - metricLabels := prometheus.Labels{"changefeed": pulsarSink.changefeedID.Name()} - beforeWriteBytes := counterValueForLabels(t, registry, "ticdc_sink_write_bytes_total", metricLabels) pulsarSink.AddDMLEvent(dmlEvent) - callbacks := make([]func(), 0, 2) - for range 2 { - select { - case callback := <-producer.callbackCh: - callbacks = append(callbacks, callback) - case <-time.After(5 * time.Second): - t.Fatal("timed out waiting for Pulsar messages") - } - } - require.Equal(t, beforeWriteBytes, counterValueForLabels(t, registry, "ticdc_sink_write_bytes_total", metricLabels)) - for _, callback := range callbacks { - require.NotNil(t, callback) - callback() - } + time.Sleep(1 * time.Second) ddlEvent2.PostFlush() - require.Len(t, producer.GetAllEvents(), 2) + require.Len(t, pulsarSink.dmlProducer.(*mockProducer).GetAllEvents(), 2) require.Len(t, pulsarSink.ddlProducer.(*mockProducer).GetAllEvents(), 1) require.Equal(t, count.Load(), int64(3)) - require.Equal(t, float64(dmlEvent.GetSize()), counterValueForLabels(t, registry, "ticdc_sink_write_bytes_total", metricLabels)-beforeWriteBytes) -} - -func counterValueForLabels( - t *testing.T, - registry *prometheus.Registry, - metricName string, - labels prometheus.Labels, -) float64 { - t.Helper() - - metricFamilies, err := registry.Gather() - require.NoError(t, err) - for _, metricFamily := range metricFamilies { - if metricFamily.GetName() != metricName { - continue - } - for _, metric := range metricFamily.Metric { - matchedLabels := 0 - for _, label := range metric.Label { - if value, ok := labels[label.GetName()]; ok { - if value != label.GetValue() { - break - } - matchedLabels++ - } - } - if matchedLabels == len(labels) { - return metric.GetCounter().GetValue() - } - } - } - return 0 } func TestPulsarSinkBatchConfig(t *testing.T) { diff --git a/pkg/metrics/init_test.go b/pkg/metrics/init_test.go deleted file mode 100644 index 938c150f12..0000000000 --- a/pkg/metrics/init_test.go +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright 2026 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, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package metrics - -import ( - "testing" - - "github.com/prometheus/client_golang/prometheus" -) - -func TestInitMetrics(t *testing.T) { - registry := prometheus.NewRegistry() - InitMetrics(registry) -} diff --git a/pkg/sink/mysql/mysql_writer_test.go b/pkg/sink/mysql/mysql_writer_test.go index 5cc0b17e32..d931c5cdff 100644 --- a/pkg/sink/mysql/mysql_writer_test.go +++ b/pkg/sink/mysql/mysql_writer_test.go @@ -38,7 +38,6 @@ import ( "github.com/pingcap/tidb/pkg/dxf/framework/handle" timodel "github.com/pingcap/tidb/pkg/meta/model" "github.com/pingcap/tidb/pkg/sessionctx/vardef" - "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/require" ) @@ -198,77 +197,6 @@ func TestMysqlWriter_FlushDML_DuplicateEntryRetry(t *testing.T) { require.NoError(t, err) } -func TestAffectedRowsRecordedAfterCommit(t *testing.T) { - registry := prometheus.NewRegistry() - statistics.InitMetrics(registry) - - writer, db, mock := newTestMysqlWriter(t) - defer db.Close() - writer.cfg.CachePrepStmts = false - writer.cfg.MultiStmtEnable = false - writer.statistics.Close() - - changefeedID := common.NewChangefeedID4Test("test", t.Name()) - writer.statistics = statistics.New(changefeedID, common.DefaultKeyspaceID) - metricLabels := prometheus.Labels{ - "changefeed": changefeedID.Name(), - "count_type": "actual", - "row_type": common.RowTypeInsert.String(), - } - - dmls := &preparedDMLs{ - sqls: []string{"INSERT INTO t VALUES (?)"}, - values: [][]any{{1}}, - rowTypes: []common.RowType{common.RowTypeInsert}, - rowCount: 1, - } - - mock.ExpectBegin() - mock.ExpectExec("INSERT INTO t VALUES (?)").WithArgs(1).WillReturnResult(sqlmock.NewResult(0, 1)) - mock.ExpectCommit().WillReturnError(errors.New("commit failed")) - require.Error(t, writer.execDMLWithMaxRetries(dmls)) - require.Zero(t, counterValueForLabels(t, registry, "ticdc_sink_dml_event_affected_row_count", metricLabels)) - - mock.ExpectBegin() - mock.ExpectExec("INSERT INTO t VALUES (?)").WithArgs(1).WillReturnResult(sqlmock.NewResult(0, 1)) - mock.ExpectCommit() - require.NoError(t, writer.execDMLWithMaxRetries(dmls)) - require.Equal(t, float64(1), counterValueForLabels(t, registry, "ticdc_sink_dml_event_affected_row_count", metricLabels)) - require.NoError(t, mock.ExpectationsWereMet()) -} - -func counterValueForLabels( - t *testing.T, - registry *prometheus.Registry, - metricName string, - labels prometheus.Labels, -) float64 { - t.Helper() - - metricFamilies, err := registry.Gather() - require.NoError(t, err) - for _, metricFamily := range metricFamilies { - if metricFamily.GetName() != metricName { - continue - } - for _, metric := range metricFamily.Metric { - matchedLabels := 0 - for _, label := range metric.Label { - if value, ok := labels[label.GetName()]; ok { - if value != label.GetValue() { - break - } - matchedLabels++ - } - } - if matchedLabels == len(labels) { - return metric.GetCounter().GetValue() - } - } - } - return 0 -} - func TestMysqlWriter_FlushMultiDML(t *testing.T) { writer, db, mock := newTestMysqlWriter(t) defer db.Close() diff --git a/pkg/statistics/metrics.go b/pkg/statistics/metrics.go index b70a007886..f1fec3bc89 100644 --- a/pkg/statistics/metrics.go +++ b/pkg/statistics/metrics.go @@ -15,8 +15,6 @@ package statistics import ( - "strconv" - "github.com/pingcap/ticdc/pkg/config/kerneltype" "github.com/prometheus/client_golang/prometheus" ) @@ -98,7 +96,3 @@ func getKeyspaceLabel() string { } return "namespace" } - -func formatKeyspaceID(keyspaceID uint32) string { - return strconv.FormatUint(uint64(keyspaceID), 10) -} diff --git a/pkg/statistics/statistics.go b/pkg/statistics/statistics.go index 3ea04396f1..45a5daa094 100644 --- a/pkg/statistics/statistics.go +++ b/pkg/statistics/statistics.go @@ -15,6 +15,7 @@ package statistics import ( "fmt" + "strconv" "strings" "sync" "time" @@ -27,7 +28,7 @@ import ( func New(changefeed common.ChangeFeedID, keyspaceID uint32) *Statistics { statistics := &Statistics{ changefeedID: changefeed, - keyspaceID: formatKeyspaceID(keyspaceID), + keyspaceID: strconv.FormatUint(uint64(keyspaceID), 10), ddlTypes: sync.Map{}, rowsAffectedMap: sync.Map{}, } diff --git a/pkg/statistics/statistics_test.go b/pkg/statistics/statistics_test.go deleted file mode 100644 index e01e09d995..0000000000 --- a/pkg/statistics/statistics_test.go +++ /dev/null @@ -1,49 +0,0 @@ -// Copyright 2026 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, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package statistics - -import ( - "testing" - - "github.com/pingcap/ticdc/pkg/common" - "github.com/prometheus/client_golang/prometheus" - dto "github.com/prometheus/client_model/go" - "github.com/stretchr/testify/require" -) - -func TestExecBatchHistogramKeyspaceIDLabel(t *testing.T) { - const keyspaceID uint32 = 123 - changefeedID := common.NewChangefeedID4Test(t.Name()+"-keyspace", t.Name()+"-changefeed") - statistics := New( - changefeedID, - keyspaceID, - ) - t.Cleanup(statistics.Close) - require.NoError(t, statistics.RecordBatchExecution(func() (int, int64, error) { - return 2, 10, nil - })) - - labelValues := []string{changefeedID.Keyspace(), changefeedID.Name(), formatKeyspaceID(keyspaceID)} - observer, err := execBatchHistogram.GetMetricWithLabelValues(labelValues...) - require.NoError(t, err) - metric, ok := observer.(prometheus.Metric) - require.True(t, ok) - metricDTO := &dto.Metric{} - require.NoError(t, metric.Write(metricDTO)) - require.Equal(t, uint64(1), metricDTO.GetHistogram().GetSampleCount()) - - statistics.Close() - require.False(t, execBatchHistogram.DeleteLabelValues(labelValues...)) -} From 9cea971623449c9a4cf85420b4fe1f5d721db3f4 Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Thu, 30 Jul 2026 16:45:55 +0800 Subject: [PATCH 10/17] track dml event raw bytes in the statistics --- .../sink/cloudstorage/buffer_manager.go | 9 ++-- downstreamadapter/sink/cloudstorage/sink.go | 1 + downstreamadapter/sink/cloudstorage/task.go | 20 ++++----- downstreamadapter/sink/cloudstorage/writer.go | 5 +-- .../sink/cloudstorage/writer_test.go | 6 +-- downstreamadapter/sink/helper/mq_row_event.go | 3 -- downstreamadapter/sink/kafka/sink.go | 10 ++--- downstreamadapter/sink/pulsar/sink.go | 10 ++--- metrics/grafana/ticdc_new_arch.json | 2 +- .../ticdc_new_arch_next_gen.json | 2 +- .../ticdc_new_arch_with_keyspace_name.json | 2 +- pkg/common/event/row_change.go | 1 - pkg/sink/codec/encoder_group.go | 15 +++---- pkg/statistics/statistics.go | 11 +++++ pkg/statistics/statistics_test.go | 41 +++++++++++++++++++ 15 files changed, 84 insertions(+), 54 deletions(-) create mode 100644 pkg/statistics/statistics_test.go diff --git a/downstreamadapter/sink/cloudstorage/buffer_manager.go b/downstreamadapter/sink/cloudstorage/buffer_manager.go index be7d8232b4..fa49c404e1 100644 --- a/downstreamadapter/sink/cloudstorage/buffer_manager.go +++ b/downstreamadapter/sink/cloudstorage/buffer_manager.go @@ -177,10 +177,9 @@ type tableBatches struct { } type tableBatch struct { - size uint64 - approximateSize int64 - tableInfo *common.TableInfo - entries []*spool.Entry + size uint64 + tableInfo *common.TableInfo + entries []*spool.Entry } func newTableBatches() tableBatches { @@ -204,7 +203,6 @@ func (t *tableBatches) addEntry(event *task, entry *spool.Entry) { tableTask := t.tables[table] tableTask.size += entry.FileBytes() - tableTask.approximateSize += event.approximateSize tableTask.entries = append(tableTask.entries, entry) t.nBytes += entry.FileBytes() } @@ -287,7 +285,6 @@ func (b *payloadBuilder) Build() *payload { tableInfo: b.batch.tableInfo, data: b.buf.Bytes(), rowsCount: b.rowsCount, - approximateSize: b.batch.approximateSize, nBytes: b.nBytes, entries: b.batch.entries, postFlushCallbacks: b.postFlushCallbacks, diff --git a/downstreamadapter/sink/cloudstorage/sink.go b/downstreamadapter/sink/cloudstorage/sink.go index d67af4e21e..499438bf67 100644 --- a/downstreamadapter/sink/cloudstorage/sink.go +++ b/downstreamadapter/sink/cloudstorage/sink.go @@ -206,6 +206,7 @@ func (s *sink) AddDMLEvent(event *commonEvent.DMLEvent) { zap.String("dispatcher", event.GetDispatcherID().String())) return } + s.statistics.TrackDMLEvent(event) s.dmlWriters.addDMLEvent(event) } diff --git a/downstreamadapter/sink/cloudstorage/task.go b/downstreamadapter/sink/cloudstorage/task.go index 218785dd70..63ef6acd2b 100644 --- a/downstreamadapter/sink/cloudstorage/task.go +++ b/downstreamadapter/sink/cloudstorage/task.go @@ -38,12 +38,11 @@ type task struct { dispatcherID commonType.DispatcherID // DML-only fields. - postEnqueue func() // Transaction enqueue callback. - tableInfo *commonType.TableInfo // Table info used after event is released. - versionedTable cloudstorage.VersionedTableName // Versioned output identity for the DML event. - approximateSize int64 // Approximate size of the original DML event. - rowEvents []*commonEvent.RowEvent // Row events to encode and flush. - encodedMsgs []*common.Message // Encoded result built from event. + postEnqueue func() // Transaction enqueue callback. + tableInfo *commonType.TableInfo // Table info used after event is released. + versionedTable cloudstorage.VersionedTableName // Versioned output identity for the DML event. + rowEvents []*commonEvent.RowEvent // Row events to encode and flush. + encodedMsgs []*common.Message // Encoded result built from event. // Flush-only field. marker *flushMarker // Barrier marker used by FlushDMLBeforeBlock. @@ -56,11 +55,10 @@ func newDMLTask( ) *task { postEnqueue, postFlush := event.DetachPostCallbacks() return &task{ - kind: taskKindDML, - postEnqueue: postEnqueue, - tableInfo: event.TableInfo, - versionedTable: version, - approximateSize: event.GetSize(), + kind: taskKindDML, + postEnqueue: postEnqueue, + tableInfo: event.TableInfo, + versionedTable: version, // Storage txn encoders attach only the last row callback to the built // batch message, so the callback is triggered once per encoded txn // message. Kafka uses row-level callbacks and counts all rows before diff --git a/downstreamadapter/sink/cloudstorage/writer.go b/downstreamadapter/sink/cloudstorage/writer.go index 4a4cdf19a9..ed346c651f 100644 --- a/downstreamadapter/sink/cloudstorage/writer.go +++ b/downstreamadapter/sink/cloudstorage/writer.go @@ -63,7 +63,6 @@ type payload struct { tableInfo *common.TableInfo data []byte rowsCount int - approximateSize int64 nBytes int64 entries []*spool.Entry postFlushCallbacks []func() @@ -230,7 +229,7 @@ func (d *writer) writeDataFile(ctx context.Context, dataFilePath, indexFilePath if err != nil { return 0, 0, err } - return payload.rowsCount, payload.approximateSize, nil + return payload.rowsCount, 0, nil } writer, err := d.storage.Create(ctx, dataFilePath, &storeapi.WriterOption{ @@ -257,7 +256,7 @@ func (d *writer) writeDataFile(ctx context.Context, dataFilePath, indexFilePath zap.String("path", dataFilePath), zap.Error(err)) return 0, 0, err } - return payload.rowsCount, payload.approximateSize, nil + return payload.rowsCount, 0, nil }) if err != nil { return err diff --git a/downstreamadapter/sink/cloudstorage/writer_test.go b/downstreamadapter/sink/cloudstorage/writer_test.go index 4d3e259c45..4747ae8084 100644 --- a/downstreamadapter/sink/cloudstorage/writer_test.go +++ b/downstreamadapter/sink/cloudstorage/writer_test.go @@ -426,12 +426,8 @@ func TestWriterPostFlushDoesNotRunPausedPostEnqueue(t *testing.T) { defer spoolBuffer.Release(secondEntry) require.Equal(t, int64(0), secondEnqueued.Load()) - payload, err := buildPayload(spoolBuffer, &tableBatch{ - approximateSize: 123, - entries: []*spool.Entry{secondEntry}, - }) + payload, err := buildPayload(spoolBuffer, &tableBatch{entries: []*spool.Entry{secondEntry}}) require.NoError(t, err) - require.Equal(t, int64(123), payload.approximateSize) require.Len(t, payload.postFlushCallbacks, 1) for _, postFlushCallback := range payload.postFlushCallbacks { diff --git a/downstreamadapter/sink/helper/mq_row_event.go b/downstreamadapter/sink/helper/mq_row_event.go index 6dcdb5a73c..bbd8de207c 100644 --- a/downstreamadapter/sink/helper/mq_row_event.go +++ b/downstreamadapter/sink/helper/mq_row_event.go @@ -28,7 +28,6 @@ func NewMQRowEvents( ) ([]*commonEvent.MQRowEvent, error) { callback := NewPostFlushRowCallback(event, uint64(event.Len())) events := make([]*commonEvent.MQRowEvent, 0, event.Len()) - approximateSize := event.GetSize() if selector == nil { selector = columnselector.NewDefaultColumnSelector() } @@ -58,14 +57,12 @@ func NewMQRowEvents( TableInfo: event.TableInfo, StartTs: event.StartTs, CommitTs: event.CommitTs, - ApproximateSize: approximateSize, Event: row, Callback: callback, ColumnSelector: selector, Checksum: row.Checksum, }, }) - approximateSize = 0 } return events, nil } diff --git a/downstreamadapter/sink/kafka/sink.go b/downstreamadapter/sink/kafka/sink.go index 0807740d5e..436da77f33 100644 --- a/downstreamadapter/sink/kafka/sink.go +++ b/downstreamadapter/sink/kafka/sink.go @@ -317,6 +317,7 @@ func (s *sink) calculateKeyPartitions(ctx context.Context) error { zap.String("changefeed", s.changefeedID.Name())) return nil } + s.statistics.TrackDMLEvent(event) schema := event.TableInfo.GetSchemaName() table := event.TableInfo.GetTableName() topic := s.comp.eventRouter.GetTopicForRowChange(schema, table) @@ -444,16 +445,13 @@ func (s *sink) sendMessages(ctx context.Context) error { if err = future.Ready(ctx); err != nil { return err } - for i, message := range future.Messages { + for _, message := range future.Messages { start := time.Now() - rows, writeBytes := message.GetRowsCount(), int64(0) - if i == 0 { - writeBytes = future.ApproximateSize - } + rows := message.GetRowsCount() callback := message.Callback message.Callback = func() { _ = s.statistics.RecordBatchExecution(func() (int, int64, error) { - return rows, writeBytes, nil + return rows, 0, nil }) if callback != nil { callback() diff --git a/downstreamadapter/sink/pulsar/sink.go b/downstreamadapter/sink/pulsar/sink.go index 5c53add02a..d463c2ec8d 100644 --- a/downstreamadapter/sink/pulsar/sink.go +++ b/downstreamadapter/sink/pulsar/sink.go @@ -396,6 +396,7 @@ func (s *sink) calculateKeyPartitions(ctx context.Context) error { zap.String("changefeed", s.changefeedID.Name())) return nil } + s.statistics.TrackDMLEvent(event) schema := event.TableInfo.GetSchemaName() table := event.TableInfo.GetTableName() topic := s.comp.eventRouter.GetTopicForRowChange(schema, table) @@ -532,16 +533,13 @@ func (s *sink) sendMessages(ctx context.Context) error { if err = future.Ready(ctx); err != nil { return errors.Trace(err) } - for i, message := range future.Messages { + for _, message := range future.Messages { start := time.Now() - rows, writeBytes := message.GetRowsCount(), int64(0) - if i == 0 { - writeBytes = future.ApproximateSize - } + rows := message.GetRowsCount() callback := message.Callback message.Callback = func() { _ = s.statistics.RecordBatchExecution(func() (int, int64, error) { - return rows, writeBytes, nil + return rows, 0, nil }) if callback != nil { callback() diff --git a/metrics/grafana/ticdc_new_arch.json b/metrics/grafana/ticdc_new_arch.json index 9e94ea7444..2ed048ed14 100644 --- a/metrics/grafana/ticdc_new_arch.json +++ b/metrics/grafana/ticdc_new_arch.json @@ -28342,5 +28342,5 @@ "timezone": "browser", "title": "${DS_TEST-CLUSTER}-TiCDC-New-Arch", "uid": "YiGL8hBZ0aac", - "version": 41 + "version": 42 } diff --git a/metrics/nextgengrafana/ticdc_new_arch_next_gen.json b/metrics/nextgengrafana/ticdc_new_arch_next_gen.json index 9805d306c1..7b76420eca 100644 --- a/metrics/nextgengrafana/ticdc_new_arch_next_gen.json +++ b/metrics/nextgengrafana/ticdc_new_arch_next_gen.json @@ -28342,5 +28342,5 @@ "timezone": "browser", "title": "${DS_TEST-CLUSTER}-TiCDC-New-Arch", "uid": "YiGL8hBZ0aac", - "version": 41 + "version": 42 } diff --git a/metrics/nextgengrafana/ticdc_new_arch_with_keyspace_name.json b/metrics/nextgengrafana/ticdc_new_arch_with_keyspace_name.json index 7c24188a00..4a0a38e698 100644 --- a/metrics/nextgengrafana/ticdc_new_arch_with_keyspace_name.json +++ b/metrics/nextgengrafana/ticdc_new_arch_with_keyspace_name.json @@ -11749,5 +11749,5 @@ "timezone": "browser", "title": "${DS_TEST-CLUSTER}-TiCDC-New-Arch-KeyspaceName", "uid": "lGT5hED6vqTn", - "version": 41 + "version": 42 } diff --git a/pkg/common/event/row_change.go b/pkg/common/event/row_change.go index 11454e27a0..1d16eb358a 100644 --- a/pkg/common/event/row_change.go +++ b/pkg/common/event/row_change.go @@ -93,7 +93,6 @@ type RowEvent struct { TableInfo *common.TableInfo StartTs uint64 CommitTs uint64 - ApproximateSize int64 Event RowChange ColumnSelector Selector Callback func() diff --git a/pkg/sink/codec/encoder_group.go b/pkg/sink/codec/encoder_group.go index fa3a506463..eeeca21a20 100644 --- a/pkg/sink/codec/encoder_group.go +++ b/pkg/sink/codec/encoder_group.go @@ -216,25 +216,20 @@ func (g *encoderGroup) cleanMetrics() { // future is a wrapper of the result of encoding events // It's used to notify the caller that the result is ready. type future struct { - Key commonEvent.TopicPartitionKey - ApproximateSize int64 - events []*commonEvent.RowEvent - Messages []*common.Message - done chan struct{} + Key commonEvent.TopicPartitionKey + events []*commonEvent.RowEvent + Messages []*common.Message + done chan struct{} } func newFuture(key commonEvent.TopicPartitionKey, events ...*commonEvent.RowEvent, ) *future { - future := &future{ + return &future{ Key: key, events: events, done: make(chan struct{}), } - for _, event := range events { - future.ApproximateSize += event.ApproximateSize - } - return future } // Ready waits until the response is ready, should be called before consuming the future. diff --git a/pkg/statistics/statistics.go b/pkg/statistics/statistics.go index 45a5daa094..41ff89c80a 100644 --- a/pkg/statistics/statistics.go +++ b/pkg/statistics/statistics.go @@ -21,6 +21,7 @@ import ( "time" "github.com/pingcap/ticdc/pkg/common" + commonEvent "github.com/pingcap/ticdc/pkg/common/event" "github.com/prometheus/client_golang/prometheus" ) @@ -81,6 +82,16 @@ func (b *Statistics) RecordBatchExecution(executor func() (int, int64, error)) e return nil } +// TrackDMLEvent records the raw size of a DML event after the whole transaction +// has been flushed to downstream. The size is snapshotted here so the callback +// does not retain the event. +func (b *Statistics) TrackDMLEvent(event *commonEvent.DMLEvent) { + writeBytes := event.GetSize() + event.PushFrontFlushFunc(func() { + b.metricTotalWriteBytesCnt.Add(float64(writeBytes)) + }) +} + // RecordDDLExecution record the time cost of execute ddl func (b *Statistics) RecordDDLExecution(executor func() (string, error)) error { b.metricExecDDLRunningCnt.Inc() diff --git a/pkg/statistics/statistics_test.go b/pkg/statistics/statistics_test.go new file mode 100644 index 0000000000..0209ecdf32 --- /dev/null +++ b/pkg/statistics/statistics_test.go @@ -0,0 +1,41 @@ +// Copyright 2026 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, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package statistics + +import ( + "testing" + + "github.com/pingcap/ticdc/pkg/common" + commonEvent "github.com/pingcap/ticdc/pkg/common/event" + "github.com/prometheus/client_golang/prometheus/testutil" + "github.com/stretchr/testify/require" +) + +func TestTrackDMLEventRecordsRawBytesOnPostFlush(t *testing.T) { + changefeedID := common.NewChangefeedID4Test("test", t.Name()) + statistics := New(changefeedID, common.DefaultKeyspaceID) + t.Cleanup(statistics.Close) + + event := commonEvent.NewDMLEvent(common.NewDispatcherID(), 1, 1, 2, nil) + event.ApproximateSize = 123 + statistics.TrackDMLEvent(event) + + require.Zero(t, testutil.ToFloat64(statistics.metricTotalWriteBytesCnt)) + + // The tracked size is a snapshot, so the callback does not retain the event. + event.ApproximateSize = 456 + event.PostFlush() + require.Equal(t, float64(123), testutil.ToFloat64(statistics.metricTotalWriteBytesCnt)) +} From faee63ae9e0791a4d04e8ae225ce30d4ed7e7e77 Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Thu, 30 Jul 2026 17:50:59 +0800 Subject: [PATCH 11/17] track dml event raw bytes in the statistics --- downstreamadapter/sink/blackhole/sink.go | 5 +++-- downstreamadapter/sink/cloudstorage/writer.go | 14 +++++++------- downstreamadapter/sink/kafka/sink.go | 10 +++++----- downstreamadapter/sink/mysql/sink.go | 1 + downstreamadapter/sink/pulsar/sink.go | 10 +++++----- pkg/sink/mysql/mysql_writer.go | 4 ++-- pkg/sink/mysql/mysql_writer_dml_exec.go | 6 +++--- pkg/statistics/statistics.go | 8 ++++---- pkg/statistics/statistics_test.go | 3 +++ 9 files changed, 33 insertions(+), 28 deletions(-) diff --git a/downstreamadapter/sink/blackhole/sink.go b/downstreamadapter/sink/blackhole/sink.go index 2f05104f0e..599fd61006 100644 --- a/downstreamadapter/sink/blackhole/sink.go +++ b/downstreamadapter/sink/blackhole/sink.go @@ -54,6 +54,7 @@ func (s *Sink) AddDMLEvent(event *commonEvent.DMLEvent) { // ref: https://github.com/pingcap/ticdc/blob/da834db76e0662ff15ef12645d1f37bfa6506d83/tests/integration_tests/lossy_ddl/run.sh#L23 // Use zap.Stringer to call String() method which applies log redaction log.Debug("BlackHoleSink: WriteEvents", zap.Stringer("dml", event)) + s.statistics.TrackDMLEvent(event) s.eventCh.Push(event) } @@ -104,8 +105,8 @@ func (s *Sink) Run(ctx context.Context) error { log.Info("blackhole sink event channel closed") return nil } - err := s.statistics.RecordBatchExecution(func() (int, int64, error) { - return int(event.Len()), event.GetSize(), nil + err := s.statistics.RecordBatchExecution(func() (int, error) { + return int(event.Len()), nil }) if err != nil { return err diff --git a/downstreamadapter/sink/cloudstorage/writer.go b/downstreamadapter/sink/cloudstorage/writer.go index ed346c651f..79630a41a5 100644 --- a/downstreamadapter/sink/cloudstorage/writer.go +++ b/downstreamadapter/sink/cloudstorage/writer.go @@ -223,20 +223,20 @@ func (d *writer) writeDataFile(ctx context.Context, dataFilePath, indexFilePath changefeed := d.changeFeedID.Name() start := time.Now() - err := d.statistics.RecordBatchExecution(func() (int, int64, error) { + err := d.statistics.RecordBatchExecution(func() (int, error) { if d.config.FlushConcurrency <= 1 { err := d.storage.WriteFile(ctx, dataFilePath, payload.data) if err != nil { - return 0, 0, err + return 0, err } - return payload.rowsCount, 0, nil + return payload.rowsCount, nil } writer, err := d.storage.Create(ctx, dataFilePath, &storeapi.WriterOption{ Concurrency: d.config.FlushConcurrency, }) if err != nil { - return 0, 0, err + return 0, err } _, err = writer.Write(ctx, payload.data) @@ -247,16 +247,16 @@ func (d *writer) writeDataFile(ctx context.Context, dataFilePath, indexFilePath zap.String("keyspace", keyspace), zap.String("changefeed", changefeed), zap.String("path", dataFilePath), zap.Error(closeErr)) } - return 0, 0, err + return 0, err } if err = writer.Close(ctx); err != nil { log.Error("failed to close concurrency writer", zap.String("keyspace", keyspace), zap.String("changefeed", changefeed), zap.String("path", dataFilePath), zap.Error(err)) - return 0, 0, err + return 0, err } - return payload.rowsCount, 0, nil + return payload.rowsCount, nil }) if err != nil { return err diff --git a/downstreamadapter/sink/kafka/sink.go b/downstreamadapter/sink/kafka/sink.go index 436da77f33..544472f960 100644 --- a/downstreamadapter/sink/kafka/sink.go +++ b/downstreamadapter/sink/kafka/sink.go @@ -245,6 +245,7 @@ func (s *sink) IsNormal() bool { } func (s *sink) AddDMLEvent(event *commonEvent.DMLEvent) { + s.statistics.TrackDMLEvent(event) s.eventChan.Push(event) } @@ -317,7 +318,6 @@ func (s *sink) calculateKeyPartitions(ctx context.Context) error { zap.String("changefeed", s.changefeedID.Name())) return nil } - s.statistics.TrackDMLEvent(event) schema := event.TableInfo.GetSchemaName() table := event.TableInfo.GetTableName() topic := s.comp.eventRouter.GetTopicForRowChange(schema, table) @@ -450,8 +450,8 @@ func (s *sink) sendMessages(ctx context.Context) error { rows := message.GetRowsCount() callback := message.Callback message.Callback = func() { - _ = s.statistics.RecordBatchExecution(func() (int, int64, error) { - return rows, 0, nil + _ = s.statistics.RecordBatchExecution(func() (int, error) { + return rows, nil }) if callback != nil { callback() @@ -469,8 +469,8 @@ func (s *sink) sendMessages(ctx context.Context) error { zap.String("keyspace", s.changefeedID.Keyspace()), zap.String("changefeed", s.changefeedID.Name()), zap.Error(err)) - return s.statistics.RecordBatchExecution(func() (int, int64, error) { - return 0, 0, err + return s.statistics.RecordBatchExecution(func() (int, error) { + return 0, err }) } metricSendMessageDuration.Observe(time.Since(start).Seconds()) diff --git a/downstreamadapter/sink/mysql/sink.go b/downstreamadapter/sink/mysql/sink.go index 442cdf7091..c233fca7ef 100644 --- a/downstreamadapter/sink/mysql/sink.go +++ b/downstreamadapter/sink/mysql/sink.go @@ -321,6 +321,7 @@ func (s *Sink) SetTableSchemaStore(tableSchemaStore *commonEvent.TableSchemaStor } func (s *Sink) AddDMLEvent(event *commonEvent.DMLEvent) { + s.statistics.TrackDMLEvent(event) s.conflictDetector.Add(event) } diff --git a/downstreamadapter/sink/pulsar/sink.go b/downstreamadapter/sink/pulsar/sink.go index d463c2ec8d..c408132cdc 100644 --- a/downstreamadapter/sink/pulsar/sink.go +++ b/downstreamadapter/sink/pulsar/sink.go @@ -198,6 +198,7 @@ func (s *sink) IsNormal() bool { } func (s *sink) AddDMLEvent(event *commonEvent.DMLEvent) { + s.statistics.TrackDMLEvent(event) s.eventChan.Push(event) } @@ -396,7 +397,6 @@ func (s *sink) calculateKeyPartitions(ctx context.Context) error { zap.String("changefeed", s.changefeedID.Name())) return nil } - s.statistics.TrackDMLEvent(event) schema := event.TableInfo.GetSchemaName() table := event.TableInfo.GetTableName() topic := s.comp.eventRouter.GetTopicForRowChange(schema, table) @@ -538,8 +538,8 @@ func (s *sink) sendMessages(ctx context.Context) error { rows := message.GetRowsCount() callback := message.Callback message.Callback = func() { - _ = s.statistics.RecordBatchExecution(func() (int, int64, error) { - return rows, 0, nil + _ = s.statistics.RecordBatchExecution(func() (int, error) { + return rows, nil }) if callback != nil { callback() @@ -548,8 +548,8 @@ func (s *sink) sendMessages(ctx context.Context) error { message.SetPartitionKey(future.Key.PartitionKey) if err = s.dmlProducer.asyncSendMessage(ctx, future.Key.Topic, message); err != nil { - return s.statistics.RecordBatchExecution(func() (int, int64, error) { - return 0, 0, errors.Trace(err) + return s.statistics.RecordBatchExecution(func() (int, error) { + return 0, errors.Trace(err) }) } metricSendMessageDuration.Observe(time.Since(start).Seconds()) diff --git a/pkg/sink/mysql/mysql_writer.go b/pkg/sink/mysql/mysql_writer.go index f2d9078a05..adb09abf54 100644 --- a/pkg/sink/mysql/mysql_writer.go +++ b/pkg/sink/mysql/mysql_writer.go @@ -239,8 +239,8 @@ func (w *Writer) Flush(events []*commonEvent.DMLEvent) error { } else { w.tryDryRunBlock() - err = w.statistics.RecordBatchExecution(func() (int, int64, error) { - return dmls.rowCount, dmls.approximateSize, nil + err = w.statistics.RecordBatchExecution(func() (int, error) { + return dmls.rowCount, nil }) } diff --git a/pkg/sink/mysql/mysql_writer_dml_exec.go b/pkg/sink/mysql/mysql_writer_dml_exec.go index 7b949b811d..9467fbf9ad 100644 --- a/pkg/sink/mysql/mysql_writer_dml_exec.go +++ b/pkg/sink/mysql/mysql_writer_dml_exec.go @@ -47,7 +47,7 @@ func (w *Writer) execDMLWithMaxRetries(dmls *preparedDMLs) error { writeTimeout, _ := time.ParseDuration(w.cfg.WriteTimeout) writeTimeout += networkDriftDuration - tryExec := func() (int, int64, error) { + tryExec := func() (int, error) { start := time.Now() defer func() { if time.Since(start) > w.cfg.SlowQuery { @@ -90,9 +90,9 @@ func (w *Writer) execDMLWithMaxRetries(dmls *preparedDMLs) error { return nil }) if err != nil { - return 0, 0, err + return 0, err } - return dmls.rowCount, dmls.approximateSize, nil + return dmls.rowCount, nil } return retry.Do(w.ctx, func() error { failpoint.Inject("MySQLSinkTxnRandomError", func() { diff --git a/pkg/statistics/statistics.go b/pkg/statistics/statistics.go index 41ff89c80a..0376f59501 100644 --- a/pkg/statistics/statistics.go +++ b/pkg/statistics/statistics.go @@ -70,15 +70,15 @@ type Statistics struct { metricExecErrCntForDML prometheus.Counter } -// RecordBatchExecution stats batch executors which return (batchRowCount, batchWriteBytes, error). -func (b *Statistics) RecordBatchExecution(executor func() (int, int64, error)) error { - batchSize, batchWriteBytes, err := executor() +// RecordBatchExecution records batch rows and execution errors. +// Raw bytes are recorded by TrackDMLEvent after the transaction is flushed. +func (b *Statistics) RecordBatchExecution(executor func() (int, error)) error { + batchSize, err := executor() if err != nil { b.metricExecErrCntForDML.Inc() return err } b.metricExecBatchHis.Observe(float64(batchSize)) - b.metricTotalWriteBytesCnt.Add(float64(batchWriteBytes)) return nil } diff --git a/pkg/statistics/statistics_test.go b/pkg/statistics/statistics_test.go index 0209ecdf32..e32b434439 100644 --- a/pkg/statistics/statistics_test.go +++ b/pkg/statistics/statistics_test.go @@ -31,6 +31,9 @@ func TestTrackDMLEventRecordsRawBytesOnPostFlush(t *testing.T) { event := commonEvent.NewDMLEvent(common.NewDispatcherID(), 1, 1, 2, nil) event.ApproximateSize = 123 statistics.TrackDMLEvent(event) + require.NoError(t, statistics.RecordBatchExecution(func() (int, error) { + return int(event.Len()), nil + })) require.Zero(t, testutil.ToFloat64(statistics.metricTotalWriteBytesCnt)) From 232108ce0e290264c4862ac550fb1cd6b96c1e2c Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Thu, 30 Jul 2026 18:08:04 +0800 Subject: [PATCH 12/17] refactor the statistics method --- downstreamadapter/sink/blackhole/sink.go | 4 ++-- downstreamadapter/sink/cloudstorage/writer.go | 14 +++++++------- downstreamadapter/sink/kafka/sink.go | 8 ++++---- downstreamadapter/sink/pulsar/sink.go | 8 ++++---- pkg/sink/mysql/mysql_writer.go | 4 ++-- pkg/sink/mysql/mysql_writer_dml_exec.go | 8 ++++---- pkg/statistics/statistics.go | 6 +++--- pkg/statistics/statistics_test.go | 4 ++-- 8 files changed, 28 insertions(+), 28 deletions(-) diff --git a/downstreamadapter/sink/blackhole/sink.go b/downstreamadapter/sink/blackhole/sink.go index 599fd61006..ec81ebc9af 100644 --- a/downstreamadapter/sink/blackhole/sink.go +++ b/downstreamadapter/sink/blackhole/sink.go @@ -105,8 +105,8 @@ func (s *Sink) Run(ctx context.Context) error { log.Info("blackhole sink event channel closed") return nil } - err := s.statistics.RecordBatchExecution(func() (int, error) { - return int(event.Len()), nil + err := s.statistics.RecordBatchExecution(int(event.Len()), func() error { + return nil }) if err != nil { return err diff --git a/downstreamadapter/sink/cloudstorage/writer.go b/downstreamadapter/sink/cloudstorage/writer.go index 79630a41a5..1f16678da6 100644 --- a/downstreamadapter/sink/cloudstorage/writer.go +++ b/downstreamadapter/sink/cloudstorage/writer.go @@ -223,20 +223,20 @@ func (d *writer) writeDataFile(ctx context.Context, dataFilePath, indexFilePath changefeed := d.changeFeedID.Name() start := time.Now() - err := d.statistics.RecordBatchExecution(func() (int, error) { + err := d.statistics.RecordBatchExecution(payload.rowsCount, func() error { if d.config.FlushConcurrency <= 1 { err := d.storage.WriteFile(ctx, dataFilePath, payload.data) if err != nil { - return 0, err + return err } - return payload.rowsCount, nil + return nil } writer, err := d.storage.Create(ctx, dataFilePath, &storeapi.WriterOption{ Concurrency: d.config.FlushConcurrency, }) if err != nil { - return 0, err + return err } _, err = writer.Write(ctx, payload.data) @@ -247,16 +247,16 @@ func (d *writer) writeDataFile(ctx context.Context, dataFilePath, indexFilePath zap.String("keyspace", keyspace), zap.String("changefeed", changefeed), zap.String("path", dataFilePath), zap.Error(closeErr)) } - return 0, err + return err } if err = writer.Close(ctx); err != nil { log.Error("failed to close concurrency writer", zap.String("keyspace", keyspace), zap.String("changefeed", changefeed), zap.String("path", dataFilePath), zap.Error(err)) - return 0, err + return err } - return payload.rowsCount, nil + return nil }) if err != nil { return err diff --git a/downstreamadapter/sink/kafka/sink.go b/downstreamadapter/sink/kafka/sink.go index 544472f960..9e3da9b619 100644 --- a/downstreamadapter/sink/kafka/sink.go +++ b/downstreamadapter/sink/kafka/sink.go @@ -450,8 +450,8 @@ func (s *sink) sendMessages(ctx context.Context) error { rows := message.GetRowsCount() callback := message.Callback message.Callback = func() { - _ = s.statistics.RecordBatchExecution(func() (int, error) { - return rows, nil + _ = s.statistics.RecordBatchExecution(rows, func() error { + return nil }) if callback != nil { callback() @@ -469,8 +469,8 @@ func (s *sink) sendMessages(ctx context.Context) error { zap.String("keyspace", s.changefeedID.Keyspace()), zap.String("changefeed", s.changefeedID.Name()), zap.Error(err)) - return s.statistics.RecordBatchExecution(func() (int, error) { - return 0, err + return s.statistics.RecordBatchExecution(rows, func() error { + return err }) } metricSendMessageDuration.Observe(time.Since(start).Seconds()) diff --git a/downstreamadapter/sink/pulsar/sink.go b/downstreamadapter/sink/pulsar/sink.go index c408132cdc..b9b1e5c886 100644 --- a/downstreamadapter/sink/pulsar/sink.go +++ b/downstreamadapter/sink/pulsar/sink.go @@ -538,8 +538,8 @@ func (s *sink) sendMessages(ctx context.Context) error { rows := message.GetRowsCount() callback := message.Callback message.Callback = func() { - _ = s.statistics.RecordBatchExecution(func() (int, error) { - return rows, nil + _ = s.statistics.RecordBatchExecution(rows, func() error { + return nil }) if callback != nil { callback() @@ -548,8 +548,8 @@ func (s *sink) sendMessages(ctx context.Context) error { message.SetPartitionKey(future.Key.PartitionKey) if err = s.dmlProducer.asyncSendMessage(ctx, future.Key.Topic, message); err != nil { - return s.statistics.RecordBatchExecution(func() (int, error) { - return 0, errors.Trace(err) + return s.statistics.RecordBatchExecution(rows, func() error { + return errors.Trace(err) }) } metricSendMessageDuration.Observe(time.Since(start).Seconds()) diff --git a/pkg/sink/mysql/mysql_writer.go b/pkg/sink/mysql/mysql_writer.go index adb09abf54..41b562cb4a 100644 --- a/pkg/sink/mysql/mysql_writer.go +++ b/pkg/sink/mysql/mysql_writer.go @@ -239,8 +239,8 @@ func (w *Writer) Flush(events []*commonEvent.DMLEvent) error { } else { w.tryDryRunBlock() - err = w.statistics.RecordBatchExecution(func() (int, error) { - return dmls.rowCount, nil + err = w.statistics.RecordBatchExecution(dmls.rowCount, func() error { + return nil }) } diff --git a/pkg/sink/mysql/mysql_writer_dml_exec.go b/pkg/sink/mysql/mysql_writer_dml_exec.go index 9467fbf9ad..c048fbc46a 100644 --- a/pkg/sink/mysql/mysql_writer_dml_exec.go +++ b/pkg/sink/mysql/mysql_writer_dml_exec.go @@ -47,7 +47,7 @@ func (w *Writer) execDMLWithMaxRetries(dmls *preparedDMLs) error { writeTimeout, _ := time.ParseDuration(w.cfg.WriteTimeout) writeTimeout += networkDriftDuration - tryExec := func() (int, error) { + tryExec := func() error { start := time.Now() defer func() { if time.Since(start) > w.cfg.SlowQuery { @@ -90,9 +90,9 @@ func (w *Writer) execDMLWithMaxRetries(dmls *preparedDMLs) error { return nil }) if err != nil { - return 0, err + return err } - return dmls.rowCount, nil + return nil } return retry.Do(w.ctx, func() error { failpoint.Inject("MySQLSinkTxnRandomError", func() { @@ -117,7 +117,7 @@ func (w *Writer) execDMLWithMaxRetries(dmls *preparedDMLs) error { failpoint.Return(err) }) - err := w.statistics.RecordBatchExecution(tryExec) + err := w.statistics.RecordBatchExecution(dmls.rowCount, tryExec) if err != nil { return errors.Trace(w.logDMLTxnErr(err, time.Now(), w.ChangefeedID.String(), dmls)) } diff --git a/pkg/statistics/statistics.go b/pkg/statistics/statistics.go index 0376f59501..ecf2707a0f 100644 --- a/pkg/statistics/statistics.go +++ b/pkg/statistics/statistics.go @@ -70,10 +70,10 @@ type Statistics struct { metricExecErrCntForDML prometheus.Counter } -// RecordBatchExecution records batch rows and execution errors. +// RecordBatchExecution records batch rows for successful executions and counts errors. // Raw bytes are recorded by TrackDMLEvent after the transaction is flushed. -func (b *Statistics) RecordBatchExecution(executor func() (int, error)) error { - batchSize, err := executor() +func (b *Statistics) RecordBatchExecution(batchSize int, executor func() error) error { + err := executor() if err != nil { b.metricExecErrCntForDML.Inc() return err diff --git a/pkg/statistics/statistics_test.go b/pkg/statistics/statistics_test.go index e32b434439..57d8819e21 100644 --- a/pkg/statistics/statistics_test.go +++ b/pkg/statistics/statistics_test.go @@ -31,8 +31,8 @@ func TestTrackDMLEventRecordsRawBytesOnPostFlush(t *testing.T) { event := commonEvent.NewDMLEvent(common.NewDispatcherID(), 1, 1, 2, nil) event.ApproximateSize = 123 statistics.TrackDMLEvent(event) - require.NoError(t, statistics.RecordBatchExecution(func() (int, error) { - return int(event.Len()), nil + require.NoError(t, statistics.RecordBatchExecution(int(event.Len()), func() error { + return nil })) require.Zero(t, testutil.ToFloat64(statistics.metricTotalWriteBytesCnt)) From fb82feae65f99b757f3fdb1b9dca71af47a87b54 Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Thu, 30 Jul 2026 19:38:26 +0800 Subject: [PATCH 13/17] remove useless test --- pkg/statistics/statistics_test.go | 44 ------------------------------- 1 file changed, 44 deletions(-) delete mode 100644 pkg/statistics/statistics_test.go diff --git a/pkg/statistics/statistics_test.go b/pkg/statistics/statistics_test.go deleted file mode 100644 index 57d8819e21..0000000000 --- a/pkg/statistics/statistics_test.go +++ /dev/null @@ -1,44 +0,0 @@ -// Copyright 2026 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, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package statistics - -import ( - "testing" - - "github.com/pingcap/ticdc/pkg/common" - commonEvent "github.com/pingcap/ticdc/pkg/common/event" - "github.com/prometheus/client_golang/prometheus/testutil" - "github.com/stretchr/testify/require" -) - -func TestTrackDMLEventRecordsRawBytesOnPostFlush(t *testing.T) { - changefeedID := common.NewChangefeedID4Test("test", t.Name()) - statistics := New(changefeedID, common.DefaultKeyspaceID) - t.Cleanup(statistics.Close) - - event := commonEvent.NewDMLEvent(common.NewDispatcherID(), 1, 1, 2, nil) - event.ApproximateSize = 123 - statistics.TrackDMLEvent(event) - require.NoError(t, statistics.RecordBatchExecution(int(event.Len()), func() error { - return nil - })) - - require.Zero(t, testutil.ToFloat64(statistics.metricTotalWriteBytesCnt)) - - // The tracked size is a snapshot, so the callback does not retain the event. - event.ApproximateSize = 456 - event.PostFlush() - require.Equal(t, float64(123), testutil.ToFloat64(statistics.metricTotalWriteBytesCnt)) -} From 85aba48dc69bd1dc5fca12ebcdbc1a87d53af694 Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Fri, 31 Jul 2026 15:41:15 +0800 Subject: [PATCH 14/17] update tests and simplify code --- downstreamadapter/sink/blackhole/sink.go | 7 +- downstreamadapter/sink/cloudstorage/writer.go | 71 ++++++++++--------- downstreamadapter/sink/kafka/sink.go | 9 +-- downstreamadapter/sink/kafka/sink_test.go | 25 +++++-- .../sink/pulsar/mock_producer.go | 16 ++++- downstreamadapter/sink/pulsar/sink.go | 10 ++- downstreamadapter/sink/pulsar/sink_test.go | 24 +++++-- metrics/grafana/ticdc_new_arch.json | 1 + .../ticdc_new_arch_next_gen.json | 1 + .../ticdc_new_arch_with_keyspace_name.json | 1 + pkg/sink/mysql/mysql_writer.go | 4 +- pkg/sink/mysql/mysql_writer_dml_exec.go | 3 +- pkg/statistics/metrics.go | 2 +- pkg/statistics/statistics.go | 20 +++--- 14 files changed, 115 insertions(+), 79 deletions(-) diff --git a/downstreamadapter/sink/blackhole/sink.go b/downstreamadapter/sink/blackhole/sink.go index ec81ebc9af..6934b3b73d 100644 --- a/downstreamadapter/sink/blackhole/sink.go +++ b/downstreamadapter/sink/blackhole/sink.go @@ -105,12 +105,7 @@ func (s *Sink) Run(ctx context.Context) error { log.Info("blackhole sink event channel closed") return nil } - err := s.statistics.RecordBatchExecution(int(event.Len()), func() error { - return nil - }) - if err != nil { - return err - } + s.statistics.RecordDMLResult(int(event.Len()), nil) event.PostFlush() } } diff --git a/downstreamadapter/sink/cloudstorage/writer.go b/downstreamadapter/sink/cloudstorage/writer.go index 1f16678da6..727b9f11d3 100644 --- a/downstreamadapter/sink/cloudstorage/writer.go +++ b/downstreamadapter/sink/cloudstorage/writer.go @@ -223,41 +223,8 @@ func (d *writer) writeDataFile(ctx context.Context, dataFilePath, indexFilePath changefeed := d.changeFeedID.Name() start := time.Now() - err := d.statistics.RecordBatchExecution(payload.rowsCount, func() error { - if d.config.FlushConcurrency <= 1 { - err := d.storage.WriteFile(ctx, dataFilePath, payload.data) - if err != nil { - return err - } - return nil - } - - writer, err := d.storage.Create(ctx, dataFilePath, &storeapi.WriterOption{ - Concurrency: d.config.FlushConcurrency, - }) - if err != nil { - return err - } - - _, err = writer.Write(ctx, payload.data) - if err != nil { - closeErr := writer.Close(ctx) - if closeErr != nil { - log.Warn("failed to close writer after write failure", - zap.String("keyspace", keyspace), zap.String("changefeed", changefeed), - zap.String("path", dataFilePath), zap.Error(closeErr)) - } - return err - } - - if err = writer.Close(ctx); err != nil { - log.Error("failed to close concurrency writer", - zap.String("keyspace", keyspace), zap.String("changefeed", changefeed), - zap.String("path", dataFilePath), zap.Error(err)) - return err - } - return nil - }) + err := d.writeData(ctx, dataFilePath, payload.data) + d.statistics.RecordDMLResult(payload.rowsCount, err) if err != nil { return err } @@ -289,6 +256,40 @@ func (d *writer) writeDataFile(ctx context.Context, dataFilePath, indexFilePath return nil } +func (d *writer) writeData(ctx context.Context, dataFilePath string, data []byte) error { + if d.config.FlushConcurrency <= 1 { + return d.storage.WriteFile(ctx, dataFilePath, data) + } + + writer, err := d.storage.Create(ctx, dataFilePath, &storeapi.WriterOption{ + Concurrency: d.config.FlushConcurrency, + }) + if err != nil { + return err + } + + _, err = writer.Write(ctx, data) + if err != nil { + closeErr := writer.Close(ctx) + if closeErr != nil { + log.Warn("failed to close writer after write failure", + zap.String("keyspace", d.changeFeedID.Keyspace()), + zap.String("changefeed", d.changeFeedID.Name()), + zap.String("path", dataFilePath), zap.Error(closeErr)) + } + return err + } + + if err = writer.Close(ctx); err != nil { + log.Error("failed to close concurrency writer", + zap.String("keyspace", d.changeFeedID.Keyspace()), + zap.String("changefeed", d.changeFeedID.Name()), + zap.String("path", dataFilePath), zap.Error(err)) + return err + } + return nil +} + func (d *writer) enqueueTask(ctx context.Context, t *task) error { return d.bufferManager.enqueueTask(ctx, t) } diff --git a/downstreamadapter/sink/kafka/sink.go b/downstreamadapter/sink/kafka/sink.go index f3dee94cda..4eba0c3a46 100644 --- a/downstreamadapter/sink/kafka/sink.go +++ b/downstreamadapter/sink/kafka/sink.go @@ -434,9 +434,7 @@ func (s *sink) sendMessages(ctx context.Context) error { rows := message.GetRowsCount() callback := message.Callback message.Callback = func() { - _ = s.statistics.RecordBatchExecution(rows, func() error { - return nil - }) + s.statistics.RecordDMLResult(rows, nil) if callback != nil { callback() } @@ -448,9 +446,8 @@ func (s *sink) sendMessages(ctx context.Context) error { future.Key.Topic, future.Key.Partition, message); err != nil { - return s.statistics.RecordBatchExecution(rows, func() error { - return err - }) + s.statistics.RecordDMLResult(rows, err) + return err } metricSendMessageDuration.Observe(time.Since(start).Seconds()) } diff --git a/downstreamadapter/sink/kafka/sink_test.go b/downstreamadapter/sink/kafka/sink_test.go index 8b205c1649..90cdd94c31 100644 --- a/downstreamadapter/sink/kafka/sink_test.go +++ b/downstreamadapter/sink/kafka/sink_test.go @@ -265,7 +265,7 @@ func TestKafkaSinkBasicFunctionality(t *testing.T) { job := helper.DDL2Job(createTableSQL) require.NotNil(t, job) - var count atomic.Int64 + var count, dmlFlushCount atomic.Int64 ddlEvent := &commonEvent.DDLEvent{ Query: job.Query, SchemaName: job.SchemaName, @@ -302,7 +302,10 @@ func TestKafkaSinkBasicFunctionality(t *testing.T) { "insert into t values (1, 'test')", "insert into t values (2, 'test2');") dmlEvent.PostTxnFlushed = []func(){ - func() { count.Add(1) }, + func() { + count.Add(1) + dmlFlushCount.Add(1) + }, } dmlEvent.CommitTs = 2 @@ -310,6 +313,7 @@ func TestKafkaSinkBasicFunctionality(t *testing.T) { ctrl := gomock.NewController(t) asyncProducer := kafka.NewMockAsyncProducer(ctrl) syncProducer := kafka.NewMockSyncProducer(ctrl) + ackCh := make(chan func(), 2) asyncProducer.EXPECT().AsyncRunCallback(gomock.Any()).Return(nil).AnyTimes() asyncProducer.EXPECT().AsyncSend(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). DoAndReturn(func( @@ -318,9 +322,7 @@ func TestKafkaSinkBasicFunctionality(t *testing.T) { _ int32, message *codecCommon.Message, ) error { - if message.Callback != nil { - message.Callback() - } + ackCh <- message.Callback return nil }).Times(2) asyncProducer.EXPECT().Close().AnyTimes() @@ -337,6 +339,19 @@ func TestKafkaSinkBasicFunctionality(t *testing.T) { kafkaSink.AddDMLEvent(dmlEvent) + require.Eventually(t, func() bool { + return len(ackCh) == 2 + }, 5*time.Second, 10*time.Millisecond) + require.Zero(t, dmlFlushCount.Load()) + + (<-ackCh)() + require.Zero(t, dmlFlushCount.Load()) + + (<-ackCh)() + require.Eventually(t, func() bool { + return dmlFlushCount.Load() == 1 + }, 5*time.Second, 10*time.Millisecond) + ddlEvent2.PostFlush() require.Eventually(t, diff --git a/downstreamadapter/sink/pulsar/mock_producer.go b/downstreamadapter/sink/pulsar/mock_producer.go index a71d6a1247..31cf213790 100644 --- a/downstreamadapter/sink/pulsar/mock_producer.go +++ b/downstreamadapter/sink/pulsar/mock_producer.go @@ -30,6 +30,8 @@ var ( type mockProducer struct { mu sync.Mutex events map[string][]*pulsar.ProducerMessage + // ackCh lets tests delay callbacks until they simulate a successful broker ack. + ackCh chan func() } func newMockDDLProducer() ddlProducer { @@ -74,12 +76,18 @@ func (p *mockProducer) GetProducerByTopic(_ string) (producer pulsar.Producer, e func (p *mockProducer) asyncSendMessage(_ context.Context, topic string, message *common.Message, ) error { p.mu.Lock() - defer p.mu.Unlock() data := &pulsar.ProducerMessage{ Payload: message.Value, Key: message.GetPartitionKey(), } p.events[topic] = append(p.events[topic], data) + ackCh := p.ackCh + p.mu.Unlock() + + if ackCh != nil { + ackCh <- message.Callback + return nil + } if message.Callback != nil { message.Callback() } @@ -93,11 +101,15 @@ func (m *mockProducer) run(_ context.Context) error { // Close close all producers func (p *mockProducer) close() { + p.mu.Lock() + defer p.mu.Unlock() p.events = make(map[string][]*pulsar.ProducerMessage) } // GetAllEvents returns the events received by the mock producer. func (p *mockProducer) GetAllEvents() []*pulsar.ProducerMessage { + p.mu.Lock() + defer p.mu.Unlock() var events []*pulsar.ProducerMessage for _, v := range p.events { events = append(events, v...) @@ -107,5 +119,7 @@ func (p *mockProducer) GetAllEvents() []*pulsar.ProducerMessage { // GetEvents returns the event filtered by the key. func (p *mockProducer) GetEvents(topic string) []*pulsar.ProducerMessage { + p.mu.Lock() + defer p.mu.Unlock() return p.events[topic] } diff --git a/downstreamadapter/sink/pulsar/sink.go b/downstreamadapter/sink/pulsar/sink.go index b9b1e5c886..103c5e5383 100644 --- a/downstreamadapter/sink/pulsar/sink.go +++ b/downstreamadapter/sink/pulsar/sink.go @@ -538,9 +538,7 @@ func (s *sink) sendMessages(ctx context.Context) error { rows := message.GetRowsCount() callback := message.Callback message.Callback = func() { - _ = s.statistics.RecordBatchExecution(rows, func() error { - return nil - }) + s.statistics.RecordDMLResult(rows, nil) if callback != nil { callback() } @@ -548,9 +546,9 @@ func (s *sink) sendMessages(ctx context.Context) error { message.SetPartitionKey(future.Key.PartitionKey) if err = s.dmlProducer.asyncSendMessage(ctx, future.Key.Topic, message); err != nil { - return s.statistics.RecordBatchExecution(rows, func() error { - return errors.Trace(err) - }) + err = errors.Trace(err) + s.statistics.RecordDMLResult(rows, err) + return err } metricSendMessageDuration.Observe(time.Since(start).Seconds()) } diff --git a/downstreamadapter/sink/pulsar/sink_test.go b/downstreamadapter/sink/pulsar/sink_test.go index 543fc43846..2f1c46890b 100644 --- a/downstreamadapter/sink/pulsar/sink_test.go +++ b/downstreamadapter/sink/pulsar/sink_test.go @@ -74,7 +74,7 @@ func TestPulsarSinkBasicFunctionality(t *testing.T) { pulsarSink, err := newPulsarSinkForTest(t) require.NoError(t, err) - var count atomic.Int64 + var count, dmlFlushCount atomic.Int64 helper := commonEvent.NewEventTestHelper(t) defer helper.Close() @@ -116,19 +116,35 @@ func TestPulsarSinkBasicFunctionality(t *testing.T) { dmlEvent := helper.DML2Event("test", "t", "insert into t values (1, 'test')", "insert into t values (2, 'test2');") dmlEvent.PostTxnFlushed = []func(){ - func() { count.Add(1) }, + func() { + count.Add(1) + dmlFlushCount.Add(1) + }, } dmlEvent.CommitTs = 2 + producer := pulsarSink.dmlProducer.(*mockProducer) + producer.ackCh = make(chan func(), 2) err = pulsarSink.WriteBlockEvent(ddlEvent) require.NoError(t, err) pulsarSink.AddDMLEvent(dmlEvent) - time.Sleep(1 * time.Second) + require.Eventually(t, func() bool { + return len(producer.ackCh) == 2 + }, 5*time.Second, 10*time.Millisecond) + require.Zero(t, dmlFlushCount.Load()) + + (<-producer.ackCh)() + require.Zero(t, dmlFlushCount.Load()) + + (<-producer.ackCh)() + require.Eventually(t, func() bool { + return dmlFlushCount.Load() == 1 + }, 5*time.Second, 10*time.Millisecond) ddlEvent2.PostFlush() - require.Len(t, pulsarSink.dmlProducer.(*mockProducer).GetAllEvents(), 2) + require.Len(t, producer.GetAllEvents(), 2) require.Len(t, pulsarSink.ddlProducer.(*mockProducer).GetAllEvents(), 1) require.Equal(t, count.Load(), int64(3)) diff --git a/metrics/grafana/ticdc_new_arch.json b/metrics/grafana/ticdc_new_arch.json index 2ed048ed14..a7ef6a1d22 100644 --- a/metrics/grafana/ticdc_new_arch.json +++ b/metrics/grafana/ticdc_new_arch.json @@ -630,6 +630,7 @@ "dashLength": 10, "dashes": false, "datasource": "${DS_TEST-CLUSTER}", + "description": "Approximate raw-entry bytes of DML events successfully written to downstream.", "fieldConfig": { "defaults": {}, "overrides": [] diff --git a/metrics/nextgengrafana/ticdc_new_arch_next_gen.json b/metrics/nextgengrafana/ticdc_new_arch_next_gen.json index 7b76420eca..6d709b5ad9 100644 --- a/metrics/nextgengrafana/ticdc_new_arch_next_gen.json +++ b/metrics/nextgengrafana/ticdc_new_arch_next_gen.json @@ -630,6 +630,7 @@ "dashLength": 10, "dashes": false, "datasource": "${DS_TEST-CLUSTER}", + "description": "Approximate raw-entry bytes of DML events successfully written to downstream.", "fieldConfig": { "defaults": {}, "overrides": [] diff --git a/metrics/nextgengrafana/ticdc_new_arch_with_keyspace_name.json b/metrics/nextgengrafana/ticdc_new_arch_with_keyspace_name.json index 4a0a38e698..1432b9b26b 100644 --- a/metrics/nextgengrafana/ticdc_new_arch_with_keyspace_name.json +++ b/metrics/nextgengrafana/ticdc_new_arch_with_keyspace_name.json @@ -441,6 +441,7 @@ "dashLength": 10, "dashes": false, "datasource": "${DS_TEST-CLUSTER}", + "description": "Approximate raw-entry bytes of DML events successfully written to downstream.", "fieldConfig": { "defaults": {}, "overrides": [] diff --git a/pkg/sink/mysql/mysql_writer.go b/pkg/sink/mysql/mysql_writer.go index 41b562cb4a..028846f3e8 100644 --- a/pkg/sink/mysql/mysql_writer.go +++ b/pkg/sink/mysql/mysql_writer.go @@ -239,9 +239,7 @@ func (w *Writer) Flush(events []*commonEvent.DMLEvent) error { } else { w.tryDryRunBlock() - err = w.statistics.RecordBatchExecution(dmls.rowCount, func() error { - return nil - }) + w.statistics.RecordDMLResult(dmls.rowCount, nil) } if err != nil { diff --git a/pkg/sink/mysql/mysql_writer_dml_exec.go b/pkg/sink/mysql/mysql_writer_dml_exec.go index c048fbc46a..72de1c052e 100644 --- a/pkg/sink/mysql/mysql_writer_dml_exec.go +++ b/pkg/sink/mysql/mysql_writer_dml_exec.go @@ -117,7 +117,8 @@ func (w *Writer) execDMLWithMaxRetries(dmls *preparedDMLs) error { failpoint.Return(err) }) - err := w.statistics.RecordBatchExecution(dmls.rowCount, tryExec) + err := tryExec() + w.statistics.RecordDMLResult(dmls.rowCount, err) if err != nil { return errors.Trace(w.logDMLTxnErr(err, time.Now(), w.ChangefeedID.String(), dmls)) } diff --git a/pkg/statistics/metrics.go b/pkg/statistics/metrics.go index f1fec3bc89..b6dfa0b69f 100644 --- a/pkg/statistics/metrics.go +++ b/pkg/statistics/metrics.go @@ -59,7 +59,7 @@ var ( Namespace: "ticdc", Subsystem: "sink", Name: "write_bytes_total", - Help: "Total number of bytes written by sink", + Help: "Total approximate raw bytes of DML events successfully written to downstream.", }, []string{getKeyspaceLabel(), "changefeed"}) execDMLEventRowsAffectedCounter = prometheus.NewCounterVec( diff --git a/pkg/statistics/statistics.go b/pkg/statistics/statistics.go index ecf2707a0f..195978153f 100644 --- a/pkg/statistics/statistics.go +++ b/pkg/statistics/statistics.go @@ -70,24 +70,22 @@ type Statistics struct { metricExecErrCntForDML prometheus.Counter } -// RecordBatchExecution records batch rows for successful executions and counts errors. -// Raw bytes are recorded by TrackDMLEvent after the transaction is flushed. -func (b *Statistics) RecordBatchExecution(batchSize int, executor func() error) error { - err := executor() +// RecordDMLResult records row counts for successful DML executions and counts errors. +// DML event bytes are recorded by TrackDMLEvent after the transaction is flushed. +func (b *Statistics) RecordDMLResult(rowCount int, err error) { if err != nil { b.metricExecErrCntForDML.Inc() - return err + return } - b.metricExecBatchHis.Observe(float64(batchSize)) - return nil + b.metricExecBatchHis.Observe(float64(rowCount)) } -// TrackDMLEvent records the raw size of a DML event after the whole transaction -// has been flushed to downstream. The size is snapshotted here so the callback -// does not retain the event. +// TrackDMLEvent records the approximate size reported by a DML event after the +// whole transaction has been flushed to downstream. The size is snapshotted +// here so the callback does not retain the event. func (b *Statistics) TrackDMLEvent(event *commonEvent.DMLEvent) { writeBytes := event.GetSize() - event.PushFrontFlushFunc(func() { + event.AddPostFlushFunc(func() { b.metricTotalWriteBytesCnt.Add(float64(writeBytes)) }) } From e4824f37e6526e9195d40eced0999bf8cbd3f7e4 Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Fri, 31 Jul 2026 18:54:50 +0800 Subject: [PATCH 15/17] adjust cloud storage sink code --- downstreamadapter/sink/cloudstorage/writer.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/downstreamadapter/sink/cloudstorage/writer.go b/downstreamadapter/sink/cloudstorage/writer.go index 727b9f11d3..55e7b96066 100644 --- a/downstreamadapter/sink/cloudstorage/writer.go +++ b/downstreamadapter/sink/cloudstorage/writer.go @@ -218,13 +218,13 @@ func (d *writer) discardEntries(entries []*spool.Entry) { } } -func (d *writer) writeDataFile(ctx context.Context, dataFilePath, indexFilePath string, payload *payload) error { - keyspace := d.changeFeedID.Keyspace() - changefeed := d.changeFeedID.Name() - start := time.Now() +func (d *writer) writeDataFile(ctx context.Context, dataFilePath, indexFilePath string, payload *payload) (err error) { + defer func() { + d.statistics.RecordDMLResult(payload.rowsCount, err) + }() - err := d.writeData(ctx, dataFilePath, payload.data) - d.statistics.RecordDMLResult(payload.rowsCount, err) + start := time.Now() + err = d.writeData(ctx, dataFilePath, payload.data) if err != nil { return err } @@ -232,8 +232,8 @@ func (d *writer) writeDataFile(ctx context.Context, dataFilePath, indexFilePath err = d.storage.WriteFile(ctx, indexFilePath, []byte(path.Base(dataFilePath)+"\n")) if err != nil { log.Error("failed to write index file to external storage", - zap.String("keyspace", keyspace), - zap.String("changefeed", changefeed), + zap.String("keyspace", d.changeFeedID.Keyspace()), + zap.String("changefeed", d.changeFeedID.Name()), zap.String("path", indexFilePath), zap.Int("shardID", d.shardID), zap.Error(err)) From 3bfa658909b17c00c5a3b3cbee525549ee8d1b1d Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Mon, 3 Aug 2026 14:43:03 +0800 Subject: [PATCH 16/17] simplify the code further --- downstreamadapter/sink/kafka/helper.go | 4 +- downstreamadapter/sink/kafka/sink.go | 22 +++------ downstreamadapter/sink/kafka/sink_test.go | 4 +- downstreamadapter/sink/pulsar/dml_producer.go | 20 +++++++- downstreamadapter/sink/pulsar/helper.go | 3 ++ downstreamadapter/sink/pulsar/sink.go | 11 +---- pkg/sink/kafka/sarama_async_producer.go | 47 ++++++++++++++++--- pkg/sink/kafka/sarama_factory.go | 25 ++++++++++ pkg/statistics/statistics.go | 6 ++- 9 files changed, 105 insertions(+), 37 deletions(-) diff --git a/downstreamadapter/sink/kafka/helper.go b/downstreamadapter/sink/kafka/helper.go index 37327548d4..b307c9ba7f 100644 --- a/downstreamadapter/sink/kafka/helper.go +++ b/downstreamadapter/sink/kafka/helper.go @@ -27,6 +27,7 @@ import ( 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/statistics" "github.com/pingcap/tidb/br/pkg/utils" ) @@ -58,6 +59,7 @@ func newKafkaSinkComponent( changefeedID common.ChangeFeedID, sinkURI *url.URL, sinkConfig *config.SinkConfig, + stat *statistics.Statistics, ) (components, config.Protocol, error) { var ( comp components @@ -85,7 +87,7 @@ func newKafkaSinkComponent( } options.Topic = topic - comp.factory, err = kafka.NewSaramaFactory(ctx, options, changefeedID) + comp.factory, err = kafka.NewSaramaFactoryWithStatistics(ctx, options, changefeedID, stat) if err != nil { return comp, protocol, err } diff --git a/downstreamadapter/sink/kafka/sink.go b/downstreamadapter/sink/kafka/sink.go index 4eba0c3a46..478e2b2a62 100644 --- a/downstreamadapter/sink/kafka/sink.go +++ b/downstreamadapter/sink/kafka/sink.go @@ -157,11 +157,13 @@ func Verify(ctx context.Context, changefeedID common.ChangeFeedID, uri *url.URL, func New( ctx context.Context, changefeedID common.ChangeFeedID, sinkURI *url.URL, sinkConfig *config.SinkConfig, keyspaceID uint32, ) (*sink, error) { - comp, protocol, err := newKafkaSinkComponent(ctx, changefeedID, sinkURI, sinkConfig) + stat := statistics.New(changefeedID, keyspaceID) + comp, protocol, err := newKafkaSinkComponent(ctx, changefeedID, sinkURI, sinkConfig, stat) if err != nil { + stat.Close() return nil, err } - return newWithComponents(ctx, changefeedID, keyspaceID, protocol, comp) + return newWithComponents(ctx, changefeedID, keyspaceID, protocol, comp, stat) } func newWithComponents( @@ -170,8 +172,8 @@ func newWithComponents( keyspaceID uint32, protocol config.Protocol, comp components, + stat *statistics.Statistics, ) (*sink, error) { - statistics := statistics.New(changefeedID, keyspaceID) var ( err error asyncProducer kafka.AsyncProducer @@ -188,7 +190,7 @@ func newWithComponents( asyncProducer.Close() } comp.close() - statistics.Close() + stat.Close() }() asyncProducer, err = comp.factory.AsyncProducer(ctx) @@ -209,7 +211,7 @@ func newWithComponents( partitionRule: helper.GetDDLDispatchRule(protocol), protocol: protocol, comp: comp, - statistics: statistics, + statistics: stat, checkpointChan: make(chan uint64, 16), eventChan: chann.NewUnlimitedChannelDefault[*commonEvent.DMLEvent](), @@ -431,22 +433,12 @@ func (s *sink) sendMessages(ctx context.Context) error { } for _, message := range future.Messages { start := time.Now() - rows := message.GetRowsCount() - callback := message.Callback - message.Callback = func() { - s.statistics.RecordDMLResult(rows, nil) - if callback != nil { - callback() - } - } - message.SetPartitionKey(future.Key.PartitionKey) if err = s.dmlProducer.AsyncSend( ctx, future.Key.Topic, future.Key.Partition, message); err != nil { - s.statistics.RecordDMLResult(rows, err) return err } metricSendMessageDuration.Observe(time.Since(start).Seconds()) diff --git a/downstreamadapter/sink/kafka/sink_test.go b/downstreamadapter/sink/kafka/sink_test.go index 90cdd94c31..eb27f84435 100644 --- a/downstreamadapter/sink/kafka/sink_test.go +++ b/downstreamadapter/sink/kafka/sink_test.go @@ -35,6 +35,7 @@ import ( "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/statistics" "github.com/stretchr/testify/require" "go.uber.org/atomic" ) @@ -228,7 +229,8 @@ func newKafkaSinkForTestWithProducers(ctx context.Context, } }() - s, err := newWithComponents(ctx, changefeedID, common.DefaultKeyspaceID, protocol, comp) + s, err := newWithComponents(ctx, changefeedID, common.DefaultKeyspaceID, protocol, comp, + statistics.New(changefeedID, common.DefaultKeyspaceID)) if err != nil { return nil, err } diff --git a/downstreamadapter/sink/pulsar/dml_producer.go b/downstreamadapter/sink/pulsar/dml_producer.go index f311ed7935..afebea277f 100644 --- a/downstreamadapter/sink/pulsar/dml_producer.go +++ b/downstreamadapter/sink/pulsar/dml_producer.go @@ -27,6 +27,7 @@ import ( "github.com/pingcap/ticdc/pkg/errors" "github.com/pingcap/ticdc/pkg/sink/codec/common" "github.com/pingcap/ticdc/pkg/sink/pulsar" + "github.com/pingcap/ticdc/pkg/statistics" "go.uber.org/zap" ) @@ -52,6 +53,8 @@ type dmlProducers struct { producers *lru.Cache comp component + // statistics is owned and closed by the sink. + statistics *statistics.Statistics // closedMu is used to protect `closed`. // We need to ensure that closed producers are never written to. @@ -104,6 +107,7 @@ func newDMLProducers( p := &dmlProducers{ changefeedID: changefeedID, comp: comp, + statistics: comp.statistics, producers: producers, closed: false, failpointCh: failpointCh, @@ -138,14 +142,18 @@ func (p *dmlProducers) asyncSendMessage( // If producers are closed, we should skip the message and return an error. if p.closed { - return errors.ErrPulsarProducerClosed.GenWithStackByArgs() + err := errors.ErrPulsarProducerClosed.GenWithStackByArgs() + p.recordDMLResult(message.GetRowsCount(), err) + return err } failpoint.Inject("PulsarSinkAsyncSendError", func() { // simulate sending message to input channel successfully but flushing // message to Pulsar meets error log.Info("PulsarSinkAsyncSendError error injected", zap.String("keyspace", p.changefeedID.Keyspace()), zap.String("changefeed", p.changefeedID.ID().String())) - p.failpointCh <- errors.New("pulsar sink injected error") + err := errors.New("pulsar sink injected error") + p.recordDMLResult(message.GetRowsCount(), err) + p.failpointCh <- err failpoint.Return(nil) }) data := &pulsarClient.ProducerMessage{ @@ -155,6 +163,7 @@ func (p *dmlProducers) asyncSendMessage( producer, err := p.getProducerByTopic(topic) if err != nil { + p.recordDMLResult(message.GetRowsCount(), err) return err } @@ -162,6 +171,7 @@ func (p *dmlProducers) asyncSendMessage( producer.SendAsync(ctx, data, func(_ pulsarClient.MessageID, m *pulsarClient.ProducerMessage, err error) { + p.recordDMLResult(message.GetRowsCount(), err) // fail if err != nil { e := errors.WrapError(errors.ErrPulsarAsyncSendMessage, err) @@ -194,6 +204,12 @@ func (p *dmlProducers) asyncSendMessage( return nil } +func (p *dmlProducers) recordDMLResult(rowCount int, err error) { + if p.statistics != nil { + p.statistics.RecordDMLResult(rowCount, err) + } +} + func (p *dmlProducers) close() { // We have to hold the lock to synchronize closing with writing. if p == nil { return diff --git a/downstreamadapter/sink/pulsar/helper.go b/downstreamadapter/sink/pulsar/helper.go index b2676b1daa..1e5a19816b 100644 --- a/downstreamadapter/sink/pulsar/helper.go +++ b/downstreamadapter/sink/pulsar/helper.go @@ -29,6 +29,7 @@ import ( "github.com/pingcap/ticdc/pkg/sink/codec" codecCommon "github.com/pingcap/ticdc/pkg/sink/codec/common" "github.com/pingcap/ticdc/pkg/sink/pulsar" + "github.com/pingcap/ticdc/pkg/statistics" putil "github.com/pingcap/ticdc/pkg/util" "go.uber.org/zap" ) @@ -41,6 +42,8 @@ type component struct { eventRouter *eventrouter.EventRouter topicManager topicmanager.TopicManager client pulsarClient.Client + // statistics is a construction dependency owned by the sink. + statistics *statistics.Statistics } func (c component) close() { diff --git a/downstreamadapter/sink/pulsar/sink.go b/downstreamadapter/sink/pulsar/sink.go index 103c5e5383..ad2b0dfa98 100644 --- a/downstreamadapter/sink/pulsar/sink.go +++ b/downstreamadapter/sink/pulsar/sink.go @@ -152,6 +152,7 @@ func newWithComponent( failpointCh := make(chan error, 1) stat = statistics.New(changefeedID, keyspaceID) + comp.statistics = stat dmlProducer, err = newDMLProducer(changefeedID, comp, failpointCh) if err != nil { return nil, err @@ -535,19 +536,9 @@ func (s *sink) sendMessages(ctx context.Context) error { } for _, message := range future.Messages { start := time.Now() - rows := message.GetRowsCount() - callback := message.Callback - message.Callback = func() { - s.statistics.RecordDMLResult(rows, nil) - if callback != nil { - callback() - } - } - message.SetPartitionKey(future.Key.PartitionKey) if err = s.dmlProducer.asyncSendMessage(ctx, future.Key.Topic, message); err != nil { err = errors.Trace(err) - s.statistics.RecordDMLResult(rows, err) return err } metricSendMessageDuration.Observe(time.Since(start).Seconds()) diff --git a/pkg/sink/kafka/sarama_async_producer.go b/pkg/sink/kafka/sarama_async_producer.go index d3e1781c71..0edc5b5fd4 100644 --- a/pkg/sink/kafka/sarama_async_producer.go +++ b/pkg/sink/kafka/sarama_async_producer.go @@ -22,6 +22,7 @@ import ( "github.com/pingcap/ticdc/pkg/common" "github.com/pingcap/ticdc/pkg/errors" codecCommon "github.com/pingcap/ticdc/pkg/sink/codec/common" + "github.com/pingcap/ticdc/pkg/statistics" "go.uber.org/atomic" "go.uber.org/zap" ) @@ -30,11 +31,13 @@ type saramaAsyncProducer struct { client sarama.Client producer sarama.AsyncProducer changefeedID common.ChangeFeedID + statistics *statistics.Statistics closed *atomic.Bool } type messageMetadata struct { + rowCount int callback func() logInfo *codecCommon.MessageLogInfo } @@ -102,8 +105,11 @@ func (p *saramaAsyncProducer) AsyncRunCallback( if ack != nil { switch meta := ack.Metadata.(type) { case *messageMetadata: - if meta != nil && meta.callback != nil { - meta.callback() + if meta != nil { + p.recordDMLResult(meta.rowCount, nil) + if meta.callback != nil { + meta.callback() + } } default: log.Error("kafka producer received unknown message metadata type", @@ -119,7 +125,9 @@ func (p *saramaAsyncProducer) AsyncRunCallback( if err == nil { return nil } - return p.handleProducerError(err) + producerErr := p.handleProducerError(err) + p.recordDMLResult(extractRowCount(err.Msg), producerErr) + return producerErr } } } @@ -140,9 +148,12 @@ func (p *saramaAsyncProducer) AsyncSend( ctx context.Context, topic string, partition int32, message *codecCommon.Message, ) error { if p.closed.Load() { - return errors.ErrKafkaSinkClosed.GenWithStackByArgs() + err := errors.ErrKafkaSinkClosed.GenWithStackByArgs() + p.recordDMLResult(message.GetRowsCount(), err) + return err } meta := &messageMetadata{ + rowCount: message.GetRowsCount(), callback: message.Callback, logInfo: message.LogInfo, } @@ -155,13 +166,37 @@ func (p *saramaAsyncProducer) AsyncSend( } select { case <-ctx.Done(): - return context.Cause(ctx) + err := context.Cause(ctx) + p.recordDMLResult(message.GetRowsCount(), err) + return err case p.producer.Input() <- msg: } return nil } +func (p *saramaAsyncProducer) recordDMLResult(rowCount int, err error) { + if p.statistics != nil { + p.statistics.RecordDMLResult(rowCount, err) + } +} + +func extractRowCount(msg *sarama.ProducerMessage) int { + meta := extractMessageMetadata(msg) + if meta == nil { + return 0 + } + return meta.rowCount +} + func extractLogInfo(msg *sarama.ProducerMessage) *codecCommon.MessageLogInfo { + meta := extractMessageMetadata(msg) + if meta == nil { + return nil + } + return meta.logInfo +} + +func extractMessageMetadata(msg *sarama.ProducerMessage) *messageMetadata { if msg == nil { return nil } @@ -169,5 +204,5 @@ func extractLogInfo(msg *sarama.ProducerMessage) *codecCommon.MessageLogInfo { if !ok || meta == nil { return nil } - return meta.logInfo + return meta } diff --git a/pkg/sink/kafka/sarama_factory.go b/pkg/sink/kafka/sarama_factory.go index 8f73ca70b5..54bc73de35 100644 --- a/pkg/sink/kafka/sarama_factory.go +++ b/pkg/sink/kafka/sarama_factory.go @@ -21,6 +21,7 @@ import ( "github.com/pingcap/log" "github.com/pingcap/ticdc/pkg/common" "github.com/pingcap/ticdc/pkg/errors" + "github.com/pingcap/ticdc/pkg/statistics" "github.com/rcrowley/go-metrics" "go.uber.org/atomic" "go.uber.org/zap" @@ -30,6 +31,7 @@ type saramaFactory struct { changefeedID common.ChangeFeedID option *options metricRegistry metrics.Registry + statistics *statistics.Statistics } // NewSaramaFactory constructs a Factory with sarama implementation. @@ -37,6 +39,27 @@ func NewSaramaFactory( ctx context.Context, o *options, changefeedID common.ChangeFeedID, +) (Factory, error) { + return newSaramaFactory(ctx, o, changefeedID, nil) +} + +// NewSaramaFactoryWithStatistics constructs a Factory for a running sink. +// The factory passes statistics to its DML producer without changing the +// producer interface. The sink remains responsible for closing statistics. +func NewSaramaFactoryWithStatistics( + ctx context.Context, + o *options, + changefeedID common.ChangeFeedID, + stat *statistics.Statistics, +) (Factory, error) { + return newSaramaFactory(ctx, o, changefeedID, stat) +} + +func newSaramaFactory( + ctx context.Context, + o *options, + changefeedID common.ChangeFeedID, + stat *statistics.Statistics, ) (Factory, error) { start := time.Now() config, err := newSaramaConfig(ctx, o) @@ -80,6 +103,7 @@ func NewSaramaFactory( changefeedID: changefeedID, option: o, metricRegistry: metrics.NewRegistry(), + statistics: stat, }, nil } @@ -178,6 +202,7 @@ func (f *saramaFactory) AsyncProducer(ctx context.Context) (AsyncProducer, error client: client, producer: p, changefeedID: f.changefeedID, + statistics: f.statistics, closed: atomic.NewBool(false), }, nil } diff --git a/pkg/statistics/statistics.go b/pkg/statistics/statistics.go index 195978153f..f4c065e705 100644 --- a/pkg/statistics/statistics.go +++ b/pkg/statistics/statistics.go @@ -70,8 +70,10 @@ type Statistics struct { metricExecErrCntForDML prometheus.Counter } -// RecordDMLResult records row counts for successful DML executions and counts errors. -// DML event bytes are recorded by TrackDMLEvent after the transaction is flushed. +// RecordDMLResult records the result of one downstream DML execution attempt. +// Successful attempts contribute their row count; failed attempts increment the +// DML execution error counter. DML event bytes are tracked separately because +// they are recorded only after the whole transaction is flushed. func (b *Statistics) RecordDMLResult(rowCount int, err error) { if err != nil { b.metricExecErrCntForDML.Inc() From 2bb3f91604d68b9824f71f9c65761df3bcf84744 Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Mon, 3 Aug 2026 15:17:08 +0800 Subject: [PATCH 17/17] further adjus the code --- downstreamadapter/sink/kafka/helper.go | 2 +- downstreamadapter/sink/kafka/sink.go | 2 +- downstreamadapter/sink/pulsar/dml_producer.go | 28 +++-- .../sink/pulsar/dml_producer_test.go | 116 +++++++++++++++--- pkg/sink/kafka/sarama_async_producer.go | 44 ++++--- pkg/sink/kafka/sarama_async_producer_test.go | 110 +++++++++++++++++ pkg/sink/kafka/sarama_factory.go | 22 +--- pkg/sink/mysql/affected_rows.go | 92 ++++++++++++++ pkg/sink/mysql/mysql_writer.go | 7 ++ pkg/sink/mysql/mysql_writer_dml_exec.go | 18 ++- pkg/statistics/metrics.go | 9 -- pkg/statistics/statistics.go | 47 +------ pkg/statistics/statistics_test.go | 82 +++++++++++++ server/metrics.go | 2 + 14 files changed, 458 insertions(+), 123 deletions(-) create mode 100644 pkg/sink/kafka/sarama_async_producer_test.go create mode 100644 pkg/sink/mysql/affected_rows.go create mode 100644 pkg/statistics/statistics_test.go diff --git a/downstreamadapter/sink/kafka/helper.go b/downstreamadapter/sink/kafka/helper.go index b307c9ba7f..7cf1c34f3e 100644 --- a/downstreamadapter/sink/kafka/helper.go +++ b/downstreamadapter/sink/kafka/helper.go @@ -87,7 +87,7 @@ func newKafkaSinkComponent( } options.Topic = topic - comp.factory, err = kafka.NewSaramaFactoryWithStatistics(ctx, options, changefeedID, stat) + comp.factory, err = kafka.NewSaramaFactory(ctx, options, changefeedID, stat) if err != nil { return comp, protocol, err } diff --git a/downstreamadapter/sink/kafka/sink.go b/downstreamadapter/sink/kafka/sink.go index 478e2b2a62..912f30fedb 100644 --- a/downstreamadapter/sink/kafka/sink.go +++ b/downstreamadapter/sink/kafka/sink.go @@ -112,7 +112,7 @@ func Verify(ctx context.Context, changefeedID common.ChangeFeedID, uri *url.URL, return err } - factory, err := kafka.NewSaramaFactory(ctx, options, changefeedID) + factory, err := kafka.NewSaramaFactory(ctx, options, changefeedID, nil) if err != nil { return err } diff --git a/downstreamadapter/sink/pulsar/dml_producer.go b/downstreamadapter/sink/pulsar/dml_producer.go index afebea277f..e010bb3957 100644 --- a/downstreamadapter/sink/pulsar/dml_producer.go +++ b/downstreamadapter/sink/pulsar/dml_producer.go @@ -142,9 +142,7 @@ func (p *dmlProducers) asyncSendMessage( // If producers are closed, we should skip the message and return an error. if p.closed { - err := errors.ErrPulsarProducerClosed.GenWithStackByArgs() - p.recordDMLResult(message.GetRowsCount(), err) - return err + return p.handleSendFailure(message, errors.ErrPulsarProducerClosed.GenWithStackByArgs()) } failpoint.Inject("PulsarSinkAsyncSendError", func() { // simulate sending message to input channel successfully but flushing @@ -152,7 +150,7 @@ func (p *dmlProducers) asyncSendMessage( log.Info("PulsarSinkAsyncSendError error injected", zap.String("keyspace", p.changefeedID.Keyspace()), zap.String("changefeed", p.changefeedID.ID().String())) err := errors.New("pulsar sink injected error") - p.recordDMLResult(message.GetRowsCount(), err) + p.handleSendFailure(message, err) p.failpointCh <- err failpoint.Return(nil) }) @@ -163,15 +161,14 @@ func (p *dmlProducers) asyncSendMessage( producer, err := p.getProducerByTopic(topic) if err != nil { - p.recordDMLResult(message.GetRowsCount(), err) - return err + return p.handleSendFailure(message, err) } // if for stress test record , add count to message callback function producer.SendAsync(ctx, data, func(_ pulsarClient.MessageID, m *pulsarClient.ProducerMessage, err error) { - p.recordDMLResult(message.GetRowsCount(), err) + p.handleAsyncSendResult(message, err) // fail if err != nil { e := errors.WrapError(errors.ErrPulsarAsyncSendMessage, err) @@ -194,7 +191,6 @@ func (p *dmlProducers) asyncSendMessage( } } else if message.Callback != nil { // success - message.Callback() pulsar.IncPublishedDMLSuccess(topic, p.changefeedID.String()) } }) @@ -210,6 +206,22 @@ func (p *dmlProducers) recordDMLResult(rowCount int, err error) { } } +// handleAsyncSendResult records the result of an asynchronous send and runs +// the message callback on success. It only touches ticdc-owned types so the +// statistics behavior can be unit-tested without any broker machinery. +func (p *dmlProducers) handleAsyncSendResult(message *common.Message, err error) { + p.recordDMLResult(message.GetRowsCount(), err) + if err == nil && message.Callback != nil { + message.Callback() + } +} + +// handleSendFailure records a failed send attempt and returns the error. +func (p *dmlProducers) handleSendFailure(message *common.Message, err error) error { + p.recordDMLResult(message.GetRowsCount(), err) + return err +} + func (p *dmlProducers) close() { // We have to hold the lock to synchronize closing with writing. if p == nil { return diff --git a/downstreamadapter/sink/pulsar/dml_producer_test.go b/downstreamadapter/sink/pulsar/dml_producer_test.go index d2f03a415f..14ed5de095 100644 --- a/downstreamadapter/sink/pulsar/dml_producer_test.go +++ b/downstreamadapter/sink/pulsar/dml_producer_test.go @@ -1,4 +1,4 @@ -// Copyright 2023 PingCAP, Inc. +// Copyright 2026 PingCAP, Inc. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -8,29 +8,117 @@ // // Unless required by applicable law or agreed to in writing, software // distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. package pulsar import ( - "context" + "errors" "testing" - "github.com/pingcap/ticdc/pkg/sink/codec/common" - "github.com/pingcap/ticdc/pkg/util" + "github.com/pingcap/ticdc/pkg/common" + codecCommon "github.com/pingcap/ticdc/pkg/sink/codec/common" + "github.com/pingcap/ticdc/pkg/statistics" + "github.com/prometheus/client_golang/prometheus" + dto "github.com/prometheus/client_model/go" "github.com/stretchr/testify/require" ) -func TestPulsarSyncAsyncSendMessage(t *testing.T) { - t.Parallel() - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - p := newMockDMLProducer() - err := p.asyncSendMessage(ctx, "test", &common.Message{ - Value: []byte("this value for test input data"), - PartitionKey: util.AddressOf("test_key"), - }) +func gatherMetric( + t *testing.T, reg *prometheus.Registry, name string, labelValues ...string, +) *dto.Metric { + t.Helper() + require.Lenf(t, labelValues, len(labelValues)&^1, "labelValues must be key/value pairs") + mfs, err := reg.Gather() require.NoError(t, err) + for _, mf := range mfs { + if mf.GetName() != name { + continue + } + for _, m := range mf.GetMetric() { + matched := true + for i := 0; i < len(labelValues); i += 2 { + found := false + for _, lp := range m.GetLabel() { + if lp.GetName() == labelValues[i] && lp.GetValue() == labelValues[i+1] { + found = true + break + } + } + if !found { + matched = false + break + } + } + if matched { + return m + } + } + } + return nil +} + +func newTestMessage(rows int, callback func()) *codecCommon.Message { + message := &codecCommon.Message{Key: []byte("k"), Value: []byte("v")} + message.SetRowsCount(rows) + message.Callback = callback + return message +} + +func newTestDMLProducers(t *testing.T, changefeed string) (*dmlProducers, *prometheus.Registry) { + t.Helper() + reg := prometheus.NewRegistry() + statistics.InitMetrics(reg) + stat := statistics.New(common.NewChangefeedID4Test("test-keyspace", changefeed), 123) + t.Cleanup(stat.Close) + return &dmlProducers{statistics: stat}, reg +} + +func TestHandleAsyncSendResultSuccess(t *testing.T) { + p, reg := newTestDMLProducers(t, "async-send-success") + + callbackCalled := make(chan struct{}) + message := newTestMessage(2, func() { close(callbackCalled) }) + p.handleAsyncSendResult(message, nil) + + <-callbackCalled + // The row count is observed into the batch histogram on success. + hist := gatherMetric(t, reg, "ticdc_sink_batch_row_count", + "namespace", "test-keyspace", "changefeed", "async-send-success") + require.NotNil(t, hist) + require.Equal(t, uint64(1), hist.GetHistogram().GetSampleCount()) + require.Equal(t, float64(2), hist.GetHistogram().GetSampleSum()) +} + +func TestHandleAsyncSendResultErrorSkipsCallback(t *testing.T) { + p, reg := newTestDMLProducers(t, "async-send-error") + + message := newTestMessage(4, func() { t.Fatal("callback must not run on failure") }) + p.handleAsyncSendResult(message, errors.New("broker boom")) + + errMetric := gatherMetric(t, reg, "ticdc_sink_execution_error", + "namespace", "test-keyspace", "changefeed", "async-send-error", "event_type", "dml") + require.NotNil(t, errMetric) + require.Equal(t, float64(1), errMetric.GetCounter().GetValue()) + // No rows are observed for failed attempts; the histogram series exists + // (created eagerly by New) but has no samples. + hist := gatherMetric(t, reg, "ticdc_sink_batch_row_count", + "namespace", "test-keyspace", "changefeed", "async-send-error") + require.NotNil(t, hist) + require.Equal(t, uint64(0), hist.GetHistogram().GetSampleCount()) +} + +func TestHandleSendFailureRecordsErrorAndReturnsIt(t *testing.T) { + p, reg := newTestDMLProducers(t, "send-failure") + + sentinel := errors.New("producer closed") + message := newTestMessage(4, nil) + require.ErrorIs(t, p.handleSendFailure(message, sentinel), sentinel) + + errMetric := gatherMetric(t, reg, "ticdc_sink_execution_error", + "namespace", "test-keyspace", "changefeed", "send-failure", "event_type", "dml") + require.NotNil(t, errMetric) + require.Equal(t, float64(1), errMetric.GetCounter().GetValue()) } diff --git a/pkg/sink/kafka/sarama_async_producer.go b/pkg/sink/kafka/sarama_async_producer.go index 0edc5b5fd4..fcf8ec2f4c 100644 --- a/pkg/sink/kafka/sarama_async_producer.go +++ b/pkg/sink/kafka/sarama_async_producer.go @@ -103,18 +103,13 @@ func (p *saramaAsyncProducer) AsyncRunCallback( return context.Cause(ctx) case ack := <-p.producer.Successes(): if ack != nil { - switch meta := ack.Metadata.(type) { - case *messageMetadata: - if meta != nil { - p.recordDMLResult(meta.rowCount, nil) - if meta.callback != nil { - meta.callback() - } - } - default: + meta, ok := ack.Metadata.(*messageMetadata) + if !ok { log.Error("kafka producer received unknown message metadata type", zap.Any("metadata", ack.Metadata)) + continue } + p.handleSuccess(meta) } case err := <-p.producer.Errors(): // We should not wrap a nil pointer if the pointer @@ -125,9 +120,7 @@ func (p *saramaAsyncProducer) AsyncRunCallback( if err == nil { return nil } - producerErr := p.handleProducerError(err) - p.recordDMLResult(extractRowCount(err.Msg), producerErr) - return producerErr + return p.handleFailure(extractRowCount(err.Msg), p.handleProducerError(err)) } } } @@ -148,9 +141,7 @@ func (p *saramaAsyncProducer) AsyncSend( ctx context.Context, topic string, partition int32, message *codecCommon.Message, ) error { if p.closed.Load() { - err := errors.ErrKafkaSinkClosed.GenWithStackByArgs() - p.recordDMLResult(message.GetRowsCount(), err) - return err + return p.handleFailure(message.GetRowsCount(), errors.ErrKafkaSinkClosed.GenWithStackByArgs()) } meta := &messageMetadata{ rowCount: message.GetRowsCount(), @@ -166,14 +157,31 @@ func (p *saramaAsyncProducer) AsyncSend( } select { case <-ctx.Done(): - err := context.Cause(ctx) - p.recordDMLResult(message.GetRowsCount(), err) - return err + return p.handleFailure(message.GetRowsCount(), context.Cause(ctx)) case p.producer.Input() <- msg: } return nil } +// handleSuccess records a successful delivery and runs the message callback. +// It only touches ticdc-owned types so the statistics behavior can be +// unit-tested without any broker machinery. +func (p *saramaAsyncProducer) handleSuccess(meta *messageMetadata) { + if meta == nil { + return + } + p.recordDMLResult(meta.rowCount, nil) + if meta.callback != nil { + meta.callback() + } +} + +// handleFailure records a failed delivery attempt and returns the error. +func (p *saramaAsyncProducer) handleFailure(rowCount int, err error) error { + p.recordDMLResult(rowCount, err) + return err +} + func (p *saramaAsyncProducer) recordDMLResult(rowCount int, err error) { if p.statistics != nil { p.statistics.RecordDMLResult(rowCount, err) diff --git a/pkg/sink/kafka/sarama_async_producer_test.go b/pkg/sink/kafka/sarama_async_producer_test.go new file mode 100644 index 0000000000..e1704b3d39 --- /dev/null +++ b/pkg/sink/kafka/sarama_async_producer_test.go @@ -0,0 +1,110 @@ +// Copyright 2026 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, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package kafka + +import ( + "errors" + "testing" + + "github.com/pingcap/ticdc/pkg/common" + "github.com/pingcap/ticdc/pkg/statistics" + "github.com/prometheus/client_golang/prometheus" + dto "github.com/prometheus/client_model/go" + "github.com/stretchr/testify/require" +) + +// gatherMetric returns the metric of the given name whose labels match +// labelValues (alternating key/value pairs), or nil if it does not exist. +func gatherMetric( + t *testing.T, reg *prometheus.Registry, name string, labelValues ...string, +) *dto.Metric { + t.Helper() + require.Lenf(t, labelValues, len(labelValues)&^1, "labelValues must be key/value pairs") + mfs, err := reg.Gather() + require.NoError(t, err) + for _, mf := range mfs { + if mf.GetName() != name { + continue + } + for _, m := range mf.GetMetric() { + matched := true + for i := 0; i < len(labelValues); i += 2 { + found := false + for _, lp := range m.GetLabel() { + if lp.GetName() == labelValues[i] && lp.GetValue() == labelValues[i+1] { + found = true + break + } + } + if !found { + matched = false + break + } + } + if matched { + return m + } + } + } + return nil +} + +func newTestStatistics(t *testing.T, changefeed string) (*saramaAsyncProducer, *prometheus.Registry) { + t.Helper() + reg := prometheus.NewRegistry() + statistics.InitMetrics(reg) + stat := statistics.New(common.NewChangefeedID4Test("test-keyspace", changefeed), 123) + t.Cleanup(stat.Close) + return &saramaAsyncProducer{statistics: stat}, reg +} + +func TestHandleSuccessRecordsRowsAndRunsCallback(t *testing.T) { + p, reg := newTestStatistics(t, "handle-success") + + callbackCalled := make(chan struct{}) + p.handleSuccess(&messageMetadata{rowCount: 3, callback: func() { close(callbackCalled) }}) + + <-callbackCalled + // The row count is observed into the batch histogram on success. + hist := gatherMetric(t, reg, "ticdc_sink_batch_row_count", + "namespace", "test-keyspace", "changefeed", "handle-success") + require.NotNil(t, hist) + require.Equal(t, uint64(1), hist.GetHistogram().GetSampleCount()) + require.Equal(t, float64(3), hist.GetHistogram().GetSampleSum()) +} + +func TestHandleSuccessNilMetaIsNoop(t *testing.T) { + p, _ := newTestStatistics(t, "handle-success-nil") + require.NotPanics(t, func() { p.handleSuccess(nil) }) +} + +func TestHandleFailureRecordsErrorAndReturnsIt(t *testing.T) { + p, reg := newTestStatistics(t, "handle-failure") + + sentinel := errors.New("broker boom") + require.ErrorIs(t, p.handleFailure(5, sentinel), sentinel) + + // The failed message increments the DML error counter and observes no rows. + errMetric := gatherMetric(t, reg, "ticdc_sink_execution_error", + "namespace", "test-keyspace", "changefeed", "handle-failure", "event_type", "dml") + require.NotNil(t, errMetric) + require.Equal(t, float64(1), errMetric.GetCounter().GetValue()) + // No rows are observed for failed attempts; the histogram series exists + // (created eagerly by New) but has no samples. + hist := gatherMetric(t, reg, "ticdc_sink_batch_row_count", + "namespace", "test-keyspace", "changefeed", "handle-failure") + require.NotNil(t, hist) + require.Equal(t, uint64(0), hist.GetHistogram().GetSampleCount()) +} diff --git a/pkg/sink/kafka/sarama_factory.go b/pkg/sink/kafka/sarama_factory.go index 54bc73de35..d1c316b83f 100644 --- a/pkg/sink/kafka/sarama_factory.go +++ b/pkg/sink/kafka/sarama_factory.go @@ -35,30 +35,12 @@ type saramaFactory struct { } // NewSaramaFactory constructs a Factory with sarama implementation. +// stat is passed to the DML producer for sink statistics. It may be nil only +// for paths that never send DML messages, such as Verify. func NewSaramaFactory( ctx context.Context, o *options, changefeedID common.ChangeFeedID, -) (Factory, error) { - return newSaramaFactory(ctx, o, changefeedID, nil) -} - -// NewSaramaFactoryWithStatistics constructs a Factory for a running sink. -// The factory passes statistics to its DML producer without changing the -// producer interface. The sink remains responsible for closing statistics. -func NewSaramaFactoryWithStatistics( - ctx context.Context, - o *options, - changefeedID common.ChangeFeedID, - stat *statistics.Statistics, -) (Factory, error) { - return newSaramaFactory(ctx, o, changefeedID, stat) -} - -func newSaramaFactory( - ctx context.Context, - o *options, - changefeedID common.ChangeFeedID, stat *statistics.Statistics, ) (Factory, error) { start := time.Now() diff --git a/pkg/sink/mysql/affected_rows.go b/pkg/sink/mysql/affected_rows.go new file mode 100644 index 0000000000..da60c1d4fd --- /dev/null +++ b/pkg/sink/mysql/affected_rows.go @@ -0,0 +1,92 @@ +// Copyright 2026 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, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package mysql + +import ( + "strings" + "sync" + + "github.com/pingcap/ticdc/pkg/common" + "github.com/pingcap/ticdc/pkg/config/kerneltype" + "github.com/prometheus/client_golang/prometheus" +) + +// execDMLEventRowsAffectedCounter records the affected row counts reported by +// the downstream MySQL, which is a MySQL-sink-specific metric. +var execDMLEventRowsAffectedCounter = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: "ticdc", + Subsystem: "sink", + Name: "dml_event_affected_row_count", + Help: "Total count of affected rows.", + }, []string{getKeyspaceLabel(), "changefeed", "count_type", "row_type"}) + +// InitMetrics registers the MySQL sink metrics. +func InitMetrics(registry *prometheus.Registry) { + registry.MustRegister(execDMLEventRowsAffectedCounter) +} + +// affectedRowsRecorder accumulates affected row statistics for one changefeed. +type affectedRowsRecorder struct { + keyspace string + changefeed string + rowsAffectedMap sync.Map +} + +func newAffectedRowsRecorder(changefeedID common.ChangeFeedID) *affectedRowsRecorder { + return &affectedRowsRecorder{ + keyspace: changefeedID.Keyspace(), + changefeed: changefeedID.Name(), + } +} + +func (r *affectedRowsRecorder) recordTotalRowsAffected(actualRowsAffected, expectedRowsAffected int64) { + r.getRowsAffected("actual", "total").Add(float64(actualRowsAffected)) + r.getRowsAffected("expected", "total").Add(float64(expectedRowsAffected)) +} + +func (r *affectedRowsRecorder) recordRowsAffected(rowsAffected int64, rowType common.RowType) { + r.getRowsAffected("actual", rowType.String()).Add(float64(rowsAffected)) + r.getRowsAffected("expected", rowType.String()).Add(1) + r.recordTotalRowsAffected(rowsAffected, 1) +} + +func (r *affectedRowsRecorder) getRowsAffected(countType, rowType string) prometheus.Counter { + key := countType + "-" + rowType + counter, loaded := r.rowsAffectedMap.Load(key) + if !loaded { + counter := execDMLEventRowsAffectedCounter.WithLabelValues(r.keyspace, r.changefeed, countType, rowType) + r.rowsAffectedMap.Store(key, counter) + return counter + } + return counter.(prometheus.Counter) +} + +// close removes the per-changefeed metric series. +func (r *affectedRowsRecorder) close() { + r.rowsAffectedMap.Range(func(key, value any) bool { + countTypeAndRowType := key.(string) + splitTypes := strings.Split(countTypeAndRowType, "-") + execDMLEventRowsAffectedCounter.DeleteLabelValues(r.keyspace, r.changefeed, splitTypes[0], splitTypes[1]) + return true + }) +} + +func getKeyspaceLabel() string { + if kerneltype.IsNextGen() { + return "keyspace_name" + } + return "namespace" +} diff --git a/pkg/sink/mysql/mysql_writer.go b/pkg/sink/mysql/mysql_writer.go index 028846f3e8..b92bbd76ed 100644 --- a/pkg/sink/mysql/mysql_writer.go +++ b/pkg/sink/mysql/mysql_writer.go @@ -66,6 +66,9 @@ type Writer struct { statistics *statistics.Statistics + // affectedRows records the affected row counts reported by the downstream. + affectedRows *affectedRowsRecorder + // activeActiveSyncStatsCollector accumulates conflict statistics from TiDB session // variable @@tidb_cdc_active_active_sync_stats. It is shared across all DML writers // in a sink. @@ -109,6 +112,7 @@ func NewWriter( ddlTsTableInit: false, stmtCache: cfg.stmtCache, statistics: statistics, + affectedRows: newAffectedRowsRecorder(changefeedID), maxDDLTsBatch: cfg.MaxTxnRow, dmlSession: *NewDMLSession(dmlConnIdleTimeout), isInErrorCausedSafeMode: false, @@ -291,6 +295,9 @@ func (w *Writer) tryDryRunBlock() { } func (w *Writer) Close() { + if w.affectedRows != nil { + w.affectedRows.close() + } if w.stmtCache != nil { w.stmtCache.Purge() } diff --git a/pkg/sink/mysql/mysql_writer_dml_exec.go b/pkg/sink/mysql/mysql_writer_dml_exec.go index 72de1c052e..fa60705010 100644 --- a/pkg/sink/mysql/mysql_writer_dml_exec.go +++ b/pkg/sink/mysql/mysql_writer_dml_exec.go @@ -62,7 +62,7 @@ func (w *Writer) execDMLWithMaxRetries(dmls *preparedDMLs) error { return errors.Trace(err) } - affectedRows, err := w.sequenceExecute(dmls, tx, writeTimeout) + err = w.sequenceExecute(dmls, tx, writeTimeout) if err != nil { return err } @@ -70,9 +70,6 @@ func (w *Writer) execDMLWithMaxRetries(dmls *preparedDMLs) error { if err = tx.Commit(); err != nil { return err } - for i, rows := range affectedRows { - w.statistics.RecordRowsAffected(rows, dmls.rowTypes[i]) - } log.Debug("Exec Rows succeeded", zap.Any("rowCount", dmls.rowCount), zap.Int("writerID", w.id)) return nil @@ -132,8 +129,7 @@ func (w *Writer) execDMLWithMaxRetries(dmls *preparedDMLs) error { // sequenceExecute runs each SQL sequentially inside a transaction. func (w *Writer) sequenceExecute( dmls *preparedDMLs, tx *sql.Tx, writeTimeout time.Duration, -) (map[int]int64, error) { - affectedRows := make(map[int]int64, len(dmls.sqls)) +) error { for i, query := range dmls.sqls { args := dmls.values[i] log.Debug("exec row", zap.String("sql", query), zap.String("args", util.RedactArgs(args)), zap.Int("writerID", w.id)) @@ -172,16 +168,16 @@ func (w *Writer) sequenceExecute( } } cancelFunc() - return nil, errors.WrapError(errors.ErrMySQLTxnError, errors.WithMessage(execError, fmt.Sprintf("Failed to execute DMLs, query info:%s, args:%v; ", query, util.RedactArgs(args)))) + return errors.WrapError(errors.ErrMySQLTxnError, errors.WithMessage(execError, fmt.Sprintf("Failed to execute DMLs, query info:%s, args:%v; ", query, util.RedactArgs(args)))) } - if rows, err := res.RowsAffected(); err != nil { + if rowsAffected, err := res.RowsAffected(); err != nil { log.Warn("get rows affected rows failed", zap.Error(err)) } else { - affectedRows[i] = rows + w.affectedRows.recordRowsAffected(rowsAffected, dmls.rowTypes[i]) } cancelFunc() } - return affectedRows, nil + return nil } // multiStmtExecute runs SQLs using the multi-statements protocol with an implicit transaction. @@ -219,7 +215,7 @@ func (w *Writer) multiStmtExecute( if rowsAffected, err := res.RowsAffected(); err != nil { log.Warn("get rows affected rows failed", zap.Error(err)) } else { - w.statistics.RecordTotalRowsAffected(rowsAffected, int64(len(dmls.sqls))) + w.affectedRows.recordTotalRowsAffected(rowsAffected, int64(len(dmls.sqls))) } return nil } diff --git a/pkg/statistics/metrics.go b/pkg/statistics/metrics.go index b6dfa0b69f..ab1dd8bad7 100644 --- a/pkg/statistics/metrics.go +++ b/pkg/statistics/metrics.go @@ -62,14 +62,6 @@ var ( Help: "Total approximate raw bytes of DML events successfully written to downstream.", }, []string{getKeyspaceLabel(), "changefeed"}) - execDMLEventRowsAffectedCounter = prometheus.NewCounterVec( - prometheus.CounterOpts{ - Namespace: "ticdc", - Subsystem: "sink", - Name: "dml_event_affected_row_count", - Help: "Total count of affected rows.", - }, []string{getKeyspaceLabel(), "changefeed", "count_type", "row_type"}) - executionErrorCounter = prometheus.NewCounterVec( prometheus.CounterOpts{ Namespace: "ticdc", @@ -86,7 +78,6 @@ func InitMetrics(registry *prometheus.Registry) { registry.MustRegister(execDDLCounter) registry.MustRegister(execBatchHistogram) registry.MustRegister(totalWriteBytesCounter) - registry.MustRegister(execDMLEventRowsAffectedCounter) registry.MustRegister(executionErrorCounter) } diff --git a/pkg/statistics/statistics.go b/pkg/statistics/statistics.go index f4c065e705..81b73117fb 100644 --- a/pkg/statistics/statistics.go +++ b/pkg/statistics/statistics.go @@ -14,9 +14,7 @@ package statistics import ( - "fmt" "strconv" - "strings" "sync" "time" @@ -28,10 +26,9 @@ import ( // New creates a Statistics. func New(changefeed common.ChangeFeedID, keyspaceID uint32) *Statistics { statistics := &Statistics{ - changefeedID: changefeed, - keyspaceID: strconv.FormatUint(uint64(keyspaceID), 10), - ddlTypes: sync.Map{}, - rowsAffectedMap: sync.Map{}, + changefeedID: changefeed, + keyspaceID: strconv.FormatUint(uint64(keyspaceID), 10), + ddlTypes: sync.Map{}, } keyspace := changefeed.Keyspace() @@ -49,10 +46,9 @@ func New(changefeed common.ChangeFeedID, keyspaceID uint32) *Statistics { // Statistics maintains some status and metrics of the Sink // Note: All methods of Statistics should be thread-safe. type Statistics struct { - changefeedID common.ChangeFeedID - keyspaceID string - ddlTypes sync.Map - rowsAffectedMap sync.Map + changefeedID common.ChangeFeedID + keyspaceID string + ddlTypes sync.Map // metricExecDDLHis records each DDL execution time duration. metricExecDDLHis prometheus.Observer @@ -114,30 +110,6 @@ func (b *Statistics) RecordDDLExecution(executor func() (string, error)) error { return nil } -func (b *Statistics) RecordTotalRowsAffected(actualRowsAffected, expectedRowsAffected int64) { - b.getRowsAffected("actual", "total").Add(float64(actualRowsAffected)) - b.getRowsAffected("expected", "total").Add(float64(expectedRowsAffected)) -} - -func (b *Statistics) RecordRowsAffected(rowsAffected int64, rowType common.RowType) { - b.getRowsAffected("actual", rowType.String()).Add(float64(rowsAffected)) - b.getRowsAffected("expected", rowType.String()).Add(1) - b.RecordTotalRowsAffected(rowsAffected, 1) -} - -func (b *Statistics) getRowsAffected(countType, rowType string) prometheus.Counter { - key := fmt.Sprintf("%s-%s", countType, rowType) - counter, loaded := b.rowsAffectedMap.Load(key) - if !loaded { - keyspace := b.changefeedID.Keyspace() - changefeedID := b.changefeedID.Name() - counter := execDMLEventRowsAffectedCounter.WithLabelValues(keyspace, changefeedID, countType, rowType) - b.rowsAffectedMap.Store(key, counter) - return counter - } - return counter.(prometheus.Counter) -} - // Close release some internal resources. func (b *Statistics) Close() { keyspace := b.changefeedID.Keyspace() @@ -152,12 +124,5 @@ func (b *Statistics) Close() { execDDLCounter.DeleteLabelValues(keyspace, changefeedID, ddlType) return true }) - b.rowsAffectedMap.Range(func(key, value any) bool { - countTypeAndRowType := key.(string) - splitTypes := strings.Split(countTypeAndRowType, "-") - countType, rowType := splitTypes[0], splitTypes[1] - execDMLEventRowsAffectedCounter.DeleteLabelValues(keyspace, changefeedID, countType, rowType) - return true - }) totalWriteBytesCounter.DeleteLabelValues(keyspace, changefeedID) } diff --git a/pkg/statistics/statistics_test.go b/pkg/statistics/statistics_test.go new file mode 100644 index 0000000000..09ab841d83 --- /dev/null +++ b/pkg/statistics/statistics_test.go @@ -0,0 +1,82 @@ +// Copyright 2026 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, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package statistics + +import ( + "errors" + "testing" + + "github.com/pingcap/ticdc/pkg/common" + commonEvent "github.com/pingcap/ticdc/pkg/common/event" + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/testutil" + dto "github.com/prometheus/client_model/go" + "github.com/stretchr/testify/require" +) + +func newTestEvent(size int64) *commonEvent.DMLEvent { + event := commonEvent.NewDMLEvent(common.NewDispatcherID(), 1, 1, 2, nil) + event.ApproximateSize = size + return event +} + +func TestRecordDMLResult(t *testing.T) { + stat := New(common.NewChangefeedID4Test("test-keyspace", "record-dml-result"), 123) + defer stat.Close() + + // failed attempts increment the DML error counter and do not observe rows. + stat.RecordDMLResult(10, errors.New("boom")) + require.Equal(t, float64(1), testutil.ToFloat64(stat.metricExecErrCntForDML)) + + // successful attempts observe the row count into the batch histogram. + stat.RecordDMLResult(2, nil) + var m dto.Metric + require.NoError(t, stat.metricExecBatchHis.(prometheus.Metric).Write(&m)) + require.Equal(t, uint64(1), m.Histogram.GetSampleCount()) + require.Equal(t, float64(2), m.Histogram.GetSampleSum()) +} + +func TestTrackDMLEventCountsBytesOnPostFlush(t *testing.T) { + stat := New(common.NewChangefeedID4Test("test-keyspace", "track-dml-event"), 123) + defer stat.Close() + + event := newTestEvent(1024) + stat.TrackDMLEvent(event) + + // Bytes are only counted after the event is flushed. + require.Zero(t, testutil.ToFloat64(stat.metricTotalWriteBytesCnt)) + event.PostFlush() + require.Equal(t, float64(1024), testutil.ToFloat64(stat.metricTotalWriteBytesCnt)) +} + +func TestCloseDeletesMetricSeries(t *testing.T) { + stat := New(common.NewChangefeedID4Test("test-keyspace", "close-deletes"), 123) + + stat.RecordDMLResult(1, nil) + stat.RecordDMLResult(1, errors.New("boom")) + event := newTestEvent(100) + stat.TrackDMLEvent(event) + event.PostFlush() + + require.Equal(t, 1, testutil.CollectAndCount(execBatchHistogram)) + // New() eagerly creates the ddl series, RecordDMLResult adds the dml one. + require.Equal(t, 2, testutil.CollectAndCount(executionErrorCounter)) + require.Equal(t, 1, testutil.CollectAndCount(totalWriteBytesCounter)) + + stat.Close() + require.Equal(t, 0, testutil.CollectAndCount(execBatchHistogram)) + require.Equal(t, 0, testutil.CollectAndCount(executionErrorCounter)) + require.Equal(t, 0, testutil.CollectAndCount(totalWriteBytesCounter)) +} diff --git a/server/metrics.go b/server/metrics.go index 989a2beda0..88f55d514b 100644 --- a/server/metrics.go +++ b/server/metrics.go @@ -16,6 +16,7 @@ package server import ( "github.com/pingcap/ticdc/pkg/common/event" "github.com/pingcap/ticdc/pkg/metrics" + mysql "github.com/pingcap/ticdc/pkg/sink/mysql" "github.com/prometheus/client_golang/prometheus" ) @@ -24,4 +25,5 @@ var registry = prometheus.NewRegistry() func init() { metrics.InitMetrics(registry) event.InitEventMetrics(registry) + mysql.InitMetrics(registry) }