diff --git a/downstreamadapter/dispatchermanager/dispatcher_manager.go b/downstreamadapter/dispatchermanager/dispatcher_manager.go index f389e6fc52..baa8c72fbf 100644 --- a/downstreamadapter/dispatchermanager/dispatcher_manager.go +++ b/downstreamadapter/dispatchermanager/dispatcher_manager.go @@ -99,6 +99,9 @@ type DispatcherManager struct { maintainerEpoch uint64 maintainerID node.ID } + // MaintainerFenceMu serializes maintainer owner/epoch changes with request + // fence checks and scheduler side effects. + MaintainerFenceMu sync.Mutex pdClock pdutil.Clock @@ -112,15 +115,12 @@ type DispatcherManager struct { dispatcherMap *DispatcherMap[*dispatcher.EventDispatcher] // redoDispatcherMap restore all the redo dispatchers in the DispatcherManager, including table trigger redo dispatcher redoDispatcherMap *DispatcherMap[*dispatcher.RedoDispatcher] - // currentOperatorMap stores at most one in-flight scheduling request per dispatcherID (event and redo). + // currentOperatorMap stores one in-flight scheduling request per dispatcherID. // - // It is used for: - // - suppressing duplicate maintainer requests for the same dispatcher, - // - reporting unfinished requests during bootstrap so a new maintainer can restore operators, - // - cleaning up remove requests when a dispatcher is fully removed. - // - // Entries must be deleted on completion (create -> after creation; remove -> on cleanup), otherwise - // future maintainer requests for the same dispatcherID will be ignored. + // The value carries sender and maintainer epoch so bootstrap recovery can + // return only current-epoch operators, and precheck can replace stale entries. + // Entries must be deleted on completion, otherwise future requests for the + // same dispatcherID will be ignored. currentOperatorMap sync.Map // map[common.DispatcherID]SchedulerDispatcherRequest (in dispatcher manager, not heartbeatpb) // schemaIDToDispatchers is shared in the DispatcherManager, // it store all the infos about schemaID->Dispatchers @@ -208,6 +208,7 @@ func NewDispatcherManager( tableTriggerRedoDispatcherID *heartbeatpb.DispatcherID, startTs uint64, maintainerID node.ID, + maintainerEpoch uint64, newChangefeed bool, registerInitializing func(*DispatcherManager) bool, ) (manager *DispatcherManager, err error) { @@ -255,8 +256,10 @@ func NewDispatcherManager( metricRedoCreateDispatcherDuration: metrics.CreateDispatcherDuration.WithLabelValues(changefeedID.Keyspace(), changefeedID.Name(), "redoDispatcher"), } - // Set the epoch and maintainerID of the event dispatcher manager - manager.meta.maintainerEpoch = cfConfig.Epoch + // Trust only the explicit request maintainer epoch for receiver fencing. The + // config epoch may be newer than an old rolling-upgrade request and must not + // turn epoch 0 compatibility traffic into strict-mode traffic. + manager.meta.maintainerEpoch = maintainerEpoch manager.meta.maintainerID = maintainerID cleanupManager := manager defer func() { @@ -451,7 +454,7 @@ func (e *DispatcherManager) NewTableTriggerEventDispatcher(id *heartbeatpb.Dispa infos := map[common.DispatcherID]dispatcherCreateInfo{} dispatcherID := common.NewDispatcherIDFromPB(id) infos[dispatcherID] = dispatcherCreateInfo{ - Id: dispatcherID, + ID: dispatcherID, TableSpan: common.KeyspaceDDLSpan(e.keyspaceID), StartTs: startTs, SchemaID: 0, diff --git a/downstreamadapter/dispatchermanager/dispatcher_manager_helper.go b/downstreamadapter/dispatchermanager/dispatcher_manager_helper.go index 64731249cd..31daf67bb8 100644 --- a/downstreamadapter/dispatchermanager/dispatcher_manager_helper.go +++ b/downstreamadapter/dispatchermanager/dispatcher_manager_helper.go @@ -82,7 +82,7 @@ func prepareCreateDispatcher[T dispatcher.Dispatcher](infos map[common.Dispatche schemaIds := make([]int64, 0, len(infos)) skipDMLAsStartTsList := make([]bool, 0, len(infos)) for _, info := range infos { - id := info.Id + id := info.ID if _, ok := dispatcherMap.Get(id); ok { continue } @@ -266,17 +266,7 @@ func removeDispatcher[T dispatcher.Dispatcher](e *DispatcherManager, } } - // Submit async remove task to thread pool - task := &RemoveDispatcherTask{ - manager: e, - dispatcherItem: dispatcherItem, - retryCount: 0, - } - scheduler := GetRemoveDispatcherTaskScheduler() - taskHandle := scheduler.Submit(task, time.Now()) - - // Save taskHandle for later cancellation - e.removeTaskHandles.Store(id, taskHandle) + e.submitRemoveDispatcherTask(dispatcherItem) dispatcherItem.SetTryRemoving() @@ -296,6 +286,17 @@ func removeDispatcher[T dispatcher.Dispatcher](e *DispatcherManager, } } +func (e *DispatcherManager) submitRemoveDispatcherTask(dispatcherItem dispatcher.Dispatcher) { + task := &RemoveDispatcherTask{ + manager: e, + dispatcherItem: dispatcherItem, + retryCount: 0, + } + scheduler := GetRemoveDispatcherTaskScheduler() + taskHandle := scheduler.Submit(task, time.Now()) + e.removeTaskHandles.Store(dispatcherItem.GetId(), taskHandle) +} + // closeAllDispatchers is called when the event dispatcher manager is closing func closeAllDispatchers[T dispatcher.Dispatcher](changefeedID common.ChangeFeedID, dispatcherMap *DispatcherMap[T], diff --git a/downstreamadapter/dispatchermanager/dispatcher_manager_info.go b/downstreamadapter/dispatchermanager/dispatcher_manager_info.go index 07a5f28910..1728b5fe30 100644 --- a/downstreamadapter/dispatchermanager/dispatcher_manager_info.go +++ b/downstreamadapter/dispatchermanager/dispatcher_manager_info.go @@ -22,10 +22,10 @@ import ( "github.com/pingcap/ticdc/pkg/node" ) -// event_dispatcher_mananger_info.go is used to store the basic info and function of the event dispatcher manager +// dispatcher_manager_info.go stores the basic info and functions of the dispatcher manager. type dispatcherCreateInfo struct { - Id common.DispatcherID + ID common.DispatcherID TableSpan *heartbeatpb.TableSpan StartTs uint64 SchemaID int64 @@ -52,10 +52,40 @@ func (e *DispatcherManager) GetMaintainerID() node.ID { return e.meta.maintainerID } -func (e *DispatcherManager) SetMaintainerID(maintainerID node.ID) { +// TryUpdateMaintainer records the active maintainer owner and epoch. +// Maintainer epoch 0 is accepted only while the manager is still in compatibility +// mode. Once a non-zero epoch is known, epoch 0 must never downgrade the receiver +// back to compatibility mode. +func (e *DispatcherManager) TryUpdateMaintainer(from node.ID, maintainerEpoch uint64) bool { e.meta.Lock() defer e.meta.Unlock() - e.meta.maintainerID = maintainerID + if maintainerEpoch == 0 { + if e.meta.maintainerEpoch != 0 { + return false + } + e.meta.maintainerID = from + return true + } + if e.meta.maintainerEpoch > maintainerEpoch { + return false + } + if e.meta.maintainerEpoch == maintainerEpoch && e.meta.maintainerID != "" && e.meta.maintainerID != from { + return false + } + e.meta.maintainerEpoch = maintainerEpoch + e.meta.maintainerID = from + return true +} + +// IsMaintainerRequestAllowed reports whether a request belongs to the current +// maintainer owner/epoch view known by this dispatcher manager. +func (e *DispatcherManager) IsMaintainerRequestAllowed(from node.ID, maintainerEpoch uint64) bool { + e.meta.Lock() + defer e.meta.Unlock() + if maintainerEpoch == 0 { + return e.meta.maintainerEpoch == 0 && (e.meta.maintainerID == "" || e.meta.maintainerID == from) + } + return e.meta.maintainerEpoch == maintainerEpoch && e.meta.maintainerID == from } func (e *DispatcherManager) GetMaintainerEpoch() uint64 { diff --git a/downstreamadapter/dispatchermanager/dispatcher_manager_redo.go b/downstreamadapter/dispatchermanager/dispatcher_manager_redo.go index 87d1ba15ad..7ae077afb2 100644 --- a/downstreamadapter/dispatchermanager/dispatcher_manager_redo.go +++ b/downstreamadapter/dispatchermanager/dispatcher_manager_redo.go @@ -125,7 +125,7 @@ func (e *DispatcherManager) NewTableTriggerRedoDispatcher(id *heartbeatpb.Dispat infos := map[common.DispatcherID]dispatcherCreateInfo{} dispatcherID := common.NewDispatcherIDFromPB(id) infos[dispatcherID] = dispatcherCreateInfo{ - Id: dispatcherID, + ID: dispatcherID, TableSpan: common.KeyspaceDDLSpan(e.keyspaceID), StartTs: startTs, SchemaID: 0, diff --git a/downstreamadapter/dispatchermanager/dispatcher_manager_test.go b/downstreamadapter/dispatchermanager/dispatcher_manager_test.go index a2bb97e7e9..2759dde39b 100644 --- a/downstreamadapter/dispatchermanager/dispatcher_manager_test.go +++ b/downstreamadapter/dispatchermanager/dispatcher_manager_test.go @@ -24,7 +24,6 @@ import ( "github.com/pingcap/ticdc/downstreamadapter/eventcollector" "github.com/pingcap/ticdc/downstreamadapter/sink" "github.com/pingcap/ticdc/downstreamadapter/sink/mock" - "github.com/pingcap/ticdc/downstreamadapter/sink/mysql" "github.com/pingcap/ticdc/heartbeatpb" "github.com/pingcap/ticdc/logservice/schemastore" "github.com/pingcap/ticdc/pkg/common" @@ -37,7 +36,6 @@ import ( "github.com/pingcap/ticdc/pkg/node" "github.com/pingcap/ticdc/pkg/pdutil" "github.com/pingcap/ticdc/pkg/routing" - mysqlcfg "github.com/pingcap/ticdc/pkg/sink/mysql" "github.com/pingcap/ticdc/pkg/util" "github.com/pingcap/ticdc/utils/threadpool" "github.com/stretchr/testify/require" @@ -500,20 +498,9 @@ func TestMergeDispatcherInvalidIDs(t *testing.T) { func TestTryCloseRemovedRequestAfterClosedReturnsImmediatelyAndTriggersCleanup(t *testing.T) { changefeedID := common.NewChangeFeedIDWithName("test", common.DefaultKeyspaceName) - mysqlConfig := mysqlcfg.New() - mysqlConfig.EnableDDLTs = false - mysqlSink := mysql.NewMySQLSink( - context.Background(), - changefeedID, - mysqlConfig, - nil, - false, - false, - time.Minute, - ) manager := &DispatcherManager{ changefeedID: changefeedID, - sink: mysqlSink, + sink: newDispatcherManagerTestSink(t, common.BlackHoleSinkType), } manager.closed.Store(true) @@ -664,6 +651,7 @@ func TestNewDispatcherManagerReturnsFenceErrorWhenInitializingRegistrationReject nil, 1, node.ID("maintainer"), + 1, true, func(manager *DispatcherManager) bool { hookCalled.Store(true) @@ -795,7 +783,7 @@ func TestCreateDispatcherByInfoKeepsCreateOperatorWhenFenced(t *testing.T) { manager := createTestManager(t) manager.writePathClosed.Store(true) dispatcherID := common.NewDispatcherID() - createReq := NewSchedulerDispatcherRequest(&heartbeatpb.ScheduleDispatcherRequest{ + createReq := NewSchedulerDispatcherRequest(node.ID("maintainer"), &heartbeatpb.ScheduleDispatcherRequest{ ChangefeedID: manager.changefeedID.ToPB(), Config: &heartbeatpb.DispatcherConfig{ DispatcherID: dispatcherID.ToPB(), @@ -812,7 +800,7 @@ func TestCreateDispatcherByInfoKeepsCreateOperatorWhenFenced(t *testing.T) { createDispatcherByInfo(manager, map[common.DispatcherID]dispatcherCreateInfo{ dispatcherID: { - Id: dispatcherID, + ID: dispatcherID, TableSpan: &heartbeatpb.TableSpan{ TableID: 1, }, diff --git a/downstreamadapter/dispatchermanager/heartbeat_collector.go b/downstreamadapter/dispatchermanager/heartbeat_collector.go index 67417face8..3b58a50645 100644 --- a/downstreamadapter/dispatchermanager/heartbeat_collector.go +++ b/downstreamadapter/dispatchermanager/heartbeat_collector.go @@ -263,12 +263,12 @@ func (c *HeartBeatCollector) RecvMessages(_ context.Context, msg *messaging.Targ heartbeatResponse := msg.Message[0].(*heartbeatpb.HeartBeatResponse) c.heartBeatResponseDynamicStream.Push( common.NewChangefeedGIDFromPB(heartbeatResponse.ChangefeedID), - NewHeartBeatResponse(heartbeatResponse)) + NewHeartBeatResponse(msg.From, heartbeatResponse)) case messaging.TypeScheduleDispatcherRequest: schedulerDispatcherRequest := msg.Message[0].(*heartbeatpb.ScheduleDispatcherRequest) c.schedulerDispatcherRequestDynamicStream.Push( common.NewChangefeedGIDFromPB(schedulerDispatcherRequest.ChangefeedID), - NewSchedulerDispatcherRequest(schedulerDispatcherRequest)) + NewSchedulerDispatcherRequest(msg.From, schedulerDispatcherRequest)) // TODO: check metrics metrics.HandleDispatcherRequsetCounter.WithLabelValues("default", schedulerDispatcherRequest.ChangefeedID.Name, "receive").Inc() case messaging.TypeCheckpointTsMessage: @@ -290,7 +290,7 @@ func (c *HeartBeatCollector) RecvMessages(_ context.Context, msg *messaging.Targ mergeDispatcherRequest := msg.Message[0].(*heartbeatpb.MergeDispatcherRequest) c.mergeDispatcherRequestDynamicStream.Push( common.NewChangefeedGIDFromPB(mergeDispatcherRequest.ChangefeedID), - NewMergeDispatcherRequest(mergeDispatcherRequest)) + NewMergeDispatcherRequest(msg.From, mergeDispatcherRequest)) default: log.Warn("unknown message type, ignore it", zap.String("type", msg.Type.String()), diff --git a/downstreamadapter/dispatchermanager/helper.go b/downstreamadapter/dispatchermanager/helper.go index 659a7a3eed..0f96cd7909 100644 --- a/downstreamadapter/dispatchermanager/helper.go +++ b/downstreamadapter/dispatchermanager/helper.go @@ -25,6 +25,7 @@ import ( "github.com/pingcap/ticdc/heartbeatpb" "github.com/pingcap/ticdc/pkg/common" "github.com/pingcap/ticdc/pkg/config" + "github.com/pingcap/ticdc/pkg/node" "github.com/pingcap/ticdc/pkg/util" "github.com/pingcap/ticdc/utils/dynstream" "go.uber.org/zap" @@ -41,22 +42,22 @@ type DispatcherMap[T dispatcher.Dispatcher] struct { // When some new dispatcher(table) is being added, the maintainer will block the forward of changefeed's checkpointTs // until the maintainer receive the message that the new dispatcher's component status change to working. // - // Besides, there is no strict order of the heartbeat message and the table status messages, which is means - // it can happen that when dispatcher A is created, event dispatcher manager may first send a table status message - // to show the new dispatcher is working, and then send a heartbeat message of the current watermark, - // which is calculated without the new disaptcher. - // When the checkpointTs of the watermark is large than the startTs of the new dispatcher, - // the watermark of next heartbeat, which calculated with the new dispatcher can be less than the previous watermark. - // Then it can cause the fallback of changefeed's checkpointTs. - // To avoid fallback, we add a seq number in each heartbeat message(both collect from collectComponentStatusWhenChanged and aggregateDispatcherHeartbeats) - // When a table is added the seq number will be increase, - // and when the maintainer receive the outdate seq, it will know the heartbeat message is outdate and ignore it. + // Besides, heartbeat messages and table status messages have no strict order. + // After dispatcher A is created, the event dispatcher manager may first send + // a table status message showing the new dispatcher is working, and then send + // a heartbeat with the current watermark calculated without the new dispatcher. + // If that watermark checkpointTs is larger than the new dispatcher's startTs, + // the next heartbeat calculated with the new dispatcher can be lower and cause + // the changefeed checkpointTs to fall back. + // To avoid fallback, each heartbeat carries a seq number collected from + // collectComponentStatusWhenChanged and aggregateDispatcherHeartbeats. When a + // table is added, the seq number increases, so the maintainer can ignore + // outdated heartbeat messages. // In this way, even the above case happens, the changefeed's checkpointTs will not fallback. // - // Here we don't need to make seq changes always atmoic with the m changed. - // Our target is just : - // The seq get from ForEach should be smaller than the seq get from Set - // when ForEach is not access the new dispatcher just Set. + // Here we don't need to make seq changes always atomic with the map changes. + // Our target is only that the seq from ForEach is smaller than the seq from + // Set when ForEach does not access the newly added dispatcher. // So we add seq after the dispatcher is add in the m for Set, and get the seq before do range for ForRange. seq atomic.Uint64 } @@ -179,11 +180,14 @@ func newSchedulerDispatcherRequestDynamicStream() dynstream.DynamicStream[int, c } type SchedulerDispatcherRequest struct { + From node.ID *heartbeatpb.ScheduleDispatcherRequest } -func NewSchedulerDispatcherRequest(req *heartbeatpb.ScheduleDispatcherRequest) SchedulerDispatcherRequest { - return SchedulerDispatcherRequest{req} +// NewSchedulerDispatcherRequest carries the sender node with the schedule +// request so dispatcher-manager admission can fence stale maintainers. +func NewSchedulerDispatcherRequest(from node.ID, req *heartbeatpb.ScheduleDispatcherRequest) SchedulerDispatcherRequest { + return SchedulerDispatcherRequest{From: from, ScheduleDispatcherRequest: req} } type SchedulerDispatcherRequestHandler struct{} @@ -205,10 +209,8 @@ func (h *SchedulerDispatcherRequestHandler) Path(scheduleDispatcherRequest Sched // Some requests are intentionally dropped (see preCheckForSchedulerHandler / handleScheduleRemove) to avoid // leaking operator entries in cases where we have no cleanup callback (e.g. remove a non-existent dispatcher). func (h *SchedulerDispatcherRequestHandler) Handle(dispatcherManager *DispatcherManager, reqs ...SchedulerDispatcherRequest) bool { - if len(reqs) == 0 { - // dynstream guarantees len(events)>0, but guard defensively to avoid panics if that contract changes. - return false - } + dispatcherManager.MaintainerFenceMu.Lock() + defer dispatcherManager.MaintainerFenceMu.Unlock() // `dynstream` guarantees per-path serialization: for a given changefeed (Path), // SchedulerDispatcherRequestHandler.Handle will not be executed concurrently. This matters for reasoning: @@ -220,14 +222,14 @@ func (h *SchedulerDispatcherRequestHandler) Handle(dispatcherManager *Dispatcher infos := map[common.DispatcherID]dispatcherCreateInfo{} redoInfos := map[common.DispatcherID]dispatcherCreateInfo{} for _, req := range reqs { - operatorKey, ok := preCheckForSchedulerHandler(req, dispatcherManager) + dispatcherID, ok := preCheckForSchedulerHandler(req, dispatcherManager) if !ok { continue } switch req.ScheduleAction { case heartbeatpb.ScheduleAction_Create: // Store the add operator and create an info for later create dispatcher. - handleScheduleCreate(dispatcherManager, req, operatorKey, infos, redoInfos) + handleScheduleCreate(dispatcherManager, req, dispatcherID, infos, redoInfos) case heartbeatpb.ScheduleAction_Remove: // Remove is non-batchable (see GetType), so reqs should contain exactly one request. if len(reqs) != 1 { @@ -235,7 +237,7 @@ func (h *SchedulerDispatcherRequestHandler) Handle(dispatcherManager *Dispatcher } // Store the remove operator (when applicable) and remove the dispatcher directly. // The remove operator will be deleted after the dispatcher is removed from dispatcherMap. - handleScheduleRemove(dispatcherManager, req, operatorKey) + handleScheduleRemove(dispatcherManager, req, dispatcherID) default: log.Panic("unknown schedule action", zap.Int("action", int(req.ScheduleAction))) } @@ -252,11 +254,10 @@ func (h *SchedulerDispatcherRequestHandler) Handle(dispatcherManager *Dispatcher // preCheckForSchedulerHandler validates a scheduling request and decides whether it should be applied. // -// It returns the stable key used in currentOperatorMap (dispatcherID), and a boolean indicating whether the -// request should proceed. The precheck filters out: +// It returns the dispatcherID used as currentOperatorMap key. The precheck filters out: // - invalid requests (nil request/config/dispatcherID), // - redo requests when redo is disabled, -// - duplicate Create requests for the same dispatcherID while another operator is in-flight, +// - stale maintainer requests and duplicate Create requests, // - Create requests for an already-existing dispatcher (idempotent no-op). // // Note: Remove requests are allowed to proceed even if the dispatcher doesn't exist (we still want to emit a @@ -271,32 +272,46 @@ func preCheckForSchedulerHandler(req SchedulerDispatcherRequest, dispatcherManag log.Warn("scheduleDispatcherRequest config is nil, skip") return common.DispatcherID{}, false } - operatorKey := common.NewDispatcherIDFromPB(req.Config.DispatcherID) - if operatorKey.IsZero() { + dispatcherID := common.NewDispatcherIDFromPB(req.Config.DispatcherID) + if dispatcherID.IsZero() { log.Warn("scheduleDispatcherRequest has no valid operator key, skip") return common.DispatcherID{}, false } - + if !dispatcherManager.IsMaintainerRequestAllowed(req.From, req.MaintainerEpoch) { + log.Warn("drop stale schedule dispatcher request", + zap.String("changefeedID", req.ChangefeedID.String()), + zap.String("dispatcherID", dispatcherID.String()), + zap.String("from", req.From.String()), + zap.Uint64("requestMaintainerEpoch", req.MaintainerEpoch), + zap.Uint64("currentMaintainerEpoch", dispatcherManager.GetMaintainerEpoch()), + zap.String("currentMaintainer", dispatcherManager.GetMaintainerID().String())) + return common.DispatcherID{}, false + } isRedo := common.IsRedoMode(req.Config.Mode) if isRedo && !dispatcherManager.IsRedoReady() { return common.DispatcherID{}, false } - if _, operatorExists := dispatcherManager.currentOperatorMap.Load(operatorKey); operatorExists { - // Create requests must be serialized per dispatcherID; otherwise we can end up creating multiple - // dispatchers for the same span/dispatcherID. - if req.ScheduleAction == heartbeatpb.ScheduleAction_Create { - return common.DispatcherID{}, false + if existing, operatorExists := dispatcherManager.currentOperatorMap.Load(dispatcherID); operatorExists { + existingReq := existing.(SchedulerDispatcherRequest) + if !dispatcherManager.IsMaintainerRequestAllowed(existingReq.From, existingReq.MaintainerEpoch) { + dispatcherManager.currentOperatorMap.Delete(dispatcherID) + } else { + // Create requests must be serialized per dispatcherID; otherwise we can end up creating multiple + // dispatchers for the same span/dispatcherID. + if req.ScheduleAction == heartbeatpb.ScheduleAction_Create { + return common.DispatcherID{}, false + } + // Remove requests are allowed to proceed: removeDispatcher is idempotent and the incoming request + // may carry a newer OperatorType for maintainer bootstrap/failover reconstruction. } - // Remove requests are allowed to proceed: removeDispatcher is idempotent and the incoming request - // may carry a newer OperatorType for maintainer bootstrap/failover reconstruction. } // Check whether the dispatcher exists locally. This is used to treat Create as idempotent. var dispatcherExists bool if isRedo { - _, dispatcherExists = dispatcherManager.redoDispatcherMap.Get(operatorKey) + _, dispatcherExists = dispatcherManager.redoDispatcherMap.Get(dispatcherID) } else { - _, dispatcherExists = dispatcherManager.dispatcherMap.Get(operatorKey) + _, dispatcherExists = dispatcherManager.dispatcherMap.Get(dispatcherID) } // Action-aware precheck: @@ -310,27 +325,26 @@ func preCheckForSchedulerHandler(req SchedulerDispatcherRequest, dispatcherManag case heartbeatpb.ScheduleAction_Remove: } - return operatorKey, true + return dispatcherID, true } func handleScheduleCreate( dispatcherManager *DispatcherManager, req SchedulerDispatcherRequest, - operatorKey common.DispatcherID, + dispatcherID common.DispatcherID, infos map[common.DispatcherID]dispatcherCreateInfo, redoInfos map[common.DispatcherID]dispatcherCreateInfo, ) { config := req.Config - dispatcherID := common.NewDispatcherIDFromPB(config.DispatcherID) info := dispatcherCreateInfo{ - Id: dispatcherID, + ID: dispatcherID, TableSpan: config.Span, StartTs: config.StartTs, SchemaID: config.SchemaID, SkipDMLAsStartTs: config.SkipDMLAsStartTs, } if common.IsRedoMode(config.Mode) { - dispatcherManager.currentOperatorMap.Store(operatorKey, req) + dispatcherManager.currentOperatorMap.Store(dispatcherID, req) log.Debug("store current working add operator for redo dispatcher", zap.String("changefeedID", req.ChangefeedID.String()), zap.String("dispatcherID", dispatcherID.String()), @@ -338,7 +352,7 @@ func handleScheduleCreate( ) redoInfos[dispatcherID] = info } else { - dispatcherManager.currentOperatorMap.Store(operatorKey, req) + dispatcherManager.currentOperatorMap.Store(dispatcherID, req) log.Debug("store current working add operator", zap.String("changefeedID", req.ChangefeedID.String()), zap.String("dispatcherID", dispatcherID.String()), @@ -351,10 +365,9 @@ func handleScheduleCreate( func handleScheduleRemove( dispatcherManager *DispatcherManager, req SchedulerDispatcherRequest, - operatorKey common.DispatcherID, + dispatcherID common.DispatcherID, ) { config := req.Config - dispatcherID := common.NewDispatcherIDFromPB(config.DispatcherID) if common.IsRedoMode(config.Mode) { // If redo is disabled or the dispatcher does not exist, do not store the remove operator. // Otherwise, the operator may never be cleaned up because cleanRedoDispatcher won't be called. @@ -362,7 +375,7 @@ func handleScheduleRemove( return } if _, exists := dispatcherManager.redoDispatcherMap.Get(dispatcherID); exists { - dispatcherManager.currentOperatorMap.Store(operatorKey, req) + dispatcherManager.currentOperatorMap.Store(dispatcherID, req) log.Debug("store current working remove operator for redo dispatcher", zap.String("changefeedID", req.ChangefeedID.String()), zap.String("dispatcherID", dispatcherID.String()), @@ -382,7 +395,7 @@ func handleScheduleRemove( // If the dispatcher does not exist, do not store the remove operator. // Otherwise, the operator may never be cleaned up because cleanEventDispatcher won't be called. if _, exists := dispatcherManager.dispatcherMap.Get(dispatcherID); exists { - dispatcherManager.currentOperatorMap.Store(operatorKey, req) + dispatcherManager.currentOperatorMap.Store(dispatcherID, req) log.Debug("store current working remove operator", zap.String("changefeedID", req.ChangefeedID.String()), zap.String("dispatcherID", dispatcherID.String()), @@ -445,21 +458,21 @@ func deleteCreatedOperators[T dispatcher.Dispatcher]( dispatcherKind string, ) { for _, info := range infos { - if _, exists := dispatcherMap.Get(info.Id); !exists { + if _, exists := dispatcherMap.Get(info.ID); !exists { continue } // Create requests are stored in currentOperatorMap before creation and // should be deleted only after the dispatcher is actually created. - if v, ok := dispatcherManager.currentOperatorMap.Load(info.Id); ok { + if v, ok := dispatcherManager.currentOperatorMap.Load(info.ID); ok { req := v.(SchedulerDispatcherRequest) if req.ScheduleAction == heartbeatpb.ScheduleAction_Create { log.Debug("delete current working add operator", zap.String("changefeedID", dispatcherManager.changefeedID.String()), - zap.String("dispatcherID", info.Id.String()), + zap.String("dispatcherID", info.ID.String()), zap.String("dispatcherKind", dispatcherKind), zap.Any("operator", req), ) - dispatcherManager.currentOperatorMap.Delete(info.Id) + dispatcherManager.currentOperatorMap.Delete(info.ID) } } } @@ -507,11 +520,14 @@ func newHeartBeatResponseDynamicStream(dds dynstream.DynamicStream[common.GID, c } type HeartBeatResponse struct { + From node.ID *heartbeatpb.HeartBeatResponse } -func NewHeartBeatResponse(resp *heartbeatpb.HeartBeatResponse) HeartBeatResponse { - return HeartBeatResponse{resp} +// NewHeartBeatResponse carries the sender node with the heartbeat so stale +// maintainer responses cannot update dispatcher state. +func NewHeartBeatResponse(from node.ID, resp *heartbeatpb.HeartBeatResponse) HeartBeatResponse { + return HeartBeatResponse{From: from, HeartBeatResponse: resp} } type HeartBeatResponseHandler struct { @@ -532,6 +548,11 @@ func (h *HeartBeatResponseHandler) Handle(dispatcherManager *DispatcherManager, panic("invalid response count") } heartbeatResponse := resps[0] + dispatcherManager.MaintainerFenceMu.Lock() + defer dispatcherManager.MaintainerFenceMu.Unlock() + if !isHeartBeatResponseAllowed(dispatcherManager, heartbeatResponse) { + return false + } dispatcherStatuses := heartbeatResponse.GetDispatcherStatuses() for _, dispatcherStatus := range dispatcherStatuses { influencedDispatchersType := dispatcherStatus.InfluencedDispatchers.InfluenceType @@ -569,6 +590,21 @@ func (h *HeartBeatResponseHandler) Handle(dispatcherManager *DispatcherManager, return false } +// isHeartBeatResponseAllowed drops dispatcher heartbeats from stale maintainers +// before they can update table state or complete scheduler operators. +func isHeartBeatResponseAllowed(dispatcherManager *DispatcherManager, heartbeatResponse HeartBeatResponse) bool { + if dispatcherManager.IsMaintainerRequestAllowed(heartbeatResponse.From, heartbeatResponse.MaintainerEpoch) { + return true + } + log.Warn("drop stale heartbeat response", + zap.String("changefeedID", heartbeatResponse.ChangefeedID.String()), + zap.String("from", heartbeatResponse.From.String()), + zap.Uint64("responseMaintainerEpoch", heartbeatResponse.MaintainerEpoch), + zap.Uint64("currentMaintainerEpoch", dispatcherManager.GetMaintainerEpoch()), + zap.String("currentMaintainer", dispatcherManager.GetMaintainerID().String())) + return false +} + func (h *HeartBeatResponseHandler) GetSize(event HeartBeatResponse) int { return 0 } func (h *HeartBeatResponseHandler) IsPaused(event HeartBeatResponse) bool { return false } func (h *HeartBeatResponseHandler) GetArea(_ common.GID, _ *DispatcherManager) int { @@ -784,11 +820,14 @@ func newMergeDispatcherRequestDynamicStream() dynstream.DynamicStream[int, commo } type MergeDispatcherRequest struct { + From node.ID *heartbeatpb.MergeDispatcherRequest } -func NewMergeDispatcherRequest(req *heartbeatpb.MergeDispatcherRequest) MergeDispatcherRequest { - return MergeDispatcherRequest{req} +// NewMergeDispatcherRequest carries the sender node together with the request +// so the handler can apply the dispatcher-manager maintainer fence. +func NewMergeDispatcherRequest(from node.ID, req *heartbeatpb.MergeDispatcherRequest) MergeDispatcherRequest { + return MergeDispatcherRequest{From: from, MergeDispatcherRequest: req} } type MergeDispatcherRequestHandler struct{} @@ -803,6 +842,17 @@ func (h *MergeDispatcherRequestHandler) Handle(dispatcherManager *DispatcherMana } mergeDispatcherRequest := reqs[0] + dispatcherManager.MaintainerFenceMu.Lock() + defer dispatcherManager.MaintainerFenceMu.Unlock() + if !dispatcherManager.IsMaintainerRequestAllowed(mergeDispatcherRequest.From, mergeDispatcherRequest.MaintainerEpoch) { + log.Warn("drop stale merge dispatcher request", + zap.String("changefeedID", mergeDispatcherRequest.ChangefeedID.String()), + zap.String("from", mergeDispatcherRequest.From.String()), + zap.Uint64("requestMaintainerEpoch", mergeDispatcherRequest.MaintainerEpoch), + zap.Uint64("currentMaintainerEpoch", dispatcherManager.GetMaintainerEpoch()), + zap.String("currentMaintainer", dispatcherManager.GetMaintainerID().String())) + return false + } dispatcherIDs := make([]common.DispatcherID, 0, len(mergeDispatcherRequest.DispatcherIDs)) for _, id := range mergeDispatcherRequest.DispatcherIDs { dispatcherIDs = append(dispatcherIDs, common.NewDispatcherIDFromPB(id)) diff --git a/downstreamadapter/dispatchermanager/helper_test.go b/downstreamadapter/dispatchermanager/helper_test.go index 15cb2ea0a6..e23f1f81a9 100644 --- a/downstreamadapter/dispatchermanager/helper_test.go +++ b/downstreamadapter/dispatchermanager/helper_test.go @@ -24,6 +24,7 @@ import ( "github.com/pingcap/ticdc/downstreamadapter/sink/redo" "github.com/pingcap/ticdc/heartbeatpb" "github.com/pingcap/ticdc/pkg/common" + "github.com/pingcap/ticdc/pkg/node" "github.com/stretchr/testify/require" ) @@ -160,7 +161,7 @@ func TestPreCheckForSchedulerHandler_RemoveAllowedWhenDispatcherMissing(t *testi dispatcherMap: newDispatcherMap[*dispatcher.EventDispatcher](), } - removeReq := NewSchedulerDispatcherRequest(&heartbeatpb.ScheduleDispatcherRequest{ + removeReq := NewSchedulerDispatcherRequest("node1", &heartbeatpb.ScheduleDispatcherRequest{ ChangefeedID: &heartbeatpb.ChangefeedID{Keyspace: "test-namespace", Name: "test-changefeed"}, Config: &heartbeatpb.DispatcherConfig{ DispatcherID: dispatcherID.ToPB(), @@ -190,7 +191,7 @@ func TestPreCheckForSchedulerHandler_CreateSkippedWhenDispatcherExists(t *testin } dm.dispatcherMap.Set(dispatcherID, &dispatcher.EventDispatcher{}) - createReq := NewSchedulerDispatcherRequest(&heartbeatpb.ScheduleDispatcherRequest{ + createReq := NewSchedulerDispatcherRequest("node1", &heartbeatpb.ScheduleDispatcherRequest{ ChangefeedID: &heartbeatpb.ChangefeedID{Keyspace: "test-namespace", Name: "test-changefeed"}, Config: &heartbeatpb.DispatcherConfig{ DispatcherID: dispatcherID.ToPB(), @@ -204,6 +205,162 @@ func TestPreCheckForSchedulerHandler_CreateSkippedWhenDispatcherExists(t *testin require.False(t, ok) } +func TestPreCheckForSchedulerHandler_MaintainerEpochFence(t *testing.T) { + t.Parallel() + + dispatcherID := common.NewDispatcherID() + currentDM := &DispatcherManager{ + changefeedID: common.NewChangeFeedIDWithName("test-changefeed", "test-namespace"), + dispatcherMap: newDispatcherMap[*dispatcher.EventDispatcher](), + } + currentDM.meta.maintainerID = "current-maintainer" + currentDM.meta.maintainerEpoch = 2 + + newReq := func(epoch uint64) *heartbeatpb.ScheduleDispatcherRequest { + return &heartbeatpb.ScheduleDispatcherRequest{ + ChangefeedID: &heartbeatpb.ChangefeedID{Keyspace: "test-namespace", Name: "test-changefeed"}, + Config: &heartbeatpb.DispatcherConfig{ + DispatcherID: dispatcherID.ToPB(), + Mode: 0, + }, + ScheduleAction: heartbeatpb.ScheduleAction_Create, + OperatorType: heartbeatpb.OperatorType_O_Add, + MaintainerEpoch: epoch, + } + } + + _, ok := preCheckForSchedulerHandler(NewSchedulerDispatcherRequest("old-maintainer", newReq(1)), currentDM) + require.False(t, ok) + + operatorKey, ok := preCheckForSchedulerHandler(NewSchedulerDispatcherRequest("current-maintainer", newReq(2)), currentDM) + require.True(t, ok) + require.Equal(t, dispatcherID, operatorKey) + + _, ok = preCheckForSchedulerHandler(NewSchedulerDispatcherRequest("current-maintainer", newReq(0)), currentDM) + require.False(t, ok) + + compatDM := &DispatcherManager{ + changefeedID: currentDM.changefeedID, + dispatcherMap: newDispatcherMap[*dispatcher.EventDispatcher](), + } + compatDM.meta.maintainerID = "current-maintainer" + operatorKey, ok = preCheckForSchedulerHandler(NewSchedulerDispatcherRequest("current-maintainer", newReq(0)), compatDM) + require.True(t, ok) + require.Equal(t, dispatcherID, operatorKey) + + currentDM.currentOperatorMap.Store(dispatcherID, NewSchedulerDispatcherRequest("old-maintainer", newReq(1))) + operatorKey, ok = preCheckForSchedulerHandler(NewSchedulerDispatcherRequest("current-maintainer", newReq(2)), currentDM) + require.True(t, ok) + require.Equal(t, dispatcherID, operatorKey) + _, exists := currentDM.currentOperatorMap.Load(dispatcherID) + require.False(t, exists) +} + +func TestDispatcherManagerTryUpdateMaintainerEpoch(t *testing.T) { + t.Parallel() + + strictDM := &DispatcherManager{} + strictDM.meta.maintainerID = "current-maintainer" + strictDM.meta.maintainerEpoch = 2 + + require.False(t, strictDM.TryUpdateMaintainer("current-maintainer", 0)) + require.Equal(t, node.ID("current-maintainer"), strictDM.GetMaintainerID()) + require.Equal(t, uint64(2), strictDM.GetMaintainerEpoch()) + + compatDM := &DispatcherManager{} + compatDM.meta.maintainerID = "old-maintainer" + + require.True(t, compatDM.TryUpdateMaintainer("new-maintainer", 0)) + require.Equal(t, node.ID("new-maintainer"), compatDM.GetMaintainerID()) + require.Zero(t, compatDM.GetMaintainerEpoch()) +} + +func TestHeartBeatResponseAllowedByMaintainerEpoch(t *testing.T) { + t.Parallel() + + changefeedID := common.NewChangeFeedIDWithName("test-changefeed", "test-namespace") + strictDM := &DispatcherManager{ + changefeedID: changefeedID, + } + strictDM.meta.maintainerID = "current-maintainer" + strictDM.meta.maintainerEpoch = 2 + + require.False(t, isHeartBeatResponseAllowed(strictDM, NewHeartBeatResponse( + node.ID("old-maintainer"), + &heartbeatpb.HeartBeatResponse{ + ChangefeedID: changefeedID.ToPB(), + MaintainerEpoch: 1, + DispatcherStatuses: []*heartbeatpb.DispatcherStatus{ + { + InfluencedDispatchers: &heartbeatpb.InfluencedDispatchers{ + InfluenceType: heartbeatpb.InfluenceType_All, + }, + Action: &heartbeatpb.DispatcherAction{Action: heartbeatpb.Action_Pass}, + }, + }, + }, + ))) + require.False(t, isHeartBeatResponseAllowed(strictDM, NewHeartBeatResponse( + node.ID("current-maintainer"), + &heartbeatpb.HeartBeatResponse{ + ChangefeedID: changefeedID.ToPB(), + MaintainerEpoch: 0, + }, + ))) + require.True(t, isHeartBeatResponseAllowed(strictDM, NewHeartBeatResponse( + node.ID("current-maintainer"), + &heartbeatpb.HeartBeatResponse{ + ChangefeedID: changefeedID.ToPB(), + MaintainerEpoch: 2, + }, + ))) + + compatDM := &DispatcherManager{ + changefeedID: changefeedID, + } + compatDM.meta.maintainerID = "compat-maintainer" + require.True(t, isHeartBeatResponseAllowed(compatDM, NewHeartBeatResponse( + node.ID("compat-maintainer"), + &heartbeatpb.HeartBeatResponse{ + ChangefeedID: changefeedID.ToPB(), + MaintainerEpoch: 0, + }, + ))) +} + +func TestHeartBeatResponseHandlerDropsStaleMaintainerEpoch(t *testing.T) { + t.Parallel() + + changefeedID := common.NewChangeFeedIDWithName("test-changefeed", "test-namespace") + dispatcherID := common.NewDispatcherID() + dispatcherManager := &DispatcherManager{ + changefeedID: changefeedID, + } + dispatcherManager.meta.maintainerID = "current-maintainer" + dispatcherManager.meta.maintainerEpoch = 2 + + handler := &HeartBeatResponseHandler{} + staleResponse := NewHeartBeatResponse( + node.ID("old-maintainer"), + &heartbeatpb.HeartBeatResponse{ + ChangefeedID: changefeedID.ToPB(), + MaintainerEpoch: 1, + DispatcherStatuses: []*heartbeatpb.DispatcherStatus{ + { + InfluencedDispatchers: &heartbeatpb.InfluencedDispatchers{ + InfluenceType: heartbeatpb.InfluenceType_Normal, + DispatcherIDs: []*heartbeatpb.DispatcherID{dispatcherID.ToPB()}, + }, + Action: &heartbeatpb.DispatcherAction{Action: heartbeatpb.Action_Pass}, + }, + }, + }) + + require.NotPanics(t, func() { + require.False(t, handler.Handle(dispatcherManager, staleResponse)) + }) +} + func TestDispatcherManagerIsRedoReadyRequiresPublication(t *testing.T) { t.Parallel() diff --git a/downstreamadapter/dispatcherorchestrator/dispatcher_orchestrator.go b/downstreamadapter/dispatcherorchestrator/dispatcher_orchestrator.go index 04c41524ad..bd5afde268 100644 --- a/downstreamadapter/dispatcherorchestrator/dispatcher_orchestrator.go +++ b/downstreamadapter/dispatcherorchestrator/dispatcher_orchestrator.go @@ -20,6 +20,7 @@ import ( "sync/atomic" "time" + "github.com/gogo/protobuf/proto" "github.com/pingcap/log" "github.com/pingcap/ticdc/downstreamadapter/dispatcher" "github.com/pingcap/ticdc/downstreamadapter/dispatchermanager" @@ -39,11 +40,15 @@ import ( // for different change feeds based on maintainer bootstrap messages. type DispatcherOrchestrator struct { mc messaging.MessageCenter - mutex sync.Mutex // protect dispatcherManagers + mutex sync.Mutex // protect dispatcherManagers and closedMaintainerEpochs dispatcherManagers map[common.ChangeFeedID]*dispatchermanager.DispatcherManager // initializingDispatcherManagers tracks managers that have been allocated // but are not yet visible in dispatcherManagers. initializingDispatcherManagers map[common.ChangeFeedID]*dispatchermanager.DispatcherManager + // closedMaintainerEpochs remembers the highest epoch that closed a manager. + // Map presence is meaningful because epoch 0 is a valid compatibility epoch. + // The tombstone prevents a delayed old bootstrap from recreating the manager after close. + closedMaintainerEpochs map[common.ChangeFeedID]uint64 // shards partition changefeed control messages by changefeed ID. Each shard keeps // the existing FIFO queue semantics, while different shards can process messages @@ -71,6 +76,7 @@ func New() *DispatcherOrchestrator { mc: appcontext.GetService[messaging.MessageCenter](appcontext.MessageCenter), dispatcherManagers: make(map[common.ChangeFeedID]*dispatchermanager.DispatcherManager), initializingDispatcherManagers: make(map[common.ChangeFeedID]*dispatchermanager.DispatcherManager), + closedMaintainerEpochs: make(map[common.ChangeFeedID]uint64), shards: make([]*orchestratorShard, dispatcherOrchestratorShardCount), } for i := range m.shards { @@ -181,22 +187,31 @@ func (m *DispatcherOrchestrator) handleBootstrapRequest( return nil } cfId := common.NewChangefeedIDFromPB(req.ChangefeedID) - - cfConfig := &config.ChangefeedConfig{} - if err := json.Unmarshal(req.Config, cfConfig); err != nil { - log.Panic("failed to unmarshal changefeed config", - zap.String("changefeedID", cfId.Name()), zap.Any("data", req.Config), zap.Error(err)) - } + maintainerEpoch := req.MaintainerEpoch // Keep the map lock scoped to dispatcherManagers lookups and updates only. // NewDispatcherManager may perform expensive downstream initialization, so it // must run outside the mutex to let unrelated shards progress concurrently. m.mutex.Lock() manager, exists := m.dispatcherManagers[cfId] + closedEpoch, closed := m.closedMaintainerEpochs[cfId] m.mutex.Unlock() var err error if !exists { + if closed && (maintainerEpoch == 0 || maintainerEpoch <= closedEpoch) { + log.Warn("drop stale maintainer bootstrap request after close", + zap.String("changefeed", cfId.Name()), + zap.String("from", from.String()), + zap.Uint64("requestMaintainerEpoch", maintainerEpoch), + zap.Uint64("closedMaintainerEpoch", closedEpoch)) + return nil + } + cfConfig := &config.ChangefeedConfig{} + if err := json.Unmarshal(req.Config, cfConfig); err != nil { + log.Panic("failed to unmarshal changefeed config", + zap.String("changefeedID", cfId.Name()), zap.Any("data", req.Config), zap.Error(err)) + } start := time.Now() var initializingManager *dispatchermanager.DispatcherManager manager, err = dispatchermanager.NewDispatcherManager( @@ -207,6 +222,7 @@ func (m *DispatcherOrchestrator) handleBootstrapRequest( req.TableTriggerRedoDispatcherId, req.StartTs, from, + maintainerEpoch, req.IsNewChangefeed, func(manager *dispatchermanager.DispatcherManager) bool { initializingManager = manager @@ -229,7 +245,8 @@ func (m *DispatcherOrchestrator) handleBootstrapRequest( appcontext.GetService[*dispatchermanager.HeartBeatCollector](appcontext.HeartbeatCollector).RemoveDispatcherManager(cfId) response := &heartbeatpb.MaintainerBootstrapResponse{ - ChangefeedID: req.ChangefeedID, + ChangefeedID: req.ChangefeedID, + MaintainerEpoch: maintainerEpoch, Err: &heartbeatpb.RunningError{ Time: time.Now().String(), Node: from.String(), @@ -249,12 +266,23 @@ func (m *DispatcherOrchestrator) handleBootstrapRequest( return nil } m.dispatcherManagers[cfId] = manager + delete(m.closedMaintainerEpochs, cfId) m.mutex.Unlock() metrics.DispatcherManagerGauge.WithLabelValues(cfId.Keyspace(), cfId.Name()).Inc() - } else { - if m.fenced.Load() { - return nil - } + } + + manager.MaintainerFenceMu.Lock() + if !manager.TryUpdateMaintainer(from, maintainerEpoch) { + log.Warn("drop stale maintainer bootstrap request", + zap.String("changefeed", cfId.Name()), + zap.String("from", from.String()), + zap.Uint64("requestMaintainerEpoch", maintainerEpoch), + zap.Uint64("currentMaintainerEpoch", manager.GetMaintainerEpoch()), + zap.String("currentMaintainer", manager.GetMaintainerID().String())) + manager.MaintainerFenceMu.Unlock() + return nil + } + if exists { // Check and potentially add a table trigger event dispatcher. // This is necessary during maintainer node migration, as the existing // dispatcher manager on the new node may not have a table trigger @@ -275,7 +303,8 @@ func (m *DispatcherOrchestrator) handleBootstrapRequest( } log.Error("failed to create new table trigger event dispatcher", zap.Stringer("changefeedID", cfId), zap.Error(err)) - return m.handleDispatcherError(from, req.ChangefeedID, err) + manager.MaintainerFenceMu.Unlock() + return m.handleDispatcherError(from, req.ChangefeedID, maintainerEpoch, err) } } } @@ -295,26 +324,15 @@ func (m *DispatcherOrchestrator) handleBootstrapRequest( } log.Error("failed to create new table trigger redo dispatcher", zap.Stringer("changefeedID", cfId), zap.Error(err)) - return m.handleDispatcherError(from, req.ChangefeedID, err) + manager.MaintainerFenceMu.Unlock() + return m.handleDispatcherError(from, req.ChangefeedID, maintainerEpoch, err) } } } } - if manager.GetMaintainerID() != from { - manager.SetMaintainerID(from) - log.Info("maintainer changed", - zap.String("changefeed", cfId.Name()), zap.String("maintainer", from.String())) - } - - // FIXME(fizz): This is a temporary check to ensure the maintainer epoch is consistent. - // I will remove this after fully testing the new maintainer epoch mechanism. - if manager.GetMaintainerEpoch() != cfConfig.Epoch { - log.Error("maintainer epoch changed, this should not happen, please report this issue", - zap.String("changefeed", cfId.Name()), zap.Uint64("epoch", cfConfig.Epoch)) - } - if m.fenced.Load() { + manager.MaintainerFenceMu.Unlock() manager.LocalFence() return nil } @@ -332,6 +350,7 @@ func (m *DispatcherOrchestrator) handleBootstrapRequest( } } response := createBootstrapResponse(req.ChangefeedID, manager, startTs, redoStartTs) + manager.MaintainerFenceMu.Unlock() return m.sendResponse(from, messaging.MaintainerManagerTopic, response) } @@ -357,6 +376,17 @@ func (m *DispatcherOrchestrator) handlePostBootstrapRequest( zap.Any("changefeedID", cfId.Name())) return nil } + manager.MaintainerFenceMu.Lock() + if !manager.IsMaintainerRequestAllowed(from, req.MaintainerEpoch) { + log.Warn("drop stale maintainer post bootstrap request", + zap.String("changefeed", cfId.Name()), + zap.String("from", from.String()), + zap.Uint64("requestMaintainerEpoch", req.MaintainerEpoch), + zap.Uint64("currentMaintainerEpoch", manager.GetMaintainerEpoch()), + zap.String("currentMaintainer", manager.GetMaintainerID().String())) + manager.MaintainerFenceMu.Unlock() + return nil + } if manager.GetTableTriggerEventDispatcher().GetId() != common.NewDispatcherIDFromPB(req.TableTriggerEventDispatcherId) { log.Error("Receive post bootstrap request but the table trigger event dispatcher id is not match", @@ -370,7 +400,8 @@ func (m *DispatcherOrchestrator) handlePostBootstrapRequest( GenWithStackByArgs("Receive post bootstrap request but the table trigger event dispatcher id is not match") response := &heartbeatpb.MaintainerPostBootstrapResponse{ - ChangefeedID: req.ChangefeedID, + ChangefeedID: req.ChangefeedID, + MaintainerEpoch: req.MaintainerEpoch, Err: &heartbeatpb.RunningError{ Time: time.Now().String(), Node: from.String(), @@ -379,6 +410,7 @@ func (m *DispatcherOrchestrator) handlePostBootstrapRequest( }, } + manager.MaintainerFenceMu.Unlock() return m.sendResponse(from, messaging.MaintainerManagerTopic, response) } @@ -392,7 +424,8 @@ func (m *DispatcherOrchestrator) handlePostBootstrapRequest( } log.Error("failed to initialize table trigger event dispatcher", zap.Any("changefeedID", cfId.Name()), zap.Error(err)) - return m.handleDispatcherError(from, req.ChangefeedID, err) + manager.MaintainerFenceMu.Unlock() + return m.handleDispatcherError(from, req.ChangefeedID, req.MaintainerEpoch, err) } if manager.IsRedoReady() { err := manager.InitalizeTableTriggerRedoDispatcher(req.RedoSchemas) @@ -404,11 +437,13 @@ func (m *DispatcherOrchestrator) handlePostBootstrapRequest( } log.Error("failed to initialize table trigger redo dispatcher", zap.Any("changefeedID", cfId.Name()), zap.Error(err)) - return m.handleDispatcherError(from, req.ChangefeedID, err) + manager.MaintainerFenceMu.Unlock() + return m.handleDispatcherError(from, req.ChangefeedID, req.MaintainerEpoch, err) } } if m.fenced.Load() { + manager.MaintainerFenceMu.Unlock() manager.LocalFence() return nil } @@ -416,7 +451,9 @@ func (m *DispatcherOrchestrator) handlePostBootstrapRequest( response := &heartbeatpb.MaintainerPostBootstrapResponse{ ChangefeedID: req.ChangefeedID, TableTriggerEventDispatcherId: req.TableTriggerEventDispatcherId, + MaintainerEpoch: req.MaintainerEpoch, } + manager.MaintainerFenceMu.Unlock() return m.sendResponse(from, messaging.MaintainerManagerTopic, response) } @@ -426,21 +463,50 @@ func (m *DispatcherOrchestrator) handleCloseRequest( ) error { cfId := common.NewChangefeedIDFromPB(req.ChangefeedID) response := &heartbeatpb.MaintainerCloseResponse{ - ChangefeedID: req.ChangefeedID, - Success: true, + ChangefeedID: req.ChangefeedID, + Success: true, + MaintainerEpoch: req.MaintainerEpoch, } m.mutex.Lock() - if manager, ok := m.dispatcherManagers[cfId]; ok { - if closed := manager.TryClose(req.Removed); closed { - delete(m.dispatcherManagers, cfId) - metrics.DispatcherManagerGauge.WithLabelValues(cfId.Keyspace(), cfId.Name()).Dec() - response.Success = true + manager, ok := m.dispatcherManagers[cfId] + if !ok { + m.recordClosedMaintainerEpochLocked(cfId, req.MaintainerEpoch, req.Removed) + } + m.mutex.Unlock() + + if ok { + // Do not hold the orchestrator-wide map lock while waiting for the + // per-changefeed fence; a slow manager must not block unrelated changefeeds. + decGauge := false + manager.MaintainerFenceMu.Lock() + if manager.IsMaintainerRequestAllowed(from, req.MaintainerEpoch) { + if closed := manager.TryClose(req.Removed); closed { + m.mutex.Lock() + delete(m.dispatcherManagers, cfId) + m.recordClosedMaintainerEpochLocked(cfId, req.MaintainerEpoch, req.Removed) + m.mutex.Unlock() + decGauge = true + response.Success = true + } else { + response.Success = false + } } else { - response.Success = false + // The active manager belongs to a newer maintainer. Do not close it, but + // acknowledge the stale sender so removal-only maintainers can stop retrying. + response.Success = true + log.Warn("drop stale maintainer close request", + zap.String("changefeed", cfId.Name()), + zap.String("from", from.String()), + zap.Uint64("requestMaintainerEpoch", req.MaintainerEpoch), + zap.Uint64("currentMaintainerEpoch", manager.GetMaintainerEpoch()), + zap.String("currentMaintainer", manager.GetMaintainerID().String())) + } + manager.MaintainerFenceMu.Unlock() + if decGauge { + metrics.DispatcherManagerGauge.WithLabelValues(cfId.Keyspace(), cfId.Name()).Dec() } } - m.mutex.Unlock() log.Info("try close dispatcher manager", zap.String("changefeed", cfId.String()), zap.Bool("success", response.Success)) @@ -511,14 +577,30 @@ func (m *DispatcherOrchestrator) removeInitializingDispatcherManager( } } +// recordClosedMaintainerEpochLocked remembers closed maintainer generations so +// later bootstrap requests from older owners cannot recreate a closed manager. +func (m *DispatcherOrchestrator) recordClosedMaintainerEpochLocked(cfID common.ChangeFeedID, maintainerEpoch uint64, removed bool) { + if maintainerEpoch == 0 && !removed { + // Epoch 0 has no ordering information. Keep permanent tombstones only + // for removal so mixed-version resume can still bootstrap in compat mode. + return + } + closedEpoch, ok := m.closedMaintainerEpochs[cfID] + if ok && closedEpoch >= maintainerEpoch { + return + } + m.closedMaintainerEpochs[cfID] = maintainerEpoch +} + func createBootstrapResponse( changefeedID *heartbeatpb.ChangefeedID, manager *dispatchermanager.DispatcherManager, startTs, redoStartTs uint64, ) *heartbeatpb.MaintainerBootstrapResponse { response := &heartbeatpb.MaintainerBootstrapResponse{ - ChangefeedID: changefeedID, - Spans: make([]*heartbeatpb.BootstrapTableSpan, 0, manager.GetDispatcherMap().Len()), + ChangefeedID: changefeedID, + Spans: make([]*heartbeatpb.BootstrapTableSpan, 0, manager.GetDispatcherMap().Len()), + MaintainerEpoch: manager.GetMaintainerEpoch(), } // table trigger event dispatcher startTs @@ -582,10 +664,12 @@ func (m *DispatcherOrchestrator) Close() { func (m *DispatcherOrchestrator) handleDispatcherError( from node.ID, changefeedID *heartbeatpb.ChangefeedID, + maintainerEpoch uint64, err error, ) error { response := &heartbeatpb.MaintainerBootstrapResponse{ - ChangefeedID: changefeedID, + ChangefeedID: changefeedID, + MaintainerEpoch: maintainerEpoch, Err: &heartbeatpb.RunningError{ Time: time.Now().String(), Node: from.String(), @@ -638,7 +722,7 @@ func retrieveOperatorsForBootstrapResponse( manager *dispatchermanager.DispatcherManager, response *heartbeatpb.MaintainerBootstrapResponse, ) { - manager.GetCurrentOperatorMap().Range(func(key, value any) bool { + manager.GetCurrentOperatorMap().Range(func(_, value any) bool { req := value.(dispatchermanager.SchedulerDispatcherRequest) dispatcherID := common.NewDispatcherIDFromPB(req.Config.DispatcherID) if common.IsRedoMode(req.Config.Mode) { @@ -666,12 +750,8 @@ func retrieveOperatorsForBootstrapResponse( ) } } - response.Operators = append(response.Operators, &heartbeatpb.ScheduleDispatcherRequest{ - ChangefeedID: req.ChangefeedID, - Config: req.Config, - ScheduleAction: req.ScheduleAction, - OperatorType: req.OperatorType, - }) + response.Operators = append(response.Operators, + proto.Clone(req.ScheduleDispatcherRequest).(*heartbeatpb.ScheduleDispatcherRequest)) return true }) } diff --git a/downstreamadapter/dispatcherorchestrator/helper.go b/downstreamadapter/dispatcherorchestrator/helper.go index e036ced60a..31aba845f5 100644 --- a/downstreamadapter/dispatcherorchestrator/helper.go +++ b/downstreamadapter/dispatcherorchestrator/helper.go @@ -34,9 +34,8 @@ type pendingMessageKey struct { // Once Pop returns a message, the key leaves the pending set immediately, so the next // retry can queue one more request for the next processing round. // -// For MaintainerCloseRequest, we treat removed=true as stronger semantics than removed=false. -// While a request is still queued, a later removed=true request replaces removed=false in -// that queued slot so the next execution still observes the stronger semantics. +// For MaintainerCloseRequest, removed=true has stronger semantics than removed=false. +// It can upgrade a queued request only when it does not move the maintainer epoch backward. type pendingMessageQueue struct { mu sync.Mutex pending map[pendingMessageKey]*messaging.TargetMessage @@ -72,6 +71,9 @@ func (q *pendingMessageQueue) TryEnqueue(key pendingMessageKey, msg *messaging.T } func shouldReplacePendingMessage(key pendingMessageKey, oldMsg, newMsg *messaging.TargetMessage) bool { + if shouldReplaceByMaintainerEpoch(oldMsg, newMsg) { + return true + } if key.msgType != messaging.TypeMaintainerCloseRequest { return false } @@ -86,8 +88,35 @@ func shouldReplacePendingMessage(key pendingMessageKey, oldMsg, newMsg *messagin if !ok1 || !ok2 { return false } - // Only upgrade semantics: allow removed=true to override removed=false. - return !oldReq.Removed && newReq.Removed + // Only upgrade semantics: allow removed=true to override removed=false without + // letting a stale removed request overwrite a newer epoch close. + return !oldReq.Removed && newReq.Removed && newReq.MaintainerEpoch >= oldReq.MaintainerEpoch +} + +// shouldReplaceByMaintainerEpoch lets a newer maintainer generation replace an +// older queued control message for the same changefeed and message type. +func shouldReplaceByMaintainerEpoch(oldMsg, newMsg *messaging.TargetMessage) bool { + oldMaintainerEpoch, oldOK := pendingMessageMaintainerEpoch(oldMsg) + newMaintainerEpoch, newOK := pendingMessageMaintainerEpoch(newMsg) + return oldOK && newOK && newMaintainerEpoch > oldMaintainerEpoch +} + +// pendingMessageMaintainerEpoch extracts the maintainer epoch from messages +// whose ordering must be fenced by maintainer generation. +func pendingMessageMaintainerEpoch(msg *messaging.TargetMessage) (uint64, bool) { + if msg == nil || len(msg.Message) == 0 { + return 0, false + } + switch req := msg.Message[0].(type) { + case *heartbeatpb.MaintainerBootstrapRequest: + return req.MaintainerEpoch, true + case *heartbeatpb.MaintainerPostBootstrapRequest: + return req.MaintainerEpoch, true + case *heartbeatpb.MaintainerCloseRequest: + return req.MaintainerEpoch, true + default: + return 0, false + } } // Pop blocks until a message is available or the queue is closed. diff --git a/maintainer/barrier.go b/maintainer/barrier.go index f4a35325e8..9948fca20e 100644 --- a/maintainer/barrier.go +++ b/maintainer/barrier.go @@ -174,6 +174,7 @@ func (b *Barrier) HandleStatus(from node.ID, ChangefeedID: request.ChangefeedID, DispatcherStatuses: dispatcherStatus, Mode: b.mode, + MaintainerEpoch: b.operatorController.MaintainerEpoch(), }) msgs := []*messaging.TargetMessage{msg} @@ -185,6 +186,7 @@ func (b *Barrier) HandleStatus(from node.ID, ChangefeedID: request.ChangefeedID, DispatcherStatuses: action, Mode: b.mode, + MaintainerEpoch: b.operatorController.MaintainerEpoch(), }) msgs = append(msgs, msg) } diff --git a/maintainer/barrier_event.go b/maintainer/barrier_event.go index 85f93c7fcd..a91dced148 100644 --- a/maintainer/barrier_event.go +++ b/maintainer/barrier_event.go @@ -802,7 +802,8 @@ func (be *BarrierEvent) newWriterActionMessage(capture node.ID, mode int64) *mes }, }, }, - Mode: mode, + Mode: mode, + MaintainerEpoch: be.operatorController.MaintainerEpoch(), }) return msg } @@ -827,7 +828,8 @@ func (be *BarrierEvent) newPassActionMessage(capture node.ID, mode int64) *messa InfluencedDispatchers: influenced, }, }, - Mode: mode, + Mode: mode, + MaintainerEpoch: be.operatorController.MaintainerEpoch(), }) } diff --git a/maintainer/barrier_test.go b/maintainer/barrier_test.go index d0f9a63c63..ee4ac4e783 100644 --- a/maintainer/barrier_test.go +++ b/maintainer/barrier_test.go @@ -1410,7 +1410,7 @@ func TestUpdateCheckpointTs(t *testing.T) { require.Equal(t, resp.DispatcherStatuses[1].Action.Action, heartbeatpb.Action_Write) require.False(t, resp.DispatcherStatuses[1].Action.IsSyncPoint) // the checkpoint ts is updated - scheduleMsg := ddlSpan.NewAddDispatcherMessage("node1", heartbeatpb.OperatorType_O_Add) + scheduleMsg := ddlSpan.NewAddDispatcherMessage("node1", heartbeatpb.OperatorType_O_Add, 7) require.Equal(t, uint64(9), scheduleMsg.Message[0].(*heartbeatpb.ScheduleDispatcherRequest).Config.StartTs, false) require.NotEqual(t, uint64(0), scheduleMsg.Message[0].(*heartbeatpb.ScheduleDispatcherRequest).Config.StartTs, false) } diff --git a/maintainer/maintainer.go b/maintainer/maintainer.go index 3e5bd75a2a..96d89a429b 100644 --- a/maintainer/maintainer.go +++ b/maintainer/maintainer.go @@ -205,7 +205,7 @@ func NewMaintainer(cfID common.ChangeFeedID, eventCh: chann.NewAutoDrainChann[*Event](), startCheckpointTs: checkpointTs, controller: NewController(cfID, checkpointTs, taskScheduler, - info.Config, ddlSpan, redoDDLSpan, conf.AddTableBatchSize, time.Duration(conf.CheckBalanceInterval), refresher, keyspaceMeta, enableRedo, conf.BalanceMoveBatchSize), + info.Config, ddlSpan, redoDDLSpan, conf.AddTableBatchSize, time.Duration(conf.CheckBalanceInterval), refresher, keyspaceMeta, enableRedo, conf.BalanceMoveBatchSize, info.Epoch), mc: mc, removed: atomic.NewBool(false), nodeManager: nodeManager, @@ -288,11 +288,13 @@ func NewMaintainerForRemove(cfID common.ChangeFeedID, selfNode *node.Info, taskScheduler threadpool.ThreadPool, keyspaceID uint32, + maintainerEpoch uint64, ) *Maintainer { unused := &config.ChangeFeedInfo{ ChangefeedID: cfID, SinkURI: "", Config: config.GetDefaultReplicaConfig(), + Epoch: maintainerEpoch, } m := NewMaintainer(cfID, conf, unused, selfNode, taskScheduler, 1, false, keyspaceID) m.cascadeRemoving.Store(true) @@ -385,12 +387,13 @@ func (m *Maintainer) GetMaintainerStatus() *heartbeatpb.MaintainerStatus { } status := &heartbeatpb.MaintainerStatus{ - ChangefeedID: m.changefeedID.ToPB(), - State: heartbeatpb.ComponentState(m.scheduleState.Load()), - CheckpointTs: m.controller.spanController.GetMaintainerCommittedCheckpointTs(), - Err: runningErrors, - BootstrapDone: m.initialized.Load(), - LastSyncedTs: m.getWatermark().LastSyncedTs, + ChangefeedID: m.changefeedID.ToPB(), + State: heartbeatpb.ComponentState(m.scheduleState.Load()), + CheckpointTs: m.controller.spanController.GetMaintainerCommittedCheckpointTs(), + Err: runningErrors, + BootstrapDone: m.initialized.Load(), + LastSyncedTs: m.getWatermark().LastSyncedTs, + MaintainerEpoch: m.currentMaintainerEpoch(), } drainTarget, drainEpoch := m.controller.getDispatcherDrainTarget() if !drainTarget.IsEmpty() && drainEpoch > 0 { @@ -492,6 +495,13 @@ func (m *Maintainer) cleanupMetrics() { metrics.TableCountGauge.DeleteLabelValues(keyspace, name, "redo") } +func (m *Maintainer) markRemoved() { + if !m.removed.CompareAndSwap(false, true) { + return + } + metrics.MaintainerGauge.WithLabelValues(m.changefeedID.Keyspace(), m.changefeedID.Name()).Dec() +} + func (m *Maintainer) onInit() bool { err := m.initialize() if err != nil { @@ -526,6 +536,14 @@ func (m *Maintainer) onMessage(msg *messaging.TargetMessage) { m.onMaintainerCloseResponse(msg.From, resp) case messaging.TypeRemoveMaintainerRequest: req := msg.Message[0].(*heartbeatpb.RemoveMaintainerRequest) + if !m.isMaintainerEpochRequestAllowed(req.MaintainerEpoch) { + log.Warn("drop stale remove maintainer request", + zap.Stringer("changefeedID", m.changefeedID), + zap.Stringer("from", msg.From), + zap.Uint64("requestMaintainerEpoch", req.MaintainerEpoch), + zap.Uint64("currentMaintainerEpoch", m.currentMaintainerEpoch())) + return + } m.onRemoveMaintainer(req.Cascade, req.Removed) case messaging.TypeCheckpointTsMessage: req := msg.Message[0].(*heartbeatpb.CheckpointTsMessage) @@ -562,9 +580,8 @@ func (m *Maintainer) onRemoveMaintainer(cascade, changefeedRemoved bool) { m.controller.EnterRemovingMode(allowedDispatcherIDs...) closed := m.tryCloseChangefeed() if closed { - m.removed.Store(true) + m.markRemoved() m.scheduleState.Store(int32(heartbeatpb.ComponentState_Stopped)) - metrics.MaintainerGauge.WithLabelValues(m.changefeedID.Keyspace(), m.changefeedID.Name()).Dec() log.Info("changefeed maintainer closed", zap.Stringer("changefeedID", m.changefeedID), zap.Uint64("checkpointTs", m.getWatermark().CheckpointTs), zap.Bool("removed", m.removed.Load())) } @@ -861,6 +878,13 @@ func (m *Maintainer) sendMessages(msgs []*messaging.TargetMessage) { } } +func (m *Maintainer) currentMaintainerEpoch() uint64 { + if m.info == nil { + return 0 + } + return m.info.Epoch +} + func (m *Maintainer) onHeartbeatRequest(msg *messaging.TargetMessage) { // ignore the heartbeat if the maintainer not bootstrapped if !m.initialized.Load() { @@ -957,6 +981,10 @@ func (m *Maintainer) onMaintainerBootstrapResponse(msg *messaging.TargetMessage) zap.Stringer("sourceNodeID", msg.From)) resp := msg.Message[0].(*heartbeatpb.MaintainerBootstrapResponse) + if !m.isMaintainerEpochResponseAllowed(resp.MaintainerEpoch) { + m.logDroppedMaintainerResponse("bootstrap", msg.From, resp.MaintainerEpoch) + return + } if resp.Err != nil { log.Warn("maintainer bootstrap failed", zap.Stringer("changefeedID", m.changefeedID), @@ -980,6 +1008,10 @@ func (m *Maintainer) onMaintainerPostBootstrapResponse(msg *messaging.TargetMess zap.Stringer("changefeedID", m.changefeedID), zap.Any("server", msg.From)) resp := msg.Message[0].(*heartbeatpb.MaintainerPostBootstrapResponse) + if !m.isMaintainerEpochResponseAllowed(resp.MaintainerEpoch) { + m.logDroppedMaintainerResponse("post-bootstrap", msg.From, resp.MaintainerEpoch) + return + } if resp.Err != nil { log.Warn("maintainer post bootstrap failed", zap.Stringer("changefeedID", m.changefeedID), @@ -991,6 +1023,36 @@ func (m *Maintainer) onMaintainerPostBootstrapResponse(msg *messaging.TargetMess m.postBootstrapMsg = nil } +// isMaintainerEpochResponseAllowed accepts current-generation responses while +// preserving epoch-0 compatibility during rolling upgrades. +func (m *Maintainer) isMaintainerEpochResponseAllowed(responseEpoch uint64) bool { + return common.MaintainerEpochMatches(responseEpoch, m.currentMaintainerEpoch()) +} + +// isMaintainerEpochRequestAllowed fences dispatcher-manager requests that can +// close or mutate local dispatcher state on behalf of a maintainer generation. +func (m *Maintainer) isMaintainerEpochRequestAllowed(requestEpoch uint64) bool { + currentEpoch := m.currentMaintainerEpoch() + if requestEpoch == 0 { + // Epoch 0 is only valid while this maintainer is still in compatibility + // mode. A strict maintainer must not accept an unfenced tombstone. + return currentEpoch == 0 + } + // A strict request can still control a compatibility maintainer during + // rolling upgrade, but strict maintainers require an exact epoch match. + return currentEpoch == 0 || requestEpoch == currentEpoch +} + +// logDroppedMaintainerResponse records responses rejected by maintainer epoch fencing. +func (m *Maintainer) logDroppedMaintainerResponse(responseType string, from node.ID, responseEpoch uint64) { + log.Warn("drop stale maintainer response", + zap.Stringer("changefeedID", m.changefeedID), + zap.String("responseType", responseType), + zap.Stringer("from", from), + zap.Uint64("responseMaintainerEpoch", responseEpoch), + zap.Uint64("currentMaintainerEpoch", m.currentMaintainerEpoch())) +} + // isMysqlCompatible returns true if the sinkURIStr is mysql compatible. func isMysqlCompatible(sinkURIStr string) (bool, error) { sinkURI, err := url.Parse(sinkURIStr) @@ -1060,6 +1122,20 @@ func (m *Maintainer) sendPostBootstrapRequest() { } func (m *Maintainer) onMaintainerCloseResponse(from node.ID, response *heartbeatpb.MaintainerCloseResponse) { + if !m.isMaintainerEpochResponseAllowed(response.MaintainerEpoch) { + m.logDroppedMaintainerResponse("close", from, response.MaintainerEpoch) + return + } + if !m.removing.Load() { + // Close responses only complete an active remove flow. A delayed compat + // response from a superseded maintainer can share this changefeed ID. + log.Warn("drop unexpected maintainer close response", + zap.Stringer("changefeedID", m.changefeedID), + zap.Stringer("from", from), + zap.Uint64("responseMaintainerEpoch", response.MaintainerEpoch), + zap.Uint64("currentMaintainerEpoch", m.currentMaintainerEpoch())) + return + } if response.Success { m.closedNodes[from] = struct{}{} m.onRemoveMaintainer(m.cascadeRemoving.Load(), m.changefeedRemoved.Load()) @@ -1115,8 +1191,9 @@ func (m *Maintainer) trySendMaintainerCloseRequestToAllNode() bool { n, messaging.DispatcherManagerManagerTopic, &heartbeatpb.MaintainerCloseRequest{ - ChangefeedID: m.changefeedID.ToPB(), - Removed: m.changefeedRemoved.Load(), + ChangefeedID: m.changefeedID.ToPB(), + Removed: m.changefeedRemoved.Load(), + MaintainerEpoch: m.currentMaintainerEpoch(), })) } } @@ -1177,6 +1254,7 @@ func (m *Maintainer) createBootstrapMessageFactory() bootstrap.NewBootstrapReque TableTriggerRedoDispatcherId: nil, IsNewChangefeed: false, KeyspaceId: m.info.KeyspaceID, + MaintainerEpoch: m.currentMaintainerEpoch(), } // only send dispatcher targetNodeID to dispatcher manager on the same node diff --git a/maintainer/maintainer_controller.go b/maintainer/maintainer_controller.go index ea5381f81f..1660a9ba2f 100644 --- a/maintainer/maintainer_controller.go +++ b/maintainer/maintainer_controller.go @@ -15,6 +15,7 @@ package maintainer import ( "sync" + "sync/atomic" "time" "github.com/pingcap/log" @@ -58,8 +59,9 @@ type Controller struct { splitter *split.Splitter - replicaConfig *config.ReplicaConfig - changefeedID common.ChangeFeedID + replicaConfig *config.ReplicaConfig + changefeedID common.ChangeFeedID + maintainerEpoch atomic.Uint64 taskPool threadpool.ThreadPool @@ -94,6 +96,7 @@ func NewController(changefeedID common.ChangeFeedID, keyspaceMeta common.KeyspaceMeta, enableRedo bool, balanceMoveBatchSize int, + maintainerEpoch uint64, ) *Controller { mc := appcontext.GetService[messaging.MessageCenter](appcontext.MessageCenter) @@ -160,6 +163,7 @@ func NewController(changefeedID common.ChangeFeedID, controller.drainState, balanceMoveBatchSize, ) + controller.SetMaintainerEpoch(maintainerEpoch) return controller } @@ -170,6 +174,20 @@ func (c *Controller) SetErrorReporter(reportError func(error)) { } } +// SetMaintainerEpoch propagates the changefeed epoch used to fence +// dispatcher-manager control requests from stale maintainers. +func (c *Controller) SetMaintainerEpoch(maintainerEpoch uint64) { + c.maintainerEpoch.Store(maintainerEpoch) + c.operatorController.SetMaintainerEpoch(maintainerEpoch) + if c.redoOperatorController != nil { + c.redoOperatorController.SetMaintainerEpoch(maintainerEpoch) + } +} + +func (c *Controller) currentMaintainerEpoch() uint64 { + return c.maintainerEpoch.Load() +} + // HandleStatus handle the status report from the node. func (c *Controller) HandleStatus(from node.ID, statusList []*heartbeatpb.TableSpanStatus) { c.handleStatus(from, statusList, true) @@ -217,7 +235,16 @@ func (c *Controller) handleStatus(from node.ID, statusList []*heartbeatpb.TableS zap.Any("status", status), zap.String("dispatcherID", dispatcherID.String())) // If the span is not found but status is Working, we need to remove it from dispatcher. - _ = c.messageCenter.SendCommand(replica.NewRemoveDispatcherMessage(from, c.changefeedID, status.ID, nil, status.Mode, heartbeatpb.OperatorType_O_Remove)) + msg := replica.NewRemoveDispatcherMessage( + from, + c.changefeedID, + status.ID, + nil, + status.Mode, + heartbeatpb.OperatorType_O_Remove, + c.currentMaintainerEpoch(), + ) + _ = c.messageCenter.SendCommand(msg) } continue } diff --git a/maintainer/maintainer_controller_bootstrap.go b/maintainer/maintainer_controller_bootstrap.go index 3df71f2911..96277ec281 100644 --- a/maintainer/maintainer_controller_bootstrap.go +++ b/maintainer/maintainer_controller_bootstrap.go @@ -153,6 +153,7 @@ func (c *Controller) FinishBootstrap( TableTriggerEventDispatcherId: c.spanController.GetDDLDispatcherID().ToPB(), Schemas: c.prepareSchemaInfoResponse(schemaInfos), RedoSchemas: c.prepareSchemaInfoResponse(redoSchemaInfos), + MaintainerEpoch: c.currentMaintainerEpoch(), }, nil } @@ -836,8 +837,14 @@ func (c *Controller) handleCurrentWorkingAdd( // 3. If the original operator is split, which is a remove + add + add..., // same as move, just finish the add part. case heartbeatpb.OperatorType_O_Add, heartbeatpb.OperatorType_O_Move, heartbeatpb.OperatorType_O_Split: - op := operator.NewAddDispatcherOperator(spanController, replicaSet, node, heartbeatpb.OperatorType_O_Add) operatorController := c.getOperatorController(req.Config.Mode) + op := operator.NewAddDispatcherOperator( + spanController, + replicaSet, + node, + heartbeatpb.OperatorType_O_Add, + operatorController.MaintainerEpoch(), + ) if ok := operatorController.AddOperator(op); !ok { log.Error("add operator failed when dealing current working operators in bootstrap, should not happen", zap.String("nodeID", node.String()), @@ -876,6 +883,7 @@ func (c *Controller) handleCurrentWorkingRemove( spanController, replicaSet, heartbeatpb.OperatorType_O_Remove, + operatorController.MaintainerEpoch(), nil, ) if ok := operatorController.AddOperator(op); !ok { @@ -897,6 +905,7 @@ func (c *Controller) handleCurrentWorkingRemove( spanController, replicaSet, req.OperatorType, + operatorController.MaintainerEpoch(), func() { // post finish // Mark the span absent only if it still exists. A concurrent DDL may have already removed it, // and we must not reintroduce a ghost entry into spanController. diff --git a/maintainer/maintainer_controller_helper.go b/maintainer/maintainer_controller_helper.go index 85db5b2b44..a319a3cdc9 100644 --- a/maintainer/maintainer_controller_helper.go +++ b/maintainer/maintainer_controller_helper.go @@ -212,7 +212,14 @@ func (c *Controller) splitTableByRegionCount(tableID int64, mode int64) error { } splitTableSpans := splitter.Split(context.Background(), wholeSpan, 0, split.SplitTypeRegionCount) - op := operator.NewSplitDispatcherOperator(spanController, replications[0], splitTableSpans, []node.ID{}, nil) + op := operator.NewSplitDispatcherOperator( + spanController, + replications[0], + splitTableSpans, + []node.ID{}, + operatorController.MaintainerEpoch(), + nil, + ) ret := operatorController.AddOperator(op) if !ret { return errors.ErrOperatorIsNil.GenWithStackByArgs("unexpected error in create split dispatcher operator") diff --git a/maintainer/maintainer_controller_test.go b/maintainer/maintainer_controller_test.go index d1226bd72f..56327458bc 100644 --- a/maintainer/maintainer_controller_test.go +++ b/maintainer/maintainer_controller_test.go @@ -76,7 +76,7 @@ func TestSchedule(t *testing.T) { CheckpointTs: 1, }, "node1", false) refresher := replica.NewRegionCountRefresher(cfID, time.Minute) - controller := NewController(cfID, 1, nil, replicaConfig, ddlSpan, nil, 9, time.Minute, refresher, common.DefaultKeyspace, false, testBalanceMoveBatchSize) + controller := NewController(cfID, 1, nil, replicaConfig, ddlSpan, nil, 9, time.Minute, refresher, common.DefaultKeyspace, false, testBalanceMoveBatchSize, 0) for i := 0; i < 10; i++ { controller.spanController.AddNewTable(commonEvent.Table{ SchemaID: 1, @@ -96,6 +96,48 @@ func TestSchedule(t *testing.T) { require.Equal(t, 3, controller.spanController.GetTaskSizeByNodeID("node3")) } +func TestNewControllerInitializesMaintainerEpoch(t *testing.T) { + testutil.SetUpTestServices(t) + cfID := common.NewChangeFeedIDWithName("test", common.DefaultKeyspaceName) + ddlDispatcherID := common.NewDispatcherID() + redoDDLDispatcherID := common.NewDispatcherID() + ddlSpan := replica.NewWorkingSpanReplication(cfID, ddlDispatcherID, + common.DDLSpanSchemaID, + common.KeyspaceDDLSpan(common.DefaultKeyspaceID), &heartbeatpb.TableSpanStatus{ + ID: ddlDispatcherID.ToPB(), + ComponentStatus: heartbeatpb.ComponentState_Working, + CheckpointTs: 1, + }, "node1", false) + redoDDLSpan := replica.NewWorkingSpanReplication(cfID, redoDDLDispatcherID, + common.DDLSpanSchemaID, + common.KeyspaceDDLSpan(common.DefaultKeyspaceID), &heartbeatpb.TableSpanStatus{ + ID: redoDDLDispatcherID.ToPB(), + ComponentStatus: heartbeatpb.ComponentState_Working, + CheckpointTs: 1, + }, "node1", false) + const maintainerEpoch = uint64(42) + + controller := NewController( + cfID, + 1, + nil, + replicaConfig, + ddlSpan, + redoDDLSpan, + 9, + time.Minute, + replica.NewRegionCountRefresher(cfID, time.Minute), + common.DefaultKeyspace, + true, + testBalanceMoveBatchSize, + maintainerEpoch, + ) + + require.Equal(t, maintainerEpoch, controller.currentMaintainerEpoch()) + require.Equal(t, maintainerEpoch, controller.operatorController.MaintainerEpoch()) + require.Equal(t, maintainerEpoch, controller.redoOperatorController.MaintainerEpoch()) +} + // This case test the scenario that the balance scheduler when a new node join in. // In this case, the num of split tables is more than the num of nodes, // and we can select appropriate split spans to move @@ -122,7 +164,7 @@ func TestBalanceGroupsNewNodeAdd_SplitsTableMoreThanNodeNum(t *testing.T) { MinTrafficPercentage: util.AddressOf(0.8), MaxTrafficPercentage: util.AddressOf(1.2), }, - }, ddlSpan, nil, 1000, 0, refresher, common.DefaultKeyspace, false, testBalanceMoveBatchSize) + }, ddlSpan, nil, 1000, 0, refresher, common.DefaultKeyspace, false, testBalanceMoveBatchSize, 0) nodeID := node.ID("node1") for i := range 100 { @@ -254,7 +296,7 @@ func TestBalanceGroupsNewNodeAdd_SplitsTableLessThanNodeNum(t *testing.T) { MinTrafficPercentage: util.AddressOf(0.8), MaxTrafficPercentage: util.AddressOf(1.2), }, - }, ddlSpan, nil, 1000, 0, refresher, common.DefaultKeyspace, false, testBalanceMoveBatchSize) + }, ddlSpan, nil, 1000, 0, refresher, common.DefaultKeyspace, false, testBalanceMoveBatchSize, 0) regionCache := appcontext.GetService[*testutil.MockCache](appcontext.RegionCache) @@ -376,7 +418,7 @@ func TestSplitBalanceGroupsWithNodeRemove(t *testing.T) { MinTrafficPercentage: util.AddressOf(0.8), MaxTrafficPercentage: util.AddressOf(1.2), }, - }, ddlSpan, nil, 1000, 0, refresher, common.DefaultKeyspace, false, testBalanceMoveBatchSize) + }, ddlSpan, nil, 1000, 0, refresher, common.DefaultKeyspace, false, testBalanceMoveBatchSize, 0) nodeIDList := []node.ID{"node1", "node2", "node3"} for i := 0; i < 100; i++ { @@ -478,7 +520,7 @@ func TestSplitTableBalanceWhenTrafficUnbalanced(t *testing.T) { MinTrafficPercentage: util.AddressOf(0.8), MaxTrafficPercentage: util.AddressOf(1.2), }, - }, ddlSpan, nil, 1000, 0, refresher, common.DefaultKeyspace, false, testBalanceMoveBatchSize) + }, ddlSpan, nil, 1000, 0, refresher, common.DefaultKeyspace, false, testBalanceMoveBatchSize, 0) nodeIDList := []node.ID{"node1", "node2", "node3"} // make a group @@ -1079,7 +1121,7 @@ func TestBalance(t *testing.T) { CheckpointTs: 1, }, "node1", false) refresher := replica.NewRegionCountRefresher(cfID, time.Minute) - s := NewController(cfID, 1, nil, replicaConfig, ddlSpan, nil, 1000, 0, refresher, common.DefaultKeyspace, false, testBalanceMoveBatchSize) + s := NewController(cfID, 1, nil, replicaConfig, ddlSpan, nil, 1000, 0, refresher, common.DefaultKeyspace, false, testBalanceMoveBatchSize, 0) for i := 0; i < 100; i++ { sz := common.TableIDToComparableSpan(common.DefaultKeyspaceID, int64(i)) span := &heartbeatpb.TableSpan{TableID: sz.TableID, StartKey: sz.StartKey, EndKey: sz.EndKey} @@ -1185,7 +1227,7 @@ func TestDefaultSpanIntoSplit(t *testing.T) { MinTrafficPercentage: util.AddressOf(0.8), MaxTrafficPercentage: util.AddressOf(1.2), }, - }, ddlSpan, nil, 1000, 0, refresher, common.DefaultKeyspace, false, testBalanceMoveBatchSize) + }, ddlSpan, nil, 1000, 0, refresher, common.DefaultKeyspace, false, testBalanceMoveBatchSize, 0) totalSpan := common.TableIDToComparableSpan(common.DefaultKeyspaceID, 1) span := &heartbeatpb.TableSpan{TableID: int64(1), StartKey: totalSpan.StartKey, EndKey: totalSpan.EndKey} dispatcherID := common.NewDispatcherID() @@ -1327,7 +1369,7 @@ func TestStoppedWhenMoving(t *testing.T) { CheckpointTs: 1, }, "node1", false) refresher := replica.NewRegionCountRefresher(cfID, time.Minute) - s := NewController(cfID, 1, nil, replicaConfig, ddlSpan, nil, 1000, 0, refresher, common.DefaultKeyspace, false, testBalanceMoveBatchSize) + s := NewController(cfID, 1, nil, replicaConfig, ddlSpan, nil, 1000, 0, refresher, common.DefaultKeyspace, false, testBalanceMoveBatchSize, 0) for i := 0; i < 2; i++ { sz := common.TableIDToComparableSpan(common.DefaultKeyspaceID, int64(i)) span := &heartbeatpb.TableSpan{TableID: sz.TableID, StartKey: sz.StartKey, EndKey: sz.EndKey} @@ -1380,7 +1422,7 @@ func TestFinishBootstrap(t *testing.T) { }, "node1", false) refresher := replica.NewRegionCountRefresher(cfID, time.Minute) s := NewController(cfID, 1, &mockThreadPool{}, - config.GetDefaultReplicaConfig(), ddlSpan, nil, 1000, 0, refresher, common.DefaultKeyspace, false, testBalanceMoveBatchSize) + config.GetDefaultReplicaConfig(), ddlSpan, nil, 1000, 0, refresher, common.DefaultKeyspace, false, testBalanceMoveBatchSize, 0) totalSpan := common.TableIDToComparableSpan(common.DefaultKeyspaceID, 1) span := &heartbeatpb.TableSpan{TableID: int64(1), StartKey: totalSpan.StartKey, EndKey: totalSpan.EndKey} schemaStore := eventservice.NewMockSchemaStore() @@ -1453,7 +1495,7 @@ func TestFinishBootstrapReturnsErrorWhenCheckpointMissing(t *testing.T) { }, "node1", false) refresher := replica.NewRegionCountRefresher(cfID, time.Minute) controller := NewController(cfID, 1, &mockThreadPool{}, - config.GetDefaultReplicaConfig(), ddlSpan, nil, 1000, 0, refresher, common.DefaultKeyspace, false, testBalanceMoveBatchSize) + config.GetDefaultReplicaConfig(), ddlSpan, nil, 1000, 0, refresher, common.DefaultKeyspace, false, testBalanceMoveBatchSize, 0) postBootstrapRequest, err := controller.FinishBootstrap(map[node.ID]*heartbeatpb.MaintainerBootstrapResponse{ "node1": { @@ -1517,7 +1559,7 @@ func TestFinishBootstrapSkipsStaleCreateOperatorForDroppedTable(t *testing.T) { }, "node1", false) refresher := replica.NewRegionCountRefresher(cfID, time.Minute) s := NewController(cfID, 1, &mockThreadPool{}, - config.GetDefaultReplicaConfig(), ddlSpan, nil, 1000, 0, refresher, common.DefaultKeyspace, false, testBalanceMoveBatchSize) + config.GetDefaultReplicaConfig(), ddlSpan, nil, 1000, 0, refresher, common.DefaultKeyspace, false, testBalanceMoveBatchSize, 0) // The schema-store snapshot is empty at bootstrap startTs, which models a table // that has already been dropped before failover recovery starts. @@ -1597,7 +1639,7 @@ func TestSplitTableWhenBootstrapFinished(t *testing.T) { MaxTrafficPercentage: util.AddressOf(1.2), } refresher := replica.NewRegionCountRefresher(cfID, time.Minute) - s := NewController(cfID, 1, nil, defaultConfig, ddlSpan, nil, 1000, 0, refresher, common.DefaultKeyspace, false, testBalanceMoveBatchSize) + s := NewController(cfID, 1, nil, defaultConfig, ddlSpan, nil, 1000, 0, refresher, common.DefaultKeyspace, false, testBalanceMoveBatchSize, 0) s.taskPool = &mockThreadPool{} schemaStore := eventservice.NewMockSchemaStore() schemaStore.SetTables( @@ -1777,7 +1819,7 @@ func TestLargeTableInitialization(t *testing.T) { MinTrafficPercentage: util.AddressOf(0.8), MaxTrafficPercentage: util.AddressOf(1.2), }, - }, ddlSpan, nil, 1000, 0, refresher, common.DefaultKeyspace, false, testBalanceMoveBatchSize) + }, ddlSpan, nil, 1000, 0, refresher, common.DefaultKeyspace, false, testBalanceMoveBatchSize, 0) // Create a large table with 10000 regions totalSpan := common.TableIDToComparableSpan(common.DefaultKeyspaceID, int64(1)) diff --git a/maintainer/maintainer_manager_maintainers.go b/maintainer/maintainer_manager_maintainers.go index ae53e97900..6a19897343 100644 --- a/maintainer/maintainer_manager_maintainers.go +++ b/maintainer/maintainer_manager_maintainers.go @@ -51,6 +51,10 @@ type managerMaintainerSet struct { // taskScheduler is shared by all local maintainers to run background tasks. taskScheduler threadpool.ThreadPool + // registryMu serializes registry mutations that create, replace, or fully + // close maintainers because maintainer metrics share changefeed labels across + // epochs. + registryMu sync.Mutex // registry is the in-memory changefeedID -> maintainer mapping. registry sync.Map } @@ -179,10 +183,16 @@ func (p *managerMaintainerSet) handleAddMaintainer( getDrainTarget func() (node.ID, uint64), ) *heartbeatpb.MaintainerStatus { changefeedID := common.NewChangefeedIDFromPB(req.Id) - if _, ok := p.registry.Load(changefeedID); ok { + if req.CheckpointTs == 0 { + log.Error("ignore add maintainer request with invalid checkpointTs", + zap.Stringer("changefeedID", changefeedID), + zap.Uint64("checkpointTs", req.CheckpointTs)) + return nil + } + requestEpoch := req.MaintainerEpoch + if !p.mayRegisterMaintainerForAdd(changefeedID, requestEpoch) { return nil } - info := &config.ChangeFeedInfo{} if err := json.Unmarshal(req.Config, info); err != nil { log.Error("ignore add maintainer request with invalid config", @@ -191,22 +201,19 @@ func (p *managerMaintainerSet) handleAddMaintainer( zap.Error(err)) return nil } - if req.CheckpointTs == 0 { - log.Error("ignore add maintainer request with invalid checkpointTs", - zap.Stringer("changefeedID", changefeedID), - zap.Uint64("checkpointTs", req.CheckpointTs)) - return nil + // The wire epoch is the sender capability signal. If an old coordinator sends + // epoch 0, keep the maintainer in compatibility mode even when the serialized + // config still carries a non-zero ChangeFeedInfo epoch. + info.Epoch = requestEpoch + // Create the maintainer only after epoch admission so normal duplicate + // add retries do not start short-lived goroutines or metrics. + newMaintainer := func() *Maintainer { + return NewMaintainer(changefeedID, p.conf, info, p.nodeInfo, p.taskScheduler, req.CheckpointTs, req.IsNewChangefeed, req.KeyspaceId) } - maintainer := NewMaintainer(changefeedID, p.conf, info, p.nodeInfo, p.taskScheduler, req.CheckpointTs, req.IsNewChangefeed, req.KeyspaceId) - registered, loaded := p.registry.LoadOrStore(changefeedID, maintainer) - if loaded { - // Duplicate add requests can race on the same changefeed. Drop the loser and - // stop the redundant maintainer immediately so background goroutines do not leak. - maintainer.Close() + registeredMaintainer := p.registerMaintainerForAdd(changefeedID, requestEpoch, newMaintainer) + if registeredMaintainer == nil { return nil } - - registeredMaintainer := registered.(*Maintainer) // Register the maintainer before seeding the drain snapshot so concurrent // manager-level drain fanout can always observe it in the registry. target, epoch := getDrainTarget() @@ -215,6 +222,96 @@ func (p *managerMaintainerSet) handleAddMaintainer( return nil } +// mayRegisterMaintainerForAdd performs a cheap admission check before decoding +// config and constructing a maintainer. +func (p *managerMaintainerSet) mayRegisterMaintainerForAdd( + changefeedID common.ChangeFeedID, + requestEpoch uint64, +) bool { + registered, loaded := p.registry.Load(changefeedID) + if !loaded { + return true + } + existing := registered.(*Maintainer) + allowed := canRegisterAfterExistingMaintainer(existing, requestEpoch) + if !allowed { + logRejectedAddMaintainer(changefeedID, existing, requestEpoch) + } + return allowed +} + +// registerMaintainerForAdd installs a newly created maintainer after rechecking +// epoch and stopped-state admission under the registry mutation lock. +func (p *managerMaintainerSet) registerMaintainerForAdd( + changefeedID common.ChangeFeedID, + requestEpoch uint64, + newMaintainer func() *Maintainer, +) *Maintainer { + p.registryMu.Lock() + defer p.registryMu.Unlock() + + registered, loaded := p.registry.Load(changefeedID) + if !loaded { + maintainer := newMaintainer() + p.registry.Store(changefeedID, maintainer) + return maintainer + } + existing := registered.(*Maintainer) + if !canRegisterAfterExistingMaintainer(existing, requestEpoch) { + logRejectedAddMaintainer(changefeedID, existing, requestEpoch) + return nil + } + // The old maintainer has fully stopped, so it is safe to release the + // shared metric labels before the new maintainer creates its own metric + // children for the same changefeed. + existing.Close() + maintainer := newMaintainer() + p.registry.Store(changefeedID, maintainer) + return maintainer +} + +// canRegisterAfterExistingMaintainer reports whether an add request can replace +// the existing local maintainer without overlapping two live owners. +func canRegisterAfterExistingMaintainer(existing *Maintainer, requestEpoch uint64) bool { + if !isMaintainerFullyStopped(existing) { + return false + } + return isNewerMaintainerEpoch(existing.currentMaintainerEpoch(), requestEpoch) +} + +// isNewerMaintainerEpoch applies strict epoch ordering for replacement adds. +func isNewerMaintainerEpoch(existingEpoch, requestEpoch uint64) bool { + if requestEpoch == 0 { + return false + } + if existingEpoch == 0 { + return true + } + return requestEpoch > existingEpoch +} + +// isMaintainerFullyStopped reports whether the old maintainer has finished its +// remove flow and released scheduler ownership. +func isMaintainerFullyStopped(maintainer *Maintainer) bool { + return maintainer.removed.Load() && + heartbeatpb.ComponentState(maintainer.scheduleState.Load()) == heartbeatpb.ComponentState_Stopped +} + +// logRejectedAddMaintainer emits detail only for newer requests blocked by a +// still-running local maintainer. +func logRejectedAddMaintainer(changefeedID common.ChangeFeedID, existing *Maintainer, requestEpoch uint64) { + existingEpoch := existing.currentMaintainerEpoch() + if requestEpoch <= existingEpoch || isMaintainerFullyStopped(existing) { + return + } + log.Warn("reject add maintainer request because existing maintainer is still running", + zap.Stringer("changefeedID", changefeedID), + zap.Uint64("requestMaintainerEpoch", requestEpoch), + zap.Uint64("existingMaintainerEpoch", existingEpoch), + zap.Bool("existingRemoved", existing.removed.Load()), + zap.String("existingState", heartbeatpb.ComponentState(existing.scheduleState.Load()).String())) +} + // handleRemoveMaintainer handles both normal remove and cascade-remove flows. func (p *managerMaintainerSet) handleRemoveMaintainer(msg *messaging.TargetMessage) *heartbeatpb.MaintainerStatus { req := msg.Message[0].(*heartbeatpb.RemoveMaintainerRequest) @@ -227,15 +324,28 @@ func (p *managerMaintainerSet) handleRemoveMaintainer(msg *messaging.TargetMessa zap.Stringer("changefeedID", changefeedID), zap.Any("request", req)) return &heartbeatpb.MaintainerStatus{ - ChangefeedID: req.GetId(), - State: heartbeatpb.ComponentState_Stopped, + ChangefeedID: req.GetId(), + State: heartbeatpb.ComponentState_Stopped, + MaintainerEpoch: req.MaintainerEpoch, } } // It's cascade remove, we should remove the dispatcher from all node. // Here we create a maintainer to run the remove dispatcher logic. - maintainer = NewMaintainerForRemove(changefeedID, p.conf, p.nodeInfo, p.taskScheduler, req.KeyspaceId) - p.registry.Store(changefeedID, maintainer) + p.registryMu.Lock() + maintainer, ok = p.registry.Load(changefeedID) + if !ok { + maintainer = NewMaintainerForRemove( + changefeedID, + p.conf, + p.nodeInfo, + p.taskScheduler, + req.KeyspaceId, + req.MaintainerEpoch, + ) + p.registry.Store(changefeedID, maintainer) + } + p.registryMu.Unlock() } maintainer.(*Maintainer).pushEvent(&Event{ changefeedID: changefeedID, @@ -270,19 +380,33 @@ func (p *managerMaintainerSet) buildHeartbeat() *heartbeatpb.MaintainerHeartbeat // cleanupRemovedMaintainers closes maintainers after their remove flow has finished. func (p *managerMaintainerSet) cleanupRemovedMaintainers() { p.registry.Range(func(key, value interface{}) bool { - cf := value.(*Maintainer) - if cf.removed.Load() { - cf.Close() - log.Info("maintainer removed, remove it from dynamic stream", - zap.Stringer("changefeedID", cf.changefeedID), - zap.Uint64("checkpointTs", cf.getWatermark().CheckpointTs), - ) - p.registry.Delete(key) - } + p.cleanupRemovedMaintainer(key, value) return true }) } +// cleanupRemovedMaintainer removes only the registry entry that still owns the +// shared changefeed metric labels observed by Range. +func (p *managerMaintainerSet) cleanupRemovedMaintainer(key, value interface{}) { + p.registryMu.Lock() + defer p.registryMu.Unlock() + + cf := value.(*Maintainer) + if !cf.removed.Load() { + return + } + // Range can observe a removed maintainer just before a newer epoch replaces it. + // Only the value still stored in the registry owns the shared metric labels. + if !p.registry.CompareAndDelete(key, cf) { + return + } + cf.Close() + log.Info("maintainer removed, remove it from dynamic stream", + zap.Stringer("changefeedID", cf.changefeedID), + zap.Uint64("checkpointTs", cf.getWatermark().CheckpointTs), + ) +} + // applyDispatcherDrainTarget fans out the latest node-scoped drain target to // every currently active maintainer. func (p *managerMaintainerSet) applyDispatcherDrainTarget(target node.ID, epoch uint64) { diff --git a/maintainer/maintainer_manager_test.go b/maintainer/maintainer_manager_test.go index f6efa3baf8..fb957c37b0 100644 --- a/maintainer/maintainer_manager_test.go +++ b/maintainer/maintainer_manager_test.go @@ -17,6 +17,7 @@ import ( "context" "encoding/json" "net" + "strconv" "sync" "testing" "time" @@ -37,10 +38,12 @@ import ( "github.com/pingcap/ticdc/pkg/liveness" "github.com/pingcap/ticdc/pkg/messaging" "github.com/pingcap/ticdc/pkg/messaging/proto" + "github.com/pingcap/ticdc/pkg/metrics" "github.com/pingcap/ticdc/pkg/node" "github.com/pingcap/ticdc/pkg/orchestrator" "github.com/pingcap/ticdc/pkg/pdutil" "github.com/pingcap/ticdc/server/watcher" + promtestutil "github.com/prometheus/client_golang/prometheus/testutil" "github.com/stretchr/testify/require" "google.golang.org/grpc" ) @@ -71,6 +74,211 @@ func runCancelable(t *testing.T, ctx context.Context, run func(context.Context) }) } +func newAddMaintainerRequestForEpoch( + t *testing.T, + cfID common.ChangeFeedID, + configEpoch uint64, + requestEpoch uint64, +) *heartbeatpb.AddMaintainerRequest { + t.Helper() + + info := &config.ChangeFeedInfo{ + ChangefeedID: cfID, + Config: config.GetDefaultReplicaConfig(), + Epoch: configEpoch, + } + data, err := json.Marshal(info) + require.NoError(t, err) + return &heartbeatpb.AddMaintainerRequest{ + Id: cfID.ToPB(), + Config: data, + CheckpointTs: 10, + KeyspaceId: common.DefaultKeyspaceID, + MaintainerEpoch: requestEpoch, + } +} + +func newManagerMaintainerSetForAddTest(t *testing.T) *managerMaintainerSet { + t.Helper() + + testutil.SetUpTestServices(t) + selfNode := node.NewInfo("", "") + maintainers := newManagerMaintainerSet(config.NewDefaultSchedulerConfig(), selfNode) + t.Cleanup(maintainers.closeAll) + return maintainers +} + +func cleanupMaintainerMetricsForTest(t *testing.T, cfID common.ChangeFeedID) { + t.Helper() + + cleanup := func() { + keyspace := cfID.Keyspace() + name := cfID.Name() + metrics.MaintainerGauge.DeleteLabelValues(keyspace, name) + metrics.MaintainerCheckpointTsGauge.DeleteLabelValues(keyspace, name) + metrics.MaintainerCheckpointTsLagGauge.DeleteLabelValues(keyspace, name) + metrics.MaintainerHandleEventDuration.DeleteLabelValues(keyspace, name) + metrics.MaintainerEventChLenGauge.DeleteLabelValues(keyspace, name) + metrics.MaintainerResolvedTsGauge.DeleteLabelValues(keyspace, name) + metrics.MaintainerResolvedTsLagGauge.DeleteLabelValues(keyspace, name) + + metrics.TableStateGauge.DeleteLabelValues(keyspace, name, "Absent", "default") + metrics.TableStateGauge.DeleteLabelValues(keyspace, name, "Absent", "redo") + metrics.TableStateGauge.DeleteLabelValues(keyspace, name, "Working", "default") + metrics.TableStateGauge.DeleteLabelValues(keyspace, name, "Working", "redo") + + metrics.ScheduleTaskGauge.DeleteLabelValues(keyspace, name, "default") + metrics.ScheduleTaskGauge.DeleteLabelValues(keyspace, name, "redo") + metrics.SpanCountGauge.DeleteLabelValues(keyspace, name, "default") + metrics.SpanCountGauge.DeleteLabelValues(keyspace, name, "redo") + metrics.TableCountGauge.DeleteLabelValues(keyspace, name, "default") + metrics.TableCountGauge.DeleteLabelValues(keyspace, name, "redo") + } + cleanup() + t.Cleanup(cleanup) +} + +func TestManagerMaintainerSet_AddMaintainerRejectsLiveNewerEpoch(t *testing.T) { + maintainers := newManagerMaintainerSetForAddTest(t) + cfID := common.NewChangeFeedIDWithName("reject-live-newer-epoch", common.DefaultKeyspaceName) + cleanupMaintainerMetricsForTest(t, cfID) + noDrainTarget := func() (node.ID, uint64) { return "", 0 } + keyspace, changefeed := cfID.Keyspace(), cfID.Name() + + maintainers.handleAddMaintainer(newAddMaintainerRequestForEpoch(t, cfID, 1, 1), noDrainTarget) + oldMaintainer, ok := maintainers.getMaintainer(cfID) + require.True(t, ok) + require.Equal(t, uint64(1), oldMaintainer.currentMaintainerEpoch()) + require.Equal(t, float64(1), promtestutil.ToFloat64(metrics.MaintainerGauge.WithLabelValues(keyspace, changefeed))) + + maintainers.handleAddMaintainer(newAddMaintainerRequestForEpoch(t, cfID, 2, 2), noDrainTarget) + currentMaintainer, ok := maintainers.getMaintainer(cfID) + require.True(t, ok) + require.True(t, oldMaintainer == currentMaintainer) + require.Equal(t, uint64(1), currentMaintainer.currentMaintainerEpoch()) + require.Equal(t, float64(1), promtestutil.ToFloat64(metrics.MaintainerGauge.WithLabelValues(keyspace, changefeed))) + + currentMaintainer.checkpointTsGauge.Set(123) + require.Equal(t, float64(123), promtestutil.ToFloat64(metrics.MaintainerCheckpointTsGauge.WithLabelValues(keyspace, changefeed))) +} + +func TestManagerMaintainerSet_AddMaintainerAfterStoppedKeepsReplacement(t *testing.T) { + maintainers := newManagerMaintainerSetForAddTest(t) + cfID := common.NewChangeFeedIDWithName("stopped-maintainer-replacement", common.DefaultKeyspaceName) + cleanupMaintainerMetricsForTest(t, cfID) + noDrainTarget := func() (node.ID, uint64) { return "", 0 } + keyspace, changefeed := cfID.Keyspace(), cfID.Name() + + maintainers.handleAddMaintainer(newAddMaintainerRequestForEpoch(t, cfID, 1, 1), noDrainTarget) + oldMaintainer, ok := maintainers.getMaintainer(cfID) + require.True(t, ok) + + oldMaintainer.markRemoved() + oldMaintainer.scheduleState.Store(int32(heartbeatpb.ComponentState_Stopped)) + + maintainers.handleAddMaintainer(newAddMaintainerRequestForEpoch(t, cfID, 2, 2), noDrainTarget) + currentMaintainer, ok := maintainers.getMaintainer(cfID) + require.True(t, ok) + require.False(t, oldMaintainer == currentMaintainer) + require.Equal(t, uint64(2), currentMaintainer.currentMaintainerEpoch()) + require.Equal(t, float64(1), promtestutil.ToFloat64(metrics.MaintainerGauge.WithLabelValues(keyspace, changefeed))) + + currentMaintainer.checkpointTsGauge.Set(456) + maintainers.cleanupRemovedMaintainer(cfID, oldMaintainer) + maintainerAfterStaleCleanup, ok := maintainers.getMaintainer(cfID) + require.True(t, ok) + require.True(t, currentMaintainer == maintainerAfterStaleCleanup) + require.Equal(t, float64(456), promtestutil.ToFloat64(metrics.MaintainerCheckpointTsGauge.WithLabelValues(keyspace, changefeed))) + + currentMaintainer.markRemoved() + currentMaintainer.scheduleState.Store(int32(heartbeatpb.ComponentState_Stopped)) + maintainers.cleanupRemovedMaintainer(cfID, currentMaintainer) + _, ok = maintainers.getMaintainer(cfID) + require.False(t, ok) +} + +func TestManagerMaintainerSet_AddMaintainerKeepsCompatibilityEpoch(t *testing.T) { + maintainers := newManagerMaintainerSetForAddTest(t) + cfID := common.NewChangeFeedIDWithName("test", common.DefaultKeyspaceName) + noDrainTarget := func() (node.ID, uint64) { return "", 0 } + + maintainers.handleAddMaintainer(newAddMaintainerRequestForEpoch(t, cfID, 3, 0), noDrainTarget) + compatMaintainer, ok := maintainers.getMaintainer(cfID) + require.True(t, ok) + require.Zero(t, compatMaintainer.currentMaintainerEpoch()) + + maintainers.handleAddMaintainer(newAddMaintainerRequestForEpoch(t, cfID, 4, 0), noDrainTarget) + compatMaintainerAfterRetry, ok := maintainers.getMaintainer(cfID) + require.True(t, ok) + require.True(t, compatMaintainer == compatMaintainerAfterRetry) +} + +func TestManagerMaintainerSet_AddMaintainerRejectsOlderEpoch(t *testing.T) { + maintainers := newManagerMaintainerSetForAddTest(t) + cfID := common.NewChangeFeedIDWithName("test", common.DefaultKeyspaceName) + noDrainTarget := func() (node.ID, uint64) { return "", 0 } + + maintainers.handleAddMaintainer(newAddMaintainerRequestForEpoch(t, cfID, 2, 2), noDrainTarget) + currentMaintainer, ok := maintainers.getMaintainer(cfID) + require.True(t, ok) + require.Equal(t, uint64(2), currentMaintainer.currentMaintainerEpoch()) + + maintainers.handleAddMaintainer(newAddMaintainerRequestForEpoch(t, cfID, 1, 1), noDrainTarget) + maintainerAfterOldAdd, ok := maintainers.getMaintainer(cfID) + require.True(t, ok) + require.True(t, currentMaintainer == maintainerAfterOldAdd) + + maintainers.handleAddMaintainer(newAddMaintainerRequestForEpoch(t, cfID, 3, 0), noDrainTarget) + maintainerAfterCompatAdd, ok := maintainers.getMaintainer(cfID) + require.True(t, ok) + require.True(t, currentMaintainer == maintainerAfterCompatAdd) +} + +func TestManagerMaintainerSet_AddMaintainerDoesNotCreateRejectedDuplicate(t *testing.T) { + maintainers := newManagerMaintainerSetForAddTest(t) + cfID := common.NewChangeFeedIDWithName("test", common.DefaultKeyspaceName) + noDrainTarget := func() (node.ID, uint64) { return "", 0 } + + maintainers.handleAddMaintainer(newAddMaintainerRequestForEpoch(t, cfID, 2, 2), noDrainTarget) + currentMaintainer, ok := maintainers.getMaintainer(cfID) + require.True(t, ok) + require.Equal(t, uint64(2), currentMaintainer.currentMaintainerEpoch()) + + rejectedEpochs := []uint64{3, 2, 1, 0} + for _, requestEpoch := range rejectedEpochs { + t.Run("requestEpoch"+strconv.FormatUint(requestEpoch, 10), func(t *testing.T) { + require.False(t, maintainers.mayRegisterMaintainerForAdd(cfID, requestEpoch)) + registeredMaintainer := maintainers.registerMaintainerForAdd(cfID, requestEpoch, func() *Maintainer { + t.Fatalf("registerMaintainerForAdd created maintainer for rejected request epoch %d", requestEpoch) + return nil + }) + require.Nil(t, registeredMaintainer) + maintainerAfterRejectedAdd, ok := maintainers.getMaintainer(cfID) + require.True(t, ok) + require.True(t, currentMaintainer == maintainerAfterRejectedAdd) + }) + } +} + +func TestManagerMaintainerSet_RemoveMissingMaintainerReportsRequestEpoch(t *testing.T) { + maintainers := newManagerMaintainerSetForAddTest(t) + cfID := common.NewChangeFeedIDWithName("remove-missing", common.DefaultKeyspaceName) + req := &heartbeatpb.RemoveMaintainerRequest{ + Id: cfID.ToPB(), + MaintainerEpoch: 7, + } + msg := messaging.NewSingleTargetMessage( + node.ID("self"), + messaging.MaintainerManagerTopic, + req, + ) + + status := maintainers.handleRemoveMaintainer(msg) + require.NotNil(t, status) + require.Equal(t, heartbeatpb.ComponentState_Stopped, status.State) + require.Equal(t, uint64(7), status.MaintainerEpoch) +} + // This is a integration test for maintainer manager, it may consume a lot of time. // scale out/in close, add/remove tables func TestMaintainerSchedulesNodeChanges(t *testing.T) { diff --git a/maintainer/maintainer_test.go b/maintainer/maintainer_test.go index 434221c4ce..242c25af2f 100644 --- a/maintainer/maintainer_test.go +++ b/maintainer/maintainer_test.go @@ -138,9 +138,10 @@ func (m *mockDispatcherManager) onBootstrapRequest(msg *messaging.TargetMessage) req := msg.Message[0].(*heartbeatpb.MaintainerBootstrapRequest) m.maintainerID = msg.From response := &heartbeatpb.MaintainerBootstrapResponse{ - ChangefeedID: req.ChangefeedID, - Spans: m.bootstrapTables, - CheckpointTs: req.StartTs, + ChangefeedID: req.ChangefeedID, + Spans: m.bootstrapTables, + CheckpointTs: req.StartTs, + MaintainerEpoch: req.MaintainerEpoch, } m.changefeedID = req.ChangefeedID m.checkpointTs = req.StartTs @@ -171,6 +172,7 @@ func (m *mockDispatcherManager) onPostBootstrapRequest(msg *messaging.TargetMess ChangefeedID: req.ChangefeedID, TableTriggerEventDispatcherId: req.TableTriggerEventDispatcherId, Err: nil, + MaintainerEpoch: req.MaintainerEpoch, } err := m.mc.SendCommand(messaging.NewSingleTargetMessage( m.maintainerID, @@ -240,8 +242,9 @@ func (m *mockDispatcherManager) onDispatchRequest( func (m *mockDispatcherManager) onMaintainerCloseRequest(msg *messaging.TargetMessage) { _ = m.mc.SendCommand(messaging.NewSingleTargetMessage(msg.From, messaging.MaintainerTopic, &heartbeatpb.MaintainerCloseResponse{ - ChangefeedID: msg.Message[0].(*heartbeatpb.MaintainerCloseRequest).ChangefeedID, - Success: true, + ChangefeedID: msg.Message[0].(*heartbeatpb.MaintainerCloseRequest).ChangefeedID, + Success: true, + MaintainerEpoch: msg.Message[0].(*heartbeatpb.MaintainerCloseRequest).MaintainerEpoch, })) } @@ -260,6 +263,67 @@ func (m *mockDispatcherManager) sendHeartbeat() { } } +func TestMaintainerPostBootstrapResponseRequiresCurrentEpoch(t *testing.T) { + cfID := common.NewChangeFeedIDWithName("test", common.DefaultKeyspaceName) + m := &Maintainer{ + changefeedID: cfID, + info: &config.ChangeFeedInfo{Epoch: 2}, + postBootstrapMsg: &heartbeatpb.MaintainerPostBootstrapRequest{ + ChangefeedID: cfID.ToPB(), + MaintainerEpoch: 2, + }, + } + + m.onMaintainerPostBootstrapResponse(messaging.NewSingleTargetMessage( + node.ID("current"), + messaging.MaintainerManagerTopic, + &heartbeatpb.MaintainerPostBootstrapResponse{ + ChangefeedID: cfID.ToPB(), + MaintainerEpoch: 1, + }, + )) + require.NotNil(t, m.postBootstrapMsg) + + m.onMaintainerPostBootstrapResponse(messaging.NewSingleTargetMessage( + node.ID("current"), + messaging.MaintainerManagerTopic, + &heartbeatpb.MaintainerPostBootstrapResponse{ + ChangefeedID: cfID.ToPB(), + MaintainerEpoch: 2, + }, + )) + require.Nil(t, m.postBootstrapMsg) +} + +func TestMaintainerEpochRequestRequiresCompatOrCurrentEpoch(t *testing.T) { + compatMaintainer := &Maintainer{info: &config.ChangeFeedInfo{}} + require.True(t, compatMaintainer.isMaintainerEpochRequestAllowed(0)) + require.True(t, compatMaintainer.isMaintainerEpochRequestAllowed(2)) + + strictMaintainer := &Maintainer{info: &config.ChangeFeedInfo{Epoch: 2}} + require.False(t, strictMaintainer.isMaintainerEpochRequestAllowed(0)) + require.False(t, strictMaintainer.isMaintainerEpochRequestAllowed(1)) + require.True(t, strictMaintainer.isMaintainerEpochRequestAllowed(2)) +} + +func TestMaintainerCloseResponseIgnoredBeforeRemoving(t *testing.T) { + cfID := common.NewChangeFeedIDWithName("test", common.DefaultKeyspaceName) + m := &Maintainer{ + changefeedID: cfID, + info: &config.ChangeFeedInfo{Epoch: 2}, + closedNodes: make(map[node.ID]struct{}), + } + + m.onMaintainerCloseResponse(node.ID("old"), &heartbeatpb.MaintainerCloseResponse{ + ChangefeedID: cfID.ToPB(), + Success: true, + MaintainerEpoch: 0, + }) + + require.Empty(t, m.closedNodes) + require.False(t, m.removing.Load()) +} + func TestMaintainerSchedule(t *testing.T) { // This test exercises a single-node maintainer lifecycle: // 1) Bootstrap a changefeed via the dispatcher manager mock. @@ -383,6 +447,7 @@ func TestMaintainer_GetMaintainerStatusUsesCommittedCheckpoint(t *testing.T) { m := &Maintainer{ changefeedID: cfID, + info: &config.ChangeFeedInfo{Epoch: 3}, controller: &Controller{ spanController: spanController, }, @@ -398,6 +463,7 @@ func TestMaintainer_GetMaintainerStatusUsesCommittedCheckpoint(t *testing.T) { status := m.GetMaintainerStatus() require.Equal(t, uint64(20), status.CheckpointTs) require.Equal(t, uint64(50), status.LastSyncedTs) + require.Equal(t, uint64(3), status.MaintainerEpoch) } func TestMaintainerHeartbeatDuringRemovingSkipsFailoverRecovery(t *testing.T) { @@ -421,7 +487,7 @@ func TestMaintainerHeartbeatDuringRemovingSkipsFailoverRecovery(t *testing.T) { }, captureID, false) refresher := replica.NewRegionCountRefresher(cfID, time.Minute) controller := NewController(cfID, 10, &mockThreadPool{}, - config.GetDefaultReplicaConfig(), ddlSpan, nil, 1000, 0, refresher, common.DefaultKeyspace, false, testBalanceMoveBatchSize) + config.GetDefaultReplicaConfig(), ddlSpan, nil, 1000, 0, refresher, common.DefaultKeyspace, false, testBalanceMoveBatchSize, 1) totalSpan := common.TableIDToComparableSpan(common.DefaultKeyspaceID, 1) dispatcherID := common.NewDispatcherID() @@ -611,6 +677,7 @@ func TestMaintainerCalculateNewCheckpointTs(t *testing.T) { replicaSet, selfNodeID, heartbeatpb.OperatorType_O_Add, + m.controller.currentMaintainerEpoch(), ))) m.removing.Store(true) diff --git a/maintainer/operator/operator_add.go b/maintainer/operator/operator_add.go index d9abe841f4..ef36cee6d5 100644 --- a/maintainer/operator/operator_add.go +++ b/maintainer/operator/operator_add.go @@ -57,7 +57,8 @@ type AddDispatcherOperator struct { // (for example merge) don't go through ScheduleDispatcherRequest and therefore don't need operatorType here. operatorType heartbeatpb.OperatorType - sendThrottler sendThrottler + maintainerEpoch uint64 + sendThrottler sendThrottler } func NewAddDispatcherOperator( @@ -65,13 +66,15 @@ func NewAddDispatcherOperator( replicaSet *replica.SpanReplication, dest node.ID, operatorType heartbeatpb.OperatorType, + maintainerEpoch uint64, ) *AddDispatcherOperator { return &AddDispatcherOperator{ - replicaSet: replicaSet, - dest: dest, - spanController: spanController, - operatorType: operatorType, - sendThrottler: newSendThrottler(), + replicaSet: replicaSet, + dest: dest, + spanController: spanController, + operatorType: operatorType, + maintainerEpoch: maintainerEpoch, + sendThrottler: newSendThrottler(), } } @@ -107,7 +110,7 @@ func (m *AddDispatcherOperator) Schedule() *messaging.TargetMessage { if !m.sendThrottler.shouldSend() { return nil } - return m.replicaSet.NewAddDispatcherMessage(m.dest, m.operatorType) + return m.replicaSet.NewAddDispatcherMessage(m.dest, m.operatorType, m.maintainerEpoch) } // OnNodeRemove is called when node offline, and the replicaset must already move to absent status and will be scheduled again diff --git a/maintainer/operator/operator_add_test.go b/maintainer/operator/operator_add_test.go index 6e8cc8ef7b..21aa53fcf5 100644 --- a/maintainer/operator/operator_add_test.go +++ b/maintainer/operator/operator_add_test.go @@ -56,7 +56,7 @@ func TestAddOperator_DestNodeRemoved(t *testing.T) { absentReplicaSet := newAddTestReplicaSet(spanController, changefeedID) - op := NewAddDispatcherOperator(spanController, absentReplicaSet, nodeB, heartbeatpb.OperatorType_O_Add) + op := NewAddDispatcherOperator(spanController, absentReplicaSet, nodeB, heartbeatpb.OperatorType_O_Add, 7) require.NotNil(t, op) op.Start() @@ -66,6 +66,8 @@ func TestAddOperator_DestNodeRemoved(t *testing.T) { msg := op.Schedule() require.NotNil(t, msg) require.Equal(t, nodeB.String(), msg.To.String()) + req := msg.Message[0].(*heartbeatpb.ScheduleDispatcherRequest) + require.Equal(t, uint64(7), req.MaintainerEpoch) // Node B is removed before it reports working status op.OnNodeRemove(nodeB) @@ -87,7 +89,7 @@ func TestAddOperator_DestReportsWorking(t *testing.T) { spanController, changefeedID, _, _, nodeB := setupTestEnvironment(t) absentReplicaSet := newAddTestReplicaSet(spanController, changefeedID) - op := NewAddDispatcherOperator(spanController, absentReplicaSet, nodeB, heartbeatpb.OperatorType_O_Add) + op := NewAddDispatcherOperator(spanController, absentReplicaSet, nodeB, heartbeatpb.OperatorType_O_Add, 7) require.NotNil(t, op) op.Start() @@ -119,7 +121,7 @@ func TestAddOperator_DestReportsRemoved(t *testing.T) { spanController, changefeedID, _, _, nodeB := setupTestEnvironment(t) absentReplicaSet := newAddTestReplicaSet(spanController, changefeedID) - op := NewAddDispatcherOperator(spanController, absentReplicaSet, nodeB, heartbeatpb.OperatorType_O_Add) + op := NewAddDispatcherOperator(spanController, absentReplicaSet, nodeB, heartbeatpb.OperatorType_O_Add, 7) require.NotNil(t, op) op.Start() @@ -149,7 +151,7 @@ func TestAddOperator_StoppedStatusIgnored(t *testing.T) { spanController, changefeedID, _, _, nodeB := setupTestEnvironment(t) absentReplicaSet := newAddTestReplicaSet(spanController, changefeedID) - op := NewAddDispatcherOperator(spanController, absentReplicaSet, nodeB, heartbeatpb.OperatorType_O_Add) + op := NewAddDispatcherOperator(spanController, absentReplicaSet, nodeB, heartbeatpb.OperatorType_O_Add, 7) require.NotNil(t, op) op.Start() @@ -176,7 +178,7 @@ func TestAddOperator_TaskRemovedDoesNotReintroduceSpan(t *testing.T) { spanController, changefeedID, _, _, nodeB := setupTestEnvironment(t) absentReplicaSet := newAddTestReplicaSet(spanController, changefeedID) - op := NewAddDispatcherOperator(spanController, absentReplicaSet, nodeB, heartbeatpb.OperatorType_O_Add) + op := NewAddDispatcherOperator(spanController, absentReplicaSet, nodeB, heartbeatpb.OperatorType_O_Add, 7) require.NotNil(t, op) op.Start() diff --git a/maintainer/operator/operator_controller.go b/maintainer/operator/operator_controller.go index f876b41dd6..0158d15f67 100644 --- a/maintainer/operator/operator_controller.go +++ b/maintainer/operator/operator_controller.go @@ -16,13 +16,13 @@ package operator import ( "container/heap" "sync" + "sync/atomic" "time" "github.com/pingcap/log" "github.com/pingcap/ticdc/heartbeatpb" "github.com/pingcap/ticdc/maintainer/replica" "github.com/pingcap/ticdc/maintainer/span" - "github.com/pingcap/ticdc/maintainer/split" "github.com/pingcap/ticdc/pkg/common" appcontext "github.com/pingcap/ticdc/pkg/common/context" "github.com/pingcap/ticdc/pkg/messaging" @@ -45,13 +45,13 @@ var _ operator.Controller[common.DispatcherID, *heartbeatpb.TableSpanStatus] = & // Controller is the operator controller, it manages all operators. // And the Controller is responsible for the execution of the operator. type Controller struct { - role string - changefeedID common.ChangeFeedID - batchSize int - messageCenter messaging.MessageCenter - spanController *span.Controller - nodeManager *watcher.NodeManager - splitter *split.Splitter + role string + changefeedID common.ChangeFeedID + batchSize int + messageCenter messaging.MessageCenter + spanController *span.Controller + nodeManager *watcher.NodeManager + maintainerEpoch atomic.Uint64 // admissionMu serializes removing-mode quiesce with normal operator side effects. // A normal operator must hold the read side from its final allow check through @@ -136,6 +136,16 @@ func (oc *Controller) isQuiescing() bool { return oc.quiescing } +// SetMaintainerEpoch sets the epoch used by scheduler requests. +func (oc *Controller) SetMaintainerEpoch(maintainerEpoch uint64) { + oc.maintainerEpoch.Store(maintainerEpoch) +} + +// MaintainerEpoch returns the epoch used by maintainer-to-dispatcher-manager requests. +func (oc *Controller) MaintainerEpoch() uint64 { + return oc.maintainerEpoch.Load() +} + // Execute poll the operator from the queue and execute it // It will be called in the thread pool. func (oc *Controller) Execute() time.Time { @@ -183,7 +193,12 @@ func (oc *Controller) scheduleOperator(op operator.Operator[common.DispatcherID, func (oc *Controller) RemoveTasksBySchemaID(schemaID int64) { tasks := oc.spanController.GetRemoveTasksBySchemaID(schemaID) for _, task := range tasks { - oc.removeReplicaSet(newRemoveDispatcherOperator(oc.spanController, task, heartbeatpb.OperatorType_O_Remove)) + oc.removeReplicaSet(newRemoveDispatcherOperator( + oc.spanController, + task, + heartbeatpb.OperatorType_O_Remove, + oc.MaintainerEpoch(), + )) } oc.spanController.RemoveBySchemaID(schemaID) } @@ -201,7 +216,12 @@ func (oc *Controller) RemoveTasksBySchemaID(schemaID int64) { func (oc *Controller) RemoveTasksByTableIDs(tables ...int64) { tasks := oc.spanController.GetRemoveTasksByTableIDs(tables...) for _, task := range tasks { - oc.removeReplicaSet(newRemoveDispatcherOperator(oc.spanController, task, heartbeatpb.OperatorType_O_Remove)) + oc.removeReplicaSet(newRemoveDispatcherOperator( + oc.spanController, + task, + heartbeatpb.OperatorType_O_Remove, + oc.MaintainerEpoch(), + )) } oc.spanController.RemoveByTableIDs(tables...) } @@ -524,7 +544,7 @@ func (oc *Controller) checkAffectedNodes(op operator.Operator[common.DispatcherI } func (oc *Controller) NewMoveOperator(replicaSet *replica.SpanReplication, origin, dest node.ID) operator.Operator[common.DispatcherID, *heartbeatpb.TableSpanStatus] { - return NewMoveDispatcherOperator(oc.spanController, replicaSet, origin, dest) + return NewMoveDispatcherOperator(oc.spanController, replicaSet, origin, dest, oc.MaintainerEpoch()) } func checkMergeOperator(affectedReplicaSets []*replica.SpanReplication) bool { @@ -585,7 +605,7 @@ func (oc *Controller) AddMergeOperator( } } - mergeOperator := NewMergeDispatcherOperator(oc.spanController, affectedReplicaSets, operators) + mergeOperator := NewMergeDispatcherOperator(oc.spanController, affectedReplicaSets, operators, oc.MaintainerEpoch()) ret := oc.AddOperator(mergeOperator) if !ret { log.Error("failed to add merge dispatcher operator", diff --git a/maintainer/operator/operator_controller_test.go b/maintainer/operator/operator_controller_test.go index 576a6bc532..32f9a77f3f 100644 --- a/maintainer/operator/operator_controller_test.go +++ b/maintainer/operator/operator_controller_test.go @@ -62,8 +62,8 @@ func TestController_CountInflightDrainMovesFromNode(t *testing.T) { }) oc := NewOperatorController(changefeedID, spanController, 1, common.DefaultMode) - require.True(t, oc.AddOperator(NewMoveDispatcherOperator(spanController, replicaSet, nodeA, nodeB))) - require.True(t, oc.AddOperator(NewMoveDispatcherOperator(spanController, otherReplicaSet, nodeB, nodeA))) + require.True(t, oc.AddOperator(NewMoveDispatcherOperator(spanController, replicaSet, nodeA, nodeB, 7))) + require.True(t, oc.AddOperator(NewMoveDispatcherOperator(spanController, otherReplicaSet, nodeB, nodeA, 7))) require.Equal(t, 1, oc.CountInflightDrainMovesFromNode(nodeA)) require.Equal(t, 1, oc.CountInflightDrainMovesFromNode(nodeB)) @@ -280,7 +280,7 @@ func TestController_PostFinishCalledOnceOnReplace(t *testing.T) { }() <-op.isFinishedCalled - oc.removeReplicaSet(newRemoveDispatcherOperator(spanController, replicaSet, heartbeatpb.OperatorType_O_Remove)) + oc.removeReplicaSet(newRemoveDispatcherOperator(spanController, replicaSet, heartbeatpb.OperatorType_O_Remove, 7)) wg.Wait() require.Equal(t, int32(1), op.postFinishCount.Load()) @@ -327,6 +327,16 @@ func TestController_AddMergeOperatorFailureCleansOccupyOperators(t *testing.T) { require.Equal(t, 1, oc.OperatorSize()) } +func TestMergeDispatcherOperatorScheduleMaintainerEpoch(t *testing.T) { + spanController, toMergedReplicaSets, occupyOperators, _ := setupMergeTestEnvironment(t) + + op := NewMergeDispatcherOperator(spanController, toMergedReplicaSets, occupyOperators, 7) + msg := op.Schedule() + + req := msg.Message[0].(*heartbeatpb.MergeDispatcherRequest) + require.Equal(t, uint64(7), req.MaintainerEpoch) +} + func TestController_RemoveReplicaSet_ReplacesRemoveOperatorOnTaskRemoved(t *testing.T) { // Scenario: the barrier can enqueue the same remove task multiple times during failover/bootstrap. // Steps: @@ -346,10 +356,11 @@ func TestController_RemoveReplicaSet_ReplacesRemoveOperatorOnTaskRemoved(t *test spanController, replicaSet, heartbeatpb.OperatorType_O_Move, + 7, func() { postFinishCount.Add(1) }, ))) - oc.removeReplicaSet(newRemoveDispatcherOperator(spanController, replicaSet, heartbeatpb.OperatorType_O_Remove)) + oc.removeReplicaSet(newRemoveDispatcherOperator(spanController, replicaSet, heartbeatpb.OperatorType_O_Remove, 7)) require.Equal(t, int32(0), postFinishCount.Load()) require.NotNil(t, oc.GetOperator(replicaSet.ID)) diff --git a/maintainer/operator/operator_merge.go b/maintainer/operator/operator_merge.go index fb7b281de3..ea4797f6c6 100644 --- a/maintainer/operator/operator_merge.go +++ b/maintainer/operator/operator_merge.go @@ -41,10 +41,11 @@ import ( // - OnNodeRemove(originNode): abort merge, mark old replicas absent, and remove the merged replica. // - OnTaskRemoved(): abort merge due to DDL and clean up without clearing node binding of old replicas. type MergeDispatcherOperator struct { - spanController *span.Controller - originNode node.ID - id common.DispatcherID - dispatcherIDs []*heartbeatpb.DispatcherID + spanController *span.Controller + originNode node.ID + id common.DispatcherID + dispatcherIDs []*heartbeatpb.DispatcherID + maintainerEpoch uint64 // aborted indicates the merge should not be applied successfully. It can be set by OnNodeRemove // or OnTaskRemoved. When aborted is true, PostFinish follows the abort path. @@ -101,6 +102,7 @@ func NewMergeDispatcherOperator( spanController *span.Controller, toMergedReplicaSets []*replica.SpanReplication, occupyOperators []operator.Operator[common.DispatcherID, *heartbeatpb.TableSpanStatus], + maintainerEpoch uint64, ) *MergeDispatcherOperator { toMergedSpans := make([]*heartbeatpb.TableSpan, 0, len(toMergedReplicaSets)) for _, replicaSet := range toMergedReplicaSets { @@ -140,6 +142,7 @@ func NewMergeDispatcherOperator( originNode: nodeID, id: newDispatcherID, dispatcherIDs: dispatcherIDs, + maintainerEpoch: maintainerEpoch, toMergedReplicaSets: toMergedReplicaSets, checkpointTs: 0, mergedSpanInfo: spansInfo, @@ -211,6 +214,7 @@ func (m *MergeDispatcherOperator) Schedule() *messaging.TargetMessage { DispatcherIDs: m.dispatcherIDs, MergedDispatcherID: m.id.ToPB(), Mode: m.newReplicaSet.GetMode(), + MaintainerEpoch: m.maintainerEpoch, }) } diff --git a/maintainer/operator/operator_merge_test.go b/maintainer/operator/operator_merge_test.go index 93769c43a0..3bd415031a 100644 --- a/maintainer/operator/operator_merge_test.go +++ b/maintainer/operator/operator_merge_test.go @@ -147,7 +147,7 @@ func setupLargeMergeTestEnvironment( func TestMergeOperator_NodeRemovedBeforeWorking(t *testing.T) { spanController, toMergedReplicaSets, occupyOperators, nodeA := setupMergeTestEnvironment(t) - op := NewMergeDispatcherOperator(spanController, toMergedReplicaSets, occupyOperators) + op := NewMergeDispatcherOperator(spanController, toMergedReplicaSets, occupyOperators, 7) require.NotNil(t, op) op.Start() @@ -188,7 +188,7 @@ func TestMergeOperator_NodeRemovedBeforeWorking(t *testing.T) { func TestMergeOperator_TaskRemovedByDDLBeforeWorking(t *testing.T) { spanController, toMergedReplicaSets, occupyOperators, nodeA := setupMergeTestEnvironment(t) - op := NewMergeDispatcherOperator(spanController, toMergedReplicaSets, occupyOperators) + op := NewMergeDispatcherOperator(spanController, toMergedReplicaSets, occupyOperators, 7) require.NotNil(t, op) op.Start() @@ -218,7 +218,7 @@ func TestMergeOperator_TaskRemovedByDDLBeforeWorking(t *testing.T) { func TestMergeOperator_NewReplicaSetCheckpointTsUsesMinOfMergedReplicas(t *testing.T) { spanController, toMergedReplicaSets, occupyOperators, _ := setupMergeTestEnvironmentWithCheckpointTs(t, 1500, 1000) - op := NewMergeDispatcherOperator(spanController, toMergedReplicaSets, occupyOperators) + op := NewMergeDispatcherOperator(spanController, toMergedReplicaSets, occupyOperators, 7) require.NotNil(t, op) // The merged replica should inherit a safe checkpointTs to avoid regressing global checkpoint. require.Equal(t, uint64(1000), op.newReplicaSet.GetStatus().GetCheckpointTs()) @@ -231,7 +231,7 @@ func TestMergeOperator_NewReplicaSetCheckpointTsUsesMinOfMergedReplicas(t *testi func TestMergeOperator_SuccessfulMerge(t *testing.T) { spanController, toMergedReplicaSets, occupyOperators, nodeA := setupMergeTestEnvironment(t) - op := NewMergeDispatcherOperator(spanController, toMergedReplicaSets, occupyOperators) + op := NewMergeDispatcherOperator(spanController, toMergedReplicaSets, occupyOperators, 7) require.NotNil(t, op) op.Start() @@ -267,7 +267,7 @@ func TestMergeOperator_PostFinishReleasesOccupyAfterRemovingOldReplicas(t *testi spanController, toMergedReplicaSets, occupyOperators, nodeA := setupLargeMergeTestEnvironment(t, 4096) - op := NewMergeDispatcherOperator(spanController, toMergedReplicaSets, occupyOperators) + op := NewMergeDispatcherOperator(spanController, toMergedReplicaSets, occupyOperators, 7) require.NotNil(t, op) op.Start() @@ -311,7 +311,7 @@ func TestMergeOperator_PostFinishReleasesOccupyAfterRemovingOldReplicas(t *testi func TestMergeOperator_NodeRemovedAfterWorking(t *testing.T) { spanController, toMergedReplicaSets, occupyOperators, nodeA := setupMergeTestEnvironment(t) - op := NewMergeDispatcherOperator(spanController, toMergedReplicaSets, occupyOperators) + op := NewMergeDispatcherOperator(spanController, toMergedReplicaSets, occupyOperators, 7) require.NotNil(t, op) op.Start() @@ -348,7 +348,7 @@ func TestMergeOperator_NodeRemovedAfterWorking(t *testing.T) { func TestMergeOperator_TaskRemovedByDDLAfterWorking(t *testing.T) { spanController, toMergedReplicaSets, occupyOperators, nodeA := setupMergeTestEnvironment(t) - op := NewMergeDispatcherOperator(spanController, toMergedReplicaSets, occupyOperators) + op := NewMergeDispatcherOperator(spanController, toMergedReplicaSets, occupyOperators, 7) require.NotNil(t, op) op.Start() diff --git a/maintainer/operator/operator_move.go b/maintainer/operator/operator_move.go index d45dec35bf..aa10836442 100644 --- a/maintainer/operator/operator_move.go +++ b/maintainer/operator/operator_move.go @@ -52,10 +52,11 @@ const ( // MoveDispatcherOperator is an operator to move a table span to the destination dispatcher type MoveDispatcherOperator struct { - replicaSet *replica.SpanReplication - spanController *span.Controller - origin node.ID - dest node.ID + replicaSet *replica.SpanReplication + spanController *span.Controller + origin node.ID + dest node.ID + maintainerEpoch uint64 // State transitions: // removeOrigin --(origin stopped)-> addDest --(dest working)-> doneSuccess @@ -97,13 +98,20 @@ func (m *MoveDispatcherOperator) finishAsAbsent() { m.state = moveStateDoneNoPostFinish } -func NewMoveDispatcherOperator(spanController *span.Controller, replicaSet *replica.SpanReplication, origin, dest node.ID) *MoveDispatcherOperator { +func NewMoveDispatcherOperator( + spanController *span.Controller, + replicaSet *replica.SpanReplication, + origin node.ID, + dest node.ID, + maintainerEpoch uint64, +) *MoveDispatcherOperator { return &MoveDispatcherOperator{ - replicaSet: replicaSet, - origin: origin, - dest: dest, - spanController: spanController, - sendThrottler: newSendThrottler(), + replicaSet: replicaSet, + origin: origin, + dest: dest, + spanController: spanController, + maintainerEpoch: maintainerEpoch, + sendThrottler: newSendThrottler(), } } @@ -152,9 +160,9 @@ func (m *MoveDispatcherOperator) Schedule() *messaging.TargetMessage { switch m.state { case moveStateAddDest: - return m.replicaSet.NewAddDispatcherMessage(m.dest, heartbeatpb.OperatorType_O_Move) + return m.replicaSet.NewAddDispatcherMessage(m.dest, heartbeatpb.OperatorType_O_Move, m.maintainerEpoch) case moveStateRemoveOrigin, moveStateAbortRemoveOrigin: - return m.replicaSet.NewRemoveDispatcherMessage(m.origin, heartbeatpb.OperatorType_O_Move) + return m.replicaSet.NewRemoveDispatcherMessage(m.origin, heartbeatpb.OperatorType_O_Move, m.maintainerEpoch) default: return nil } diff --git a/maintainer/operator/operator_move_test.go b/maintainer/operator/operator_move_test.go index 60558a9a07..a8c446e9d6 100644 --- a/maintainer/operator/operator_move_test.go +++ b/maintainer/operator/operator_move_test.go @@ -88,7 +88,7 @@ func setupTestEnvironment(t *testing.T) (*span.Controller, common.ChangeFeedID, func TestMoveOperator_DestNodeRemovedBeforeOriginStopped(t *testing.T) { spanController, _, replicaSet, nodeA, nodeB := setupTestEnvironment(t) - op := NewMoveDispatcherOperator(spanController, replicaSet, nodeA, nodeB) + op := NewMoveDispatcherOperator(spanController, replicaSet, nodeA, nodeB, 7) require.NotNil(t, op) op.Start() @@ -109,6 +109,7 @@ func TestMoveOperator_DestNodeRemovedBeforeOriginStopped(t *testing.T) { require.True(t, ok) require.Equal(t, heartbeatpb.ScheduleAction_Remove, scheduleMsg.ScheduleAction) require.Equal(t, replicaSet.ID.ToPB(), scheduleMsg.Config.DispatcherID) + require.Equal(t, uint64(7), scheduleMsg.MaintainerEpoch) absentSizeBefore := spanController.GetAbsentSize() nonWorkingStatus := &heartbeatpb.TableSpanStatus{ @@ -131,7 +132,7 @@ func TestMoveOperator_DestNodeRemovedBeforeOriginStopped(t *testing.T) { func TestMoveOperator_DestNodeRemovedAfterOriginStopped(t *testing.T) { spanController, _, replicaSet, nodeA, nodeB := setupTestEnvironment(t) - op := NewMoveDispatcherOperator(spanController, replicaSet, nodeA, nodeB) + op := NewMoveDispatcherOperator(spanController, replicaSet, nodeA, nodeB, 7) require.NotNil(t, op) op.Start() @@ -161,7 +162,7 @@ func TestMoveOperator_DestNodeRemovedAfterOriginStopped(t *testing.T) { func TestMoveOperator_OriginNodeRemovedBeforeOriginStopped(t *testing.T) { spanController, _, replicaSet, nodeA, nodeB := setupTestEnvironment(t) - op := NewMoveDispatcherOperator(spanController, replicaSet, nodeA, nodeB) + op := NewMoveDispatcherOperator(spanController, replicaSet, nodeA, nodeB, 7) require.NotNil(t, op) op.Start() @@ -204,7 +205,7 @@ func TestMoveOperator_OriginNodeRemovedBeforeOriginStopped(t *testing.T) { func TestMoveOperator_OriginNodeRemovedAfterOriginStopped(t *testing.T) { spanController, _, replicaSet, nodeA, nodeB := setupTestEnvironment(t) - op := NewMoveDispatcherOperator(spanController, replicaSet, nodeA, nodeB) + op := NewMoveDispatcherOperator(spanController, replicaSet, nodeA, nodeB, 7) require.NotNil(t, op) op.Start() @@ -254,7 +255,7 @@ func TestMoveOperator_BothNodesRemovedBeforeStartDoesNotLeaveSchedulingWithoutNo setAliveNodes(nodeManager, map[node.ID]*node.Info{}) oc := NewOperatorController(changefeedID, spanController, 1, common.DefaultMode) - op := NewMoveDispatcherOperator(spanController, replicaSet, nodeA, nodeB) + op := NewMoveDispatcherOperator(spanController, replicaSet, nodeA, nodeB, 7) require.True(t, oc.AddOperator(op)) require.Equal(t, 1, spanController.GetAbsentSize()) @@ -270,7 +271,7 @@ func TestMoveOperator_BothNodesRemovedBeforeStartDoesNotLeaveSchedulingWithoutNo func TestMoveOperator_DestThenOriginRemovedAbortsToAbsent(t *testing.T) { spanController, _, replicaSet, nodeA, nodeB := setupTestEnvironment(t) - op := NewMoveDispatcherOperator(spanController, replicaSet, nodeA, nodeB) + op := NewMoveDispatcherOperator(spanController, replicaSet, nodeA, nodeB, 7) require.NotNil(t, op) op.Start() @@ -298,7 +299,7 @@ func TestMoveOperator_TaskRemovedByDDL(t *testing.T) { spanController, _, replicaSet, nodeA, nodeB := setupTestEnvironment(t) spanController.AddReplicatingSpan(replicaSet) - op := NewMoveDispatcherOperator(spanController, replicaSet, nodeA, nodeB) + op := NewMoveDispatcherOperator(spanController, replicaSet, nodeA, nodeB, 7) require.NotNil(t, op) op.Start() diff --git a/maintainer/operator/operator_remove.go b/maintainer/operator/operator_remove.go index 43a95c21c6..5c44a0e619 100644 --- a/maintainer/operator/operator_remove.go +++ b/maintainer/operator/operator_remove.go @@ -59,33 +59,35 @@ type removeDispatcherOperator struct { // ScheduleDispatcherRequest (for example merge-related messages). operatorType heartbeatpb.OperatorType - sendThrottler sendThrottler + maintainerEpoch uint64 + sendThrottler sendThrottler } func NewRemoveDispatcherOperator( spanController *span.Controller, replicaSet *replica.SpanReplication, operatorType heartbeatpb.OperatorType, + maintainerEpoch uint64, postFinish func(), ) *removeDispatcherOperator { return &removeDispatcherOperator{ - replicaSet: replicaSet, - nodeID: replicaSet.GetNodeID(), - spanController: spanController, - postFinish: postFinish, - operatorType: operatorType, - sendThrottler: newSendThrottler(), + replicaSet: replicaSet, + nodeID: replicaSet.GetNodeID(), + spanController: spanController, + postFinish: postFinish, + operatorType: operatorType, + maintainerEpoch: maintainerEpoch, + sendThrottler: newSendThrottler(), } } -func newRemoveDispatcherOperator(spanController *span.Controller, replicaSet *replica.SpanReplication, operatorType heartbeatpb.OperatorType) *removeDispatcherOperator { - return &removeDispatcherOperator{ - replicaSet: replicaSet, - nodeID: replicaSet.GetNodeID(), - spanController: spanController, - operatorType: operatorType, - sendThrottler: newSendThrottler(), - } +func newRemoveDispatcherOperator( + spanController *span.Controller, + replicaSet *replica.SpanReplication, + operatorType heartbeatpb.OperatorType, + maintainerEpoch uint64, +) *removeDispatcherOperator { + return NewRemoveDispatcherOperator(spanController, replicaSet, operatorType, maintainerEpoch, nil) } func (m *removeDispatcherOperator) Check(from node.ID, status *heartbeatpb.TableSpanStatus) { @@ -110,7 +112,7 @@ func (m *removeDispatcherOperator) Schedule() *messaging.TargetMessage { return nil } - return m.replicaSet.NewRemoveDispatcherMessage(m.nodeID, m.operatorType) + return m.replicaSet.NewRemoveDispatcherMessage(m.nodeID, m.operatorType, m.maintainerEpoch) } // OnNodeRemove is called when node offline, and the replicaset has been removed from spanController, so it's ok. diff --git a/maintainer/operator/operator_remove_test.go b/maintainer/operator/operator_remove_test.go index 8f681016d5..2857082d37 100644 --- a/maintainer/operator/operator_remove_test.go +++ b/maintainer/operator/operator_remove_test.go @@ -28,7 +28,7 @@ import ( func TestRemoveOperator_NodeRemovedBeforeStopped(t *testing.T) { spanController, _, replicaSet, nodeA, _ := setupTestEnvironment(t) - op := newRemoveDispatcherOperator(spanController, replicaSet, heartbeatpb.OperatorType_O_Remove) + op := newRemoveDispatcherOperator(spanController, replicaSet, heartbeatpb.OperatorType_O_Remove, 7) require.NotNil(t, op) op.Start() @@ -39,6 +39,8 @@ func TestRemoveOperator_NodeRemovedBeforeStopped(t *testing.T) { require.NotNil(t, msg) require.Equal(t, messaging.TypeScheduleDispatcherRequest, msg.Type) require.Equal(t, nodeA.String(), msg.To.String()) + req := msg.Message[0].(*heartbeatpb.ScheduleDispatcherRequest) + require.Equal(t, uint64(7), req.MaintainerEpoch) // Node A is removed before it reports stopped status op.OnNodeRemove(nodeA) @@ -53,7 +55,7 @@ func TestRemoveOperator_SnapshotNodeIDAfterMarkAbsent(t *testing.T) { spanController, _, replicaSet, nodeA, _ := setupTestEnvironment(t) spanController.AddReplicatingSpan(replicaSet) - op := newRemoveDispatcherOperator(spanController, replicaSet, heartbeatpb.OperatorType_O_Remove) + op := newRemoveDispatcherOperator(spanController, replicaSet, heartbeatpb.OperatorType_O_Remove, 7) require.NotNil(t, op) spanController.MarkSpanAbsent(replicaSet) @@ -75,7 +77,7 @@ func TestRemoveOperator_SnapshotNodeIDAfterMarkAbsent(t *testing.T) { func TestRemoveOperator_NotFinishedOnWaitingMerge(t *testing.T) { spanController, _, replicaSet, nodeA, _ := setupTestEnvironment(t) - op := newRemoveDispatcherOperator(spanController, replicaSet, heartbeatpb.OperatorType_O_Remove) + op := newRemoveDispatcherOperator(spanController, replicaSet, heartbeatpb.OperatorType_O_Remove, 7) require.NotNil(t, op) waitingMergeStatus := &heartbeatpb.TableSpanStatus{ @@ -98,7 +100,7 @@ func TestRemoveOperator_NotFinishedOnWaitingMerge(t *testing.T) { func TestRemoveOperator_FinishedOnRemovedStatus(t *testing.T) { spanController, _, replicaSet, nodeA, _ := setupTestEnvironment(t) - op := newRemoveDispatcherOperator(spanController, replicaSet, heartbeatpb.OperatorType_O_Remove) + op := newRemoveDispatcherOperator(spanController, replicaSet, heartbeatpb.OperatorType_O_Remove, 7) require.NotNil(t, op) removedStatus := &heartbeatpb.TableSpanStatus{ diff --git a/maintainer/operator/operator_split.go b/maintainer/operator/operator_split.go index 636c7c2454..36fcef912b 100644 --- a/maintainer/operator/operator_split.go +++ b/maintainer/operator/operator_split.go @@ -52,6 +52,7 @@ type SplitDispatcherOperator struct { splitSpans []*heartbeatpb.TableSpan splitSpanInfo string splitTargetNodes []node.ID + maintainerEpoch uint64 // postFinish is invoked for each newly created span after ReplaceReplicaSet. It is mainly used by // the split-balance scheduler to schedule the new spans to specific nodes. // Note: when postFinish is not nil, splitTargetNodes is expected to be aligned with splitSpans. @@ -74,6 +75,7 @@ func NewSplitDispatcherOperator( replicaSet *replica.SpanReplication, splitSpans []*heartbeatpb.TableSpan, splitTargetNodes []node.ID, + maintainerEpoch uint64, postFinish func(span *replica.SpanReplication, node node.ID) bool, ) *SplitDispatcherOperator { var spansInfo strings.Builder @@ -89,6 +91,7 @@ func NewSplitDispatcherOperator( spanController: spanController, splitSpanInfo: spansInfo.String(), splitTargetNodes: splitTargetNodes, + maintainerEpoch: maintainerEpoch, postFinish: postFinish, sendThrottler: newSendThrottler(), } @@ -147,7 +150,7 @@ func (m *SplitDispatcherOperator) Schedule() *messaging.TargetMessage { if !m.sendThrottler.shouldSend() { return nil } - return m.replicaSet.NewRemoveDispatcherMessage(m.originNode, heartbeatpb.OperatorType_O_Split) + return m.replicaSet.NewRemoveDispatcherMessage(m.originNode, heartbeatpb.OperatorType_O_Split, m.maintainerEpoch) } // OnTaskRemoved is called when the task is removed by ddl diff --git a/maintainer/operator/operator_split_test.go b/maintainer/operator/operator_split_test.go index e0f3971faa..30ab9797e5 100644 --- a/maintainer/operator/operator_split_test.go +++ b/maintainer/operator/operator_split_test.go @@ -49,7 +49,7 @@ func TestSplitOperator_OriginNodeRemovedBeforeStopped(t *testing.T) { // Split targets are empty, meaning let scheduler decide splitTargetNodes := []node.ID{"", ""} - op := NewSplitDispatcherOperator(spanController, replicaSet, splitSpans, splitTargetNodes, nil) + op := NewSplitDispatcherOperator(spanController, replicaSet, splitSpans, splitTargetNodes, 7, nil) require.NotNil(t, op) op.Start() @@ -100,7 +100,7 @@ func TestSplitOperator_OriginNodeRemovedAfterStopped(t *testing.T) { // Split targets are empty, meaning let scheduler decide splitTargetNodes := []node.ID{"", ""} - op := NewSplitDispatcherOperator(spanController, replicaSet, splitSpans, splitTargetNodes, nil) + op := NewSplitDispatcherOperator(spanController, replicaSet, splitSpans, splitTargetNodes, 7, nil) require.NotNil(t, op) op.Start() @@ -153,7 +153,7 @@ func TestSplitOperator_SuccessfulSplitCreatesAbsentSpans(t *testing.T) { }, } - op := NewSplitDispatcherOperator(spanController, replicaSet, splitSpans, []node.ID{}, nil) + op := NewSplitDispatcherOperator(spanController, replicaSet, splitSpans, []node.ID{}, 7, nil) require.NotNil(t, op) op.Start() @@ -244,7 +244,7 @@ func TestSplitOperator_SuccessfulSplitToSchedulingTargets(t *testing.T) { } splitTargetNodes := []node.ID{nodeA, nodeB} - op := NewSplitDispatcherOperator(spanController, replicaSetToSplit, splitSpans, splitTargetNodes, nil) + op := NewSplitDispatcherOperator(spanController, replicaSetToSplit, splitSpans, splitTargetNodes, 7, nil) require.NotNil(t, op) op.Start() @@ -337,7 +337,7 @@ func TestSplitOperator_PostFinishCallbackFailureMarksSpanAbsent(t *testing.T) { } splitTargetNodes := []node.ID{nodeA, nodeB} - op := NewSplitDispatcherOperator(spanController, replicaSetToSplit, splitSpans, splitTargetNodes, + op := NewSplitDispatcherOperator(spanController, replicaSetToSplit, splitSpans, splitTargetNodes, 7, func(_ *replica.SpanReplication, target node.ID) bool { return target != nodeB }) @@ -394,7 +394,7 @@ func TestSplitOperator_PostFinishSkipsWhenTargetNodesMismatch(t *testing.T) { splitTargetNodes := []node.ID{nodeA} postFinishCalled := 0 - op := NewSplitDispatcherOperator(spanController, replicaSet, splitSpans, splitTargetNodes, + op := NewSplitDispatcherOperator(spanController, replicaSet, splitSpans, splitTargetNodes, 7, func(_ *replica.SpanReplication, _ node.ID) bool { postFinishCalled++ return true @@ -445,7 +445,7 @@ func TestSplitOperator_TaskRemovedByDDLDoesNotSplit(t *testing.T) { }, } - op := NewSplitDispatcherOperator(spanController, replicaSet, splitSpans, []node.ID{}, nil) + op := NewSplitDispatcherOperator(spanController, replicaSet, splitSpans, []node.ID{}, 7, nil) require.NotNil(t, op) op.Start() diff --git a/maintainer/replica/replication_span.go b/maintainer/replica/replication_span.go index e35aa9ac40..c94a8bd8d7 100644 --- a/maintainer/replica/replication_span.go +++ b/maintainer/replica/replication_span.go @@ -260,7 +260,11 @@ func (r *SpanReplication) getCommittedCheckpointTs() uint64 { // moved/recreated during an in-flight barrier (DDL or syncpoint), starting from the raw checkpoint can // violate barrier semantics (e.g. replaying events that have already been acknowledged by the barrier), // so we adjust StartTs and (for DDL) optionally skip DML at StartTs+1. -func (r *SpanReplication) NewAddDispatcherMessage(server node.ID, operatorType heartbeatpb.OperatorType) *messaging.TargetMessage { +func (r *SpanReplication) NewAddDispatcherMessage( + server node.ID, + operatorType heartbeatpb.OperatorType, + maintainerEpoch uint64, +) *messaging.TargetMessage { startTs := r.status.Load().CheckpointTs skipDMLAsStartTs := false ddlBarrierBlockTs := uint64(0) @@ -330,22 +334,35 @@ func (r *SpanReplication) NewAddDispatcherMessage(server node.ID, operatorType h SkipDMLAsStartTs: skipDMLAsStartTs, Mode: r.GetMode(), }, - ScheduleAction: heartbeatpb.ScheduleAction_Create, - OperatorType: operatorType, + ScheduleAction: heartbeatpb.ScheduleAction_Create, + OperatorType: operatorType, + MaintainerEpoch: maintainerEpoch, }) } // NewRemoveDispatcherMessage creates a ScheduleDispatcherRequest(Remove) for this span. // Span and OperatorType are included so a new maintainer can reconstruct intent during bootstrap/failover, // even if the dispatcher has already disappeared from the node span snapshot. -func (r *SpanReplication) NewRemoveDispatcherMessage(server node.ID, operatorType heartbeatpb.OperatorType) *messaging.TargetMessage { - return NewRemoveDispatcherMessage(server, r.ChangefeedID, r.ID.ToPB(), r.Span, r.GetMode(), operatorType) +func (r *SpanReplication) NewRemoveDispatcherMessage( + server node.ID, + operatorType heartbeatpb.OperatorType, + maintainerEpoch uint64, +) *messaging.TargetMessage { + return NewRemoveDispatcherMessage(server, r.ChangefeedID, r.ID.ToPB(), r.Span, r.GetMode(), operatorType, maintainerEpoch) } // NewRemoveDispatcherMessage creates a ScheduleDispatcherRequest(Remove) for a dispatcherID. // The span is optional for the dispatcher manager, but is useful for maintainer bootstrap to correlate // in-flight remove requests with table spans when the dispatcher no longer exists. -func NewRemoveDispatcherMessage(server node.ID, cfID common.ChangeFeedID, dispatcherID *heartbeatpb.DispatcherID, span *heartbeatpb.TableSpan, mode int64, operatorType heartbeatpb.OperatorType) *messaging.TargetMessage { +func NewRemoveDispatcherMessage( + server node.ID, + cfID common.ChangeFeedID, + dispatcherID *heartbeatpb.DispatcherID, + span *heartbeatpb.TableSpan, + mode int64, + operatorType heartbeatpb.OperatorType, + maintainerEpoch uint64, +) *messaging.TargetMessage { return messaging.NewSingleTargetMessage(server, messaging.HeartbeatCollectorTopic, &heartbeatpb.ScheduleDispatcherRequest{ @@ -355,7 +372,8 @@ func NewRemoveDispatcherMessage(server node.ID, cfID common.ChangeFeedID, dispat Span: span, Mode: mode, }, - ScheduleAction: heartbeatpb.ScheduleAction_Remove, - OperatorType: operatorType, + ScheduleAction: heartbeatpb.ScheduleAction_Remove, + OperatorType: operatorType, + MaintainerEpoch: maintainerEpoch, }) } diff --git a/maintainer/replica/replication_span_test.go b/maintainer/replica/replication_span_test.go index b7802485a2..151bde78aa 100644 --- a/maintainer/replica/replication_span_test.go +++ b/maintainer/replica/replication_span_test.go @@ -36,11 +36,12 @@ func TestNewRemoveDispatcherMessage(t *testing.T) { t.Parallel() replicaSet := NewSpanReplication(common.NewChangeFeedIDWithName("test", common.DefaultKeyspaceName), common.NewDispatcherID(), 1, getTableSpanByID(4), 10, common.DefaultMode, false) - msg := replicaSet.NewRemoveDispatcherMessage("node1", heartbeatpb.OperatorType_O_Remove) + msg := replicaSet.NewRemoveDispatcherMessage("node1", heartbeatpb.OperatorType_O_Remove, 7) req := msg.Message[0].(*heartbeatpb.ScheduleDispatcherRequest) require.Equal(t, heartbeatpb.ScheduleAction_Remove, req.ScheduleAction) require.Equal(t, replicaSet.ID.ToPB(), req.Config.DispatcherID) require.Equal(t, replicaSet.Span, req.Config.Span) + require.Equal(t, uint64(7), req.MaintainerEpoch) require.Equal(t, "node1", msg.To.String()) } @@ -49,7 +50,7 @@ func TestSpanReplication_NewAddDispatcherMessage(t *testing.T) { replicaSet := NewSpanReplication(common.NewChangeFeedIDWithName("test", common.DefaultKeyspaceName), common.NewDispatcherID(), 1, getTableSpanByID(4), 10, common.DefaultMode, false) - msg := replicaSet.NewAddDispatcherMessage("node1", heartbeatpb.OperatorType_O_Add) + msg := replicaSet.NewAddDispatcherMessage("node1", heartbeatpb.OperatorType_O_Add, 7) require.Equal(t, "node1", msg.To.String()) req := msg.Message[0].(*heartbeatpb.ScheduleDispatcherRequest) require.Equal(t, heartbeatpb.ScheduleAction_Create, req.ScheduleAction) @@ -57,6 +58,7 @@ func TestSpanReplication_NewAddDispatcherMessage(t *testing.T) { require.Equal(t, replicaSet.schemaID, req.Config.SchemaID) require.Equal(t, uint64(10), req.Config.StartTs) require.False(t, req.Config.SkipDMLAsStartTs) + require.Equal(t, uint64(7), req.MaintainerEpoch) } func TestSpanReplication_NewAddDispatcherMessage_ClampToCommittedCheckpoint(t *testing.T) { @@ -65,7 +67,7 @@ func TestSpanReplication_NewAddDispatcherMessage_ClampToCommittedCheckpoint(t *t replicaSet := NewSpanReplication(common.NewChangeFeedIDWithName("test", common.DefaultKeyspaceName), common.NewDispatcherID(), 1, getTableSpanByID(4), 10, common.DefaultMode, false) replicaSet.BindCommittedCheckpointTs(atomic.NewUint64(20)) - msg := replicaSet.NewAddDispatcherMessage("node1", heartbeatpb.OperatorType_O_Add) + msg := replicaSet.NewAddDispatcherMessage("node1", heartbeatpb.OperatorType_O_Add, 7) req := msg.Message[0].(*heartbeatpb.ScheduleDispatcherRequest) require.Equal(t, uint64(20), req.Config.StartTs) require.False(t, req.Config.SkipDMLAsStartTs) @@ -82,7 +84,7 @@ func TestSpanReplication_NewAddDispatcherMessage_UseBlockTsForInFlightSyncPoint( Stage: heartbeatpb.BlockStage_WAITING, }) - msg := replicaSet.NewAddDispatcherMessage("node1", heartbeatpb.OperatorType_O_Add) + msg := replicaSet.NewAddDispatcherMessage("node1", heartbeatpb.OperatorType_O_Add, 7) req := msg.Message[0].(*heartbeatpb.ScheduleDispatcherRequest) require.Equal(t, uint64(10), req.Config.StartTs) require.False(t, req.Config.SkipDMLAsStartTs) @@ -100,7 +102,7 @@ func TestSpanReplication_NewAddDispatcherMessage_UseSyncPointBlockTsWhenCommitte Stage: heartbeatpb.BlockStage_WAITING, }) - msg := replicaSet.NewAddDispatcherMessage("node1", heartbeatpb.OperatorType_O_Add) + msg := replicaSet.NewAddDispatcherMessage("node1", heartbeatpb.OperatorType_O_Add, 7) req := msg.Message[0].(*heartbeatpb.ScheduleDispatcherRequest) require.Equal(t, uint64(30), req.Config.StartTs) require.False(t, req.Config.SkipDMLAsStartTs) @@ -117,7 +119,7 @@ func TestSpanReplication_NewAddDispatcherMessage_DontUseBlockTsAfterSyncPointDon Stage: heartbeatpb.BlockStage_DONE, }) - msg := replicaSet.NewAddDispatcherMessage("node1", heartbeatpb.OperatorType_O_Add) + msg := replicaSet.NewAddDispatcherMessage("node1", heartbeatpb.OperatorType_O_Add, 7) req := msg.Message[0].(*heartbeatpb.ScheduleDispatcherRequest) require.Equal(t, uint64(20), req.Config.StartTs) require.False(t, req.Config.SkipDMLAsStartTs) @@ -134,7 +136,7 @@ func TestSpanReplication_NewAddDispatcherMessage_UseBlockTsMinusOneForDDLInFligh Stage: heartbeatpb.BlockStage_WAITING, }) - msg := replicaSet.NewAddDispatcherMessage("node1", heartbeatpb.OperatorType_O_Add) + msg := replicaSet.NewAddDispatcherMessage("node1", heartbeatpb.OperatorType_O_Add, 7) req := msg.Message[0].(*heartbeatpb.ScheduleDispatcherRequest) require.Equal(t, uint64(9), req.Config.StartTs) require.True(t, req.Config.SkipDMLAsStartTs) @@ -152,7 +154,7 @@ func TestSpanReplication_NewAddDispatcherMessage_UseCommittedCheckpointForStaleD Stage: heartbeatpb.BlockStage_WAITING, }) - msg := replicaSet.NewAddDispatcherMessage("node1", heartbeatpb.OperatorType_O_Add) + msg := replicaSet.NewAddDispatcherMessage("node1", heartbeatpb.OperatorType_O_Add, 7) req := msg.Message[0].(*heartbeatpb.ScheduleDispatcherRequest) require.Equal(t, uint64(10), req.Config.StartTs) require.False(t, req.Config.SkipDMLAsStartTs) @@ -170,7 +172,7 @@ func TestSpanReplication_NewAddDispatcherMessage_UseCommittedCheckpointForStaleD Stage: heartbeatpb.BlockStage_WRITING, }) - msg := replicaSet.NewAddDispatcherMessage("node1", heartbeatpb.OperatorType_O_Add) + msg := replicaSet.NewAddDispatcherMessage("node1", heartbeatpb.OperatorType_O_Add, 7) req := msg.Message[0].(*heartbeatpb.ScheduleDispatcherRequest) require.Equal(t, uint64(11), req.Config.StartTs) require.False(t, req.Config.SkipDMLAsStartTs) diff --git a/maintainer/scheduler/balance.go b/maintainer/scheduler/balance.go index 734482e04a..b60114e8d8 100644 --- a/maintainer/scheduler/balance.go +++ b/maintainer/scheduler/balance.go @@ -148,7 +148,14 @@ func (s *balanceScheduler) doSplit(results pkgReplica.GroupCheckResult) int { spansNum := max(result.SpanNum, len(s.nodeManager.GetAliveNodes())*2) splitSpans := s.splitter.Split(context.Background(), result.Span.Span, spansNum, result.SpanType) if len(splitSpans) > 1 { - op := operator.NewSplitDispatcherOperator(s.spanController, result.Span, splitSpans, []node.ID{}, nil) + op := operator.NewSplitDispatcherOperator( + s.spanController, + result.Span, + splitSpans, + []node.ID{}, + s.operatorController.MaintainerEpoch(), + nil, + ) ret := s.operatorController.AddOperator(op) if ret { splitCount++ @@ -160,6 +167,6 @@ func (s *balanceScheduler) doSplit(results pkgReplica.GroupCheckResult) int { } func (s *balanceScheduler) doMove(replication *replica.SpanReplication, id node.ID) bool { - op := operator.NewMoveDispatcherOperator(s.spanController, replication, replication.GetNodeID(), id) + op := s.operatorController.NewMoveOperator(replication, replication.GetNodeID(), id) return s.operatorController.AddOperator(op) } diff --git a/maintainer/scheduler/balance_splits.go b/maintainer/scheduler/balance_splits.go index 7f808a01a9..7898fdf5de 100644 --- a/maintainer/scheduler/balance_splits.go +++ b/maintainer/scheduler/balance_splits.go @@ -132,9 +132,22 @@ func (s *balanceSplitsScheduler) Execute() time.Time { case replica.OpSplit: splitSpans := s.splitter.Split(context.Background(), checkResult.SplitSpan.Span, checkResult.SpanNum, checkResult.SpanType) if len(splitSpans) > 1 { - op := operator.NewSplitDispatcherOperator(s.spanController, checkResult.SplitSpan, splitSpans, checkResult.SplitTargetNodes, func(span *replica.SpanReplication, node node.ID) bool { - return s.operatorController.AddOperator(operator.NewAddDispatcherOperator(s.spanController, span, node, heartbeatpb.OperatorType_O_Split)) - }) + op := operator.NewSplitDispatcherOperator( + s.spanController, + checkResult.SplitSpan, + splitSpans, + checkResult.SplitTargetNodes, + s.operatorController.MaintainerEpoch(), + func(span *replica.SpanReplication, node node.ID) bool { + return s.operatorController.AddOperator(operator.NewAddDispatcherOperator( + s.spanController, + span, + node, + heartbeatpb.OperatorType_O_Split, + s.operatorController.MaintainerEpoch(), + )) + }, + ) ret := s.operatorController.AddOperator(op) if ret { availableSize-- @@ -142,7 +155,7 @@ func (s *balanceSplitsScheduler) Execute() time.Time { } case replica.OpMove: for _, span := range checkResult.MoveSpans { - op := operator.NewMoveDispatcherOperator(s.spanController, span, span.GetNodeID(), checkResult.TargetNode) + op := s.operatorController.NewMoveOperator(span, span.GetNodeID(), checkResult.TargetNode) ret := s.operatorController.AddOperator(op) if ret { availableSize-- diff --git a/maintainer/scheduler/basic.go b/maintainer/scheduler/basic.go index 344452309c..449516a31d 100644 --- a/maintainer/scheduler/basic.go +++ b/maintainer/scheduler/basic.go @@ -171,7 +171,13 @@ func (s *basicScheduler) schedule( absentReplications := s.spanController.GetAbsentByGroup(groupID, availableSize) pkgScheduler.BasicSchedule(availableSize, absentReplications, nodeSize, func(replication *replica.SpanReplication, id node.ID) bool { - return s.operatorController.AddOperator(operator.NewAddDispatcherOperator(s.spanController, replication, id, heartbeatpb.OperatorType_O_Add)) + return s.operatorController.AddOperator(operator.NewAddDispatcherOperator( + s.spanController, + replication, + id, + heartbeatpb.OperatorType_O_Add, + s.operatorController.MaintainerEpoch(), + )) }) return len(absentReplications) } diff --git a/maintainer/scheduler/drain.go b/maintainer/scheduler/drain.go index 6f2e57f685..bfe07daa8d 100644 --- a/maintainer/scheduler/drain.go +++ b/maintainer/scheduler/drain.go @@ -152,7 +152,7 @@ func (s *drainScheduler) Execute() time.Time { } if s.operatorController.AddOperator( - operator.NewMoveDispatcherOperator(s.spanController, replication, target, dest), + s.operatorController.NewMoveOperator(replication, target, dest), ) { nodeTaskSize[target]-- nodeTaskSize[dest]++ diff --git a/maintainer/scheduler/drain_test.go b/maintainer/scheduler/drain_test.go index 1ee5243430..8e6f246f61 100644 --- a/maintainer/scheduler/drain_test.go +++ b/maintainer/scheduler/drain_test.go @@ -107,7 +107,7 @@ func TestDrainSchedulerIgnoresUnrelatedOperatorCapacity(t *testing.T) { onTarget := addReplicatingSpan(t, cfID, sc, 1, target) unrelated := addReplicatingSpan(t, cfID, sc, 2, other) - require.True(t, oc.AddOperator(operator.NewMoveDispatcherOperator(sc, unrelated, other, dest))) + require.True(t, oc.AddOperator(operator.NewMoveDispatcherOperator(sc, unrelated, other, dest, oc.MaintainerEpoch()))) drainState.SetSelfNodeID(self) drainState.SetDispatcherDrainTarget(target, 1) diff --git a/maintainer/span/span_controller_test.go b/maintainer/span/span_controller_test.go index 827f175e12..3453f34596 100644 --- a/maintainer/span/span_controller_test.go +++ b/maintainer/span/span_controller_test.go @@ -487,7 +487,7 @@ func TestController_BindCommittedCheckpointToManagedSpan(t *testing.T) { task := controller.GetTasksByTableID(100)[0] controller.AdvanceMaintainerCommittedCheckpointTs(20) - msg := task.NewAddDispatcherMessage("node1", heartbeatpb.OperatorType_O_Add) + msg := task.NewAddDispatcherMessage("node1", heartbeatpb.OperatorType_O_Add, 7) req := msg.Message[0].(*heartbeatpb.ScheduleDispatcherRequest) require.Equal(t, uint64(20), req.Config.StartTs) }