From 92ee0e16a1e6d08be7944db60b8c5e500d12ca50 Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Tue, 24 Feb 2026 15:38:11 +0000 Subject: [PATCH 01/20] dispatcher,event,cloudstorage: add DML two-stage ack --- .../dispatcher/basic_dispatcher.go | 12 ++---- .../sink/cloudstorage/dml_writers.go | 1 + pkg/common/event/dml_event.go | 24 ++++++++++++ pkg/common/event/dml_event_test.go | 39 +++++++++++++++++++ 4 files changed, 68 insertions(+), 8 deletions(-) diff --git a/downstreamadapter/dispatcher/basic_dispatcher.go b/downstreamadapter/dispatcher/basic_dispatcher.go index f9131c581a..c956165cc9 100644 --- a/downstreamadapter/dispatcher/basic_dispatcher.go +++ b/downstreamadapter/dispatcher/basic_dispatcher.go @@ -545,7 +545,8 @@ func (d *BasicDispatcher) HandleEvents(dispatcherEvents []DispatcherEvent, wakeC // When we handle events, we don't have any previous events still in sink. // // wakeCallback is used to wake the dynamic stream to handle the next batch events. -// It will be called when all the events are flushed to downstream successfully. +// It is triggered after DML events are enqueued to sink pipeline, while checkpoint +// still advances only after PostFlush callbacks are completed. func (d *BasicDispatcher) handleEvents(dispatcherEvents []DispatcherEvent, wakeCallback func()) bool { if d.GetRemovingStatus() { log.Warn("dispatcher is removing", zap.Any("id", d.id)) @@ -622,13 +623,8 @@ func (d *BasicDispatcher) handleEvents(dispatcherEvents []DispatcherEvent, wakeC block = true dml.ReplicatingTs = d.creationPDTs - dml.AddPostFlushFunc(func() { - // Considering dml event in sink may be written to downstream not in order, - // thus, we use tableProgress.Empty() to ensure these events are flushed to downstream completely - // and wake dynamic stream to handle the next events. - if d.tableProgress.Empty() { - dmlWakeOnce.Do(wakeCallback) - } + dml.AddPostEnqueueFunc(func() { + dmlWakeOnce.Do(wakeCallback) }) dmlEvents = append(dmlEvents, dml) case commonEvent.TypeDDLEvent: diff --git a/downstreamadapter/sink/cloudstorage/dml_writers.go b/downstreamadapter/sink/cloudstorage/dml_writers.go index bdc442bda2..7b7ffbf8df 100644 --- a/downstreamadapter/sink/cloudstorage/dml_writers.go +++ b/downstreamadapter/sink/cloudstorage/dml_writers.go @@ -116,6 +116,7 @@ func (d *dmlWriters) AddDMLEvent(event *commonEvent.DMLEvent) { _ = d.statistics.RecordBatchExecution(func() (int, int64, error) { // emit a TxnCallbackableEvent encoupled with a sequence number starting from one. d.msgCh.Push(newEventFragment(seq, tbl, event)) + event.PostEnqueue() return int(event.Len()), event.GetSize(), nil }) } diff --git a/pkg/common/event/dml_event.go b/pkg/common/event/dml_event.go index 8f18234db8..10dd667518 100644 --- a/pkg/common/event/dml_event.go +++ b/pkg/common/event/dml_event.go @@ -392,9 +392,14 @@ type DMLEvent struct { // The following fields are set and used by dispatcher. ReplicatingTs uint64 `json:"replicating_ts"` + // PostTxnEnqueued is the functions to be executed after the transaction is + // enqueued into sink internal pipeline. + PostTxnEnqueued []func() `json:"-"` // PostTxnFlushed is the functions to be executed after the transaction is flushed. // It is set and used by dispatcher. PostTxnFlushed []func() `json:"-"` + // postEnqueueCalled ensures PostTxnEnqueued callbacks are triggered at most once. + postEnqueueCalled bool `json:"-"` // eventSize is the size of the event in bytes. It is set when it's unmarshaled. eventSize int64 `json:"-"` @@ -631,11 +636,22 @@ func (t *DMLEvent) GetStartTs() common.Ts { } func (t *DMLEvent) PostFlush() { + t.PostEnqueue() for _, f := range t.PostTxnFlushed { f() } } +func (t *DMLEvent) PostEnqueue() { + if t.postEnqueueCalled { + return + } + t.postEnqueueCalled = true + for _, f := range t.PostTxnEnqueued { + f() + } +} + func (t *DMLEvent) GetSeq() uint64 { return t.Seq } @@ -656,6 +672,14 @@ func (t *DMLEvent) AddPostFlushFunc(f func()) { t.PostTxnFlushed = append(t.PostTxnFlushed, f) } +func (t *DMLEvent) ClearPostEnqueueFunc() { + t.PostTxnEnqueued = t.PostTxnEnqueued[:0] +} + +func (t *DMLEvent) AddPostEnqueueFunc(f func()) { + t.PostTxnEnqueued = append(t.PostTxnEnqueued, f) +} + // Rewind reset the offset to 0, So that the next GetNextRow will return the first row func (t *DMLEvent) Rewind() { t.offset = 0 diff --git a/pkg/common/event/dml_event_test.go b/pkg/common/event/dml_event_test.go index 555f1587af..aab7078169 100644 --- a/pkg/common/event/dml_event_test.go +++ b/pkg/common/event/dml_event_test.go @@ -15,6 +15,7 @@ package event import ( "encoding/binary" + "sync/atomic" "testing" "github.com/pingcap/ticdc/pkg/common" @@ -342,3 +343,41 @@ func TestBatchDMLEventHeaderValidation(t *testing.T) { require.Error(t, err) require.Contains(t, err.Error(), "incomplete data") } + +func TestDMLEventPostEnqueueFuncs(t *testing.T) { + t.Parallel() + + event := &DMLEvent{} + var called int64 + event.AddPostEnqueueFunc(func() { + atomic.AddInt64(&called, 1) + }) + event.AddPostEnqueueFunc(func() { + atomic.AddInt64(&called, 1) + }) + + event.PostEnqueue() + event.PostEnqueue() + + require.Equal(t, int64(2), atomic.LoadInt64(&called)) +} + +func TestDMLEventPostFlushTriggersPostEnqueueOnce(t *testing.T) { + t.Parallel() + + event := &DMLEvent{} + var enqueueCalled int64 + var flushCalled int64 + event.AddPostEnqueueFunc(func() { + atomic.AddInt64(&enqueueCalled, 1) + }) + event.AddPostFlushFunc(func() { + atomic.AddInt64(&flushCalled, 1) + }) + + event.PostFlush() + event.PostFlush() + + require.Equal(t, int64(1), atomic.LoadInt64(&enqueueCalled)) + require.Equal(t, int64(2), atomic.LoadInt64(&flushCalled)) +} From 01bd19a716aef71d088e439c266546bec7febe24 Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Wed, 25 Feb 2026 10:11:33 +0000 Subject: [PATCH 02/20] dispatcher,event,cloudstorage: fix enqueue callback semantics --- .../sink/cloudstorage/defragmenter.go | 1 + .../sink/cloudstorage/dml_writers.go | 1 - .../sink/cloudstorage/dml_writers_test.go | 41 +++++++++++++++++++ pkg/common/event/active_active.go | 5 +++ pkg/common/event/active_active_test.go | 31 ++++++++++++++ pkg/common/event/dml_event.go | 37 +++++++++++++---- pkg/common/event/dml_event_test.go | 28 +++++++++++++ 7 files changed, 136 insertions(+), 8 deletions(-) diff --git a/downstreamadapter/sink/cloudstorage/defragmenter.go b/downstreamadapter/sink/cloudstorage/defragmenter.go index 6f14990e71..7b6f9076bc 100644 --- a/downstreamadapter/sink/cloudstorage/defragmenter.go +++ b/downstreamadapter/sink/cloudstorage/defragmenter.go @@ -122,6 +122,7 @@ func (d *defragmenter) dispatchFragToDMLWorker(frag eventFragment) { d.hasher.Write([]byte(tableName.Schema), []byte(tableName.Table)) workerID := d.hasher.Sum32() % uint32(len(d.outputChs)) d.outputChs[workerID].In() <- frag + frag.event.PostEnqueue() d.lastDispatchedSeq = frag.seqNumber } diff --git a/downstreamadapter/sink/cloudstorage/dml_writers.go b/downstreamadapter/sink/cloudstorage/dml_writers.go index 7b7ffbf8df..bdc442bda2 100644 --- a/downstreamadapter/sink/cloudstorage/dml_writers.go +++ b/downstreamadapter/sink/cloudstorage/dml_writers.go @@ -116,7 +116,6 @@ func (d *dmlWriters) AddDMLEvent(event *commonEvent.DMLEvent) { _ = d.statistics.RecordBatchExecution(func() (int, int64, error) { // emit a TxnCallbackableEvent encoupled with a sequence number starting from one. d.msgCh.Push(newEventFragment(seq, tbl, event)) - event.PostEnqueue() return int(event.Len()), event.GetSize(), nil }) } diff --git a/downstreamadapter/sink/cloudstorage/dml_writers_test.go b/downstreamadapter/sink/cloudstorage/dml_writers_test.go index bd9a89a5ff..aafd385206 100644 --- a/downstreamadapter/sink/cloudstorage/dml_writers_test.go +++ b/downstreamadapter/sink/cloudstorage/dml_writers_test.go @@ -52,6 +52,47 @@ func getTableFiles(t *testing.T, tableDir string) []string { return fileNames } +func TestAddDMLEventDoesNotCallPostEnqueueBeforePipelineRun(t *testing.T) { + uri := fmt.Sprintf("file:///%s?protocol=csv", t.TempDir()) + sinkURI, err := url.Parse(uri) + require.NoError(t, err) + + replicaConfig := config.GetDefaultReplicaConfig() + err = replicaConfig.ValidateAndAdjust(sinkURI) + require.NoError(t, err) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + mockPDClock := pdutil.NewClock4Test() + appcontext.SetService(appcontext.DefaultPDClock, mockPDClock) + + cloudStorageSink, err := newSinkForTest(ctx, replicaConfig, sinkURI, nil) + require.NoError(t, err) + + tableInfo := &common.TableInfo{ + TableName: common.TableName{ + Schema: "test", + Table: "t_enqueue", + TableID: 100, + }, + } + event := commonEvent.NewDMLEvent(common.NewDispatcherID(), tableInfo.TableName.TableID, 1, 1, tableInfo) + event.TableInfoVersion = 1 + event.Length = 1 + event.ApproximateSize = 1 + + var enqueueCalled int64 + event.AddPostEnqueueFunc(func() { + atomic.AddInt64(&enqueueCalled, 1) + }) + + // Without starting sink.Run, the event should only be accepted by AddDMLEvent + // and should not be considered enqueued into downstream write pipeline yet. + cloudStorageSink.AddDMLEvent(event) + require.Equal(t, int64(0), atomic.LoadInt64(&enqueueCalled)) +} + func TestCloudStorageWriteEventsWithoutDateSeparator(t *testing.T) { parentDir := t.TempDir() diff --git a/pkg/common/event/active_active.go b/pkg/common/event/active_active.go index eac1853cc5..7277704174 100644 --- a/pkg/common/event/active_active.go +++ b/pkg/common/event/active_active.go @@ -14,6 +14,8 @@ package event import ( + "sync/atomic" + "github.com/pingcap/log" "github.com/pingcap/ticdc/pkg/common" "github.com/pingcap/ticdc/pkg/errors" @@ -443,7 +445,10 @@ func newFilteredDMLEvent( newEvent.Seq = source.Seq newEvent.Epoch = source.Epoch newEvent.ReplicatingTs = source.ReplicatingTs + newEvent.PostTxnEnqueued = source.PostTxnEnqueued newEvent.PostTxnFlushed = source.PostTxnFlushed + newEvent.postEnqueueCalled = atomic.LoadUint32(&source.postEnqueueCalled) + source.PostTxnEnqueued = nil source.PostTxnFlushed = nil newEvent.SetRows(rows) diff --git a/pkg/common/event/active_active_test.go b/pkg/common/event/active_active_test.go index b3d1633fd3..82c14d1c78 100644 --- a/pkg/common/event/active_active_test.go +++ b/pkg/common/event/active_active_test.go @@ -14,6 +14,7 @@ package event import ( + "sync/atomic" "testing" "time" @@ -316,6 +317,36 @@ func TestFilterDMLEventSoftDeleteTableMissingColumnReportsError(t *testing.T) { require.Contains(t, handledErr.Error(), SoftDeleteTimeColumn) } +func TestFilterDMLEventKeepsPostEnqueueCallbacksOnFilteredEvent(t *testing.T) { + ti := newTestTableInfo(t, true, true) + ts := newTimestampValue(time.Date(2025, time.March, 10, 0, 0, 0, 0, time.UTC)) + event := newDMLEventForTest(t, ti, + []commonpkg.RowType{commonpkg.RowTypeUpdate}, + [][]interface{}{ + {int64(1), nil}, + {int64(1), ts}, + }) + + var enqueueCalled int64 + var flushCalled int64 + event.AddPostEnqueueFunc(func() { + atomic.AddInt64(&enqueueCalled, 1) + }) + event.AddPostFlushFunc(func() { + atomic.AddInt64(&flushCalled, 1) + }) + + filtered, skip := FilterDMLEvent(event, false, nil) + require.False(t, skip) + require.NotNil(t, filtered) + require.NotEqual(t, event, filtered) + + filtered.PostEnqueue() + filtered.PostFlush() + require.Equal(t, int64(1), atomic.LoadInt64(&enqueueCalled)) + require.Equal(t, int64(1), atomic.LoadInt64(&flushCalled)) +} + func newTestTableInfo(t *testing.T, activeActive, softDelete bool) *commonpkg.TableInfo { idCol := newTestColumn(1, "id", mysql.TypeLong, mysql.PriKeyFlag) cols := []*model.ColumnInfo{idCol} diff --git a/pkg/common/event/dml_event.go b/pkg/common/event/dml_event.go index 10dd667518..7d0cd3576c 100644 --- a/pkg/common/event/dml_event.go +++ b/pkg/common/event/dml_event.go @@ -17,6 +17,7 @@ import ( "encoding/binary" "fmt" "strings" + "sync/atomic" "time" "github.com/pingcap/failpoint" @@ -392,14 +393,18 @@ type DMLEvent struct { // The following fields are set and used by dispatcher. ReplicatingTs uint64 `json:"replicating_ts"` - // PostTxnEnqueued is the functions to be executed after the transaction is - // enqueued into sink internal pipeline. + // PostTxnEnqueued contains callbacks executed when the txn is accepted by + // sink's internal pipeline (enqueue stage). + // + // Note: enqueue means "handed over to sink workers", not "durably written + // to downstream". PostTxnEnqueued []func() `json:"-"` - // PostTxnFlushed is the functions to be executed after the transaction is flushed. - // It is set and used by dispatcher. + // PostTxnFlushed contains callbacks executed when the txn is fully flushed to + // downstream (flush stage), which is stronger than enqueue and is used by + // checkpoint related logic. PostTxnFlushed []func() `json:"-"` // postEnqueueCalled ensures PostTxnEnqueued callbacks are triggered at most once. - postEnqueueCalled bool `json:"-"` + postEnqueueCalled uint32 `json:"-"` // eventSize is the size of the event in bytes. It is set when it's unmarshaled. eventSize int64 `json:"-"` @@ -635,6 +640,11 @@ func (t *DMLEvent) GetStartTs() common.Ts { return t.StartTs } +// PostFlush marks the transaction as flushed to downstream. +// +// It always calls PostEnqueue first to preserve callback order: +// enqueue callbacks run before flush callbacks, even for sinks that only invoke +// PostFlush and do not have an explicit enqueue hook. func (t *DMLEvent) PostFlush() { t.PostEnqueue() for _, f := range t.PostTxnFlushed { @@ -642,11 +652,14 @@ func (t *DMLEvent) PostFlush() { } } +// PostEnqueue marks the transaction as enqueued into sink internal pipeline. +// +// This stage does not mean data is already written to downstream. The method is +// idempotent and guarantees enqueue callbacks run at most once. func (t *DMLEvent) PostEnqueue() { - if t.postEnqueueCalled { + if !atomic.CompareAndSwapUint32(&t.postEnqueueCalled, 0, 1) { return } - t.postEnqueueCalled = true for _, f := range t.PostTxnEnqueued { f() } @@ -660,22 +673,32 @@ func (t *DMLEvent) GetEpoch() uint64 { return t.Epoch } +// PushFrontFlushFunc prepends a flush callback so it runs before existing ones. func (t *DMLEvent) PushFrontFlushFunc(f func()) { t.PostTxnFlushed = append([]func(){f}, t.PostTxnFlushed...) } +// ClearPostFlushFunc removes all registered flush callbacks. func (t *DMLEvent) ClearPostFlushFunc() { t.PostTxnFlushed = t.PostTxnFlushed[:0] } +// AddPostFlushFunc registers a callback that runs at flush stage. +// +// Use this when the callback depends on downstream persistence semantics. func (t *DMLEvent) AddPostFlushFunc(f func()) { t.PostTxnFlushed = append(t.PostTxnFlushed, f) } +// ClearPostEnqueueFunc removes all registered enqueue callbacks. func (t *DMLEvent) ClearPostEnqueueFunc() { t.PostTxnEnqueued = t.PostTxnEnqueued[:0] } +// AddPostEnqueueFunc registers a callback that runs at enqueue stage. +// +// Use this when only sink acceptance is required. For sinks with no explicit +// enqueue signal, this callback is triggered via PostFlush. func (t *DMLEvent) AddPostEnqueueFunc(f func()) { t.PostTxnEnqueued = append(t.PostTxnEnqueued, f) } diff --git a/pkg/common/event/dml_event_test.go b/pkg/common/event/dml_event_test.go index aab7078169..1fe4763347 100644 --- a/pkg/common/event/dml_event_test.go +++ b/pkg/common/event/dml_event_test.go @@ -15,6 +15,7 @@ package event import ( "encoding/binary" + "sync" "sync/atomic" "testing" @@ -381,3 +382,30 @@ func TestDMLEventPostFlushTriggersPostEnqueueOnce(t *testing.T) { require.Equal(t, int64(1), atomic.LoadInt64(&enqueueCalled)) require.Equal(t, int64(2), atomic.LoadInt64(&flushCalled)) } + +func TestDMLEventPostEnqueueConcurrentWithPostFlush(t *testing.T) { + t.Parallel() + + event := &DMLEvent{} + var enqueueCalled int64 + event.AddPostEnqueueFunc(func() { + atomic.AddInt64(&enqueueCalled, 1) + }) + + var wg sync.WaitGroup + const loops = 256 + for i := 0; i < loops; i++ { + wg.Add(2) + go func() { + defer wg.Done() + event.PostEnqueue() + }() + go func() { + defer wg.Done() + event.PostFlush() + }() + } + wg.Wait() + + require.Equal(t, int64(1), atomic.LoadInt64(&enqueueCalled)) +} From c113666aa3e9667c4cb76c8acefed8113ea0aafb Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Wed, 25 Feb 2026 10:55:34 +0000 Subject: [PATCH 03/20] 2 small changes --- downstreamadapter/sink/cloudstorage/dml_writers.go | 7 ++----- pkg/sink/cloudstorage/config.go | 2 +- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/downstreamadapter/sink/cloudstorage/dml_writers.go b/downstreamadapter/sink/cloudstorage/dml_writers.go index bdc442bda2..678c0f2b8f 100644 --- a/downstreamadapter/sink/cloudstorage/dml_writers.go +++ b/downstreamadapter/sink/cloudstorage/dml_writers.go @@ -113,11 +113,8 @@ func (d *dmlWriters) AddDMLEvent(event *commonEvent.DMLEvent) { DispatcherID: event.GetDispatcherID(), } seq := atomic.AddUint64(&d.lastSeqNum, 1) - _ = d.statistics.RecordBatchExecution(func() (int, int64, error) { - // emit a TxnCallbackableEvent encoupled with a sequence number starting from one. - d.msgCh.Push(newEventFragment(seq, tbl, event)) - return int(event.Len()), event.GetSize(), nil - }) + // emit a TxnCallbackableEvent encoupled with a sequence number starting from one. + d.msgCh.Push(newEventFragment(seq, tbl, event)) } func (d *dmlWriters) close() { diff --git a/pkg/sink/cloudstorage/config.go b/pkg/sink/cloudstorage/config.go index 4b817dd394..390c6d37c2 100644 --- a/pkg/sink/cloudstorage/config.go +++ b/pkg/sink/cloudstorage/config.go @@ -60,7 +60,7 @@ const ( // `0 0 2 * * ?` means 2:00:00 AM every day defaultFileCleanupCronSpec = "0 0 2 * * *" - defaultEnableTableAcrossNodes = true + defaultEnableTableAcrossNodes = false ) type urlConfig struct { From 6a1be2d0e66b375409d5c03871edae46aa21219d Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Wed, 25 Feb 2026 11:26:10 +0000 Subject: [PATCH 04/20] fix test --- pkg/sink/cloudstorage/config_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/sink/cloudstorage/config_test.go b/pkg/sink/cloudstorage/config_test.go index 442d95a2eb..f02cfb446b 100644 --- a/pkg/sink/cloudstorage/config_test.go +++ b/pkg/sink/cloudstorage/config_test.go @@ -41,7 +41,7 @@ func TestConfigApply(t *testing.T) { err = replicaConfig.ValidateAndAdjust(sinkURI) require.NoError(t, err) cfg := NewConfig() - err = cfg.Apply(context.TODO(), sinkURI, replicaConfig.Sink, true) + err = cfg.Apply(context.TODO(), sinkURI, replicaConfig.Sink, false) require.Nil(t, err) require.Equal(t, expected, cfg) } From 2f8b51c1d7b1bd545421d7deb86bc1be3ccc1fdd Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Thu, 26 Feb 2026 05:52:39 +0000 Subject: [PATCH 05/20] downstreamadapter,event: simplify sink wake callback flow - extract shared txn post-flush row callback helper for kafka/pulsar/redo\n- keep sink-specific wake behavior explicit via dispatcher test\n- streamline callback coverage to avoid redundant sink-level assertions --- .../dispatcher/basic_dispatcher.go | 6 +- .../dispatcher/event_dispatcher_test.go | 70 +++++++++++++++++++ downstreamadapter/sink/helper/row_callback.go | 30 ++++++++ .../sink/helper/row_callback_test.go | 42 +++++++++++ downstreamadapter/sink/kafka/sink.go | 14 +--- downstreamadapter/sink/mock_sink.go | 4 ++ downstreamadapter/sink/pulsar/sink.go | 14 +--- downstreamadapter/sink/redo/sink.go | 14 +--- 8 files changed, 154 insertions(+), 40 deletions(-) create mode 100644 downstreamadapter/sink/helper/row_callback.go create mode 100644 downstreamadapter/sink/helper/row_callback_test.go diff --git a/downstreamadapter/dispatcher/basic_dispatcher.go b/downstreamadapter/dispatcher/basic_dispatcher.go index c956165cc9..fe471d6fa1 100644 --- a/downstreamadapter/dispatcher/basic_dispatcher.go +++ b/downstreamadapter/dispatcher/basic_dispatcher.go @@ -545,8 +545,10 @@ func (d *BasicDispatcher) HandleEvents(dispatcherEvents []DispatcherEvent, wakeC // When we handle events, we don't have any previous events still in sink. // // wakeCallback is used to wake the dynamic stream to handle the next batch events. -// It is triggered after DML events are enqueued to sink pipeline, while checkpoint -// still advances only after PostFlush callbacks are completed. +// It is registered on enqueue stage, and sink implementations are responsible for +// deciding when enqueue is considered complete (some sinks trigger enqueue via +// PostFlush after downstream flush completes). +// Checkpoint still advances only after PostFlush callbacks are completed. func (d *BasicDispatcher) handleEvents(dispatcherEvents []DispatcherEvent, wakeCallback func()) bool { if d.GetRemovingStatus() { log.Warn("dispatcher is removing", zap.Any("id", d.id)) diff --git a/downstreamadapter/dispatcher/event_dispatcher_test.go b/downstreamadapter/dispatcher/event_dispatcher_test.go index 897182c951..d36d8eac99 100644 --- a/downstreamadapter/dispatcher/event_dispatcher_test.go +++ b/downstreamadapter/dispatcher/event_dispatcher_test.go @@ -14,6 +14,7 @@ package dispatcher import ( + "fmt" "math" "sync/atomic" "testing" @@ -776,6 +777,75 @@ func TestBatchDMLEventsPartialFlush(t *testing.T) { require.Equal(t, 0, len(mockSink.GetDMLs())) } +func TestDMLWakeCallbackBySinkType(t *testing.T) { + helper := commonEvent.NewEventTestHelper(t) + defer helper.Close() + + helper.Tk().MustExec("use test") + ddlJob := helper.DDL2Job("create table t(id int primary key, v int)") + require.NotNil(t, ddlJob) + + buildDMLEvent := func(commitTs uint64) *commonEvent.DMLEvent { + event := helper.DML2Event( + "test", + "t", + fmt.Sprintf("insert into t values(%d, %d)", commitTs, commitTs), + fmt.Sprintf("insert into t values(%d, %d)", commitTs+1000, commitTs+1000), + ) + require.NotNil(t, event) + event.CommitTs = commitTs + return event + } + + testCases := []struct { + name string + sinkType common.SinkType + commitTs uint64 + wakeOnEnqueue bool + }{ + { + name: "mysql wake after flush", + sinkType: common.MysqlSinkType, + commitTs: 20, + wakeOnEnqueue: false, + }, + { + name: "cloudstorage wake on enqueue", + sinkType: common.CloudStorageSinkType, + commitTs: 21, + wakeOnEnqueue: true, + }, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + mockSink := sink.NewMockSink(tc.sinkType) + tableSpan, err := getCompleteTableSpan(getTestingKeyspaceID()) + require.NoError(t, err) + dispatcher := newDispatcherForTest(mockSink, tableSpan) + + dmlEvent := buildDMLEvent(tc.commitTs) + nodeID := node.NewID() + var callbackCalled atomic.Bool + block := dispatcher.HandleEvents([]DispatcherEvent{NewDispatcherEvent(&nodeID, dmlEvent)}, func() { + callbackCalled.Store(true) + }) + require.True(t, block) + require.Equal(t, tc.wakeOnEnqueue, callbackCalled.Load()) + require.Len(t, mockSink.GetDMLs(), 1) + + if tc.wakeOnEnqueue { + return + } + + mockSink.FlushDMLs() + require.True(t, callbackCalled.Load()) + require.Len(t, mockSink.GetDMLs(), 0) + }) + } +} + // TestDispatcherSplittableCheck tests that a split table dispatcher with enableSplittableCheck=true // correctly reports an error when receiving a DDL that breaks splittable func TestDispatcherSplittableCheck(t *testing.T) { diff --git a/downstreamadapter/sink/helper/row_callback.go b/downstreamadapter/sink/helper/row_callback.go new file mode 100644 index 0000000000..07c6d4ff9a --- /dev/null +++ b/downstreamadapter/sink/helper/row_callback.go @@ -0,0 +1,30 @@ +// 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 helper + +import ( + commonEvent "github.com/pingcap/ticdc/pkg/common/event" + "go.uber.org/atomic" +) + +// NewTxnPostFlushRowCallback returns a row-level callback that triggers txn-level +// PostFlush exactly once when the callback has been invoked totalCount times. +func NewTxnPostFlushRowCallback(event *commonEvent.DMLEvent, totalCount uint64) func() { + var calledCount atomic.Uint64 + return func() { + if calledCount.Inc() == totalCount { + event.PostFlush() + } + } +} diff --git a/downstreamadapter/sink/helper/row_callback_test.go b/downstreamadapter/sink/helper/row_callback_test.go new file mode 100644 index 0000000000..c5d4058a34 --- /dev/null +++ b/downstreamadapter/sink/helper/row_callback_test.go @@ -0,0 +1,42 @@ +// 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 helper + +import ( + "testing" + + commonType "github.com/pingcap/ticdc/pkg/common" + commonEvent "github.com/pingcap/ticdc/pkg/common/event" + "github.com/stretchr/testify/require" +) + +func TestTxnPostFlushRowCallback(t *testing.T) { + event := commonEvent.NewDMLEvent(commonType.NewDispatcherID(), 1, 1, 1, nil) + + flushCount := 0 + event.AddPostFlushFunc(func() { + flushCount++ + }) + + rowCallback := NewTxnPostFlushRowCallback(event, 3) + rowCallback() + rowCallback() + require.Equal(t, 0, flushCount) + + rowCallback() + require.Equal(t, 1, flushCount) + + rowCallback() + require.Equal(t, 1, flushCount) +} diff --git a/downstreamadapter/sink/kafka/sink.go b/downstreamadapter/sink/kafka/sink.go index cc2690423f..7016347e8b 100644 --- a/downstreamadapter/sink/kafka/sink.go +++ b/downstreamadapter/sink/kafka/sink.go @@ -219,20 +219,8 @@ func (s *sink) calculateKeyPartitions(ctx context.Context) error { partitionGenerator := s.comp.eventRouter.GetPartitionGenerator(schema, table) selector := s.comp.columnSelector.Get(schema, table) - toRowCallback := func(postTxnFlushed []func(), totalCount uint64) func() { - var calledCount atomic.Uint64 - // The callback of the last row will trigger the callback of the txn. - return func() { - if calledCount.Inc() == totalCount { - for _, callback := range postTxnFlushed { - callback() - } - } - } - } - rowsCount := uint64(event.Len()) - rowCallback := toRowCallback(event.PostTxnFlushed, rowsCount) + rowCallback := helper.NewTxnPostFlushRowCallback(event, rowsCount) for { row, ok := event.GetNextRow() diff --git a/downstreamadapter/sink/mock_sink.go b/downstreamadapter/sink/mock_sink.go index 37b1cfe986..62e71c552a 100644 --- a/downstreamadapter/sink/mock_sink.go +++ b/downstreamadapter/sink/mock_sink.go @@ -32,6 +32,10 @@ func (s *mockSink) AddDMLEvent(event *commonEvent.DMLEvent) { s.mu.Lock() defer s.mu.Unlock() s.dmls = append(s.dmls, event) + // CloudStorage sink wakes dispatcher on enqueue stage. + if s.sinkType == common.CloudStorageSinkType { + event.PostEnqueue() + } } func (s *mockSink) WriteBlockEvent(event commonEvent.BlockEvent) error { diff --git a/downstreamadapter/sink/pulsar/sink.go b/downstreamadapter/sink/pulsar/sink.go index 73e1e92cb6..b2eb8b911c 100644 --- a/downstreamadapter/sink/pulsar/sink.go +++ b/downstreamadapter/sink/pulsar/sink.go @@ -329,20 +329,8 @@ func (s *sink) calculateKeyPartitions(ctx context.Context) error { partitionGenerator := s.comp.eventRouter.GetPartitionGenerator(schema, table) selector := s.comp.columnSelector.Get(schema, table) - toRowCallback := func(postTxnFlushed []func(), totalCount uint64) func() { - var calledCount atomic.Uint64 - // The callback of the last row will trigger the callback of the txn. - return func() { - if calledCount.Inc() == totalCount { - for _, callback := range postTxnFlushed { - callback() - } - } - } - } - rowsCount := uint64(event.Len()) - rowCallback := toRowCallback(event.PostTxnFlushed, rowsCount) + rowCallback := helper.NewTxnPostFlushRowCallback(event, rowsCount) for { row, ok := event.GetNextRow() diff --git a/downstreamadapter/sink/redo/sink.go b/downstreamadapter/sink/redo/sink.go index 70ce2c12a2..f714554976 100644 --- a/downstreamadapter/sink/redo/sink.go +++ b/downstreamadapter/sink/redo/sink.go @@ -18,6 +18,7 @@ import ( "time" "github.com/pingcap/log" + "github.com/pingcap/ticdc/downstreamadapter/sink/helper" "github.com/pingcap/ticdc/pkg/common" commonEvent "github.com/pingcap/ticdc/pkg/common/event" "github.com/pingcap/ticdc/pkg/config" @@ -128,19 +129,8 @@ func (s *Sink) WriteBlockEvent(event commonEvent.BlockEvent) error { func (s *Sink) AddDMLEvent(event *commonEvent.DMLEvent) { _ = s.statistics.RecordBatchExecution(func() (int, int64, error) { - toRowCallback := func(postTxnFlushed []func(), totalCount uint64) func() { - var calledCount atomic.Uint64 - // The callback of the last row will trigger the callback of the txn. - return func() { - if calledCount.Inc() == totalCount { - for _, callback := range postTxnFlushed { - callback() - } - } - } - } rowsCount := uint64(event.Len()) - rowCallback := toRowCallback(event.PostTxnFlushed, rowsCount) + rowCallback := helper.NewTxnPostFlushRowCallback(event, rowsCount) for { row, ok := event.GetNextRow() From 841178b7317dcc6fd544614b542274f14020154f Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Thu, 26 Feb 2026 06:27:27 +0000 Subject: [PATCH 06/20] some small changes --- downstreamadapter/sink/helper/row_callback.go | 4 ++-- downstreamadapter/sink/helper/row_callback_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/downstreamadapter/sink/helper/row_callback.go b/downstreamadapter/sink/helper/row_callback.go index 07c6d4ff9a..e690ec16c0 100644 --- a/downstreamadapter/sink/helper/row_callback.go +++ b/downstreamadapter/sink/helper/row_callback.go @@ -14,13 +14,13 @@ package helper import ( - commonEvent "github.com/pingcap/ticdc/pkg/common/event" + "github.com/pingcap/ticdc/pkg/common/event" "go.uber.org/atomic" ) // NewTxnPostFlushRowCallback returns a row-level callback that triggers txn-level // PostFlush exactly once when the callback has been invoked totalCount times. -func NewTxnPostFlushRowCallback(event *commonEvent.DMLEvent, totalCount uint64) func() { +func NewTxnPostFlushRowCallback(event *event.DMLEvent, totalCount uint64) func() { var calledCount atomic.Uint64 return func() { if calledCount.Inc() == totalCount { diff --git a/downstreamadapter/sink/helper/row_callback_test.go b/downstreamadapter/sink/helper/row_callback_test.go index c5d4058a34..b4996ba4a4 100644 --- a/downstreamadapter/sink/helper/row_callback_test.go +++ b/downstreamadapter/sink/helper/row_callback_test.go @@ -16,13 +16,13 @@ package helper import ( "testing" - commonType "github.com/pingcap/ticdc/pkg/common" + "github.com/pingcap/ticdc/pkg/common" commonEvent "github.com/pingcap/ticdc/pkg/common/event" "github.com/stretchr/testify/require" ) func TestTxnPostFlushRowCallback(t *testing.T) { - event := commonEvent.NewDMLEvent(commonType.NewDispatcherID(), 1, 1, 1, nil) + event := commonEvent.NewDMLEvent(common.NewDispatcherID(), 1, 1, 1, nil) flushCount := 0 event.AddPostFlushFunc(func() { From 81bbfcea165d04069bc89025908dbcc24d81d40b Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Thu, 26 Feb 2026 17:40:30 +0800 Subject: [PATCH 07/20] downstreamadapter,dispatcher: refine sink wake callbacks --- .../dispatcher/basic_dispatcher.go | 43 ++++-- .../dispatcher/event_dispatcher_test.go | 144 ++++++++++++------ downstreamadapter/sink/mock/sink_mock.go | 141 +++++++++++++++++ downstreamadapter/sink/mock_sink.go | 4 - scripts/generate-mock.sh | 1 + 5 files changed, 269 insertions(+), 64 deletions(-) create mode 100644 downstreamadapter/sink/mock/sink_mock.go diff --git a/downstreamadapter/dispatcher/basic_dispatcher.go b/downstreamadapter/dispatcher/basic_dispatcher.go index fe471d6fa1..eaf28479e1 100644 --- a/downstreamadapter/dispatcher/basic_dispatcher.go +++ b/downstreamadapter/dispatcher/basic_dispatcher.go @@ -262,9 +262,10 @@ func NewBasicDispatcher( return dispatcher } -// AddDMLEventsToSink filters events for special tables and only returns true when -// at least one event remains to be written to the downstream sink. -func (d *BasicDispatcher) AddDMLEventsToSink(events []*commonEvent.DMLEvent) bool { +// AddDMLEventsToSink filters events for special tables, registers batch wake +// callbacks, and returns true when at least one event remains to be written to +// the downstream sink. +func (d *BasicDispatcher) AddDMLEventsToSink(events []*commonEvent.DMLEvent, wakeCallback func()) bool { // Normal DML dispatch: most tables just pass through this function unchanged. // Active-active or soft-delete tables are processed by FilterDMLEvent before // being handed over to the sink (delete rows dropped; soft-delete transitions may @@ -288,9 +289,29 @@ func (d *BasicDispatcher) AddDMLEventsToSink(events []*commonEvent.DMLEvent) boo return false } + if d.sink.SinkType() == common.CloudStorageSinkType { + var remaining atomic.Int64 + remaining.Store(int64(len(filteredEvents))) + for _, event := range filteredEvents { + event.AddPostEnqueueFunc(func() { + if remaining.Add(-1) == 0 { + wakeCallback() + } + }) + } + } else { + wakeOnce := &sync.Once{} + for _, event := range filteredEvents { + event.AddPostFlushFunc(func() { + if d.tableProgress.Empty() { + wakeOnce.Do(wakeCallback) + } + }) + } + } + // for one batch events, we need to add all them in table progress first, then add them to sink - // because we need to ensure the wakeCallback only will be called when - // all these events are flushed to downstream successfully + // to avoid checkpoint jumping while events are being enqueued/flushed. for _, event := range filteredEvents { d.tableProgress.Add(event) } @@ -545,10 +566,8 @@ func (d *BasicDispatcher) HandleEvents(dispatcherEvents []DispatcherEvent, wakeC // When we handle events, we don't have any previous events still in sink. // // wakeCallback is used to wake the dynamic stream to handle the next batch events. -// It is registered on enqueue stage, and sink implementations are responsible for -// deciding when enqueue is considered complete (some sinks trigger enqueue via -// PostFlush after downstream flush completes). -// Checkpoint still advances only after PostFlush callbacks are completed. +// For non-storage sinks, we wake only after flush and tableProgress becomes empty. +// For cloud storage sink, we wake after all events in this batch are enqueued. func (d *BasicDispatcher) handleEvents(dispatcherEvents []DispatcherEvent, wakeCallback func()) bool { if d.GetRemovingStatus() { log.Warn("dispatcher is removing", zap.Any("id", d.id)) @@ -560,7 +579,6 @@ func (d *BasicDispatcher) handleEvents(dispatcherEvents []DispatcherEvent, wakeC // Only return false when all events are resolvedTs Event. block := false - dmlWakeOnce := &sync.Once{} dmlEvents := make([]*commonEvent.DMLEvent, 0, len(dispatcherEvents)) latestResolvedTs := uint64(0) // Dispatcher is ready, handle the events @@ -625,9 +643,6 @@ func (d *BasicDispatcher) handleEvents(dispatcherEvents []DispatcherEvent, wakeC block = true dml.ReplicatingTs = d.creationPDTs - dml.AddPostEnqueueFunc(func() { - dmlWakeOnce.Do(wakeCallback) - }) dmlEvents = append(dmlEvents, dml) case commonEvent.TypeDDLEvent: if len(dispatcherEvents) != 1 { @@ -711,7 +726,7 @@ func (d *BasicDispatcher) handleEvents(dispatcherEvents []DispatcherEvent, wakeC // the checkpointTs may be incorrect set as the new resolvedTs, // due to the tableProgress is empty before dml events add into sink. if len(dmlEvents) > 0 { - hasDMLToFlush := d.AddDMLEventsToSink(dmlEvents) + hasDMLToFlush := d.AddDMLEventsToSink(dmlEvents, wakeCallback) if !hasDMLToFlush { // All DML events were filtered out, so they no longer block dispatcher progress. block = false diff --git a/downstreamadapter/dispatcher/event_dispatcher_test.go b/downstreamadapter/dispatcher/event_dispatcher_test.go index d36d8eac99..228c0a2c20 100644 --- a/downstreamadapter/dispatcher/event_dispatcher_test.go +++ b/downstreamadapter/dispatcher/event_dispatcher_test.go @@ -20,8 +20,10 @@ import ( "testing" "time" + "github.com/golang/mock/gomock" "github.com/pingcap/failpoint" "github.com/pingcap/ticdc/downstreamadapter/sink" + "github.com/pingcap/ticdc/downstreamadapter/sink/mock" "github.com/pingcap/ticdc/downstreamadapter/syncpoint" "github.com/pingcap/ticdc/heartbeatpb" "github.com/pingcap/ticdc/pkg/common" @@ -777,7 +779,7 @@ func TestBatchDMLEventsPartialFlush(t *testing.T) { require.Equal(t, 0, len(mockSink.GetDMLs())) } -func TestDMLWakeCallbackBySinkType(t *testing.T) { +func TestDMLWakeCallbackNonStorageOnlyAfterTableProgressEmpty(t *testing.T) { helper := commonEvent.NewEventTestHelper(t) defer helper.Close() @@ -790,60 +792,110 @@ func TestDMLWakeCallbackBySinkType(t *testing.T) { "test", "t", fmt.Sprintf("insert into t values(%d, %d)", commitTs, commitTs), - fmt.Sprintf("insert into t values(%d, %d)", commitTs+1000, commitTs+1000), ) require.NotNil(t, event) event.CommitTs = commitTs return event } - testCases := []struct { - name string - sinkType common.SinkType - commitTs uint64 - wakeOnEnqueue bool - }{ - { - name: "mysql wake after flush", - sinkType: common.MysqlSinkType, - commitTs: 20, - wakeOnEnqueue: false, - }, - { - name: "cloudstorage wake on enqueue", - sinkType: common.CloudStorageSinkType, - commitTs: 21, - wakeOnEnqueue: true, - }, + ctrl := gomock.NewController(t) + mockSink := mock.NewMockSink(ctrl) + mockSink.EXPECT().SinkType().Return(common.MysqlSinkType).AnyTimes() + capturedDMLs := make([]*commonEvent.DMLEvent, 0, 3) + mockSink.EXPECT().AddDMLEvent(gomock.Any()).Do(func(event *commonEvent.DMLEvent) { + capturedDMLs = append(capturedDMLs, event) + }).Times(3) + tableSpan, err := getCompleteTableSpan(getTestingKeyspaceID()) + require.NoError(t, err) + dispatcher := newDispatcherForTest(mockSink, tableSpan) + + dmlEvent1 := buildDMLEvent(100) + dmlEvent2 := buildDMLEvent(101) + dmlEvent3 := buildDMLEvent(102) + nodeID := node.NewID() + dispatcherEvents := []DispatcherEvent{ + NewDispatcherEvent(&nodeID, dmlEvent1), + NewDispatcherEvent(&nodeID, dmlEvent2), + NewDispatcherEvent(&nodeID, dmlEvent3), } - for _, tc := range testCases { - tc := tc - t.Run(tc.name, func(t *testing.T) { - mockSink := sink.NewMockSink(tc.sinkType) - tableSpan, err := getCompleteTableSpan(getTestingKeyspaceID()) - require.NoError(t, err) - dispatcher := newDispatcherForTest(mockSink, tableSpan) - - dmlEvent := buildDMLEvent(tc.commitTs) - nodeID := node.NewID() - var callbackCalled atomic.Bool - block := dispatcher.HandleEvents([]DispatcherEvent{NewDispatcherEvent(&nodeID, dmlEvent)}, func() { - callbackCalled.Store(true) - }) - require.True(t, block) - require.Equal(t, tc.wakeOnEnqueue, callbackCalled.Load()) - require.Len(t, mockSink.GetDMLs(), 1) - - if tc.wakeOnEnqueue { - return - } - - mockSink.FlushDMLs() - require.True(t, callbackCalled.Load()) - require.Len(t, mockSink.GetDMLs(), 0) - }) + var callbackCalled atomic.Bool + block := dispatcher.HandleEvents(dispatcherEvents, func() { + callbackCalled.Store(true) + }) + require.True(t, block) + require.False(t, callbackCalled.Load()) + + require.Len(t, capturedDMLs, 3) + + capturedDMLs[0].PostFlush() + require.False(t, callbackCalled.Load()) + + capturedDMLs[1].PostFlush() + require.False(t, callbackCalled.Load()) + + capturedDMLs[2].PostFlush() + require.True(t, callbackCalled.Load()) + require.True(t, dispatcher.tableProgress.Empty()) +} + +func TestDMLWakeCallbackStorageAfterBatchEnqueue(t *testing.T) { + helper := commonEvent.NewEventTestHelper(t) + defer helper.Close() + + helper.Tk().MustExec("use test") + ddlJob := helper.DDL2Job("create table t(id int primary key, v int)") + require.NotNil(t, ddlJob) + + buildDMLEvent := func(commitTs uint64) *commonEvent.DMLEvent { + event := helper.DML2Event( + "test", + "t", + fmt.Sprintf("insert into t values(%d, %d)", commitTs, commitTs), + ) + require.NotNil(t, event) + event.CommitTs = commitTs + return event + } + + ctrl := gomock.NewController(t) + mockSink := mock.NewMockSink(ctrl) + mockSink.EXPECT().SinkType().Return(common.CloudStorageSinkType).AnyTimes() + capturedDMLs := make([]*commonEvent.DMLEvent, 0, 3) + mockSink.EXPECT().AddDMLEvent(gomock.Any()).Do(func(event *commonEvent.DMLEvent) { + capturedDMLs = append(capturedDMLs, event) + }).Times(3) + tableSpan, err := getCompleteTableSpan(getTestingKeyspaceID()) + require.NoError(t, err) + dispatcher := newDispatcherForTest(mockSink, tableSpan) + + dmlEvent1 := buildDMLEvent(110) + dmlEvent2 := buildDMLEvent(111) + dmlEvent3 := buildDMLEvent(112) + nodeID := node.NewID() + dispatcherEvents := []DispatcherEvent{ + NewDispatcherEvent(&nodeID, dmlEvent1), + NewDispatcherEvent(&nodeID, dmlEvent2), + NewDispatcherEvent(&nodeID, dmlEvent3), } + + var callbackCalled atomic.Bool + block := dispatcher.HandleEvents(dispatcherEvents, func() { + callbackCalled.Store(true) + }) + require.True(t, block) + require.False(t, callbackCalled.Load()) + + require.Len(t, capturedDMLs, 3) + + capturedDMLs[0].PostEnqueue() + require.False(t, callbackCalled.Load()) + + capturedDMLs[1].PostEnqueue() + require.False(t, callbackCalled.Load()) + + capturedDMLs[2].PostEnqueue() + require.True(t, callbackCalled.Load()) } // TestDispatcherSplittableCheck tests that a split table dispatcher with enableSplittableCheck=true diff --git a/downstreamadapter/sink/mock/sink_mock.go b/downstreamadapter/sink/mock/sink_mock.go new file mode 100644 index 0000000000..d5ea8540bd --- /dev/null +++ b/downstreamadapter/sink/mock/sink_mock.go @@ -0,0 +1,141 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: downstreamadapter/sink/sink.go + +// Package mock is a generated GoMock package. +package mock + +import ( + context "context" + reflect "reflect" + + gomock "github.com/golang/mock/gomock" + common "github.com/pingcap/ticdc/pkg/common" + event "github.com/pingcap/ticdc/pkg/common/event" +) + +// MockSink is a mock of Sink interface. +type MockSink struct { + ctrl *gomock.Controller + recorder *MockSinkMockRecorder +} + +// MockSinkMockRecorder is the mock recorder for MockSink. +type MockSinkMockRecorder struct { + mock *MockSink +} + +// NewMockSink creates a new mock instance. +func NewMockSink(ctrl *gomock.Controller) *MockSink { + mock := &MockSink{ctrl: ctrl} + mock.recorder = &MockSinkMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockSink) EXPECT() *MockSinkMockRecorder { + return m.recorder +} + +// AddCheckpointTs mocks base method. +func (m *MockSink) AddCheckpointTs(ts uint64) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "AddCheckpointTs", ts) +} + +// AddCheckpointTs indicates an expected call of AddCheckpointTs. +func (mr *MockSinkMockRecorder) AddCheckpointTs(ts interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddCheckpointTs", reflect.TypeOf((*MockSink)(nil).AddCheckpointTs), ts) +} + +// AddDMLEvent mocks base method. +func (m *MockSink) AddDMLEvent(event *event.DMLEvent) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "AddDMLEvent", event) +} + +// AddDMLEvent indicates an expected call of AddDMLEvent. +func (mr *MockSinkMockRecorder) AddDMLEvent(event interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddDMLEvent", reflect.TypeOf((*MockSink)(nil).AddDMLEvent), event) +} + +// Close mocks base method. +func (m *MockSink) Close(removeChangefeed bool) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "Close", removeChangefeed) +} + +// Close indicates an expected call of Close. +func (mr *MockSinkMockRecorder) Close(removeChangefeed interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Close", reflect.TypeOf((*MockSink)(nil).Close), removeChangefeed) +} + +// IsNormal mocks base method. +func (m *MockSink) IsNormal() bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IsNormal") + ret0, _ := ret[0].(bool) + return ret0 +} + +// IsNormal indicates an expected call of IsNormal. +func (mr *MockSinkMockRecorder) IsNormal() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsNormal", reflect.TypeOf((*MockSink)(nil).IsNormal)) +} + +// Run mocks base method. +func (m *MockSink) Run(ctx context.Context) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Run", ctx) + ret0, _ := ret[0].(error) + return ret0 +} + +// Run indicates an expected call of Run. +func (mr *MockSinkMockRecorder) Run(ctx interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Run", reflect.TypeOf((*MockSink)(nil).Run), ctx) +} + +// SetTableSchemaStore mocks base method. +func (m *MockSink) SetTableSchemaStore(tableSchemaStore *event.TableSchemaStore) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetTableSchemaStore", tableSchemaStore) +} + +// SetTableSchemaStore indicates an expected call of SetTableSchemaStore. +func (mr *MockSinkMockRecorder) SetTableSchemaStore(tableSchemaStore interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetTableSchemaStore", reflect.TypeOf((*MockSink)(nil).SetTableSchemaStore), tableSchemaStore) +} + +// SinkType mocks base method. +func (m *MockSink) SinkType() common.SinkType { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SinkType") + ret0, _ := ret[0].(common.SinkType) + return ret0 +} + +// SinkType indicates an expected call of SinkType. +func (mr *MockSinkMockRecorder) SinkType() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SinkType", reflect.TypeOf((*MockSink)(nil).SinkType)) +} + +// WriteBlockEvent mocks base method. +func (m *MockSink) WriteBlockEvent(event event.BlockEvent) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "WriteBlockEvent", event) + ret0, _ := ret[0].(error) + return ret0 +} + +// WriteBlockEvent indicates an expected call of WriteBlockEvent. +func (mr *MockSinkMockRecorder) WriteBlockEvent(event interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WriteBlockEvent", reflect.TypeOf((*MockSink)(nil).WriteBlockEvent), event) +} diff --git a/downstreamadapter/sink/mock_sink.go b/downstreamadapter/sink/mock_sink.go index 62e71c552a..37b1cfe986 100644 --- a/downstreamadapter/sink/mock_sink.go +++ b/downstreamadapter/sink/mock_sink.go @@ -32,10 +32,6 @@ func (s *mockSink) AddDMLEvent(event *commonEvent.DMLEvent) { s.mu.Lock() defer s.mu.Unlock() s.dmls = append(s.dmls, event) - // CloudStorage sink wakes dispatcher on enqueue stage. - if s.sinkType == common.CloudStorageSinkType { - event.PostEnqueue() - } } func (s *mockSink) WriteBlockEvent(event commonEvent.BlockEvent) error { diff --git a/scripts/generate-mock.sh b/scripts/generate-mock.sh index 3610b056ab..2b2cc79d68 100755 --- a/scripts/generate-mock.sh +++ b/scripts/generate-mock.sh @@ -36,3 +36,4 @@ fi "$MOCKGEN" -source pkg/sink/codec/simple/marshaller.go -destination pkg/sink/codec/simple/mock/marshaller.go "$MOCKGEN" -source pkg/keyspace/keyspace_manager.go -destination pkg/keyspace/keyspace_manager_mock.go -package keyspace "$MOCKGEN" -source pkg/txnutil/gc/gc_manager.go -destination pkg/txnutil/gc/gc_manager_mock.go -package gc +"$MOCKGEN" -source downstreamadapter/sink/sink.go -destination downstreamadapter/sink/mock/sink_mock.go -package mock From 3ded4e07621739788be9bc1701759777f9986447 Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Thu, 26 Feb 2026 18:48:59 +0800 Subject: [PATCH 08/20] fix code --- .../dispatcher/basic_dispatcher.go | 27 ++++++------------- pkg/common/event/dml_event.go | 9 +++---- pkg/common/event/dml_event_test.go | 18 +++++++++++++ 3 files changed, 30 insertions(+), 24 deletions(-) diff --git a/downstreamadapter/dispatcher/basic_dispatcher.go b/downstreamadapter/dispatcher/basic_dispatcher.go index eaf28479e1..dd45703ef5 100644 --- a/downstreamadapter/dispatcher/basic_dispatcher.go +++ b/downstreamadapter/dispatcher/basic_dispatcher.go @@ -289,25 +289,14 @@ func (d *BasicDispatcher) AddDMLEventsToSink(events []*commonEvent.DMLEvent, wak return false } - if d.sink.SinkType() == common.CloudStorageSinkType { - var remaining atomic.Int64 - remaining.Store(int64(len(filteredEvents))) - for _, event := range filteredEvents { - event.AddPostEnqueueFunc(func() { - if remaining.Add(-1) == 0 { - wakeCallback() - } - }) - } - } else { - wakeOnce := &sync.Once{} - for _, event := range filteredEvents { - event.AddPostFlushFunc(func() { - if d.tableProgress.Empty() { - wakeOnce.Do(wakeCallback) - } - }) - } + var remaining atomic.Int64 + remaining.Store(int64(len(filteredEvents))) + for _, event := range filteredEvents { + event.AddPostEnqueueFunc(func() { + if remaining.Add(-1) == 0 { + wakeCallback() + } + }) } // for one batch events, we need to add all them in table progress first, then add them to sink diff --git a/pkg/common/event/dml_event.go b/pkg/common/event/dml_event.go index 7d0cd3576c..99d318307f 100644 --- a/pkg/common/event/dml_event.go +++ b/pkg/common/event/dml_event.go @@ -642,14 +642,13 @@ func (t *DMLEvent) GetStartTs() common.Ts { // PostFlush marks the transaction as flushed to downstream. // -// It always calls PostEnqueue first to preserve callback order: -// enqueue callbacks run before flush callbacks, even for sinks that only invoke -// PostFlush and do not have an explicit enqueue hook. +// It runs flush callbacks first, then triggers PostEnqueue. For sinks with an +// explicit enqueue hook, PostEnqueue may already have been called before flush. func (t *DMLEvent) PostFlush() { - t.PostEnqueue() for _, f := range t.PostTxnFlushed { f() } + t.PostEnqueue() } // PostEnqueue marks the transaction as enqueued into sink internal pipeline. @@ -698,7 +697,7 @@ func (t *DMLEvent) ClearPostEnqueueFunc() { // AddPostEnqueueFunc registers a callback that runs at enqueue stage. // // Use this when only sink acceptance is required. For sinks with no explicit -// enqueue signal, this callback is triggered via PostFlush. +// enqueue signal, this callback is triggered after flush callbacks in PostFlush. func (t *DMLEvent) AddPostEnqueueFunc(f func()) { t.PostTxnEnqueued = append(t.PostTxnEnqueued, f) } diff --git a/pkg/common/event/dml_event_test.go b/pkg/common/event/dml_event_test.go index 1fe4763347..0c0df8fb85 100644 --- a/pkg/common/event/dml_event_test.go +++ b/pkg/common/event/dml_event_test.go @@ -383,6 +383,24 @@ func TestDMLEventPostFlushTriggersPostEnqueueOnce(t *testing.T) { require.Equal(t, int64(2), atomic.LoadInt64(&flushCalled)) } +func TestDMLEventPostFlushRunsFlushBeforePostEnqueueFallback(t *testing.T) { + t.Parallel() + + event := &DMLEvent{} + order := make([]string, 0, 3) + event.AddPostFlushFunc(func() { + order = append(order, "flush") + }) + event.AddPostEnqueueFunc(func() { + order = append(order, "enqueue") + }) + + event.PostFlush() + event.PostFlush() + + require.Equal(t, []string{"flush", "enqueue", "flush"}, order) +} + func TestDMLEventPostEnqueueConcurrentWithPostFlush(t *testing.T) { t.Parallel() From cdf2a98bb50032ca2cecb8e078892a590473dc17 Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Thu, 26 Feb 2026 19:32:17 +0800 Subject: [PATCH 09/20] fix --- .../dispatcher/event_dispatcher_test.go | 39 +++++++++++++------ .../sink/cloudstorage/dml_writers.go | 6 +-- pkg/common/event/active_active.go | 4 +- pkg/common/event/active_active_test.go | 14 +++---- pkg/common/event/dml_event.go | 6 +-- pkg/common/event/dml_event_test.go | 28 ++++++------- 6 files changed, 56 insertions(+), 41 deletions(-) diff --git a/downstreamadapter/dispatcher/event_dispatcher_test.go b/downstreamadapter/dispatcher/event_dispatcher_test.go index 228c0a2c20..7d8d1ce050 100644 --- a/downstreamadapter/dispatcher/event_dispatcher_test.go +++ b/downstreamadapter/dispatcher/event_dispatcher_test.go @@ -741,9 +741,9 @@ func TestBatchDMLEventsPartialFlush(t *testing.T) { dispatcher := newDispatcherForTest(mockSink, tableSpan) // Create a callback that records when it's called - var callbackCalled bool + var callbackCalled atomic.Bool wakeCallback := func() { - callbackCalled = true + callbackCalled.Store(true) } nodeID := node.NewID() @@ -755,25 +755,42 @@ func TestBatchDMLEventsPartialFlush(t *testing.T) { NewDispatcherEvent(&nodeID, dmlEvent3), } - failpoint.Enable("github.com/pingcap/ticdc/downstreamadapter/dispatcher/BlockAddDMLEvents", `pause`) + require.NoError(t, failpoint.Enable("github.com/pingcap/ticdc/downstreamadapter/dispatcher/BlockAddDMLEvents", `pause`)) + failpointEnabled := true + defer func() { + if failpointEnabled { + require.NoError(t, failpoint.Disable("github.com/pingcap/ticdc/downstreamadapter/dispatcher/BlockAddDMLEvents")) + } + }() + resultCh := make(chan bool, 1) go func() { block := dispatcher.HandleEvents(dispatcherEvents, wakeCallback) - require.Equal(t, true, block) + resultCh <- block }() - time.Sleep(1 * time.Second) - require.Equal(t, 1, len(mockSink.GetDMLs())) + require.Eventually(t, func() bool { + return len(mockSink.GetDMLs()) == 1 + }, 5*time.Second, 10*time.Millisecond) mockSink.FlushDMLs() - require.False(t, callbackCalled) + require.False(t, callbackCalled.Load()) - failpoint.Disable("github.com/pingcap/ticdc/downstreamadapter/dispatcher/BlockAddDMLEvents") + require.NoError(t, failpoint.Disable("github.com/pingcap/ticdc/downstreamadapter/dispatcher/BlockAddDMLEvents")) + failpointEnabled = false - time.Sleep(1 * time.Second) - require.Equal(t, 2, len(mockSink.GetDMLs())) + require.Eventually(t, func() bool { + return len(mockSink.GetDMLs()) == 2 + }, 5*time.Second, 10*time.Millisecond) mockSink.FlushDMLs() + require.Eventually(t, func() bool { + return callbackCalled.Load() + }, 5*time.Second, 10*time.Millisecond) // Now the callback should be called after all events are flushed - require.True(t, callbackCalled) + require.True(t, callbackCalled.Load()) + require.Eventually(t, func() bool { + return len(mockSink.GetDMLs()) == 0 + }, 5*time.Second, 10*time.Millisecond) + require.True(t, <-resultCh) // Verify that all events were actually flushed require.Equal(t, 0, len(mockSink.GetDMLs())) diff --git a/downstreamadapter/sink/cloudstorage/dml_writers.go b/downstreamadapter/sink/cloudstorage/dml_writers.go index 678c0f2b8f..4dc6e245ee 100644 --- a/downstreamadapter/sink/cloudstorage/dml_writers.go +++ b/downstreamadapter/sink/cloudstorage/dml_writers.go @@ -15,7 +15,6 @@ package cloudstorage import ( "context" - "sync/atomic" commonType "github.com/pingcap/ticdc/pkg/common" commonEvent "github.com/pingcap/ticdc/pkg/common/event" @@ -24,6 +23,7 @@ import ( "github.com/pingcap/ticdc/pkg/sink/codec/common" "github.com/pingcap/ticdc/utils/chann" "github.com/pingcap/tidb/br/pkg/storage" + "go.uber.org/atomic" "golang.org/x/sync/errgroup" ) @@ -45,7 +45,7 @@ type dmlWriters struct { writers []*writer // last sequence number - lastSeqNum uint64 + lastSeqNum atomic.Uint64 } func newDMLWriters( @@ -112,7 +112,7 @@ func (d *dmlWriters) AddDMLEvent(event *commonEvent.DMLEvent) { TableInfoVersion: event.TableInfoVersion, DispatcherID: event.GetDispatcherID(), } - seq := atomic.AddUint64(&d.lastSeqNum, 1) + seq := d.lastSeqNum.Inc() // emit a TxnCallbackableEvent encoupled with a sequence number starting from one. d.msgCh.Push(newEventFragment(seq, tbl, event)) } diff --git a/pkg/common/event/active_active.go b/pkg/common/event/active_active.go index 7277704174..2ee11c2ffb 100644 --- a/pkg/common/event/active_active.go +++ b/pkg/common/event/active_active.go @@ -14,8 +14,6 @@ package event import ( - "sync/atomic" - "github.com/pingcap/log" "github.com/pingcap/ticdc/pkg/common" "github.com/pingcap/ticdc/pkg/errors" @@ -447,7 +445,7 @@ func newFilteredDMLEvent( newEvent.ReplicatingTs = source.ReplicatingTs newEvent.PostTxnEnqueued = source.PostTxnEnqueued newEvent.PostTxnFlushed = source.PostTxnFlushed - newEvent.postEnqueueCalled = atomic.LoadUint32(&source.postEnqueueCalled) + newEvent.postEnqueueCalled.Store(source.postEnqueueCalled.Load()) source.PostTxnEnqueued = nil source.PostTxnFlushed = nil diff --git a/pkg/common/event/active_active_test.go b/pkg/common/event/active_active_test.go index 82c14d1c78..70e47b17c4 100644 --- a/pkg/common/event/active_active_test.go +++ b/pkg/common/event/active_active_test.go @@ -14,7 +14,6 @@ package event import ( - "sync/atomic" "testing" "time" @@ -26,6 +25,7 @@ import ( tidbTypes "github.com/pingcap/tidb/pkg/types" "github.com/pingcap/tidb/pkg/util/chunk" "github.com/stretchr/testify/require" + "go.uber.org/atomic" ) func TestFilterDMLEventNormalTablePassthrough(t *testing.T) { @@ -327,13 +327,13 @@ func TestFilterDMLEventKeepsPostEnqueueCallbacksOnFilteredEvent(t *testing.T) { {int64(1), ts}, }) - var enqueueCalled int64 - var flushCalled int64 + var enqueueCalled atomic.Int64 + var flushCalled atomic.Int64 event.AddPostEnqueueFunc(func() { - atomic.AddInt64(&enqueueCalled, 1) + enqueueCalled.Inc() }) event.AddPostFlushFunc(func() { - atomic.AddInt64(&flushCalled, 1) + flushCalled.Inc() }) filtered, skip := FilterDMLEvent(event, false, nil) @@ -343,8 +343,8 @@ func TestFilterDMLEventKeepsPostEnqueueCallbacksOnFilteredEvent(t *testing.T) { filtered.PostEnqueue() filtered.PostFlush() - require.Equal(t, int64(1), atomic.LoadInt64(&enqueueCalled)) - require.Equal(t, int64(1), atomic.LoadInt64(&flushCalled)) + require.Equal(t, int64(1), enqueueCalled.Load()) + require.Equal(t, int64(1), flushCalled.Load()) } func newTestTableInfo(t *testing.T, activeActive, softDelete bool) *commonpkg.TableInfo { diff --git a/pkg/common/event/dml_event.go b/pkg/common/event/dml_event.go index 99d318307f..11eb8eb37a 100644 --- a/pkg/common/event/dml_event.go +++ b/pkg/common/event/dml_event.go @@ -17,7 +17,6 @@ import ( "encoding/binary" "fmt" "strings" - "sync/atomic" "time" "github.com/pingcap/failpoint" @@ -29,6 +28,7 @@ import ( "github.com/pingcap/ticdc/pkg/util" "github.com/pingcap/tidb/pkg/types" "github.com/pingcap/tidb/pkg/util/chunk" + "go.uber.org/atomic" "go.uber.org/zap" ) @@ -404,7 +404,7 @@ type DMLEvent struct { // checkpoint related logic. PostTxnFlushed []func() `json:"-"` // postEnqueueCalled ensures PostTxnEnqueued callbacks are triggered at most once. - postEnqueueCalled uint32 `json:"-"` + postEnqueueCalled atomic.Bool `json:"-"` // eventSize is the size of the event in bytes. It is set when it's unmarshaled. eventSize int64 `json:"-"` @@ -656,7 +656,7 @@ func (t *DMLEvent) PostFlush() { // This stage does not mean data is already written to downstream. The method is // idempotent and guarantees enqueue callbacks run at most once. func (t *DMLEvent) PostEnqueue() { - if !atomic.CompareAndSwapUint32(&t.postEnqueueCalled, 0, 1) { + if !t.postEnqueueCalled.CAS(false, true) { return } for _, f := range t.PostTxnEnqueued { diff --git a/pkg/common/event/dml_event_test.go b/pkg/common/event/dml_event_test.go index 0c0df8fb85..766c20767b 100644 --- a/pkg/common/event/dml_event_test.go +++ b/pkg/common/event/dml_event_test.go @@ -16,7 +16,6 @@ package event import ( "encoding/binary" "sync" - "sync/atomic" "testing" "github.com/pingcap/ticdc/pkg/common" @@ -25,6 +24,7 @@ import ( "github.com/pingcap/tidb/pkg/types" "github.com/pingcap/tidb/pkg/util/chunk" "github.com/stretchr/testify/require" + "go.uber.org/atomic" ) func TestDMLEventBasicEncodeAndDecode(t *testing.T) { @@ -349,38 +349,38 @@ func TestDMLEventPostEnqueueFuncs(t *testing.T) { t.Parallel() event := &DMLEvent{} - var called int64 + var called atomic.Int64 event.AddPostEnqueueFunc(func() { - atomic.AddInt64(&called, 1) + called.Inc() }) event.AddPostEnqueueFunc(func() { - atomic.AddInt64(&called, 1) + called.Inc() }) event.PostEnqueue() event.PostEnqueue() - require.Equal(t, int64(2), atomic.LoadInt64(&called)) + require.Equal(t, int64(2), called.Load()) } func TestDMLEventPostFlushTriggersPostEnqueueOnce(t *testing.T) { t.Parallel() event := &DMLEvent{} - var enqueueCalled int64 - var flushCalled int64 + var enqueueCalled atomic.Int64 + var flushCalled atomic.Int64 event.AddPostEnqueueFunc(func() { - atomic.AddInt64(&enqueueCalled, 1) + enqueueCalled.Inc() }) event.AddPostFlushFunc(func() { - atomic.AddInt64(&flushCalled, 1) + flushCalled.Inc() }) event.PostFlush() event.PostFlush() - require.Equal(t, int64(1), atomic.LoadInt64(&enqueueCalled)) - require.Equal(t, int64(2), atomic.LoadInt64(&flushCalled)) + require.Equal(t, int64(1), enqueueCalled.Load()) + require.Equal(t, int64(2), flushCalled.Load()) } func TestDMLEventPostFlushRunsFlushBeforePostEnqueueFallback(t *testing.T) { @@ -405,9 +405,9 @@ func TestDMLEventPostEnqueueConcurrentWithPostFlush(t *testing.T) { t.Parallel() event := &DMLEvent{} - var enqueueCalled int64 + var enqueueCalled atomic.Int64 event.AddPostEnqueueFunc(func() { - atomic.AddInt64(&enqueueCalled, 1) + enqueueCalled.Inc() }) var wg sync.WaitGroup @@ -425,5 +425,5 @@ func TestDMLEventPostEnqueueConcurrentWithPostFlush(t *testing.T) { } wg.Wait() - require.Equal(t, int64(1), atomic.LoadInt64(&enqueueCalled)) + require.Equal(t, int64(1), enqueueCalled.Load()) } From 460ac1ec29829cf6c1e3be081815f7f55c0f3af9 Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Thu, 26 Feb 2026 20:11:55 +0800 Subject: [PATCH 10/20] fix --- downstreamadapter/dispatcher/basic_dispatcher.go | 12 ++++++------ .../dispatcher/basic_dispatcher_info.go | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/downstreamadapter/dispatcher/basic_dispatcher.go b/downstreamadapter/dispatcher/basic_dispatcher.go index dd45703ef5..45d41d2f15 100644 --- a/downstreamadapter/dispatcher/basic_dispatcher.go +++ b/downstreamadapter/dispatcher/basic_dispatcher.go @@ -17,7 +17,6 @@ import ( "fmt" "math/rand" "sync" - "sync/atomic" "time" "github.com/pingcap/failpoint" @@ -31,6 +30,7 @@ import ( "github.com/pingcap/ticdc/pkg/errors" "github.com/pingcap/tidb/pkg/parser/mysql" tidbTypes "github.com/pingcap/tidb/pkg/types" + "go.uber.org/atomic" "go.uber.org/zap" "go.uber.org/zap/zapcore" ) @@ -168,7 +168,7 @@ type BasicDispatcher struct { sink sink.Sink // the max resolvedTs received by the dispatcher - resolvedTs uint64 + resolvedTs atomic.Uint64 // blockEventStatus is used to store the current pending ddl/sync point event and its block status. blockEventStatus BlockEventStatus @@ -246,7 +246,6 @@ func NewBasicDispatcher( sharedInfo: sharedInfo, sink: sink, componentStatus: newComponentStateWithMutex(heartbeatpb.ComponentState_Initializing), - resolvedTs: startTs, isRemoving: atomic.Bool{}, duringHandleEvents: atomic.Bool{}, blockEventStatus: BlockEventStatus{blockPendingEvent: nil}, @@ -258,6 +257,7 @@ func NewBasicDispatcher( mode: mode, BootstrapState: BootstrapFinished, } + dispatcher.resolvedTs.Store(startTs) return dispatcher } @@ -293,7 +293,7 @@ func (d *BasicDispatcher) AddDMLEventsToSink(events []*commonEvent.DMLEvent, wak remaining.Store(int64(len(filteredEvents))) for _, event := range filteredEvents { event.AddPostEnqueueFunc(func() { - if remaining.Add(-1) == 0 { + if remaining.Dec() == 0 { wakeCallback() } }) @@ -483,7 +483,7 @@ func (d *BasicDispatcher) GetHeartBeatInfo(h *HeartBeatInfo) { } func (d *BasicDispatcher) GetResolvedTs() uint64 { - return atomic.LoadUint64(&d.resolvedTs) + return d.resolvedTs.Load() } func (d *BasicDispatcher) GetLastSyncedTs() uint64 { @@ -722,7 +722,7 @@ func (d *BasicDispatcher) handleEvents(dispatcherEvents []DispatcherEvent, wakeC } } if latestResolvedTs > 0 { - atomic.StoreUint64(&d.resolvedTs, latestResolvedTs) + d.resolvedTs.Store(latestResolvedTs) } return block } diff --git a/downstreamadapter/dispatcher/basic_dispatcher_info.go b/downstreamadapter/dispatcher/basic_dispatcher_info.go index 552fefab18..4845b35d97 100644 --- a/downstreamadapter/dispatcher/basic_dispatcher_info.go +++ b/downstreamadapter/dispatcher/basic_dispatcher_info.go @@ -227,7 +227,7 @@ func (d *BasicDispatcher) IsTableTriggerDispatcher() bool { // SetStartTs only be called after the dispatcher is created func (d *BasicDispatcher) SetStartTs(startTs uint64) { atomic.StoreUint64(&d.startTs, startTs) - atomic.StoreUint64(&d.resolvedTs, startTs) + d.resolvedTs.Store(startTs) } func (d *BasicDispatcher) SetCurrentPDTs(currentPDTs uint64) { From 05f0792af461dd277fdba11315bc17953d115852 Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Fri, 27 Feb 2026 10:56:34 +0800 Subject: [PATCH 11/20] cloudstorage: drain affected dispatchers before ddl write --- .../sink/cloudstorage/defragmenter.go | 68 ++++++-- .../sink/cloudstorage/dml_writers.go | 150 ++++++++++++++++-- .../cloudstorage/dml_writers_drain_test.go | 108 +++++++++++++ .../sink/cloudstorage/encoding_group.go | 10 +- downstreamadapter/sink/cloudstorage/sink.go | 7 +- .../sink/cloudstorage/sink_test.go | 60 +++++++ downstreamadapter/sink/cloudstorage/writer.go | 49 +++++- .../sink/cloudstorage/writer_test.go | 73 +++++++++ .../sink/metrics/cloudstorage.go | 9 ++ pkg/common/event/table_schema_store.go | 13 +- pkg/common/event/table_schema_store_test.go | 32 ++++ 11 files changed, 547 insertions(+), 32 deletions(-) create mode 100644 downstreamadapter/sink/cloudstorage/dml_writers_drain_test.go diff --git a/downstreamadapter/sink/cloudstorage/defragmenter.go b/downstreamadapter/sink/cloudstorage/defragmenter.go index 7b6f9076bc..206cc6a7f6 100644 --- a/downstreamadapter/sink/cloudstorage/defragmenter.go +++ b/downstreamadapter/sink/cloudstorage/defragmenter.go @@ -16,18 +16,42 @@ package cloudstorage import ( "context" + commonType "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/hash" "github.com/pingcap/ticdc/pkg/sink/cloudstorage" "github.com/pingcap/ticdc/pkg/sink/codec/common" "github.com/pingcap/ticdc/utils/chann" ) +type eventFragmentKind uint8 + +const ( + eventFragmentKindDML eventFragmentKind = iota + eventFragmentKindDrain +) + +type drainMarker struct { + dispatcherID commonType.DispatcherID + commitTs uint64 + doneCh chan error +} + +func (m *drainMarker) done(err error) { + select { + case m.doneCh <- err: + default: + } +} + // eventFragment is used to attach a sequence number to TxnCallbackableEvent. type eventFragment struct { + kind eventFragmentKind + event *commonEvent.DMLEvent versionedTable cloudstorage.VersionedTableName + dispatcherID commonType.DispatcherID + marker *drainMarker // The sequence number is mainly useful for TxnCallbackableEvent defragmentation. // e.g. TxnCallbackableEvent 1~5 are dispatched to a group of encoding workers, but the @@ -39,14 +63,43 @@ type eventFragment struct { encodedMsgs []*common.Message } -func newEventFragment(seq uint64, version cloudstorage.VersionedTableName, event *commonEvent.DMLEvent) eventFragment { +func newEventFragment( + seq uint64, + version cloudstorage.VersionedTableName, + dispatcherID commonType.DispatcherID, + event *commonEvent.DMLEvent, +) eventFragment { return eventFragment{ + kind: eventFragmentKindDML, seqNumber: seq, versionedTable: version, + dispatcherID: dispatcherID, event: event, } } +func newDrainEventFragment( + seq uint64, + dispatcherID commonType.DispatcherID, + commitTs uint64, + doneCh chan error, +) eventFragment { + return eventFragment{ + kind: eventFragmentKindDrain, + seqNumber: seq, + dispatcherID: dispatcherID, + marker: &drainMarker{ + dispatcherID: dispatcherID, + commitTs: commitTs, + doneCh: doneCh, + }, + } +} + +func (e eventFragment) isDrain() bool { + return e.kind == eventFragmentKindDrain +} + // defragmenter is used to handle event fragments which can be registered // out of order. type defragmenter struct { @@ -54,7 +107,6 @@ type defragmenter struct { future map[uint64]eventFragment inputCh <-chan eventFragment outputChs []*chann.DrainableChann[eventFragment] - hasher *hash.PositionInertia } func newDefragmenter( @@ -65,7 +117,6 @@ func newDefragmenter( future: make(map[uint64]eventFragment), inputCh: inputCh, outputChs: outputChs, - hasher: hash.NewPositionInertia(), } } @@ -117,12 +168,11 @@ func (d *defragmenter) writeMsgsConsecutive( } func (d *defragmenter) dispatchFragToDMLWorker(frag eventFragment) { - tableName := frag.versionedTable.TableNameWithPhysicTableID - d.hasher.Reset() - d.hasher.Write([]byte(tableName.Schema), []byte(tableName.Table)) - workerID := d.hasher.Sum32() % uint32(len(d.outputChs)) + workerID := commonType.GID(frag.dispatcherID).Hash(uint64(len(d.outputChs))) d.outputChs[workerID].In() <- frag - frag.event.PostEnqueue() + if !frag.isDrain() { + frag.event.PostEnqueue() + } d.lastDispatchedSeq = frag.seqNumber } diff --git a/downstreamadapter/sink/cloudstorage/dml_writers.go b/downstreamadapter/sink/cloudstorage/dml_writers.go index 4dc6e245ee..3bbaad8cde 100644 --- a/downstreamadapter/sink/cloudstorage/dml_writers.go +++ b/downstreamadapter/sink/cloudstorage/dml_writers.go @@ -15,9 +15,14 @@ package cloudstorage import ( "context" + "sort" + "sync" + "time" + sinkmetrics "github.com/pingcap/ticdc/downstreamadapter/sink/metrics" commonType "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/sink/cloudstorage" "github.com/pingcap/ticdc/pkg/sink/codec/common" @@ -29,6 +34,7 @@ import ( // dmlWriters denotes a worker responsible for writing messages to cloud storage. type dmlWriters struct { + ctx context.Context changefeedID commonType.ChangeFeedID statistics *metrics.Statistics @@ -46,9 +52,14 @@ type dmlWriters struct { // last sequence number lastSeqNum atomic.Uint64 + + dispatcherTableMu sync.RWMutex + dispatcherTableIDs map[commonType.DispatcherID]int64 + tableDispatchers map[int64]map[commonType.DispatcherID]struct{} } func newDMLWriters( + ctx context.Context, changefeedID commonType.ChangeFeedID, storage storage.ExternalStorage, config *cloudstorage.Config, @@ -69,19 +80,28 @@ func newDMLWriters( } return &dmlWriters{ + ctx: ctx, changefeedID: changefeedID, statistics: statistics, msgCh: messageCh, - encodeGroup: encoderGroup, - defragmenter: newDefragmenter(encodedOutCh, writerInputChs), - writers: writers, + encodeGroup: encoderGroup, + defragmenter: newDefragmenter(encodedOutCh, writerInputChs), + writers: writers, + dispatcherTableIDs: make(map[commonType.DispatcherID]int64), + tableDispatchers: make(map[int64]map[commonType.DispatcherID]struct{}), } } func (d *dmlWriters) Run(ctx context.Context) error { eg, ctx := errgroup.WithContext(ctx) + eg.Go(func() error { + <-ctx.Done() + d.msgCh.Close() + return nil + }) + eg.Go(func() error { return d.encodeGroup.Run(ctx) }) @@ -90,12 +110,10 @@ func (d *dmlWriters) Run(ctx context.Context) error { return d.defragmenter.Run(ctx) }) - for i := 0; i < len(d.writers); i++ { + for i := range d.writers { + writer := d.writers[i] eg.Go(func() error { - // UnlimitedChannel will block when there is no event, they cannot dirrectly find ctx.Done() - // Thus, we need to close the channel when the context is done - defer d.encodeGroup.inputCh.Close() - return d.writers[i].Run(ctx) + return writer.Run(ctx) }) } return eg.Wait() @@ -112,9 +130,123 @@ func (d *dmlWriters) AddDMLEvent(event *commonEvent.DMLEvent) { TableInfoVersion: event.TableInfoVersion, DispatcherID: event.GetDispatcherID(), } + d.recordDispatcherTable(event.GetDispatcherID(), event.PhysicalTableID) seq := d.lastSeqNum.Inc() // emit a TxnCallbackableEvent encoupled with a sequence number starting from one. - d.msgCh.Push(newEventFragment(seq, tbl, event)) + d.msgCh.Push(newEventFragment(seq, tbl, event.GetDispatcherID(), event)) +} + +func (d *dmlWriters) DrainBlockEvent(event commonEvent.BlockEvent, tableSchemaStore *commonEvent.TableSchemaStore) error { + if event == nil { + return nil + } + + start := time.Now() + defer sinkmetrics.CloudStorageDDLDrainDurationHistogram.WithLabelValues( + d.changefeedID.Keyspace(), + d.changefeedID.ID().String(), + ).Observe(time.Since(start).Seconds()) + + dispatchers := d.resolveAffectedDispatchers(event, tableSchemaStore) + doneChs := make([]chan error, 0, len(dispatchers)) + for _, dispatcherID := range dispatchers { + doneCh := make(chan error, 1) + doneChs = append(doneChs, doneCh) + seq := d.lastSeqNum.Inc() + d.msgCh.Push(newDrainEventFragment(seq, dispatcherID, event.GetCommitTs(), doneCh)) + } + + for _, doneCh := range doneChs { + select { + case err := <-doneCh: + if err != nil { + return err + } + case <-d.ctx.Done(): + return errors.Trace(d.ctx.Err()) + } + } + return nil +} + +func (d *dmlWriters) recordDispatcherTable(dispatcherID commonType.DispatcherID, tableID int64) { + d.dispatcherTableMu.Lock() + defer d.dispatcherTableMu.Unlock() + + if oldTableID, ok := d.dispatcherTableIDs[dispatcherID]; ok { + if oldTableID == tableID { + return + } + if dispatchers, ok := d.tableDispatchers[oldTableID]; ok { + delete(dispatchers, dispatcherID) + if len(dispatchers) == 0 { + delete(d.tableDispatchers, oldTableID) + } + } + } + + d.dispatcherTableIDs[dispatcherID] = tableID + dispatchers, ok := d.tableDispatchers[tableID] + if !ok { + dispatchers = make(map[commonType.DispatcherID]struct{}) + d.tableDispatchers[tableID] = dispatchers + } + dispatchers[dispatcherID] = struct{}{} +} + +func (d *dmlWriters) resolveAffectedDispatchers( + event commonEvent.BlockEvent, + tableSchemaStore *commonEvent.TableSchemaStore, +) []commonType.DispatcherID { + if event == nil { + return nil + } + + blockedTables := event.GetBlockedTables() + tableIDs := make([]int64, 0) + if blockedTables != nil { + switch blockedTables.InfluenceType { + case commonEvent.InfluenceTypeNormal: + tableIDs = append(tableIDs, blockedTables.TableIDs...) + case commonEvent.InfluenceTypeDB: + if tableSchemaStore != nil { + tableIDs = append(tableIDs, tableSchemaStore.GetNormalTableIdsByDB(blockedTables.SchemaID)...) + } + case commonEvent.InfluenceTypeAll: + if tableSchemaStore != nil { + tableIDs = append(tableIDs, tableSchemaStore.GetAllNormalTableIds()...) + } + } + } + + dispatcherSet := make(map[commonType.DispatcherID]struct{}) + d.dispatcherTableMu.RLock() + for _, tableID := range tableIDs { + for dispatcherID := range d.tableDispatchers[tableID] { + dispatcherSet[dispatcherID] = struct{}{} + } + } + if len(dispatcherSet) == 0 && blockedTables != nil && + (blockedTables.InfluenceType == commonEvent.InfluenceTypeDB || + blockedTables.InfluenceType == commonEvent.InfluenceTypeAll) { + for dispatcherID := range d.dispatcherTableIDs { + dispatcherSet[dispatcherID] = struct{}{} + } + } + d.dispatcherTableMu.RUnlock() + + if len(dispatcherSet) == 0 { + dispatcherSet[event.GetDispatcherID()] = struct{}{} + } + + dispatchers := make([]commonType.DispatcherID, 0, len(dispatcherSet)) + for dispatcherID := range dispatcherSet { + dispatchers = append(dispatchers, dispatcherID) + } + sort.Slice(dispatchers, func(i, j int) bool { + return dispatchers[i].String() < dispatchers[j].String() + }) + return dispatchers } func (d *dmlWriters) close() { diff --git a/downstreamadapter/sink/cloudstorage/dml_writers_drain_test.go b/downstreamadapter/sink/cloudstorage/dml_writers_drain_test.go new file mode 100644 index 0000000000..5af66c8b58 --- /dev/null +++ b/downstreamadapter/sink/cloudstorage/dml_writers_drain_test.go @@ -0,0 +1,108 @@ +// 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 cloudstorage + +import ( + "testing" + + "github.com/pingcap/ticdc/heartbeatpb" + commonType "github.com/pingcap/ticdc/pkg/common" + commonEvent "github.com/pingcap/ticdc/pkg/common/event" + "github.com/stretchr/testify/require" +) + +func TestResolveAffectedDispatchersByBlockedTables(t *testing.T) { + dispatcher1 := commonType.NewDispatcherID() + dispatcher2 := commonType.NewDispatcherID() + dispatcher3 := commonType.NewDispatcherID() + + workers := &dmlWriters{ + dispatcherTableIDs: make(map[commonType.DispatcherID]int64), + tableDispatchers: make(map[int64]map[commonType.DispatcherID]struct{}), + } + workers.recordDispatcherTable(dispatcher1, 101) + workers.recordDispatcherTable(dispatcher2, 201) + workers.recordDispatcherTable(dispatcher3, 102) + + tableStore := commonEvent.NewTableSchemaStore([]*heartbeatpb.SchemaInfo{ + { + SchemaID: 1, + Tables: []*heartbeatpb.TableInfo{ + {TableID: 101}, + {TableID: 102}, + }, + }, + { + SchemaID: 2, + Tables: []*heartbeatpb.TableInfo{ + {TableID: 201}, + }, + }, + }, commonType.CloudStorageSinkType, false) + + testCases := []struct { + name string + blocked *commonEvent.InfluencedTables + expected []commonType.DispatcherID + useSchema bool + }{ + { + name: "normal blocked tables", + blocked: &commonEvent.InfluencedTables{ + InfluenceType: commonEvent.InfluenceTypeNormal, + TableIDs: []int64{101}, + }, + expected: []commonType.DispatcherID{dispatcher1}, + }, + { + name: "db blocked tables", + blocked: &commonEvent.InfluencedTables{ + InfluenceType: commonEvent.InfluenceTypeDB, + SchemaID: 1, + }, + expected: []commonType.DispatcherID{dispatcher1, dispatcher3}, + useSchema: true, + }, + { + name: "all blocked tables", + blocked: &commonEvent.InfluencedTables{ + InfluenceType: commonEvent.InfluenceTypeAll, + }, + expected: []commonType.DispatcherID{dispatcher1, dispatcher2, dispatcher3}, + useSchema: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + ddl := &commonEvent.DDLEvent{ + DispatcherID: dispatcher1, + FinishedTs: 100, + BlockedTables: &commonEvent.InfluencedTables{ + InfluenceType: tc.blocked.InfluenceType, + TableIDs: tc.blocked.TableIDs, + SchemaID: tc.blocked.SchemaID, + }, + } + + var store *commonEvent.TableSchemaStore + if tc.useSchema { + store = tableStore + } + + affected := workers.resolveAffectedDispatchers(ddl, store) + require.ElementsMatch(t, tc.expected, affected) + }) + } +} diff --git a/downstreamadapter/sink/cloudstorage/encoding_group.go b/downstreamadapter/sink/cloudstorage/encoding_group.go index bcc37663e7..998f475786 100644 --- a/downstreamadapter/sink/cloudstorage/encoding_group.go +++ b/downstreamadapter/sink/cloudstorage/encoding_group.go @@ -85,11 +85,13 @@ func (eg *encodingGroup) runEncoder(ctx context.Context) error { if !ok || eg.closed.Load() { return nil } - err = encoder.AppendTxnEvent(frag.event) - if err != nil { - return err + if !frag.isDrain() { + err = encoder.AppendTxnEvent(frag.event) + if err != nil { + return err + } + frag.encodedMsgs = encoder.Build() } - frag.encodedMsgs = encoder.Build() select { case <-ctx.Done(): diff --git a/downstreamadapter/sink/cloudstorage/sink.go b/downstreamadapter/sink/cloudstorage/sink.go index cb59ce6953..0eb9c7560c 100644 --- a/downstreamadapter/sink/cloudstorage/sink.go +++ b/downstreamadapter/sink/cloudstorage/sink.go @@ -126,7 +126,7 @@ func New( cfg: cfg, cleanupJobs: cleanupJobs, storage: storage, - dmlWriters: newDMLWriters(changefeedID, storage, cfg, encoderConfig, ext, statistics), + dmlWriters: newDMLWriters(ctx, changefeedID, storage, cfg, encoderConfig, ext, statistics), checkpointChan: make(chan uint64, 16), lastSendCheckpointTsTime: time.Now(), outputRawChangeEvent: sinkConfig.CloudStorageConfig.GetOutputRawChangeEvent(), @@ -174,7 +174,10 @@ func (s *sink) WriteBlockEvent(event commonEvent.BlockEvent) error { var err error switch e := event.(type) { case *commonEvent.DDLEvent: - err = s.writeDDLEvent(e) + err = s.dmlWriters.DrainBlockEvent(e, s.tableSchemaStore) + if err == nil && !e.NotSync { + err = s.writeDDLEvent(e) + } default: log.Error("cloudstorage sink doesn't support this type of block event", zap.String("namespace", s.changefeedID.Keyspace()), diff --git a/downstreamadapter/sink/cloudstorage/sink_test.go b/downstreamadapter/sink/cloudstorage/sink_test.go index ecede56719..99d88665df 100644 --- a/downstreamadapter/sink/cloudstorage/sink_test.go +++ b/downstreamadapter/sink/cloudstorage/sink_test.go @@ -207,6 +207,66 @@ func TestWriteDDLEvent(t *testing.T) { }`, string(tableSchema)) } +func TestWriteDDLEventDrainsAffectedDispatchersFirst(t *testing.T) { + parentDir := t.TempDir() + uri := fmt.Sprintf("file:///%s?protocol=csv&flush-interval=3600s", parentDir) + sinkURI, err := url.Parse(uri) + require.NoError(t, err) + + replicaConfig := config.GetDefaultReplicaConfig() + err = replicaConfig.ValidateAndAdjust(sinkURI) + require.NoError(t, err) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + mockPDClock := pdutil.NewClock4Test() + appcontext.SetService(appcontext.DefaultPDClock, mockPDClock) + + cloudStorageSink, err := newSinkForTest(ctx, replicaConfig, sinkURI, nil) + require.NoError(t, err) + + go cloudStorageSink.Run(ctx) + + helper := commonEvent.NewEventTestHelper(t) + defer helper.Close() + + helper.Tk().MustExec("use test") + job := helper.DDL2Job("create table t_drain_before_ddl (id int primary key, v int)") + require.NotNil(t, job) + helper.ApplyJob(job) + + dispatcherID := common.NewDispatcherID() + dmlEvent := helper.DML2Event(job.SchemaName, job.TableName, "insert into t_drain_before_ddl values (1, 1)") + dmlEvent.TableInfoVersion = job.BinlogInfo.FinishedTS + dmlEvent.DispatcherID = dispatcherID + + var dmlFlushed atomic.Int64 + dmlEvent.AddPostFlushFunc(func() { + dmlFlushed.Add(1) + }) + + cloudStorageSink.AddDMLEvent(dmlEvent) + + ddlEvent := &commonEvent.DDLEvent{ + Query: "alter table t_drain_before_ddl add column c2 int", + Type: byte(timodel.ActionAddColumn), + SchemaName: job.SchemaName, + TableName: job.TableName, + FinishedTs: dmlEvent.CommitTs + 10, + TableInfo: helper.GetTableInfo(job), + DispatcherID: dispatcherID, + BlockedTables: &commonEvent.InfluencedTables{ + InfluenceType: commonEvent.InfluenceTypeNormal, + TableIDs: []int64{dmlEvent.PhysicalTableID}, + }, + } + + err = cloudStorageSink.WriteBlockEvent(ddlEvent) + require.NoError(t, err) + require.Equal(t, int64(1), dmlFlushed.Load()) +} + func TestWriteCheckpointEvent(t *testing.T) { parentDir := t.TempDir() uri := fmt.Sprintf("file:///%s?protocol=csv", parentDir) diff --git a/downstreamadapter/sink/cloudstorage/writer.go b/downstreamadapter/sink/cloudstorage/writer.go index 35eee16791..ded413ca6a 100644 --- a/downstreamadapter/sink/cloudstorage/writer.go +++ b/downstreamadapter/sink/cloudstorage/writer.go @@ -45,7 +45,7 @@ type writer struct { storage storage.ExternalStorage config *cloudstorage.Config // toBeFlushedCh contains a set of batchedTask waiting to be flushed to cloud storage. - toBeFlushedCh chan batchedTask + toBeFlushedCh chan writerTask inputCh *chann.DrainableChann[eventFragment] isClosed uint64 statistics *pmetrics.Statistics @@ -57,6 +57,11 @@ type writer struct { metricsWorkerBusyRatio prometheus.Counter } +type writerTask struct { + batch batchedTask + marker *drainMarker +} + func newWriter( id int, changefeedID commonType.ChangeFeedID, @@ -72,7 +77,7 @@ func newWriter( storage: storage, config: config, inputCh: inputCh, - toBeFlushedCh: make(chan batchedTask, 64), + toBeFlushedCh: make(chan writerTask, 64), statistics: statistics, filePathGenerator: cloudstorage.NewFilePathGenerator(changefeedID, config, storage, extension), metricWriteBytes: metrics.CloudStorageWriteBytesGauge. @@ -123,10 +128,18 @@ func (d *writer) flushMessages(ctx context.Context) error { case <-overseerTicker.C: d.metricsWorkerBusyRatio.Add(flushTimeSlice.Seconds()) flushTimeSlice = 0 - case batchedTask := <-d.toBeFlushedCh: + case task := <-d.toBeFlushedCh: if atomic.LoadUint64(&d.isClosed) == 1 { return nil } + if task.marker != nil { + task.marker.done(nil) + continue + } + batchedTask := task.batch + if len(batchedTask.batch) == 0 { + continue + } start := time.Now() for table, task := range batchedTask.batch { if len(task.msgs) == 0 { @@ -314,7 +327,7 @@ func (d *writer) genAndDispatchTask(ctx context.Context, select { case <-ctx.Done(): return errors.Trace(ctx.Err()) - case d.toBeFlushedCh <- batchedTask: + case d.toBeFlushedCh <- writerTask{batch: batchedTask}: log.Debug("flush task is emitted successfully when flush interval exceeds", zap.Int("tablesLength", len(batchedTask.batch))) batchedTask = newBatchedTask() @@ -322,7 +335,31 @@ func (d *writer) genAndDispatchTask(ctx context.Context, } case frag, ok := <-ch.Out(): if !ok || atomic.LoadUint64(&d.isClosed) == 1 { - return nil + if len(batchedTask.batch) == 0 { + return nil + } + select { + case <-ctx.Done(): + return errors.Trace(ctx.Err()) + case d.toBeFlushedCh <- writerTask{batch: batchedTask}: + return nil + } + } + if frag.isDrain() { + if len(batchedTask.batch) > 0 { + select { + case <-ctx.Done(): + return errors.Trace(ctx.Err()) + case d.toBeFlushedCh <- writerTask{batch: batchedTask}: + batchedTask = newBatchedTask() + } + } + select { + case <-ctx.Done(): + return errors.Trace(ctx.Err()) + case d.toBeFlushedCh <- writerTask{marker: frag.marker}: + } + continue } batchedTask.handleSingleTableEvent(frag) // if the file size exceeds the upper limit, emit the flush task containing the table @@ -333,7 +370,7 @@ func (d *writer) genAndDispatchTask(ctx context.Context, select { case <-ctx.Done(): return errors.Trace(ctx.Err()) - case d.toBeFlushedCh <- task: + case d.toBeFlushedCh <- writerTask{batch: task}: log.Debug("flush task is emitted successfully when file size exceeds", zap.Any("table", table), zap.Int("eventsLenth", len(task.batch[table].msgs))) diff --git a/downstreamadapter/sink/cloudstorage/writer_test.go b/downstreamadapter/sink/cloudstorage/writer_test.go index 0be8a78640..f6ab81a578 100644 --- a/downstreamadapter/sink/cloudstorage/writer_test.go +++ b/downstreamadapter/sink/cloudstorage/writer_test.go @@ -19,6 +19,7 @@ import ( "net/url" "path" "sync" + "sync/atomic" "testing" "time" @@ -130,3 +131,75 @@ func TestWriterRun(t *testing.T) { d.close() wg.Wait() } + +func TestWriterDrainMarker(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + parentDir := t.TempDir() + d := testWriter(ctx, t, parentDir) + + tidbTableInfo := &timodel.TableInfo{ + ID: 100, + Name: ast.NewCIStr("table1"), + Columns: []*timodel.ColumnInfo{ + {ID: 1, Name: ast.NewCIStr("c1"), FieldType: *types.NewFieldType(mysql.TypeLong)}, + }, + } + tableInfo := commonType.WrapTableInfo("test", tidbTableInfo) + dispatcherID := commonType.NewDispatcherID() + + var callbackCnt int64 + msg := common.NewMsg(nil, []byte(`{"id":1}`)) + msg.SetRowsCount(1) + msg.Callback = func() { + atomic.AddInt64(&callbackCnt, 1) + } + + d.inputCh.In() <- eventFragment{ + kind: eventFragmentKindDML, + seqNumber: 1, + dispatcherID: dispatcherID, + versionedTable: cloudstorage.VersionedTableName{ + TableNameWithPhysicTableID: commonType.TableName{ + Schema: "test", + Table: "table1", + TableID: 100, + }, + TableInfoVersion: 99, + DispatcherID: dispatcherID, + }, + event: &commonEvent.DMLEvent{ + PhysicalTableID: 100, + TableInfo: tableInfo, + }, + encodedMsgs: []*common.Message{msg}, + } + + doneCh := make(chan error, 1) + d.inputCh.In() <- newDrainEventFragment(2, dispatcherID, 100, doneCh) + + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + _ = d.Run(ctx) + }() + + select { + case err := <-doneCh: + require.NoError(t, err) + case <-time.After(10 * time.Second): + t.Fatal("wait drain marker timeout") + } + + require.Eventually(t, func() bool { + return atomic.LoadInt64(&callbackCnt) == 1 + }, 5*time.Second, 100*time.Millisecond) + + d.inputCh.CloseAndDrain() + d.close() + cancel() + wg.Wait() +} diff --git a/downstreamadapter/sink/metrics/cloudstorage.go b/downstreamadapter/sink/metrics/cloudstorage.go index 7fc8dae975..13a5c1e2fc 100644 --- a/downstreamadapter/sink/metrics/cloudstorage.go +++ b/downstreamadapter/sink/metrics/cloudstorage.go @@ -55,6 +55,14 @@ var ( Buckets: prometheus.ExponentialBuckets(0.001, 2.0, 13), }, []string{"namespace", "changefeed"}) + CloudStorageDDLDrainDurationHistogram = prometheus.NewHistogramVec(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "cloud_storage_ddl_drain_duration_seconds", + Help: "DDL drain duration for cloud storage sink", + Buckets: prometheus.ExponentialBuckets(0.001, 2.0, 13), + }, []string{"namespace", "changefeed"}) + // CloudStorageWorkerBusyRatio records the busy ratio of CloudStorage bgUpdateLog worker. CloudStorageWorkerBusyRatio = prometheus.NewCounterVec( prometheus.CounterOpts{ @@ -71,5 +79,6 @@ func InitCloudStorageMetrics(registry *prometheus.Registry) { registry.MustRegister(CloudStorageFileCountGauge) registry.MustRegister(CloudStorageWriteDurationHistogram) registry.MustRegister(CloudStorageFlushDurationHistogram) + registry.MustRegister(CloudStorageDDLDrainDurationHistogram) registry.MustRegister(CloudStorageWorkerBusyRatio) } diff --git a/pkg/common/event/table_schema_store.go b/pkg/common/event/table_schema_store.go index 256721b362..9d956199a3 100644 --- a/pkg/common/event/table_schema_store.go +++ b/pkg/common/event/table_schema_store.go @@ -51,6 +51,8 @@ type tableSchemaStoreRequirements struct { // - Redo pipeline: attached to redo DDL logs for InfluenceTypeDB/All expansion, and consumed // by redo applier to expand dropped tables for cleanup/skip decisions. // See pkg/common/event/redo.go and pkg/applier/redo.go. +// - Cloud storage sink: used to expand DB/All DDL affected table IDs and map to dispatcher routes +// for per-dispatcher drain before executing schema DDL file writes. // - Kafka/Pulsar (simple protocol, send-all-bootstrap-at-start): used by the table trigger dispatcher // to list all current tables and emit bootstrap schema messages at changefeed start. // See downstreamadapter/dispatcher/event_dispatcher.go. @@ -91,8 +93,15 @@ func newTableSchemaStoreRequirements( updateTableIDs: false, needTableNames: true, } - case commonType.CloudStorageSinkType, commonType.BlackHoleSinkType: - // These sinks currently do not consume TableSchemaStore metadata. + case commonType.CloudStorageSinkType: + // Cloud storage sink uses table IDs to expand DB/All DDL affected tables, + // then maps them to active dispatcher routes for per-dispatcher drain. + return tableSchemaStoreRequirements{ + needTableIDs: true, + updateTableIDs: true, + needTableNames: false, + } + case commonType.BlackHoleSinkType: return tableSchemaStoreRequirements{ needTableIDs: false, updateTableIDs: false, diff --git a/pkg/common/event/table_schema_store_test.go b/pkg/common/event/table_schema_store_test.go index fd56f35adc..1aca24651b 100644 --- a/pkg/common/event/table_schema_store_test.go +++ b/pkg/common/event/table_schema_store_test.go @@ -253,3 +253,35 @@ func TestTableSchemaStoreNonMysqlTableIDsBootstrapOnly(t *testing.T) { require.ElementsMatch(t, []int64{10, 20}, store.GetAllNormalTableIds()) } + +func TestTableSchemaStoreWhenCloudStorageSink(t *testing.T) { + schemaInfos := []*heartbeatpb.SchemaInfo{ + { + SchemaID: 1, + Tables: []*heartbeatpb.TableInfo{ + {TableID: 10}, + {TableID: 20}, + }, + }, + } + + store := NewTableSchemaStore(schemaInfos, common.CloudStorageSinkType, false) + require.ElementsMatch(t, []int64{10, 20}, store.GetAllNormalTableIds()) + + store.AddEvent(&DDLEvent{ + FinishedTs: 100, + NeedAddedTables: []Table{ + {SchemaID: 1, TableID: 30}, + }, + }) + require.ElementsMatch(t, []int64{10, 20, 30}, store.GetAllNormalTableIds()) + + store.AddEvent(&DDLEvent{ + FinishedTs: 101, + NeedDroppedTables: &InfluencedTables{ + InfluenceType: InfluenceTypeNormal, + TableIDs: []int64{20}, + }, + }) + require.ElementsMatch(t, []int64{10, 30}, store.GetAllNormalTableIds()) +} From 0b76095839cee20cd2184493aadcf0c1d4b40ed0 Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Fri, 27 Feb 2026 15:08:05 +0800 Subject: [PATCH 12/20] add pass block event to the sink interface --- .../dispatcher/basic_dispatcher.go | 20 +- .../dispatchermanager/helper_test.go | 172 +++++------------- downstreamadapter/sink/blackhole/sink.go | 4 + .../sink/cloudstorage/dml_writers.go | 123 ++----------- .../cloudstorage/dml_writers_drain_test.go | 108 ----------- downstreamadapter/sink/cloudstorage/sink.go | 12 +- .../sink/cloudstorage/sink_test.go | 7 +- downstreamadapter/sink/kafka/sink.go | 4 + downstreamadapter/sink/mock/sink_mock.go | 14 ++ downstreamadapter/sink/mock_sink.go | 4 + downstreamadapter/sink/mysql/sink.go | 4 + downstreamadapter/sink/pulsar/sink.go | 4 + downstreamadapter/sink/redo/sink.go | 4 + downstreamadapter/sink/sink.go | 1 + pkg/common/event/table_schema_store.go | 10 +- pkg/common/event/table_schema_store_test.go | 32 ---- 16 files changed, 132 insertions(+), 391 deletions(-) delete mode 100644 downstreamadapter/sink/cloudstorage/dml_writers_drain_test.go diff --git a/downstreamadapter/dispatcher/basic_dispatcher.go b/downstreamadapter/dispatcher/basic_dispatcher.go index 45d41d2f15..c7033a8713 100644 --- a/downstreamadapter/dispatcher/basic_dispatcher.go +++ b/downstreamadapter/dispatcher/basic_dispatcher.go @@ -895,7 +895,15 @@ func (d *BasicDispatcher) DealWithBlockEvent(event commonEvent.BlockEvent) { // we track it as a pending schedule-related event until the maintainer ACKs it. d.pendingACKCount.Add(1) } - err := d.AddBlockEventToSink(event) + err := d.sink.PassBlockEvent(event) + if err != nil { + if needsScheduleACKTracking { + d.pendingACKCount.Add(-1) + } + d.HandleError(err) + return + } + err = d.AddBlockEventToSink(event) if err != nil { if needsScheduleACKTracking { d.pendingACKCount.Add(-1) @@ -963,8 +971,14 @@ func (d *BasicDispatcher) DealWithBlockEvent(event commonEvent.BlockEvent) { d.holdBlockEvent(event) return } - - d.reportBlockedEventToMaintainer(event) + d.sharedInfo.GetBlockEventExecutor().Submit(d, func() { + err := d.sink.PassBlockEvent(event) + if err != nil { + d.HandleError(err) + return + } + d.reportBlockedEventToMaintainer(event) + }) } // dealing with events which update schema ids diff --git a/downstreamadapter/dispatchermanager/helper_test.go b/downstreamadapter/dispatchermanager/helper_test.go index bd3fcc1981..35bd9eddaf 100644 --- a/downstreamadapter/dispatchermanager/helper_test.go +++ b/downstreamadapter/dispatchermanager/helper_test.go @@ -15,117 +15,20 @@ package dispatchermanager import ( "context" - "sync" "testing" "time" - "github.com/pingcap/log" + "github.com/golang/mock/gomock" "github.com/pingcap/ticdc/downstreamadapter/dispatcher" + "github.com/pingcap/ticdc/downstreamadapter/sink/mock" "github.com/pingcap/ticdc/heartbeatpb" "github.com/pingcap/ticdc/pkg/common" - commonEvent "github.com/pingcap/ticdc/pkg/common/event" "github.com/stretchr/testify/require" - "go.uber.org/atomic" ) -// mockKafkaSink implements the sink.Sink interface with a blocking AddCheckpointTs method -type mockKafkaSink struct { - cancel context.CancelFunc - ctx context.Context - checkpointChan chan uint64 - isNormal *atomic.Bool - mu sync.Mutex -} - -func newMockKafkaSink(ctx context.Context, cancel context.CancelFunc) *mockKafkaSink { - return &mockKafkaSink{ - cancel: cancel, - ctx: ctx, - checkpointChan: make(chan uint64), - isNormal: atomic.NewBool(true), - } -} - -func (m *mockKafkaSink) SinkType() common.SinkType { - return common.KafkaSinkType -} - -func (m *mockKafkaSink) IsNormal() bool { - return true -} - -func (m *mockKafkaSink) AddDMLEvent(event *commonEvent.DMLEvent) { - // No-op for this test -} - -func (m *mockKafkaSink) WriteBlockEvent(event commonEvent.BlockEvent) error { - // No-op for this test - return nil -} - -// AddCheckpointTs simulates the blocking behavior when sink is closed -func (m *mockKafkaSink) AddCheckpointTs(ts uint64) { - select { - case m.checkpointChan <- ts: - case <-m.ctx.Done(): - return - } -} - -func (m *mockKafkaSink) SetTableSchemaStore(tableSchemaStore *commonEvent.TableSchemaStore) { - // No-op for this test -} - -func (m *mockKafkaSink) Close(removeChangefeed bool) { - m.mu.Lock() - defer m.mu.Unlock() - m.isNormal.Store(false) - // Don't close the channel to simulate the real bug where channel becomes orphaned -} - -func (m *mockKafkaSink) Run(ctx context.Context) error { - // Simulate consuming from checkpoint channel - for { - select { - case <-ctx.Done(): - log.Info("mockKafkaSink context done") - return ctx.Err() - case ts := <-m.checkpointChan: - // Simulate processing checkpoint - _ = ts - } - } -} - -// simulateCloseSink simulates closing the sink (e.g., when path is removed) -func (m *mockKafkaSink) CloseSinkAndCancelContext() { - m.Close(false) - m.cancel() -} - func TestCheckpointTsMessageHandlerDeadlock(t *testing.T) { t.Parallel() - // Create context for the test - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - // Create mock sink - mockSink := newMockKafkaSink(ctx, cancel) - - // Start the sink's Run method in background to consume checkpoint messages - sinkCtx, _ := context.WithCancel(ctx) - go func() { - _ = mockSink.Run(sinkCtx) - }() - - // Create a mock DispatcherManager - dispatcherManager := &DispatcherManager{ - sink: mockSink, - tableTriggerEventDispatcher: &dispatcher.EventDispatcher{}, // Non-nil to pass the check - } - - // Create CheckpointTsMessage changefeedID := &heartbeatpb.ChangefeedID{ Keyspace: "test-namespace", Name: "test-changefeed", @@ -136,12 +39,20 @@ func TestCheckpointTsMessageHandlerDeadlock(t *testing.T) { CheckpointTs: 12345, }) - // Create handler handler := &CheckpointTsMessageHandler{} - // Step 1: Normal operation should work t.Run("normal_operation", func(t *testing.T) { - // This should complete quickly + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockSink := mock.NewMockSink(ctrl) + mockSink.EXPECT().AddCheckpointTs(checkpointTsMessage.CheckpointTs).Times(1) + + dispatcherManager := &DispatcherManager{ + sink: mockSink, + tableTriggerEventDispatcher: &dispatcher.EventDispatcher{}, + } + done := make(chan bool, 1) go func() { blocking := handler.Handle(dispatcherManager, checkpointTsMessage) @@ -151,31 +62,30 @@ func TestCheckpointTsMessageHandlerDeadlock(t *testing.T) { select { case <-done: - // Success case <-time.After(1 * time.Second): t.Fatal("Normal operation took too long, unexpected") } }) - // Step 2: Simulate the deadlock scenario t.Run("deadlock_scenario", func(t *testing.T) { - // Create a new mock sink for this test to avoid interference - deadlockMockSink := newMockKafkaSink(ctx, cancel) + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + blockCh := make(chan struct{}) + deadlockMockSink := mock.NewMockSink(ctrl) + deadlockMockSink.EXPECT().AddCheckpointTs(checkpointTsMessage.CheckpointTs).Do( + func(uint64) { + <-blockCh + }, + ).Times(1) - // Create a new dispatcher manager with the deadlock sink deadlockDispatcherManager := &DispatcherManager{ sink: deadlockMockSink, tableTriggerEventDispatcher: &dispatcher.EventDispatcher{}, } - // Close the sink but does not cancel the context, so the AddCheckpointTs will block forever - deadlockMockSink.Close(false) - - // Now try to send a checkpoint message - // This should block because there's no consumer for the channel done := make(chan bool, 1) go func() { - // This should block indefinitely handler.Handle(deadlockDispatcherManager, checkpointTsMessage) done <- true }() @@ -184,37 +94,49 @@ func TestCheckpointTsMessageHandlerDeadlock(t *testing.T) { case <-done: t.Fatal("Handler completed unexpectedly - deadlock was not reproduced") case <-time.After(1 * time.Second): - // Expected: the handler should be blocked t.Log("Successfully reproduced the deadlock: handler is blocked in AddCheckpointTs") } + + close(blockCh) + select { + case <-done: + case <-time.After(1 * time.Second): + t.Fatal("handler should resume once AddCheckpointTs unblocks") + } }) - // Step 3: close the sink and cancel the context, so the AddCheckpointTs will return t.Run("deadlock_resolve_scenario", func(t *testing.T) { - // Create a new mock sink for this test to avoid interference - deadlockMockSink := newMockKafkaSink(ctx, cancel) + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + deadlockMockSink := mock.NewMockSink(ctrl) + deadlockMockSink.EXPECT().AddCheckpointTs(checkpointTsMessage.CheckpointTs).Do( + func(uint64) { + select { + case <-ctx.Done(): + return + case <-time.After(1 * time.Second): + t.Fatal("context cancellation should unblock AddCheckpointTs path") + } + }, + ).Times(1) - // Create a new dispatcher manager with the deadlock sink deadlockDispatcherManager := &DispatcherManager{ sink: deadlockMockSink, tableTriggerEventDispatcher: &dispatcher.EventDispatcher{}, } - // Close the sink but does not cancel the context, so the AddCheckpointTs will block forever - deadlockMockSink.CloseSinkAndCancelContext() - - // Now try to send a checkpoint message - // This should not block because the context is canceled done := make(chan bool, 1) go func() { - // This should not block handler.Handle(deadlockDispatcherManager, checkpointTsMessage) done <- true }() select { case <-done: - // Expected: the handler should complete without blocking. t.Log("Handler completed normally") case <-time.After(1 * time.Second): t.Fatal("deadlock: handler is blocked in AddCheckpointTs") diff --git a/downstreamadapter/sink/blackhole/sink.go b/downstreamadapter/sink/blackhole/sink.go index c07e886419..4067c95cde 100644 --- a/downstreamadapter/sink/blackhole/sink.go +++ b/downstreamadapter/sink/blackhole/sink.go @@ -53,6 +53,10 @@ func (s *sink) AddDMLEvent(event *commonEvent.DMLEvent) { s.eventCh <- event } +func (s *sink) PassBlockEvent(_ commonEvent.BlockEvent) error { + return nil +} + func (s *sink) WriteBlockEvent(event commonEvent.BlockEvent) error { switch event.GetType() { case commonEvent.TypeDDLEvent: diff --git a/downstreamadapter/sink/cloudstorage/dml_writers.go b/downstreamadapter/sink/cloudstorage/dml_writers.go index 3bbaad8cde..014266ccb9 100644 --- a/downstreamadapter/sink/cloudstorage/dml_writers.go +++ b/downstreamadapter/sink/cloudstorage/dml_writers.go @@ -15,8 +15,6 @@ package cloudstorage import ( "context" - "sort" - "sync" "time" sinkmetrics "github.com/pingcap/ticdc/downstreamadapter/sink/metrics" @@ -52,10 +50,6 @@ type dmlWriters struct { // last sequence number lastSeqNum atomic.Uint64 - - dispatcherTableMu sync.RWMutex - dispatcherTableIDs map[commonType.DispatcherID]int64 - tableDispatchers map[int64]map[commonType.DispatcherID]struct{} } func newDMLWriters( @@ -85,11 +79,9 @@ func newDMLWriters( statistics: statistics, msgCh: messageCh, - encodeGroup: encoderGroup, - defragmenter: newDefragmenter(encodedOutCh, writerInputChs), - writers: writers, - dispatcherTableIDs: make(map[commonType.DispatcherID]int64), - tableDispatchers: make(map[int64]map[commonType.DispatcherID]struct{}), + encodeGroup: encoderGroup, + defragmenter: newDefragmenter(encodedOutCh, writerInputChs), + writers: writers, } } @@ -130,13 +122,12 @@ func (d *dmlWriters) AddDMLEvent(event *commonEvent.DMLEvent) { TableInfoVersion: event.TableInfoVersion, DispatcherID: event.GetDispatcherID(), } - d.recordDispatcherTable(event.GetDispatcherID(), event.PhysicalTableID) seq := d.lastSeqNum.Inc() // emit a TxnCallbackableEvent encoupled with a sequence number starting from one. d.msgCh.Push(newEventFragment(seq, tbl, event.GetDispatcherID(), event)) } -func (d *dmlWriters) DrainBlockEvent(event commonEvent.BlockEvent, tableSchemaStore *commonEvent.TableSchemaStore) error { +func (d *dmlWriters) PassBlockEvent(event commonEvent.BlockEvent) error { if event == nil { return nil } @@ -147,106 +138,16 @@ func (d *dmlWriters) DrainBlockEvent(event commonEvent.BlockEvent, tableSchemaSt d.changefeedID.ID().String(), ).Observe(time.Since(start).Seconds()) - dispatchers := d.resolveAffectedDispatchers(event, tableSchemaStore) - doneChs := make([]chan error, 0, len(dispatchers)) - for _, dispatcherID := range dispatchers { - doneCh := make(chan error, 1) - doneChs = append(doneChs, doneCh) - seq := d.lastSeqNum.Inc() - d.msgCh.Push(newDrainEventFragment(seq, dispatcherID, event.GetCommitTs(), doneCh)) - } - - for _, doneCh := range doneChs { - select { - case err := <-doneCh: - if err != nil { - return err - } - case <-d.ctx.Done(): - return errors.Trace(d.ctx.Err()) - } - } - return nil -} - -func (d *dmlWriters) recordDispatcherTable(dispatcherID commonType.DispatcherID, tableID int64) { - d.dispatcherTableMu.Lock() - defer d.dispatcherTableMu.Unlock() - - if oldTableID, ok := d.dispatcherTableIDs[dispatcherID]; ok { - if oldTableID == tableID { - return - } - if dispatchers, ok := d.tableDispatchers[oldTableID]; ok { - delete(dispatchers, dispatcherID) - if len(dispatchers) == 0 { - delete(d.tableDispatchers, oldTableID) - } - } - } - - d.dispatcherTableIDs[dispatcherID] = tableID - dispatchers, ok := d.tableDispatchers[tableID] - if !ok { - dispatchers = make(map[commonType.DispatcherID]struct{}) - d.tableDispatchers[tableID] = dispatchers - } - dispatchers[dispatcherID] = struct{}{} -} - -func (d *dmlWriters) resolveAffectedDispatchers( - event commonEvent.BlockEvent, - tableSchemaStore *commonEvent.TableSchemaStore, -) []commonType.DispatcherID { - if event == nil { - return nil - } - - blockedTables := event.GetBlockedTables() - tableIDs := make([]int64, 0) - if blockedTables != nil { - switch blockedTables.InfluenceType { - case commonEvent.InfluenceTypeNormal: - tableIDs = append(tableIDs, blockedTables.TableIDs...) - case commonEvent.InfluenceTypeDB: - if tableSchemaStore != nil { - tableIDs = append(tableIDs, tableSchemaStore.GetNormalTableIdsByDB(blockedTables.SchemaID)...) - } - case commonEvent.InfluenceTypeAll: - if tableSchemaStore != nil { - tableIDs = append(tableIDs, tableSchemaStore.GetAllNormalTableIds()...) - } - } - } - - dispatcherSet := make(map[commonType.DispatcherID]struct{}) - d.dispatcherTableMu.RLock() - for _, tableID := range tableIDs { - for dispatcherID := range d.tableDispatchers[tableID] { - dispatcherSet[dispatcherID] = struct{}{} - } - } - if len(dispatcherSet) == 0 && blockedTables != nil && - (blockedTables.InfluenceType == commonEvent.InfluenceTypeDB || - blockedTables.InfluenceType == commonEvent.InfluenceTypeAll) { - for dispatcherID := range d.dispatcherTableIDs { - dispatcherSet[dispatcherID] = struct{}{} - } - } - d.dispatcherTableMu.RUnlock() - - if len(dispatcherSet) == 0 { - dispatcherSet[event.GetDispatcherID()] = struct{}{} - } + doneCh := make(chan error, 1) + seq := d.lastSeqNum.Inc() + d.msgCh.Push(newDrainEventFragment(seq, event.GetDispatcherID(), event.GetCommitTs(), doneCh)) - dispatchers := make([]commonType.DispatcherID, 0, len(dispatcherSet)) - for dispatcherID := range dispatcherSet { - dispatchers = append(dispatchers, dispatcherID) + select { + case err := <-doneCh: + return err + case <-d.ctx.Done(): + return errors.Trace(d.ctx.Err()) } - sort.Slice(dispatchers, func(i, j int) bool { - return dispatchers[i].String() < dispatchers[j].String() - }) - return dispatchers } func (d *dmlWriters) close() { diff --git a/downstreamadapter/sink/cloudstorage/dml_writers_drain_test.go b/downstreamadapter/sink/cloudstorage/dml_writers_drain_test.go deleted file mode 100644 index 5af66c8b58..0000000000 --- a/downstreamadapter/sink/cloudstorage/dml_writers_drain_test.go +++ /dev/null @@ -1,108 +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 cloudstorage - -import ( - "testing" - - "github.com/pingcap/ticdc/heartbeatpb" - commonType "github.com/pingcap/ticdc/pkg/common" - commonEvent "github.com/pingcap/ticdc/pkg/common/event" - "github.com/stretchr/testify/require" -) - -func TestResolveAffectedDispatchersByBlockedTables(t *testing.T) { - dispatcher1 := commonType.NewDispatcherID() - dispatcher2 := commonType.NewDispatcherID() - dispatcher3 := commonType.NewDispatcherID() - - workers := &dmlWriters{ - dispatcherTableIDs: make(map[commonType.DispatcherID]int64), - tableDispatchers: make(map[int64]map[commonType.DispatcherID]struct{}), - } - workers.recordDispatcherTable(dispatcher1, 101) - workers.recordDispatcherTable(dispatcher2, 201) - workers.recordDispatcherTable(dispatcher3, 102) - - tableStore := commonEvent.NewTableSchemaStore([]*heartbeatpb.SchemaInfo{ - { - SchemaID: 1, - Tables: []*heartbeatpb.TableInfo{ - {TableID: 101}, - {TableID: 102}, - }, - }, - { - SchemaID: 2, - Tables: []*heartbeatpb.TableInfo{ - {TableID: 201}, - }, - }, - }, commonType.CloudStorageSinkType, false) - - testCases := []struct { - name string - blocked *commonEvent.InfluencedTables - expected []commonType.DispatcherID - useSchema bool - }{ - { - name: "normal blocked tables", - blocked: &commonEvent.InfluencedTables{ - InfluenceType: commonEvent.InfluenceTypeNormal, - TableIDs: []int64{101}, - }, - expected: []commonType.DispatcherID{dispatcher1}, - }, - { - name: "db blocked tables", - blocked: &commonEvent.InfluencedTables{ - InfluenceType: commonEvent.InfluenceTypeDB, - SchemaID: 1, - }, - expected: []commonType.DispatcherID{dispatcher1, dispatcher3}, - useSchema: true, - }, - { - name: "all blocked tables", - blocked: &commonEvent.InfluencedTables{ - InfluenceType: commonEvent.InfluenceTypeAll, - }, - expected: []commonType.DispatcherID{dispatcher1, dispatcher2, dispatcher3}, - useSchema: true, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - ddl := &commonEvent.DDLEvent{ - DispatcherID: dispatcher1, - FinishedTs: 100, - BlockedTables: &commonEvent.InfluencedTables{ - InfluenceType: tc.blocked.InfluenceType, - TableIDs: tc.blocked.TableIDs, - SchemaID: tc.blocked.SchemaID, - }, - } - - var store *commonEvent.TableSchemaStore - if tc.useSchema { - store = tableStore - } - - affected := workers.resolveAffectedDispatchers(ddl, store) - require.ElementsMatch(t, tc.expected, affected) - }) - } -} diff --git a/downstreamadapter/sink/cloudstorage/sink.go b/downstreamadapter/sink/cloudstorage/sink.go index 0eb9c7560c..8598f9c327 100644 --- a/downstreamadapter/sink/cloudstorage/sink.go +++ b/downstreamadapter/sink/cloudstorage/sink.go @@ -170,14 +170,18 @@ func (s *sink) AddDMLEvent(event *commonEvent.DMLEvent) { s.dmlWriters.AddDMLEvent(event) } +func (s *sink) PassBlockEvent(event commonEvent.BlockEvent) error { + if event == nil { + return nil + } + return s.dmlWriters.PassBlockEvent(event) +} + func (s *sink) WriteBlockEvent(event commonEvent.BlockEvent) error { var err error switch e := event.(type) { case *commonEvent.DDLEvent: - err = s.dmlWriters.DrainBlockEvent(e, s.tableSchemaStore) - if err == nil && !e.NotSync { - err = s.writeDDLEvent(e) - } + err = s.writeDDLEvent(e) default: log.Error("cloudstorage sink doesn't support this type of block event", zap.String("namespace", s.changefeedID.Keyspace()), diff --git a/downstreamadapter/sink/cloudstorage/sink_test.go b/downstreamadapter/sink/cloudstorage/sink_test.go index 99d88665df..358c42e98d 100644 --- a/downstreamadapter/sink/cloudstorage/sink_test.go +++ b/downstreamadapter/sink/cloudstorage/sink_test.go @@ -207,7 +207,7 @@ func TestWriteDDLEvent(t *testing.T) { }`, string(tableSchema)) } -func TestWriteDDLEventDrainsAffectedDispatchersFirst(t *testing.T) { +func TestPassBlockEventDrainsBeforeWriteDDLEvent(t *testing.T) { parentDir := t.TempDir() uri := fmt.Sprintf("file:///%s?protocol=csv&flush-interval=3600s", parentDir) sinkURI, err := url.Parse(uri) @@ -262,9 +262,12 @@ func TestWriteDDLEventDrainsAffectedDispatchersFirst(t *testing.T) { }, } - err = cloudStorageSink.WriteBlockEvent(ddlEvent) + err = cloudStorageSink.PassBlockEvent(ddlEvent) require.NoError(t, err) require.Equal(t, int64(1), dmlFlushed.Load()) + + err = cloudStorageSink.WriteBlockEvent(ddlEvent) + require.NoError(t, err) } func TestWriteCheckpointEvent(t *testing.T) { diff --git a/downstreamadapter/sink/kafka/sink.go b/downstreamadapter/sink/kafka/sink.go index 7016347e8b..fb92032875 100644 --- a/downstreamadapter/sink/kafka/sink.go +++ b/downstreamadapter/sink/kafka/sink.go @@ -144,6 +144,10 @@ func (s *sink) AddDMLEvent(event *commonEvent.DMLEvent) { s.eventChan.Push(event) } +func (s *sink) PassBlockEvent(_ commonEvent.BlockEvent) error { + return nil +} + func (s *sink) WriteBlockEvent(event commonEvent.BlockEvent) error { var err error switch v := event.(type) { diff --git a/downstreamadapter/sink/mock/sink_mock.go b/downstreamadapter/sink/mock/sink_mock.go index d5ea8540bd..36f6c904ed 100644 --- a/downstreamadapter/sink/mock/sink_mock.go +++ b/downstreamadapter/sink/mock/sink_mock.go @@ -86,6 +86,20 @@ func (mr *MockSinkMockRecorder) IsNormal() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsNormal", reflect.TypeOf((*MockSink)(nil).IsNormal)) } +// PassBlockEvent mocks base method. +func (m *MockSink) PassBlockEvent(event event.BlockEvent) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "PassBlockEvent", event) + ret0, _ := ret[0].(error) + return ret0 +} + +// PassBlockEvent indicates an expected call of PassBlockEvent. +func (mr *MockSinkMockRecorder) PassBlockEvent(event interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PassBlockEvent", reflect.TypeOf((*MockSink)(nil).PassBlockEvent), event) +} + // Run mocks base method. func (m *MockSink) Run(ctx context.Context) error { m.ctrl.T.Helper() diff --git a/downstreamadapter/sink/mock_sink.go b/downstreamadapter/sink/mock_sink.go index 37b1cfe986..e0ef9750b3 100644 --- a/downstreamadapter/sink/mock_sink.go +++ b/downstreamadapter/sink/mock_sink.go @@ -39,6 +39,10 @@ func (s *mockSink) WriteBlockEvent(event commonEvent.BlockEvent) error { return nil } +func (s *mockSink) PassBlockEvent(_ commonEvent.BlockEvent) error { + return nil +} + func (s *mockSink) AddCheckpointTs(_ uint64) { } diff --git a/downstreamadapter/sink/mysql/sink.go b/downstreamadapter/sink/mysql/sink.go index b4a6d0f3d5..8624c6e558 100644 --- a/downstreamadapter/sink/mysql/sink.go +++ b/downstreamadapter/sink/mysql/sink.go @@ -270,6 +270,10 @@ func (s *Sink) AddDMLEvent(event *commonEvent.DMLEvent) { s.conflictDetector.Add(event) } +func (s *Sink) PassBlockEvent(_ commonEvent.BlockEvent) error { + return nil +} + func (s *Sink) WriteBlockEvent(event commonEvent.BlockEvent) error { var err error switch event.GetType() { diff --git a/downstreamadapter/sink/pulsar/sink.go b/downstreamadapter/sink/pulsar/sink.go index b2eb8b911c..64d703916a 100644 --- a/downstreamadapter/sink/pulsar/sink.go +++ b/downstreamadapter/sink/pulsar/sink.go @@ -128,6 +128,10 @@ func (s *sink) AddDMLEvent(event *commonEvent.DMLEvent) { s.eventChan.Push(event) } +func (s *sink) PassBlockEvent(_ commonEvent.BlockEvent) error { + return nil +} + func (s *sink) WriteBlockEvent(event commonEvent.BlockEvent) error { var err error switch v := event.(type) { diff --git a/downstreamadapter/sink/redo/sink.go b/downstreamadapter/sink/redo/sink.go index 8f042f191d..8b711a5615 100644 --- a/downstreamadapter/sink/redo/sink.go +++ b/downstreamadapter/sink/redo/sink.go @@ -112,6 +112,10 @@ func (s *Sink) Run(ctx context.Context) error { return err } +func (s *Sink) PassBlockEvent(_ commonEvent.BlockEvent) error { + return nil +} + func (s *Sink) WriteBlockEvent(event commonEvent.BlockEvent) error { switch e := event.(type) { case *commonEvent.DDLEvent: diff --git a/downstreamadapter/sink/sink.go b/downstreamadapter/sink/sink.go index 77a5b43fc4..e89b41b318 100644 --- a/downstreamadapter/sink/sink.go +++ b/downstreamadapter/sink/sink.go @@ -33,6 +33,7 @@ type Sink interface { IsNormal() bool AddDMLEvent(event *commonEvent.DMLEvent) + PassBlockEvent(event commonEvent.BlockEvent) error WriteBlockEvent(event commonEvent.BlockEvent) error AddCheckpointTs(ts uint64) diff --git a/pkg/common/event/table_schema_store.go b/pkg/common/event/table_schema_store.go index 9d956199a3..017ac36c1c 100644 --- a/pkg/common/event/table_schema_store.go +++ b/pkg/common/event/table_schema_store.go @@ -51,8 +51,6 @@ type tableSchemaStoreRequirements struct { // - Redo pipeline: attached to redo DDL logs for InfluenceTypeDB/All expansion, and consumed // by redo applier to expand dropped tables for cleanup/skip decisions. // See pkg/common/event/redo.go and pkg/applier/redo.go. -// - Cloud storage sink: used to expand DB/All DDL affected table IDs and map to dispatcher routes -// for per-dispatcher drain before executing schema DDL file writes. // - Kafka/Pulsar (simple protocol, send-all-bootstrap-at-start): used by the table trigger dispatcher // to list all current tables and emit bootstrap schema messages at changefeed start. // See downstreamadapter/dispatcher/event_dispatcher.go. @@ -94,11 +92,11 @@ func newTableSchemaStoreRequirements( needTableNames: true, } case commonType.CloudStorageSinkType: - // Cloud storage sink uses table IDs to expand DB/All DDL affected tables, - // then maps them to active dispatcher routes for per-dispatcher drain. + // Cloud storage sink drains by dispatcher route in PassBlockEvent and does not + // require table-id expansion from TableSchemaStore. return tableSchemaStoreRequirements{ - needTableIDs: true, - updateTableIDs: true, + needTableIDs: false, + updateTableIDs: false, needTableNames: false, } case commonType.BlackHoleSinkType: diff --git a/pkg/common/event/table_schema_store_test.go b/pkg/common/event/table_schema_store_test.go index 1aca24651b..fd56f35adc 100644 --- a/pkg/common/event/table_schema_store_test.go +++ b/pkg/common/event/table_schema_store_test.go @@ -253,35 +253,3 @@ func TestTableSchemaStoreNonMysqlTableIDsBootstrapOnly(t *testing.T) { require.ElementsMatch(t, []int64{10, 20}, store.GetAllNormalTableIds()) } - -func TestTableSchemaStoreWhenCloudStorageSink(t *testing.T) { - schemaInfos := []*heartbeatpb.SchemaInfo{ - { - SchemaID: 1, - Tables: []*heartbeatpb.TableInfo{ - {TableID: 10}, - {TableID: 20}, - }, - }, - } - - store := NewTableSchemaStore(schemaInfos, common.CloudStorageSinkType, false) - require.ElementsMatch(t, []int64{10, 20}, store.GetAllNormalTableIds()) - - store.AddEvent(&DDLEvent{ - FinishedTs: 100, - NeedAddedTables: []Table{ - {SchemaID: 1, TableID: 30}, - }, - }) - require.ElementsMatch(t, []int64{10, 20, 30}, store.GetAllNormalTableIds()) - - store.AddEvent(&DDLEvent{ - FinishedTs: 101, - NeedDroppedTables: &InfluencedTables{ - InfluenceType: InfluenceTypeNormal, - TableIDs: []int64{20}, - }, - }) - require.ElementsMatch(t, []int64{10, 30}, store.GetAllNormalTableIds()) -} From 2cef2fe808f7dd912ddbd56ebb50136618c0aa9b Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Sat, 28 Feb 2026 11:26:30 +0800 Subject: [PATCH 13/20] Add more comment --- downstreamadapter/dispatcher/basic_dispatcher.go | 15 ++++++++++++++- .../sink/cloudstorage/dml_writers.go | 3 +++ downstreamadapter/sink/cloudstorage/writer.go | 2 ++ downstreamadapter/sink/sink.go | 5 +++++ pkg/common/event/dml_event.go | 2 ++ 5 files changed, 26 insertions(+), 1 deletion(-) diff --git a/downstreamadapter/dispatcher/basic_dispatcher.go b/downstreamadapter/dispatcher/basic_dispatcher.go index c7033a8713..8a179fc34c 100644 --- a/downstreamadapter/dispatcher/basic_dispatcher.go +++ b/downstreamadapter/dispatcher/basic_dispatcher.go @@ -339,7 +339,9 @@ func (d *BasicDispatcher) AddBlockEventToSink(event commonEvent.BlockEvent) erro if event.GetType() == commonEvent.TypeDDLEvent { ddl := event.(*commonEvent.DDLEvent) // If NotSync is true, it means the DDL should not be sent to downstream. - // So we just call PassBlockEventToSink to update the table progress and call the postFlush func. + // So we just call PassBlockEventToSink to finish local bookkeeping: + // mark it passed in tableProgress and trigger flush callbacks to unblock + // dispatcher progress, without sending this DDL to sink. if ddl.NotSync { log.Info("ignore DDL by NotSync", zap.Stringer("dispatcher", d.id), zap.Any("ddl", ddl)) d.PassBlockEventToSink(event) @@ -350,6 +352,12 @@ func (d *BasicDispatcher) AddBlockEventToSink(event commonEvent.BlockEvent) erro return d.sink.WriteBlockEvent(event) } +// PassBlockEventToSink is used when block event handling result is "pass" +// (for example maintainer action=Pass or DDL NotSync). +// +// It intentionally does not call sink.WriteBlockEvent. Instead, it updates +// local progress as if the event had been handled, then fires PostFlush +// callbacks so wake/checkpoint logic can continue with consistent ordering. func (d *BasicDispatcher) PassBlockEventToSink(event commonEvent.BlockEvent) { d.tableProgress.Pass(event) event.PostFlush() @@ -895,6 +903,9 @@ func (d *BasicDispatcher) DealWithBlockEvent(event commonEvent.BlockEvent) { // we track it as a pending schedule-related event until the maintainer ACKs it. d.pendingACKCount.Add(1) } + // Pre-block barrier for ordering: + // storage sink may wait until prior DML of this dispatcher are accepted + // by sink pipeline; non-storage sinks usually no-op here. err := d.sink.PassBlockEvent(event) if err != nil { if needsScheduleACKTracking { @@ -972,6 +983,8 @@ func (d *BasicDispatcher) DealWithBlockEvent(event commonEvent.BlockEvent) { return } d.sharedInfo.GetBlockEventExecutor().Submit(d, func() { + // Same pre-block barrier in the blocking path; this keeps block-event + // visibility ordered with prior DML for sinks that require draining. err := d.sink.PassBlockEvent(event) if err != nil { d.HandleError(err) diff --git a/downstreamadapter/sink/cloudstorage/dml_writers.go b/downstreamadapter/sink/cloudstorage/dml_writers.go index 014266ccb9..b911b16aac 100644 --- a/downstreamadapter/sink/cloudstorage/dml_writers.go +++ b/downstreamadapter/sink/cloudstorage/dml_writers.go @@ -140,6 +140,9 @@ func (d *dmlWriters) PassBlockEvent(event commonEvent.BlockEvent) error { doneCh := make(chan error, 1) seq := d.lastSeqNum.Inc() + // Drain marker shares the same global sequence as DML fragments. + // Defragmenter and writer will place it after all prior fragments, so once + // doneCh returns, previous DML of this dispatcher are already enqueued/drained. d.msgCh.Push(newDrainEventFragment(seq, event.GetDispatcherID(), event.GetCommitTs(), doneCh)) select { diff --git a/downstreamadapter/sink/cloudstorage/writer.go b/downstreamadapter/sink/cloudstorage/writer.go index ded413ca6a..72bdb99324 100644 --- a/downstreamadapter/sink/cloudstorage/writer.go +++ b/downstreamadapter/sink/cloudstorage/writer.go @@ -346,6 +346,8 @@ func (d *writer) genAndDispatchTask(ctx context.Context, } } if frag.isDrain() { + // Drain marker must be placed behind pending batchedTask so the caller + // observes completion only after all prior DML flush tasks are handled. if len(batchedTask.batch) > 0 { select { case <-ctx.Done(): diff --git a/downstreamadapter/sink/sink.go b/downstreamadapter/sink/sink.go index e89b41b318..4a9c769221 100644 --- a/downstreamadapter/sink/sink.go +++ b/downstreamadapter/sink/sink.go @@ -33,7 +33,12 @@ type Sink interface { IsNormal() bool AddDMLEvent(event *commonEvent.DMLEvent) + // PassBlockEvent is a pre-block hook before reporting or writing a block + // event (DDL/syncpoint). Sinks can use it as a barrier to drain/serialize + // prior DML events for ordering guarantees. Most non-storage sinks no-op. PassBlockEvent(event commonEvent.BlockEvent) error + // WriteBlockEvent writes the block event to downstream. On success, sink + // implementations are expected to call event.PostFlush(). WriteBlockEvent(event commonEvent.BlockEvent) error AddCheckpointTs(ts uint64) diff --git a/pkg/common/event/dml_event.go b/pkg/common/event/dml_event.go index 11eb8eb37a..2216fc4f2f 100644 --- a/pkg/common/event/dml_event.go +++ b/pkg/common/event/dml_event.go @@ -648,6 +648,8 @@ func (t *DMLEvent) PostFlush() { for _, f := range t.PostTxnFlushed { f() } + // Keep this fallback to preserve flush-then-wake semantics for sinks without + // an explicit enqueue stage; CAS in PostEnqueue guarantees double-calls are no-op. t.PostEnqueue() } From 8e51ebb84e459060fcdf6e1b4fa2e78c4f9d169f Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Mon, 2 Mar 2026 16:28:33 +0800 Subject: [PATCH 14/20] rename methods --- downstreamadapter/dispatcher/basic_dispatcher.go | 4 ++-- downstreamadapter/sink/blackhole/sink.go | 2 +- downstreamadapter/sink/cloudstorage/dml_writers.go | 2 +- downstreamadapter/sink/cloudstorage/sink.go | 4 ++-- downstreamadapter/sink/cloudstorage/sink_test.go | 2 +- downstreamadapter/sink/kafka/sink.go | 2 +- downstreamadapter/sink/mock/sink_mock.go | 12 ++++++------ downstreamadapter/sink/mock_sink.go | 2 +- downstreamadapter/sink/mysql/sink.go | 2 +- downstreamadapter/sink/pulsar/sink.go | 2 +- downstreamadapter/sink/redo/sink.go | 2 +- downstreamadapter/sink/sink.go | 4 ++-- pkg/common/event/table_schema_store.go | 10 +--------- 13 files changed, 21 insertions(+), 29 deletions(-) diff --git a/downstreamadapter/dispatcher/basic_dispatcher.go b/downstreamadapter/dispatcher/basic_dispatcher.go index 8a179fc34c..8b30148239 100644 --- a/downstreamadapter/dispatcher/basic_dispatcher.go +++ b/downstreamadapter/dispatcher/basic_dispatcher.go @@ -906,7 +906,7 @@ func (d *BasicDispatcher) DealWithBlockEvent(event commonEvent.BlockEvent) { // Pre-block barrier for ordering: // storage sink may wait until prior DML of this dispatcher are accepted // by sink pipeline; non-storage sinks usually no-op here. - err := d.sink.PassBlockEvent(event) + err := d.sink.FlushDMLBeforeBlock(event) if err != nil { if needsScheduleACKTracking { d.pendingACKCount.Add(-1) @@ -985,7 +985,7 @@ func (d *BasicDispatcher) DealWithBlockEvent(event commonEvent.BlockEvent) { d.sharedInfo.GetBlockEventExecutor().Submit(d, func() { // Same pre-block barrier in the blocking path; this keeps block-event // visibility ordered with prior DML for sinks that require draining. - err := d.sink.PassBlockEvent(event) + err := d.sink.FlushDMLBeforeBlock(event) if err != nil { d.HandleError(err) return diff --git a/downstreamadapter/sink/blackhole/sink.go b/downstreamadapter/sink/blackhole/sink.go index 4067c95cde..6ce095b46b 100644 --- a/downstreamadapter/sink/blackhole/sink.go +++ b/downstreamadapter/sink/blackhole/sink.go @@ -53,7 +53,7 @@ func (s *sink) AddDMLEvent(event *commonEvent.DMLEvent) { s.eventCh <- event } -func (s *sink) PassBlockEvent(_ commonEvent.BlockEvent) error { +func (s *sink) FlushDMLBeforeBlock(_ commonEvent.BlockEvent) error { return nil } diff --git a/downstreamadapter/sink/cloudstorage/dml_writers.go b/downstreamadapter/sink/cloudstorage/dml_writers.go index b911b16aac..b899b8984c 100644 --- a/downstreamadapter/sink/cloudstorage/dml_writers.go +++ b/downstreamadapter/sink/cloudstorage/dml_writers.go @@ -127,7 +127,7 @@ func (d *dmlWriters) AddDMLEvent(event *commonEvent.DMLEvent) { d.msgCh.Push(newEventFragment(seq, tbl, event.GetDispatcherID(), event)) } -func (d *dmlWriters) PassBlockEvent(event commonEvent.BlockEvent) error { +func (d *dmlWriters) FlushDMLBeforeBlock(event commonEvent.BlockEvent) error { if event == nil { return nil } diff --git a/downstreamadapter/sink/cloudstorage/sink.go b/downstreamadapter/sink/cloudstorage/sink.go index 8598f9c327..ca15f38836 100644 --- a/downstreamadapter/sink/cloudstorage/sink.go +++ b/downstreamadapter/sink/cloudstorage/sink.go @@ -170,11 +170,11 @@ func (s *sink) AddDMLEvent(event *commonEvent.DMLEvent) { s.dmlWriters.AddDMLEvent(event) } -func (s *sink) PassBlockEvent(event commonEvent.BlockEvent) error { +func (s *sink) FlushDMLBeforeBlock(event commonEvent.BlockEvent) error { if event == nil { return nil } - return s.dmlWriters.PassBlockEvent(event) + return s.dmlWriters.FlushDMLBeforeBlock(event) } func (s *sink) WriteBlockEvent(event commonEvent.BlockEvent) error { diff --git a/downstreamadapter/sink/cloudstorage/sink_test.go b/downstreamadapter/sink/cloudstorage/sink_test.go index 358c42e98d..0c9fae45c2 100644 --- a/downstreamadapter/sink/cloudstorage/sink_test.go +++ b/downstreamadapter/sink/cloudstorage/sink_test.go @@ -262,7 +262,7 @@ func TestPassBlockEventDrainsBeforeWriteDDLEvent(t *testing.T) { }, } - err = cloudStorageSink.PassBlockEvent(ddlEvent) + err = cloudStorageSink.FlushDMLBeforeBlock(ddlEvent) require.NoError(t, err) require.Equal(t, int64(1), dmlFlushed.Load()) diff --git a/downstreamadapter/sink/kafka/sink.go b/downstreamadapter/sink/kafka/sink.go index fb92032875..c5df6737a5 100644 --- a/downstreamadapter/sink/kafka/sink.go +++ b/downstreamadapter/sink/kafka/sink.go @@ -144,7 +144,7 @@ func (s *sink) AddDMLEvent(event *commonEvent.DMLEvent) { s.eventChan.Push(event) } -func (s *sink) PassBlockEvent(_ commonEvent.BlockEvent) error { +func (s *sink) FlushDMLBeforeBlock(_ commonEvent.BlockEvent) error { return nil } diff --git a/downstreamadapter/sink/mock/sink_mock.go b/downstreamadapter/sink/mock/sink_mock.go index 36f6c904ed..358f1d9894 100644 --- a/downstreamadapter/sink/mock/sink_mock.go +++ b/downstreamadapter/sink/mock/sink_mock.go @@ -86,18 +86,18 @@ func (mr *MockSinkMockRecorder) IsNormal() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsNormal", reflect.TypeOf((*MockSink)(nil).IsNormal)) } -// PassBlockEvent mocks base method. -func (m *MockSink) PassBlockEvent(event event.BlockEvent) error { +// FlushDMLBeforeBlock mocks base method. +func (m *MockSink) FlushDMLBeforeBlock(event event.BlockEvent) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "PassBlockEvent", event) + ret := m.ctrl.Call(m, "FlushDMLBeforeBlock", event) ret0, _ := ret[0].(error) return ret0 } -// PassBlockEvent indicates an expected call of PassBlockEvent. -func (mr *MockSinkMockRecorder) PassBlockEvent(event interface{}) *gomock.Call { +// FlushDMLBeforeBlock indicates an expected call of FlushDMLBeforeBlock. +func (mr *MockSinkMockRecorder) FlushDMLBeforeBlock(event interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PassBlockEvent", reflect.TypeOf((*MockSink)(nil).PassBlockEvent), event) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FlushDMLBeforeBlock", reflect.TypeOf((*MockSink)(nil).FlushDMLBeforeBlock), event) } // Run mocks base method. diff --git a/downstreamadapter/sink/mock_sink.go b/downstreamadapter/sink/mock_sink.go index e0ef9750b3..192aa04e45 100644 --- a/downstreamadapter/sink/mock_sink.go +++ b/downstreamadapter/sink/mock_sink.go @@ -39,7 +39,7 @@ func (s *mockSink) WriteBlockEvent(event commonEvent.BlockEvent) error { return nil } -func (s *mockSink) PassBlockEvent(_ commonEvent.BlockEvent) error { +func (s *mockSink) FlushDMLBeforeBlock(_ commonEvent.BlockEvent) error { return nil } diff --git a/downstreamadapter/sink/mysql/sink.go b/downstreamadapter/sink/mysql/sink.go index 8624c6e558..f1b222e3b6 100644 --- a/downstreamadapter/sink/mysql/sink.go +++ b/downstreamadapter/sink/mysql/sink.go @@ -270,7 +270,7 @@ func (s *Sink) AddDMLEvent(event *commonEvent.DMLEvent) { s.conflictDetector.Add(event) } -func (s *Sink) PassBlockEvent(_ commonEvent.BlockEvent) error { +func (s *Sink) FlushDMLBeforeBlock(_ commonEvent.BlockEvent) error { return nil } diff --git a/downstreamadapter/sink/pulsar/sink.go b/downstreamadapter/sink/pulsar/sink.go index 64d703916a..136f95ca1a 100644 --- a/downstreamadapter/sink/pulsar/sink.go +++ b/downstreamadapter/sink/pulsar/sink.go @@ -128,7 +128,7 @@ func (s *sink) AddDMLEvent(event *commonEvent.DMLEvent) { s.eventChan.Push(event) } -func (s *sink) PassBlockEvent(_ commonEvent.BlockEvent) error { +func (s *sink) FlushDMLBeforeBlock(_ commonEvent.BlockEvent) error { return nil } diff --git a/downstreamadapter/sink/redo/sink.go b/downstreamadapter/sink/redo/sink.go index 8b711a5615..646035799d 100644 --- a/downstreamadapter/sink/redo/sink.go +++ b/downstreamadapter/sink/redo/sink.go @@ -112,7 +112,7 @@ func (s *Sink) Run(ctx context.Context) error { return err } -func (s *Sink) PassBlockEvent(_ commonEvent.BlockEvent) error { +func (s *Sink) FlushDMLBeforeBlock(_ commonEvent.BlockEvent) error { return nil } diff --git a/downstreamadapter/sink/sink.go b/downstreamadapter/sink/sink.go index 4a9c769221..4b249a6b76 100644 --- a/downstreamadapter/sink/sink.go +++ b/downstreamadapter/sink/sink.go @@ -33,10 +33,10 @@ type Sink interface { IsNormal() bool AddDMLEvent(event *commonEvent.DMLEvent) - // PassBlockEvent is a pre-block hook before reporting or writing a block + // FlushDMLBeforeBlock is a pre-block hook before reporting or writing a block // event (DDL/syncpoint). Sinks can use it as a barrier to drain/serialize // prior DML events for ordering guarantees. Most non-storage sinks no-op. - PassBlockEvent(event commonEvent.BlockEvent) error + FlushDMLBeforeBlock(event commonEvent.BlockEvent) error // WriteBlockEvent writes the block event to downstream. On success, sink // implementations are expected to call event.PostFlush(). WriteBlockEvent(event commonEvent.BlockEvent) error diff --git a/pkg/common/event/table_schema_store.go b/pkg/common/event/table_schema_store.go index 017ac36c1c..588f33b7fa 100644 --- a/pkg/common/event/table_schema_store.go +++ b/pkg/common/event/table_schema_store.go @@ -91,15 +91,7 @@ func newTableSchemaStoreRequirements( updateTableIDs: false, needTableNames: true, } - case commonType.CloudStorageSinkType: - // Cloud storage sink drains by dispatcher route in PassBlockEvent and does not - // require table-id expansion from TableSchemaStore. - return tableSchemaStoreRequirements{ - needTableIDs: false, - updateTableIDs: false, - needTableNames: false, - } - case commonType.BlackHoleSinkType: + case commonType.CloudStorageSinkType, commonType.BlackHoleSinkType: return tableSchemaStoreRequirements{ needTableIDs: false, updateTableIDs: false, From c8bb04067d83c5834b7dfeee0331d9855777862e Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Mon, 2 Mar 2026 17:24:49 +0800 Subject: [PATCH 15/20] fix the code --- .../dispatcher/basic_dispatcher.go | 68 +++++++++++-------- .../dispatcher/event_dispatcher_test.go | 11 +-- downstreamadapter/dispatcher/helper.go | 8 +-- 3 files changed, 49 insertions(+), 38 deletions(-) diff --git a/downstreamadapter/dispatcher/basic_dispatcher.go b/downstreamadapter/dispatcher/basic_dispatcher.go index 8b30148239..649c8d2685 100644 --- a/downstreamadapter/dispatcher/basic_dispatcher.go +++ b/downstreamadapter/dispatcher/basic_dispatcher.go @@ -334,6 +334,13 @@ func (d *BasicDispatcher) InitializeTableSchemaStore(schemaInfo []*heartbeatpb.S } func (d *BasicDispatcher) AddBlockEventToSink(event commonEvent.BlockEvent) error { + // Keep block-event write order with prior DML. For storage sink this may wait + // until prior DML are enqueued/flushed to the sink pipeline; for non-storage + // sinks it is usually a no-op. + if err := d.sink.FlushDMLBeforeBlock(event); err != nil { + return err + } + // For ddl event, we need to check whether it should be sent to downstream. // It may be marked as not sync by filter when building the event. if event.GetType() == commonEvent.TypeDDLEvent { @@ -742,9 +749,10 @@ func (d *BasicDispatcher) handleEvents(dispatcherEvents []DispatcherEvent, wakeC // 1. If the action is a write, we need to add the ddl event to the sink for writing to downstream. // 2. If the action is a pass, we just need to pass the event // -// For Action_Write, writing the block event may involve IO (e.g. executing DDL). To avoid blocking the -// dispatcher status dynamic stream handler, we execute the write asynchronously and return await=true. -// The status path will be waked up after the write finishes. +// For block actions (write/pass), execution may involve downstream IO because we flush prior DML first. +// To avoid blocking the dispatcher status dynamic stream handler, we execute the action asynchronously +// and return await=true. +// The status path will be waked up after the action finishes. func (d *BasicDispatcher) HandleDispatcherStatus(dispatcherStatus *heartbeatpb.DispatcherStatus) (await bool) { log.Debug("dispatcher handle dispatcher status", zap.Any("dispatcherStatus", dispatcherStatus), @@ -791,17 +799,18 @@ func (d *BasicDispatcher) HandleDispatcherStatus(dispatcherStatus *heartbeatpb.D // 3. clear blockEventStatus(should be the old pending event, but clear the new one) d.blockEventStatus.clear() }) + actionCommitTs := action.CommitTs + actionIsSyncPoint := action.IsSyncPoint if action.Action == heartbeatpb.Action_Write { - actionCommitTs := action.CommitTs - actionIsSyncPoint := action.IsSyncPoint d.sharedInfo.GetBlockEventExecutor().Submit(d, func() { d.ExecuteBlockEventDDL(pendingEvent, actionCommitTs, actionIsSyncPoint) }) return true } else { - failpoint.Inject("BlockOrWaitBeforePass", nil) - d.PassBlockEventToSink(pendingEvent) - failpoint.Inject("BlockAfterPass", nil) + d.sharedInfo.GetBlockEventExecutor().Submit(d, func() { + d.PassBlockEvent(pendingEvent, actionCommitTs, actionIsSyncPoint) + }) + return true } } else { ts, ok := d.blockEventStatus.getEventCommitTs() @@ -842,7 +851,26 @@ func (d *BasicDispatcher) ExecuteBlockEventDDL(pendingEvent commonEvent.BlockEve return } failpoint.Inject("BlockOrWaitReportAfterWrite", nil) + d.reportBlockedEventDone(actionCommitTs, actionIsSyncPoint) +} +// PassBlockEvent executes maintainer Action_Pass: +// flush prior DML for ordering, then locally mark the block event as passed. +func (d *BasicDispatcher) PassBlockEvent(pendingEvent commonEvent.BlockEvent, actionCommitTs uint64, actionIsSyncPoint bool) { + failpoint.Inject("BlockOrWaitBeforePass", nil) + err := d.sink.FlushDMLBeforeBlock(pendingEvent) + if err != nil { + d.HandleError(err) + return + } + d.PassBlockEventToSink(pendingEvent) + failpoint.Inject("BlockAfterPass", nil) + d.reportBlockedEventDone(actionCommitTs, actionIsSyncPoint) +} + +// reportBlockedEventDone sends DONE status and wakes dispatcher-status stream path +// so the next status for this dispatcher can be handled. +func (d *BasicDispatcher) reportBlockedEventDone(actionCommitTs uint64, actionIsSyncPoint bool) { d.sharedInfo.blockStatusesChan <- &heartbeatpb.TableSpanBlockStatus{ ID: d.id.ToPB(), State: &heartbeatpb.State{ @@ -903,18 +931,7 @@ func (d *BasicDispatcher) DealWithBlockEvent(event commonEvent.BlockEvent) { // we track it as a pending schedule-related event until the maintainer ACKs it. d.pendingACKCount.Add(1) } - // Pre-block barrier for ordering: - // storage sink may wait until prior DML of this dispatcher are accepted - // by sink pipeline; non-storage sinks usually no-op here. - err := d.sink.FlushDMLBeforeBlock(event) - if err != nil { - if needsScheduleACKTracking { - d.pendingACKCount.Add(-1) - } - d.HandleError(err) - return - } - err = d.AddBlockEventToSink(event) + err := d.AddBlockEventToSink(event) if err != nil { if needsScheduleACKTracking { d.pendingACKCount.Add(-1) @@ -982,16 +999,7 @@ func (d *BasicDispatcher) DealWithBlockEvent(event commonEvent.BlockEvent) { d.holdBlockEvent(event) return } - d.sharedInfo.GetBlockEventExecutor().Submit(d, func() { - // Same pre-block barrier in the blocking path; this keeps block-event - // visibility ordered with prior DML for sinks that require draining. - err := d.sink.FlushDMLBeforeBlock(event) - if err != nil { - d.HandleError(err) - return - } - d.reportBlockedEventToMaintainer(event) - }) + d.reportBlockedEventToMaintainer(event) } // dealing with events which update schema ids diff --git a/downstreamadapter/dispatcher/event_dispatcher_test.go b/downstreamadapter/dispatcher/event_dispatcher_test.go index 7d8d1ce050..13231cb24b 100644 --- a/downstreamadapter/dispatcher/event_dispatcher_test.go +++ b/downstreamadapter/dispatcher/event_dispatcher_test.go @@ -387,10 +387,13 @@ func TestDispatcherHandleEvents(t *testing.T) { }, } dispatcher.HandleDispatcherStatus(dispatcherStatus) - checkpointTs, isEmpty = tableProgress.GetCheckpointTs() - require.Equal(t, true, isEmpty) - require.Equal(t, uint64(5), checkpointTs) - require.Equal(t, int32(6), count.Load()) + require.Eventually(t, func() bool { + checkpointTs, isEmpty = tableProgress.GetCheckpointTs() + if !isEmpty || checkpointTs != uint64(5) { + return false + } + return count.Load() == int32(6) + }, 5*time.Second, 10*time.Millisecond) // ===== resolved event ===== checkpointTs = dispatcher.GetCheckpointTs() diff --git a/downstreamadapter/dispatcher/helper.go b/downstreamadapter/dispatcher/helper.go index 29373edc14..177eab4ffa 100644 --- a/downstreamadapter/dispatcher/helper.go +++ b/downstreamadapter/dispatcher/helper.go @@ -346,9 +346,8 @@ func (d *DispatcherStatusWithID) GetDispatcherID() common.DispatcherID { // If so, we can cancel the resend task. // If we get a dispatcher action, we need to check whether the action is for the current pending ddl event. // If so, we can deal the ddl event based on the action. -// 1. If the action is a write, we need to add the ddl event to the sink for writing to downstream(async). -// 2. If the action is a pass, we just need to pass the event in tableProgress(for correct calculation) and -// wake the dispatcherEventsHandler to handle the event. +// 1. If the action is a write, flush prior DML and write the block event to sink(async). +// 2. If the action is a pass, flush prior DML and pass the event in tableProgress(async). type DispatcherStatusHandler struct{} func (h *DispatcherStatusHandler) Path(event DispatcherStatusWithID) common.DispatcherID { @@ -382,7 +381,8 @@ func (h *DispatcherStatusHandler) GetTimestamp(event DispatcherStatusWithID) dyn } func (h *DispatcherStatusHandler) GetType(event DispatcherStatusWithID) dynstream.EventType { - // DispatcherStatus may trigger downstream IO (e.g. executing DDL) when handling Action_Write. + // DispatcherStatus may trigger downstream IO when handling action write/pass + // because we flush prior DML before completing the block action. // Make it non-batchable to ensure we can safely return await=true for a single event. return dynstream.EventType{DataGroup: 0, Property: dynstream.NonBatchable} } From 229673c98e750c844e10f19fa838b2f3929f65ec Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Mon, 2 Mar 2026 17:54:50 +0800 Subject: [PATCH 16/20] fix the code --- .../dispatcher/basic_dispatcher.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/downstreamadapter/dispatcher/basic_dispatcher.go b/downstreamadapter/dispatcher/basic_dispatcher.go index 649c8d2685..32c265029e 100644 --- a/downstreamadapter/dispatcher/basic_dispatcher.go +++ b/downstreamadapter/dispatcher/basic_dispatcher.go @@ -562,16 +562,16 @@ func (d *BasicDispatcher) HandleEvents(dispatcherEvents []DispatcherEvent, wakeC return false } -// handleEvents can batch handle events about resolvedTs Event and DML Event. -// While for DDLEvent and SyncPointEvent, they should be handled separately, -// because they are block events. -// We ensure we only will receive one event when it's ddl event or sync point event -// by setting them with different event types in DispatcherEventsHandler.GetType -// When we handle events, we don't have any previous events still in sink. +// handleEvents handles one batch on a dispatcher path. +// DML and resolved-ts events may be batched together; DDL/syncpoint events are +// always single-event batches because they are block events. // -// wakeCallback is used to wake the dynamic stream to handle the next batch events. -// For non-storage sinks, we wake only after flush and tableProgress becomes empty. -// For cloud storage sink, we wake after all events in this batch are enqueued. +// wakeCallback is used to wake the dynamic stream to process the next batch. +// For DML batches, we wake when all accepted events in this batch reach +// PostEnqueue. Sinks with an explicit enqueue stage (for example cloud storage) +// call PostEnqueue before flush. Sinks without an explicit enqueue stage rely on +// the PostFlush fallback (PostFlush -> PostEnqueue), so wake preserves the +// historical flush-then-wake behavior. func (d *BasicDispatcher) handleEvents(dispatcherEvents []DispatcherEvent, wakeCallback func()) bool { if d.GetRemovingStatus() { log.Warn("dispatcher is removing", zap.Any("id", d.id)) From 442a724acd68cb1176b0f6bcddba05612b948b50 Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Mon, 2 Mar 2026 18:36:49 +0800 Subject: [PATCH 17/20] fix a lot of code --- .../basic_dispatcher_active_active_test.go | 5 +- .../dispatcher/event_dispatcher_test.go | 169 +++++------------- .../dispatcher/mock_sink_helper_test.go | 92 ++++++++++ .../dispatcher/redo_dispatcher_test.go | 88 +++++---- .../dispatcher_manager_test.go | 27 ++- downstreamadapter/sink/mock/sink_mock.go | 28 +-- downstreamadapter/sink/mock_sink.go | 91 ---------- 7 files changed, 230 insertions(+), 270 deletions(-) create mode 100644 downstreamadapter/dispatcher/mock_sink_helper_test.go delete mode 100644 downstreamadapter/sink/mock_sink.go diff --git a/downstreamadapter/dispatcher/basic_dispatcher_active_active_test.go b/downstreamadapter/dispatcher/basic_dispatcher_active_active_test.go index 7f53f70a3b..8f95a93b5b 100644 --- a/downstreamadapter/dispatcher/basic_dispatcher_active_active_test.go +++ b/downstreamadapter/dispatcher/basic_dispatcher_active_active_test.go @@ -15,7 +15,6 @@ package dispatcher import ( "testing" - "github.com/pingcap/ticdc/downstreamadapter/sink" "github.com/pingcap/ticdc/heartbeatpb" "github.com/pingcap/ticdc/pkg/common" commonEvent "github.com/pingcap/ticdc/pkg/common/event" @@ -147,7 +146,7 @@ func newTestBasicDispatcher(t *testing.T, sinkType common.SinkType, enableActive blockStatuses, errCh, ) - dispatcherSink := sink.NewMockSink(sinkType) + dispatcherSink := newDispatcherTestSink(t, sinkType) tableSpan := &heartbeatpb.TableSpan{TableID: 1, StartKey: []byte{0}, EndKey: []byte{1}} dispatcher := NewBasicDispatcher( common.NewDispatcherID(), @@ -159,7 +158,7 @@ func newTestBasicDispatcher(t *testing.T, sinkType common.SinkType, enableActive false, 200, common.DefaultMode, - dispatcherSink, + dispatcherSink.Sink(), sharedInfo, ) return dispatcher diff --git a/downstreamadapter/dispatcher/event_dispatcher_test.go b/downstreamadapter/dispatcher/event_dispatcher_test.go index 13231cb24b..3c574917d1 100644 --- a/downstreamadapter/dispatcher/event_dispatcher_test.go +++ b/downstreamadapter/dispatcher/event_dispatcher_test.go @@ -20,10 +20,8 @@ import ( "testing" "time" - "github.com/golang/mock/gomock" "github.com/pingcap/failpoint" "github.com/pingcap/ticdc/downstreamadapter/sink" - "github.com/pingcap/ticdc/downstreamadapter/sink/mock" "github.com/pingcap/ticdc/downstreamadapter/syncpoint" "github.com/pingcap/ticdc/heartbeatpb" "github.com/pingcap/ticdc/pkg/common" @@ -127,10 +125,10 @@ func TestDispatcherHandleEvents(t *testing.T) { tableInfo := dmlEvent.TableInfo - sink := sink.NewMockSink(common.MysqlSinkType) + testSink := newDispatcherTestSink(t, common.MysqlSinkType) tableSpan, err := getCompleteTableSpan(getTestingKeyspaceID()) require.NoError(t, err) - dispatcher := newDispatcherForTest(sink, tableSpan) + dispatcher := newDispatcherForTest(testSink.Sink(), tableSpan) require.Equal(t, uint64(0), dispatcher.GetCheckpointTs()) require.Equal(t, uint64(0), dispatcher.GetResolvedTs()) tableProgress := dispatcher.tableProgress @@ -143,7 +141,7 @@ func TestDispatcherHandleEvents(t *testing.T) { nodeID := node.NewID() block := dispatcher.HandleEvents([]DispatcherEvent{NewDispatcherEvent(&nodeID, dmlEvent)}, callback) require.Equal(t, true, block) - require.Equal(t, 1, len(sink.GetDMLs())) + require.Equal(t, 1, len(testSink.GetDMLs())) checkpointTs, isEmpty = tableProgress.GetCheckpointTs() require.Equal(t, false, isEmpty) @@ -151,8 +149,8 @@ func TestDispatcherHandleEvents(t *testing.T) { require.Equal(t, int32(0), count.Load()) // flush - sink.FlushDMLs() - require.Equal(t, 0, len(sink.GetDMLs())) + testSink.FlushDMLs() + require.Equal(t, 0, len(testSink.GetDMLs())) checkpointTs, isEmpty = tableProgress.GetCheckpointTs() require.Equal(t, true, isEmpty) require.Equal(t, uint64(1), checkpointTs) @@ -171,7 +169,7 @@ func TestDispatcherHandleEvents(t *testing.T) { block = dispatcher.HandleEvents([]DispatcherEvent{NewDispatcherEvent(&nodeID, ddlEvent)}, callback) require.Equal(t, true, block) - require.Equal(t, 0, len(sink.GetDMLs())) + require.Equal(t, 0, len(testSink.GetDMLs())) time.Sleep(5 * time.Second) // no pending event blockPendingEvent, blockStage := dispatcher.blockEventStatus.getEventAndStage() @@ -198,7 +196,7 @@ func TestDispatcherHandleEvents(t *testing.T) { } block = dispatcher.HandleEvents([]DispatcherEvent{NewDispatcherEvent(&nodeID, ddlEvent21)}, callback) require.Equal(t, true, block) - require.Equal(t, 0, len(sink.GetDMLs())) + require.Equal(t, 0, len(testSink.GetDMLs())) time.Sleep(5 * time.Second) // no pending event blockPendingEvent, blockStage = dispatcher.blockEventStatus.getEventAndStage() @@ -239,7 +237,7 @@ func TestDispatcherHandleEvents(t *testing.T) { } block = dispatcher.HandleEvents([]DispatcherEvent{NewDispatcherEvent(&nodeID, ddlEvent2)}, callback) require.Equal(t, true, block) - require.Equal(t, 0, len(sink.GetDMLs())) + require.Equal(t, 0, len(testSink.GetDMLs())) time.Sleep(5 * time.Second) // no pending event blockPendingEvent, blockStage = dispatcher.blockEventStatus.getEventAndStage() @@ -289,7 +287,7 @@ func TestDispatcherHandleEvents(t *testing.T) { } block = dispatcher.HandleEvents([]DispatcherEvent{NewDispatcherEvent(&nodeID, ddlEvent3)}, callback) require.Equal(t, true, block) - require.Equal(t, 0, len(sink.GetDMLs())) + require.Equal(t, 0, len(testSink.GetDMLs())) time.Sleep(5 * time.Second) // pending event blockPendingEvent, blockStage = dispatcher.blockEventStatus.getEventAndStage() @@ -353,7 +351,7 @@ func TestDispatcherHandleEvents(t *testing.T) { block = dispatcher.HandleEvents([]DispatcherEvent{NewDispatcherEvent(&nodeID, syncPointEvent)}, callback) require.Equal(t, true, block) time.Sleep(5 * time.Second) - require.Equal(t, 0, len(sink.GetDMLs())) + require.Equal(t, 0, len(testSink.GetDMLs())) // pending event blockPendingEvent, blockStage = dispatcher.blockEventStatus.getEventAndStage() require.NotNil(t, blockPendingEvent) @@ -403,7 +401,7 @@ func TestDispatcherHandleEvents(t *testing.T) { } block = dispatcher.HandleEvents([]DispatcherEvent{NewDispatcherEvent(&nodeID, resolvedEvent)}, callback) require.Equal(t, false, block) - require.Equal(t, 0, len(sink.GetDMLs())) + require.Equal(t, 0, len(testSink.GetDMLs())) require.Equal(t, uint64(7), dispatcher.GetResolvedTs()) checkpointTs = dispatcher.GetCheckpointTs() require.Equal(t, uint64(7), checkpointTs) @@ -419,9 +417,9 @@ func TestUncompeleteTableSpanDispatcherHandleEvents(t *testing.T) { ddlJob := helper.DDL2Job("create table t(id int primary key, v int)") require.NotNil(t, ddlJob) - sink := sink.NewMockSink(common.MysqlSinkType) + testSink := newDispatcherTestSink(t, common.MysqlSinkType) tableSpan := getUncompleteTableSpan() - dispatcher := newDispatcherForTest(sink, tableSpan) + dispatcher := newDispatcherForTest(testSink.Sink(), tableSpan) dmlEvent := helper.DML2Event("test", "t", "insert into t values(1, 1)") require.NotNil(t, dmlEvent) @@ -489,8 +487,8 @@ func TestTableTriggerEventDispatcherInMysql(t *testing.T) { count.Swap(0) ddlTableSpan := common.KeyspaceDDLSpan(common.DefaultKeyspaceID) - sink := sink.NewMockSink(common.MysqlSinkType) - tableTriggerEventDispatcher := newDispatcherForTest(sink, ddlTableSpan) + testSink := newDispatcherTestSink(t, common.MysqlSinkType) + tableTriggerEventDispatcher := newDispatcherForTest(testSink.Sink(), ddlTableSpan) require.Nil(t, tableTriggerEventDispatcher.tableSchemaStore) ok, err := tableTriggerEventDispatcher.InitializeTableSchemaStore([]*heartbeatpb.SchemaInfo{}) @@ -573,8 +571,8 @@ func TestTableTriggerEventDispatcherInKafka(t *testing.T) { count.Swap(0) ddlTableSpan := common.KeyspaceDDLSpan(common.DefaultKeyspaceID) - sink := sink.NewMockSink(common.KafkaSinkType) - tableTriggerEventDispatcher := newDispatcherForTest(sink, ddlTableSpan) + testSink := newDispatcherTestSink(t, common.KafkaSinkType) + tableTriggerEventDispatcher := newDispatcherForTest(testSink.Sink(), ddlTableSpan) require.Nil(t, tableTriggerEventDispatcher.tableSchemaStore) ok, err := tableTriggerEventDispatcher.InitializeTableSchemaStore([]*heartbeatpb.SchemaInfo{}) @@ -668,10 +666,10 @@ func TestDispatcherClose(t *testing.T) { dmlEvent.Length = 1 { - sink := sink.NewMockSink(common.MysqlSinkType) + testSink := newDispatcherTestSink(t, common.MysqlSinkType) tableSpan, err := getCompleteTableSpan(getTestingKeyspaceID()) require.NoError(t, err) - dispatcher := newDispatcherForTest(sink, tableSpan) + dispatcher := newDispatcherForTest(testSink.Sink(), tableSpan) // ===== dml event ===== nodeID := node.NewID() @@ -681,7 +679,7 @@ func TestDispatcherClose(t *testing.T) { require.Equal(t, false, ok) // flush - sink.FlushDMLs() + testSink.FlushDMLs() watermark, ok := dispatcher.TryClose() require.Equal(t, true, ok) @@ -691,10 +689,10 @@ func TestDispatcherClose(t *testing.T) { // test sink is not normal { - sink := sink.NewMockSink(common.MysqlSinkType) + testSink := newDispatcherTestSink(t, common.MysqlSinkType) tableSpan, err := getCompleteTableSpan(getTestingKeyspaceID()) require.NoError(t, err) - dispatcher := newDispatcherForTest(sink, tableSpan) + dispatcher := newDispatcherForTest(testSink.Sink(), tableSpan) // ===== dml event ===== nodeID := node.NewID() @@ -703,7 +701,7 @@ func TestDispatcherClose(t *testing.T) { _, ok := dispatcher.TryClose() require.Equal(t, false, ok) - sink.SetIsNormal(false) + testSink.SetIsNormal(false) watermark, ok := dispatcher.TryClose() require.Equal(t, true, ok) @@ -738,20 +736,13 @@ func TestBatchDMLEventsPartialFlush(t *testing.T) { dmlEvent3.CommitTs = 12 dmlEvent3.Length = 1 - mockSink := sink.NewMockSink(common.MysqlSinkType) + testSink := newDispatcherTestSink(t, common.MysqlSinkType) tableSpan, err := getCompleteTableSpan(getTestingKeyspaceID()) require.NoError(t, err) - dispatcher := newDispatcherForTest(mockSink, tableSpan) - - // Create a callback that records when it's called - var callbackCalled atomic.Bool - wakeCallback := func() { - callbackCalled.Store(true) - } + dispatcher := newDispatcherForTest(testSink.Sink(), tableSpan) nodeID := node.NewID() - - // Create dispatcher events for all three DML events + var callbackCalled atomic.Bool dispatcherEvents := []DispatcherEvent{ NewDispatcherEvent(&nodeID, dmlEvent1), NewDispatcherEvent(&nodeID, dmlEvent2), @@ -768,94 +759,33 @@ func TestBatchDMLEventsPartialFlush(t *testing.T) { resultCh := make(chan bool, 1) go func() { - block := dispatcher.HandleEvents(dispatcherEvents, wakeCallback) - resultCh <- block + resultCh <- dispatcher.HandleEvents(dispatcherEvents, func() { + callbackCalled.Store(true) + }) }() require.Eventually(t, func() bool { - return len(mockSink.GetDMLs()) == 1 + return len(testSink.GetDMLs()) == 1 }, 5*time.Second, 10*time.Millisecond) - mockSink.FlushDMLs() + + testSink.FlushDMLs() require.False(t, callbackCalled.Load()) require.NoError(t, failpoint.Disable("github.com/pingcap/ticdc/downstreamadapter/dispatcher/BlockAddDMLEvents")) failpointEnabled = false require.Eventually(t, func() bool { - return len(mockSink.GetDMLs()) == 2 + return len(testSink.GetDMLs()) == 2 }, 5*time.Second, 10*time.Millisecond) - mockSink.FlushDMLs() + + testSink.FlushDMLs() require.Eventually(t, func() bool { return callbackCalled.Load() }, 5*time.Second, 10*time.Millisecond) - // Now the callback should be called after all events are flushed - require.True(t, callbackCalled.Load()) require.Eventually(t, func() bool { - return len(mockSink.GetDMLs()) == 0 + return len(testSink.GetDMLs()) == 0 }, 5*time.Second, 10*time.Millisecond) require.True(t, <-resultCh) - - // Verify that all events were actually flushed - require.Equal(t, 0, len(mockSink.GetDMLs())) -} - -func TestDMLWakeCallbackNonStorageOnlyAfterTableProgressEmpty(t *testing.T) { - helper := commonEvent.NewEventTestHelper(t) - defer helper.Close() - - helper.Tk().MustExec("use test") - ddlJob := helper.DDL2Job("create table t(id int primary key, v int)") - require.NotNil(t, ddlJob) - - buildDMLEvent := func(commitTs uint64) *commonEvent.DMLEvent { - event := helper.DML2Event( - "test", - "t", - fmt.Sprintf("insert into t values(%d, %d)", commitTs, commitTs), - ) - require.NotNil(t, event) - event.CommitTs = commitTs - return event - } - - ctrl := gomock.NewController(t) - mockSink := mock.NewMockSink(ctrl) - mockSink.EXPECT().SinkType().Return(common.MysqlSinkType).AnyTimes() - capturedDMLs := make([]*commonEvent.DMLEvent, 0, 3) - mockSink.EXPECT().AddDMLEvent(gomock.Any()).Do(func(event *commonEvent.DMLEvent) { - capturedDMLs = append(capturedDMLs, event) - }).Times(3) - tableSpan, err := getCompleteTableSpan(getTestingKeyspaceID()) - require.NoError(t, err) - dispatcher := newDispatcherForTest(mockSink, tableSpan) - - dmlEvent1 := buildDMLEvent(100) - dmlEvent2 := buildDMLEvent(101) - dmlEvent3 := buildDMLEvent(102) - nodeID := node.NewID() - dispatcherEvents := []DispatcherEvent{ - NewDispatcherEvent(&nodeID, dmlEvent1), - NewDispatcherEvent(&nodeID, dmlEvent2), - NewDispatcherEvent(&nodeID, dmlEvent3), - } - - var callbackCalled atomic.Bool - block := dispatcher.HandleEvents(dispatcherEvents, func() { - callbackCalled.Store(true) - }) - require.True(t, block) - require.False(t, callbackCalled.Load()) - - require.Len(t, capturedDMLs, 3) - - capturedDMLs[0].PostFlush() - require.False(t, callbackCalled.Load()) - - capturedDMLs[1].PostFlush() - require.False(t, callbackCalled.Load()) - - capturedDMLs[2].PostFlush() - require.True(t, callbackCalled.Load()) require.True(t, dispatcher.tableProgress.Empty()) } @@ -878,16 +808,10 @@ func TestDMLWakeCallbackStorageAfterBatchEnqueue(t *testing.T) { return event } - ctrl := gomock.NewController(t) - mockSink := mock.NewMockSink(ctrl) - mockSink.EXPECT().SinkType().Return(common.CloudStorageSinkType).AnyTimes() - capturedDMLs := make([]*commonEvent.DMLEvent, 0, 3) - mockSink.EXPECT().AddDMLEvent(gomock.Any()).Do(func(event *commonEvent.DMLEvent) { - capturedDMLs = append(capturedDMLs, event) - }).Times(3) + testSink := newDispatcherTestSink(t, common.CloudStorageSinkType) tableSpan, err := getCompleteTableSpan(getTestingKeyspaceID()) require.NoError(t, err) - dispatcher := newDispatcherForTest(mockSink, tableSpan) + dispatcher := newDispatcherForTest(testSink.Sink(), tableSpan) dmlEvent1 := buildDMLEvent(110) dmlEvent2 := buildDMLEvent(111) @@ -906,6 +830,7 @@ func TestDMLWakeCallbackStorageAfterBatchEnqueue(t *testing.T) { require.True(t, block) require.False(t, callbackCalled.Load()) + capturedDMLs := testSink.GetDMLs() require.Len(t, capturedDMLs, 3) capturedDMLs[0].PostEnqueue() @@ -942,7 +867,7 @@ func TestDispatcherSplittableCheck(t *testing.T) { require.False(t, commonEvent.IsSplitable(commonTableInfo)) // Create a mock sink - sink := sink.NewMockSink(common.MysqlSinkType) + testSink := newDispatcherTestSink(t, common.MysqlSinkType) // Create an incomplete table span (split table) tableSpan := getUncompleteTableSpan() @@ -979,7 +904,7 @@ func TestDispatcherSplittableCheck(t *testing.T) { false, // skipSyncpointAtStartTs false, // skipDMLAsStartTs common.Ts(0), // pdTs - sink, + testSink.Sink(), sharedInfo, false, &redoTs, @@ -1047,7 +972,7 @@ func TestDispatcher_SkipDMLAsStartTs_FilterCorrectly(t *testing.T) { dmlEvent101.CommitTs = 101 dmlEvent101.Length = 1 - mockSink := sink.NewMockSink(common.MysqlSinkType) + mockSink := newDispatcherTestSink(t, common.MysqlSinkType) tableSpan, err := getCompleteTableSpan(getTestingKeyspaceID()) require.NoError(t, err) @@ -1086,7 +1011,7 @@ func TestDispatcher_SkipDMLAsStartTs_FilterCorrectly(t *testing.T) { false, // skipSyncpointAtStartTs true, // skipDMLAsStartTs = true (KEY: enable DML filtering) common.Ts(99), - mockSink, + mockSink.Sink(), sharedInfo, false, &redoTs, @@ -1131,7 +1056,7 @@ func TestDispatcher_SkipDMLAsStartTs_Disabled(t *testing.T) { dmlEvent100.CommitTs = 100 dmlEvent100.Length = 1 - mockSink := sink.NewMockSink(common.MysqlSinkType) + mockSink := newDispatcherTestSink(t, common.MysqlSinkType) tableSpan, err := getCompleteTableSpan(getTestingKeyspaceID()) require.NoError(t, err) @@ -1166,7 +1091,7 @@ func TestDispatcher_SkipDMLAsStartTs_Disabled(t *testing.T) { false, // skipSyncpointAtStartTs false, // skipDMLAsStartTs = false (KEY: DML filtering disabled) common.Ts(99), - mockSink, + mockSink.Sink(), sharedInfo, false, &redoTs, @@ -1183,8 +1108,8 @@ func TestDispatcher_SkipDMLAsStartTs_Disabled(t *testing.T) { func TestHoldBlockEventUntilNoResendTasks(t *testing.T) { keyspaceID := getTestingKeyspaceID() ddlTableSpan := common.KeyspaceDDLSpan(keyspaceID) - mockSink := sink.NewMockSink(common.MysqlSinkType) - dispatcher := newDispatcherForTest(mockSink, ddlTableSpan) + mockSink := newDispatcherTestSink(t, common.MysqlSinkType) + dispatcher := newDispatcherForTest(mockSink.Sink(), ddlTableSpan) nodeID := node.NewID() diff --git a/downstreamadapter/dispatcher/mock_sink_helper_test.go b/downstreamadapter/dispatcher/mock_sink_helper_test.go new file mode 100644 index 0000000000..709d8e3abf --- /dev/null +++ b/downstreamadapter/dispatcher/mock_sink_helper_test.go @@ -0,0 +1,92 @@ +// Copyright 2025 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 dispatcher + +import ( + "sync" + "sync/atomic" + "testing" + + "github.com/golang/mock/gomock" + "github.com/pingcap/ticdc/downstreamadapter/sink" + sinkmock "github.com/pingcap/ticdc/downstreamadapter/sink/mock" + "github.com/pingcap/ticdc/pkg/common" + commonEvent "github.com/pingcap/ticdc/pkg/common/event" +) + +// dispatcherTestSink wraps gomock sink and keeps the few stateful helpers that +// old tests used (captured DML events and normal/abnormal switch). +type dispatcherTestSink struct { + sink *sinkmock.MockSink + + mu sync.Mutex + dmls []*commonEvent.DMLEvent + isNormal atomic.Bool +} + +func newDispatcherTestSink(t *testing.T, sinkType common.SinkType) *dispatcherTestSink { + t.Helper() + + ctrl := gomock.NewController(t) + testSink := &dispatcherTestSink{ + sink: sinkmock.NewMockSink(ctrl), + dmls: make([]*commonEvent.DMLEvent, 0), + } + testSink.isNormal.Store(true) + + testSink.sink.EXPECT().SinkType().Return(sinkType).AnyTimes() + testSink.sink.EXPECT().IsNormal().DoAndReturn(func() bool { + return testSink.isNormal.Load() + }).AnyTimes() + testSink.sink.EXPECT().AddDMLEvent(gomock.Any()).Do(func(event *commonEvent.DMLEvent) { + testSink.mu.Lock() + defer testSink.mu.Unlock() + testSink.dmls = append(testSink.dmls, event) + }).AnyTimes() + testSink.sink.EXPECT().WriteBlockEvent(gomock.Any()).DoAndReturn(func(event commonEvent.BlockEvent) error { + event.PostFlush() + return nil + }).AnyTimes() + testSink.sink.EXPECT().FlushDMLBeforeBlock(gomock.Any()).Return(nil).AnyTimes() + testSink.sink.EXPECT().AddCheckpointTs(gomock.Any()).AnyTimes() + testSink.sink.EXPECT().SetTableSchemaStore(gomock.Any()).AnyTimes() + testSink.sink.EXPECT().Close(gomock.Any()).AnyTimes() + testSink.sink.EXPECT().Run(gomock.Any()).Return(nil).AnyTimes() + return testSink +} + +func (s *dispatcherTestSink) Sink() sink.Sink { + return s.sink +} + +func (s *dispatcherTestSink) SetIsNormal(isNormal bool) { + s.isNormal.Store(isNormal) +} + +func (s *dispatcherTestSink) GetDMLs() []*commonEvent.DMLEvent { + s.mu.Lock() + defer s.mu.Unlock() + dmls := make([]*commonEvent.DMLEvent, len(s.dmls)) + copy(dmls, s.dmls) + return dmls +} + +func (s *dispatcherTestSink) FlushDMLs() { + s.mu.Lock() + defer s.mu.Unlock() + for _, dml := range s.dmls { + dml.PostFlush() + } + s.dmls = s.dmls[:0] +} diff --git a/downstreamadapter/dispatcher/redo_dispatcher_test.go b/downstreamadapter/dispatcher/redo_dispatcher_test.go index ba008d3a0e..68f97016b7 100644 --- a/downstreamadapter/dispatcher/redo_dispatcher_test.go +++ b/downstreamadapter/dispatcher/redo_dispatcher_test.go @@ -80,10 +80,10 @@ func TestRedoDispatcherHandleEvents(t *testing.T) { tableInfo := dmlEvent.TableInfo - sink := sink.NewMockSink(common.MysqlSinkType) + testSink := newDispatcherTestSink(t, common.MysqlSinkType) tableSpan, err := getCompleteTableSpan(getTestingKeyspaceID()) require.NoError(t, err) - dispatcher := newRedoDispatcherForTest(sink, tableSpan) + dispatcher := newRedoDispatcherForTest(testSink.Sink(), tableSpan) require.Equal(t, uint64(0), dispatcher.GetCheckpointTs()) require.Equal(t, uint64(0), dispatcher.GetResolvedTs()) tableProgress := dispatcher.tableProgress @@ -96,7 +96,7 @@ func TestRedoDispatcherHandleEvents(t *testing.T) { nodeID := node.NewID() block := dispatcher.HandleEvents([]DispatcherEvent{NewDispatcherEvent(&nodeID, dmlEvent)}, redoCallback) require.Equal(t, true, block) - require.Equal(t, 1, len(sink.GetDMLs())) + require.Equal(t, 1, len(testSink.GetDMLs())) checkpointTs, isEmpty = tableProgress.GetCheckpointTs() require.Equal(t, false, isEmpty) @@ -104,8 +104,8 @@ func TestRedoDispatcherHandleEvents(t *testing.T) { require.Equal(t, int32(0), redoCount.Load()) // flush - sink.FlushDMLs() - require.Equal(t, 0, len(sink.GetDMLs())) + testSink.FlushDMLs() + require.Equal(t, 0, len(testSink.GetDMLs())) checkpointTs, isEmpty = tableProgress.GetCheckpointTs() require.Equal(t, true, isEmpty) require.Equal(t, uint64(1), checkpointTs) @@ -124,7 +124,7 @@ func TestRedoDispatcherHandleEvents(t *testing.T) { block = dispatcher.HandleEvents([]DispatcherEvent{NewDispatcherEvent(&nodeID, ddlEvent)}, redoCallback) require.Equal(t, true, block) - require.Equal(t, 0, len(sink.GetDMLs())) + require.Equal(t, 0, len(testSink.GetDMLs())) time.Sleep(5 * time.Second) // no pending event blockPendingEvent, blockStage := dispatcher.blockEventStatus.getEventAndStage() @@ -151,7 +151,7 @@ func TestRedoDispatcherHandleEvents(t *testing.T) { } block = dispatcher.HandleEvents([]DispatcherEvent{NewDispatcherEvent(&nodeID, ddlEvent21)}, redoCallback) require.Equal(t, true, block) - require.Equal(t, 0, len(sink.GetDMLs())) + require.Equal(t, 0, len(testSink.GetDMLs())) time.Sleep(5 * time.Second) // no pending event blockPendingEvent, blockStage = dispatcher.blockEventStatus.getEventAndStage() @@ -193,7 +193,7 @@ func TestRedoDispatcherHandleEvents(t *testing.T) { } block = dispatcher.HandleEvents([]DispatcherEvent{NewDispatcherEvent(&nodeID, ddlEvent2)}, redoCallback) require.Equal(t, true, block) - require.Equal(t, 0, len(sink.GetDMLs())) + require.Equal(t, 0, len(testSink.GetDMLs())) time.Sleep(5 * time.Second) // no pending event blockPendingEvent, blockStage = dispatcher.blockEventStatus.getEventAndStage() @@ -243,7 +243,7 @@ func TestRedoDispatcherHandleEvents(t *testing.T) { } block = dispatcher.HandleEvents([]DispatcherEvent{NewDispatcherEvent(&nodeID, ddlEvent3)}, redoCallback) require.Equal(t, true, block) - require.Equal(t, 0, len(sink.GetDMLs())) + require.Equal(t, 0, len(testSink.GetDMLs())) time.Sleep(5 * time.Second) // pending event blockPendingEvent, blockStage = dispatcher.blockEventStatus.getEventAndStage() @@ -304,7 +304,7 @@ func TestRedoDispatcherHandleEvents(t *testing.T) { } block = dispatcher.HandleEvents([]DispatcherEvent{NewDispatcherEvent(&nodeID, resolvedEvent)}, redoCallback) require.Equal(t, false, block) - require.Equal(t, 0, len(sink.GetDMLs())) + require.Equal(t, 0, len(testSink.GetDMLs())) require.Equal(t, uint64(7), dispatcher.GetResolvedTs()) checkpointTs = dispatcher.GetCheckpointTs() require.Equal(t, uint64(7), checkpointTs) @@ -319,9 +319,9 @@ func TestRedoUncompeleteTableSpanDispatcherHandleEvents(t *testing.T) { ddlJob := helper.DDL2Job("create table t(id int primary key, v int)") require.NotNil(t, ddlJob) - sink := sink.NewMockSink(common.MysqlSinkType) + testSink := newDispatcherTestSink(t, common.MysqlSinkType) tableSpan := getUncompleteTableSpan() - dispatcher := newRedoDispatcherForTest(sink, tableSpan) + dispatcher := newRedoDispatcherForTest(testSink.Sink(), tableSpan) dmlEvent := helper.DML2Event("test", "t", "insert into t values(1, 1)") require.NotNil(t, dmlEvent) @@ -389,8 +389,8 @@ func TestTableTriggerRedoDispatcherInMysql(t *testing.T) { redoCount.Store(0) ddlTableSpan := common.KeyspaceDDLSpan(common.DefaultKeyspaceID) - sink := sink.NewMockSink(common.MysqlSinkType) - tableTriggerEventDispatcher := newRedoDispatcherForTest(sink, ddlTableSpan) + testSink := newDispatcherTestSink(t, common.MysqlSinkType) + tableTriggerEventDispatcher := newRedoDispatcherForTest(testSink.Sink(), ddlTableSpan) helper := commonEvent.NewEventTestHelper(t) defer helper.Close() @@ -459,8 +459,8 @@ func TestTableTriggerRedoDispatcherInKafka(t *testing.T) { redoCount.Store(0) ddlTableSpan := common.KeyspaceDDLSpan(common.DefaultKeyspaceID) - sink := sink.NewMockSink(common.KafkaSinkType) - tableTriggerEventDispatcher := newRedoDispatcherForTest(sink, ddlTableSpan) + testSink := newDispatcherTestSink(t, common.KafkaSinkType) + tableTriggerEventDispatcher := newRedoDispatcherForTest(testSink.Sink(), ddlTableSpan) helper := commonEvent.NewEventTestHelper(t) defer helper.Close() @@ -539,10 +539,10 @@ func TestRedoDispatcherClose(t *testing.T) { dmlEvent.Length = 1 { - sink := sink.NewMockSink(common.MysqlSinkType) + testSink := newDispatcherTestSink(t, common.MysqlSinkType) tableSpan, err := getCompleteTableSpan(getTestingKeyspaceID()) require.NoError(t, err) - dispatcher := newRedoDispatcherForTest(sink, tableSpan) + dispatcher := newRedoDispatcherForTest(testSink.Sink(), tableSpan) // ===== dml event ===== nodeID := node.NewID() @@ -552,7 +552,7 @@ func TestRedoDispatcherClose(t *testing.T) { require.Equal(t, false, ok) // flush - sink.FlushDMLs() + testSink.FlushDMLs() watermark, ok := dispatcher.TryClose() require.Equal(t, true, ok) @@ -562,10 +562,10 @@ func TestRedoDispatcherClose(t *testing.T) { // test sink is not normal { - sink := sink.NewMockSink(common.MysqlSinkType) + testSink := newDispatcherTestSink(t, common.MysqlSinkType) tableSpan, err := getCompleteTableSpan(getTestingKeyspaceID()) require.NoError(t, err) - dispatcher := newRedoDispatcherForTest(sink, tableSpan) + dispatcher := newRedoDispatcherForTest(testSink.Sink(), tableSpan) // ===== dml event ===== nodeID := node.NewID() @@ -574,7 +574,7 @@ func TestRedoDispatcherClose(t *testing.T) { _, ok := dispatcher.TryClose() require.Equal(t, false, ok) - sink.SetIsNormal(false) + testSink.SetIsNormal(false) watermark, ok := dispatcher.TryClose() require.Equal(t, true, ok) @@ -607,15 +607,15 @@ func TestRedoBatchDMLEventsPartialFlush(t *testing.T) { dmlEvent3.CommitTs = 12 dmlEvent3.Length = 1 - mockSink := sink.NewMockSink(common.MysqlSinkType) + mockSink := newDispatcherTestSink(t, common.MysqlSinkType) tableSpan, err := getCompleteTableSpan(getTestingKeyspaceID()) require.NoError(t, err) - dispatcher := newRedoDispatcherForTest(mockSink, tableSpan) + dispatcher := newRedoDispatcherForTest(mockSink.Sink(), tableSpan) // Create a redoCallback that records when it's called - var redoCallbackCalled bool + var redoCallbackCalled atomic.Bool wakeredoCallback := func() { - redoCallbackCalled = true + redoCallbackCalled.Store(true) } nodeID := node.NewID() @@ -627,26 +627,40 @@ func TestRedoBatchDMLEventsPartialFlush(t *testing.T) { NewDispatcherEvent(&nodeID, dmlEvent3), } - failpoint.Enable("github.com/pingcap/ticdc/downstreamadapter/dispatcher/BlockAddDMLEvents", `pause`) + require.NoError(t, failpoint.Enable("github.com/pingcap/ticdc/downstreamadapter/dispatcher/BlockAddDMLEvents", `pause`)) + failpointEnabled := true + defer func() { + if failpointEnabled { + require.NoError(t, failpoint.Disable("github.com/pingcap/ticdc/downstreamadapter/dispatcher/BlockAddDMLEvents")) + } + }() + resultCh := make(chan bool, 1) go func() { - block := dispatcher.HandleEvents(dispatcherEvents, wakeredoCallback) - require.Equal(t, true, block) + resultCh <- dispatcher.HandleEvents(dispatcherEvents, wakeredoCallback) }() - time.Sleep(1 * time.Second) - require.Equal(t, 1, len(mockSink.GetDMLs())) + require.Eventually(t, func() bool { + return len(mockSink.GetDMLs()) == 1 + }, 5*time.Second, 10*time.Millisecond) mockSink.FlushDMLs() - require.False(t, redoCallbackCalled) + require.False(t, redoCallbackCalled.Load()) - failpoint.Disable("github.com/pingcap/ticdc/downstreamadapter/dispatcher/BlockAddDMLEvents") + require.NoError(t, failpoint.Disable("github.com/pingcap/ticdc/downstreamadapter/dispatcher/BlockAddDMLEvents")) + failpointEnabled = false - time.Sleep(1 * time.Second) - require.Equal(t, 2, len(mockSink.GetDMLs())) + require.Eventually(t, func() bool { + return len(mockSink.GetDMLs()) == 2 + }, 5*time.Second, 10*time.Millisecond) mockSink.FlushDMLs() // Now the redoCallback should be called after all events are flushed - require.True(t, redoCallbackCalled) + require.Eventually(t, func() bool { + return redoCallbackCalled.Load() + }, 5*time.Second, 10*time.Millisecond) // Verify that all events were actually flushed - require.Equal(t, 0, len(mockSink.GetDMLs())) + require.Eventually(t, func() bool { + return len(mockSink.GetDMLs()) == 0 + }, 5*time.Second, 10*time.Millisecond) + require.True(t, <-resultCh) } diff --git a/downstreamadapter/dispatchermanager/dispatcher_manager_test.go b/downstreamadapter/dispatchermanager/dispatcher_manager_test.go index 4f54b86da7..2caa3afe80 100644 --- a/downstreamadapter/dispatchermanager/dispatcher_manager_test.go +++ b/downstreamadapter/dispatchermanager/dispatcher_manager_test.go @@ -19,9 +19,11 @@ import ( "testing" "time" + "github.com/golang/mock/gomock" "github.com/pingcap/ticdc/downstreamadapter/dispatcher" "github.com/pingcap/ticdc/downstreamadapter/eventcollector" "github.com/pingcap/ticdc/downstreamadapter/sink" + sinkmock "github.com/pingcap/ticdc/downstreamadapter/sink/mock" "github.com/pingcap/ticdc/heartbeatpb" "github.com/pingcap/ticdc/pkg/common" appcontext "github.com/pingcap/ticdc/pkg/common/context" @@ -36,7 +38,25 @@ import ( "github.com/stretchr/testify/require" ) -var mockSink = sink.NewMockSink(common.BlackHoleSinkType) +func newDispatcherManagerTestSink(t *testing.T, sinkType common.SinkType) sink.Sink { + t.Helper() + + ctrl := gomock.NewController(t) + mockSink := sinkmock.NewMockSink(ctrl) + mockSink.EXPECT().SinkType().Return(sinkType).AnyTimes() + mockSink.EXPECT().IsNormal().Return(true).AnyTimes() + mockSink.EXPECT().AddDMLEvent(gomock.Any()).AnyTimes() + mockSink.EXPECT().FlushDMLBeforeBlock(gomock.Any()).Return(nil).AnyTimes() + mockSink.EXPECT().WriteBlockEvent(gomock.Any()).DoAndReturn(func(blockEvent event.BlockEvent) error { + blockEvent.PostFlush() + return nil + }).AnyTimes() + mockSink.EXPECT().AddCheckpointTs(gomock.Any()).AnyTimes() + mockSink.EXPECT().SetTableSchemaStore(gomock.Any()).AnyTimes() + mockSink.EXPECT().Close(gomock.Any()).AnyTimes() + mockSink.EXPECT().Run(gomock.Any()).Return(nil).AnyTimes() + return mockSink +} // createTestDispatcher creates a test dispatcher with given parameters func createTestDispatcher(t *testing.T, manager *DispatcherManager, id common.DispatcherID, tableID int64, startKey, endKey []byte) *dispatcher.EventDispatcher { @@ -72,7 +92,7 @@ func createTestDispatcher(t *testing.T, manager *DispatcherManager, id common.Di false, // skipSyncpointAtStartTs false, // skipDMLAsStartTs 0, // currentPDTs - mockSink, + manager.sink, sharedInfo, false, &redoTs, @@ -84,12 +104,13 @@ func createTestDispatcher(t *testing.T, manager *DispatcherManager, id common.Di // createTestManager creates a test DispatcherManager func createTestManager(t *testing.T) *DispatcherManager { changefeedID := common.NewChangeFeedIDWithName("test", common.DefaultKeyspaceName) + testSink := newDispatcherManagerTestSink(t, common.BlackHoleSinkType) manager := &DispatcherManager{ changefeedID: changefeedID, dispatcherMap: newDispatcherMap[*dispatcher.EventDispatcher](), heartbeatRequestQueue: NewHeartbeatRequestQueue(), blockStatusRequestQueue: NewBlockStatusRequestQueue(), - sink: mockSink, + sink: testSink, schemaIDToDispatchers: dispatcher.NewSchemaIDToDispatchers(), sinkQuota: util.GetOrZero(config.GetDefaultReplicaConfig().MemoryQuota), latestWatermark: NewWatermark(0), diff --git a/downstreamadapter/sink/mock/sink_mock.go b/downstreamadapter/sink/mock/sink_mock.go index 358f1d9894..8726735eb2 100644 --- a/downstreamadapter/sink/mock/sink_mock.go +++ b/downstreamadapter/sink/mock/sink_mock.go @@ -72,20 +72,6 @@ func (mr *MockSinkMockRecorder) Close(removeChangefeed interface{}) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Close", reflect.TypeOf((*MockSink)(nil).Close), removeChangefeed) } -// IsNormal mocks base method. -func (m *MockSink) IsNormal() bool { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IsNormal") - ret0, _ := ret[0].(bool) - return ret0 -} - -// IsNormal indicates an expected call of IsNormal. -func (mr *MockSinkMockRecorder) IsNormal() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsNormal", reflect.TypeOf((*MockSink)(nil).IsNormal)) -} - // FlushDMLBeforeBlock mocks base method. func (m *MockSink) FlushDMLBeforeBlock(event event.BlockEvent) error { m.ctrl.T.Helper() @@ -100,6 +86,20 @@ func (mr *MockSinkMockRecorder) FlushDMLBeforeBlock(event interface{}) *gomock.C return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FlushDMLBeforeBlock", reflect.TypeOf((*MockSink)(nil).FlushDMLBeforeBlock), event) } +// IsNormal mocks base method. +func (m *MockSink) IsNormal() bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IsNormal") + ret0, _ := ret[0].(bool) + return ret0 +} + +// IsNormal indicates an expected call of IsNormal. +func (mr *MockSinkMockRecorder) IsNormal() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsNormal", reflect.TypeOf((*MockSink)(nil).IsNormal)) +} + // Run mocks base method. func (m *MockSink) Run(ctx context.Context) error { m.ctrl.T.Helper() diff --git a/downstreamadapter/sink/mock_sink.go b/downstreamadapter/sink/mock_sink.go deleted file mode 100644 index 192aa04e45..0000000000 --- a/downstreamadapter/sink/mock_sink.go +++ /dev/null @@ -1,91 +0,0 @@ -// Copyright 2025 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 sink - -import ( - "context" - "sync" - - "github.com/pingcap/ticdc/pkg/common" - commonEvent "github.com/pingcap/ticdc/pkg/common/event" -) - -type mockSink struct { - mu sync.Mutex - dmls []*commonEvent.DMLEvent - isNormal bool - sinkType common.SinkType -} - -func (s *mockSink) AddDMLEvent(event *commonEvent.DMLEvent) { - s.mu.Lock() - defer s.mu.Unlock() - s.dmls = append(s.dmls, event) -} - -func (s *mockSink) WriteBlockEvent(event commonEvent.BlockEvent) error { - event.PostFlush() - return nil -} - -func (s *mockSink) FlushDMLBeforeBlock(_ commonEvent.BlockEvent) error { - return nil -} - -func (s *mockSink) AddCheckpointTs(_ uint64) { -} - -func (s *mockSink) SetTableSchemaStore(_ *commonEvent.TableSchemaStore) { -} - -func (s *mockSink) Close(bool) {} - -func (s *mockSink) Run(context.Context) error { - return nil -} - -func (s *mockSink) SinkType() common.SinkType { - return s.sinkType -} - -func (s *mockSink) IsNormal() bool { - return s.isNormal -} - -func (s *mockSink) SetIsNormal(isNormal bool) { - s.isNormal = isNormal -} - -func (s *mockSink) FlushDMLs() { - s.mu.Lock() - defer s.mu.Unlock() - for _, dml := range s.dmls { - dml.PostFlush() - } - s.dmls = make([]*commonEvent.DMLEvent, 0) -} - -func (s *mockSink) GetDMLs() []*commonEvent.DMLEvent { - s.mu.Lock() - defer s.mu.Unlock() - return s.dmls -} - -func NewMockSink(sinkType common.SinkType) *mockSink { - return &mockSink{ - dmls: make([]*commonEvent.DMLEvent, 0), - isNormal: true, - sinkType: sinkType, - } -} From 9993792e4ca970a1f4bf49c0fadc3f3a62fb6fda Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Mon, 2 Mar 2026 13:04:35 +0000 Subject: [PATCH 18/20] fix --- downstreamadapter/dispatcher/mock_sink_helper_test.go | 6 +++--- .../dispatchermanager/dispatcher_manager_test.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/downstreamadapter/dispatcher/mock_sink_helper_test.go b/downstreamadapter/dispatcher/mock_sink_helper_test.go index 709d8e3abf..6ed6d2117e 100644 --- a/downstreamadapter/dispatcher/mock_sink_helper_test.go +++ b/downstreamadapter/dispatcher/mock_sink_helper_test.go @@ -20,7 +20,7 @@ import ( "github.com/golang/mock/gomock" "github.com/pingcap/ticdc/downstreamadapter/sink" - sinkmock "github.com/pingcap/ticdc/downstreamadapter/sink/mock" + "github.com/pingcap/ticdc/downstreamadapter/sink/mock" "github.com/pingcap/ticdc/pkg/common" commonEvent "github.com/pingcap/ticdc/pkg/common/event" ) @@ -28,7 +28,7 @@ import ( // dispatcherTestSink wraps gomock sink and keeps the few stateful helpers that // old tests used (captured DML events and normal/abnormal switch). type dispatcherTestSink struct { - sink *sinkmock.MockSink + sink *mock.MockSink mu sync.Mutex dmls []*commonEvent.DMLEvent @@ -40,7 +40,7 @@ func newDispatcherTestSink(t *testing.T, sinkType common.SinkType) *dispatcherTe ctrl := gomock.NewController(t) testSink := &dispatcherTestSink{ - sink: sinkmock.NewMockSink(ctrl), + sink: mock.NewMockSink(ctrl), dmls: make([]*commonEvent.DMLEvent, 0), } testSink.isNormal.Store(true) diff --git a/downstreamadapter/dispatchermanager/dispatcher_manager_test.go b/downstreamadapter/dispatchermanager/dispatcher_manager_test.go index 2caa3afe80..d26a6193d8 100644 --- a/downstreamadapter/dispatchermanager/dispatcher_manager_test.go +++ b/downstreamadapter/dispatchermanager/dispatcher_manager_test.go @@ -23,7 +23,7 @@ import ( "github.com/pingcap/ticdc/downstreamadapter/dispatcher" "github.com/pingcap/ticdc/downstreamadapter/eventcollector" "github.com/pingcap/ticdc/downstreamadapter/sink" - sinkmock "github.com/pingcap/ticdc/downstreamadapter/sink/mock" + "github.com/pingcap/ticdc/downstreamadapter/sink/mock" "github.com/pingcap/ticdc/heartbeatpb" "github.com/pingcap/ticdc/pkg/common" appcontext "github.com/pingcap/ticdc/pkg/common/context" @@ -42,7 +42,7 @@ func newDispatcherManagerTestSink(t *testing.T, sinkType common.SinkType) sink.S t.Helper() ctrl := gomock.NewController(t) - mockSink := sinkmock.NewMockSink(ctrl) + mockSink := mock.NewMockSink(ctrl) mockSink.EXPECT().SinkType().Return(sinkType).AnyTimes() mockSink.EXPECT().IsNormal().Return(true).AnyTimes() mockSink.EXPECT().AddDMLEvent(gomock.Any()).AnyTimes() From a619c8f0f0ef92ce7058fccbab3dcb8d2bec7199 Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Tue, 3 Mar 2026 03:12:57 +0000 Subject: [PATCH 19/20] adjust comments --- .../dispatcher/basic_dispatcher.go | 52 +++++++++++-------- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/downstreamadapter/dispatcher/basic_dispatcher.go b/downstreamadapter/dispatcher/basic_dispatcher.go index 32c265029e..fc6f3139b1 100644 --- a/downstreamadapter/dispatcher/basic_dispatcher.go +++ b/downstreamadapter/dispatcher/basic_dispatcher.go @@ -334,13 +334,6 @@ func (d *BasicDispatcher) InitializeTableSchemaStore(schemaInfo []*heartbeatpb.S } func (d *BasicDispatcher) AddBlockEventToSink(event commonEvent.BlockEvent) error { - // Keep block-event write order with prior DML. For storage sink this may wait - // until prior DML are enqueued/flushed to the sink pipeline; for non-storage - // sinks it is usually a no-op. - if err := d.sink.FlushDMLBeforeBlock(event); err != nil { - return err - } - // For ddl event, we need to check whether it should be sent to downstream. // It may be marked as not sync by filter when building the event. if event.GetType() == commonEvent.TypeDDLEvent { @@ -351,10 +344,16 @@ func (d *BasicDispatcher) AddBlockEventToSink(event commonEvent.BlockEvent) erro // dispatcher progress, without sending this DDL to sink. if ddl.NotSync { log.Info("ignore DDL by NotSync", zap.Stringer("dispatcher", d.id), zap.Any("ddl", ddl)) - d.PassBlockEventToSink(event) - return nil + return d.PassBlockEventToSink(event) } } + // Keep block-event write order with prior DML. For storage sink this may wait + // until prior DML are enqueued/flushed to the sink pipeline; for non-storage + // sinks it is usually a no-op. + if err := d.sink.FlushDMLBeforeBlock(event); err != nil { + return err + } + d.tableProgress.Add(event) return d.sink.WriteBlockEvent(event) } @@ -365,9 +364,13 @@ func (d *BasicDispatcher) AddBlockEventToSink(event commonEvent.BlockEvent) erro // It intentionally does not call sink.WriteBlockEvent. Instead, it updates // local progress as if the event had been handled, then fires PostFlush // callbacks so wake/checkpoint logic can continue with consistent ordering. -func (d *BasicDispatcher) PassBlockEventToSink(event commonEvent.BlockEvent) { +func (d *BasicDispatcher) PassBlockEventToSink(event commonEvent.BlockEvent) error { + if err := d.sink.FlushDMLBeforeBlock(event); err != nil { + return err + } d.tableProgress.Pass(event) event.PostFlush() + return nil } // ensureActiveActiveTableInfo validates the table schema requirements for active-active mode. @@ -562,16 +565,22 @@ func (d *BasicDispatcher) HandleEvents(dispatcherEvents []DispatcherEvent, wakeC return false } -// handleEvents handles one batch on a dispatcher path. -// DML and resolved-ts events may be batched together; DDL/syncpoint events are -// always single-event batches because they are block events. +// handleEvents processes one batch for one dispatcher. +// A batch may mix DML and resolved-ts events; block events are expected +// to be handled one by one. +// +// Ordering rules: +// - Stale events are ignored. +// - DML are staged before resolved-ts is advanced. +// +// wakeCallback: +// - DML batch: fired after all accepted DML reach enqueue stage. +// - Block event: fired after block-event flush completion. +// - Storage sinks have an explicit enqueue stage before flush; non-storage +// sinks rely on flush completion to drive enqueue callbacks. // -// wakeCallback is used to wake the dynamic stream to process the next batch. -// For DML batches, we wake when all accepted events in this batch reach -// PostEnqueue. Sinks with an explicit enqueue stage (for example cloud storage) -// call PostEnqueue before flush. Sinks without an explicit enqueue stage rely on -// the PostFlush fallback (PostFlush -> PostEnqueue), so wake preserves the -// historical flush-then-wake behavior. +// Return true when this batch may still block progress (contains DML or block event); +// Return false when it is non-blocking (for example resolved-only, or all DML in this batch were filtered). func (d *BasicDispatcher) handleEvents(dispatcherEvents []DispatcherEvent, wakeCallback func()) bool { if d.GetRemovingStatus() { log.Warn("dispatcher is removing", zap.Any("id", d.id)) @@ -855,15 +864,14 @@ func (d *BasicDispatcher) ExecuteBlockEventDDL(pendingEvent commonEvent.BlockEve } // PassBlockEvent executes maintainer Action_Pass: -// flush prior DML for ordering, then locally mark the block event as passed. +// It relies on PassBlockEventToSink to preserve ordering and mark the event passed. func (d *BasicDispatcher) PassBlockEvent(pendingEvent commonEvent.BlockEvent, actionCommitTs uint64, actionIsSyncPoint bool) { failpoint.Inject("BlockOrWaitBeforePass", nil) - err := d.sink.FlushDMLBeforeBlock(pendingEvent) + err := d.PassBlockEventToSink(pendingEvent) if err != nil { d.HandleError(err) return } - d.PassBlockEventToSink(pendingEvent) failpoint.Inject("BlockAfterPass", nil) d.reportBlockedEventDone(actionCommitTs, actionIsSyncPoint) } From da1038b2f58413fc7722f0e527fc5a2885c96c33 Mon Sep 17 00:00:00 2001 From: 3AceShowHand Date: Tue, 3 Mar 2026 03:32:57 +0000 Subject: [PATCH 20/20] update comments --- .../dispatcher/basic_dispatcher.go | 22 +++++++------------ 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/downstreamadapter/dispatcher/basic_dispatcher.go b/downstreamadapter/dispatcher/basic_dispatcher.go index fc6f3139b1..0220883b63 100644 --- a/downstreamadapter/dispatcher/basic_dispatcher.go +++ b/downstreamadapter/dispatcher/basic_dispatcher.go @@ -566,21 +566,15 @@ func (d *BasicDispatcher) HandleEvents(dispatcherEvents []DispatcherEvent, wakeC } // handleEvents processes one batch for one dispatcher. -// A batch may mix DML and resolved-ts events; block events are expected -// to be handled one by one. +// the next batch of events can only be handled after the current batch is enqueued or flushed. +// A batch may mix DML and resolved-ts events; Block events are expected to be handled one by one. +// - Block events, such as DDL / Syncpoint, is sent to the sink synchronously, +// the dispatcher is blocked until the block event is flushed. +// - DML events is sent to the sink asynchronously. +// - Storage sink, the dispatcher wake up after all DML events is guaranteed enqueued. +// - Non-storage sink, the dispatche wake up after all DML events is guaranteed flushed. // -// Ordering rules: -// - Stale events are ignored. -// - DML are staged before resolved-ts is advanced. -// -// wakeCallback: -// - DML batch: fired after all accepted DML reach enqueue stage. -// - Block event: fired after block-event flush completion. -// - Storage sinks have an explicit enqueue stage before flush; non-storage -// sinks rely on flush completion to drive enqueue callbacks. -// -// Return true when this batch may still block progress (contains DML or block event); -// Return false when it is non-blocking (for example resolved-only, or all DML in this batch were filtered). +// Return true if should block the dispatcher. func (d *BasicDispatcher) handleEvents(dispatcherEvents []DispatcherEvent, wakeCallback func()) bool { if d.GetRemovingStatus() { log.Warn("dispatcher is removing", zap.Any("id", d.id))