From e3cdae405a7d293bb0b84a77ddc060fed4cb45a2 Mon Sep 17 00:00:00 2001 From: lhy1024 Date: Fri, 19 Jul 2024 16:57:59 +0800 Subject: [PATCH] schedule: fix datarace in operator.check (#8264) Signed-off-by: lhy1024 --- pkg/schedule/operator/operator.go | 5 +++-- pkg/schedule/operator/operator_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/pkg/schedule/operator/operator.go b/pkg/schedule/operator/operator.go index a8c54e824fb..4d01fd12a05 100644 --- a/pkg/schedule/operator/operator.go +++ b/pkg/schedule/operator/operator.go @@ -340,10 +340,11 @@ func (o *Operator) Check(region *core.RegionInfo) OpStep { defer func() { _ = o.CheckTimeout() }() for step := atomic.LoadInt32(&o.currentStep); int(step) < len(o.steps); step++ { if o.steps[int(step)].IsFinish(region) { - if atomic.CompareAndSwapInt64(&(o.stepsTime[step]), 0, time.Now().UnixNano()) { + current := time.Now() + if atomic.CompareAndSwapInt64(&(o.stepsTime[step]), 0, current.UnixNano()) { startTime, _ := o.getCurrentTimeAndStep() operatorStepDuration.WithLabelValues(reflect.TypeOf(o.steps[int(step)]).Name()). - Observe(time.Unix(0, o.stepsTime[step]).Sub(startTime).Seconds()) + Observe(current.Sub(startTime).Seconds()) } atomic.StoreInt32(&o.currentStep, step+1) } else { diff --git a/pkg/schedule/operator/operator_test.go b/pkg/schedule/operator/operator_test.go index 9d924738543..828ab4be27d 100644 --- a/pkg/schedule/operator/operator_test.go +++ b/pkg/schedule/operator/operator_test.go @@ -18,6 +18,7 @@ import ( "context" "encoding/json" "fmt" + "sync" "sync/atomic" "testing" "time" @@ -529,3 +530,26 @@ func (suite *operatorTestSuite) TestRecord() { suite.Equal(now, ob.FinishTime) suite.Greater(ob.duration.Seconds(), time.Second.Seconds()) } + +func (suite *operatorTestSuite) TestOperatorCheckConcurrently() { + region := suite.newTestRegion(1, 1, [2]uint64{1, 1}, [2]uint64{2, 2}) + // addPeer1, transferLeader1, removePeer3 + steps := []OpStep{ + AddPeer{ToStore: 1, PeerID: 1}, + TransferLeader{FromStore: 3, ToStore: 1}, + RemovePeer{FromStore: 3}, + } + op := NewTestOperator(1, &metapb.RegionEpoch{}, OpAdmin|OpLeader|OpRegion, steps...) + suite.Equal(constant.Urgent, op.GetPriorityLevel()) + suite.checkSteps(op, steps) + op.Start() + var wg sync.WaitGroup + for i := 0; i < 10; i++ { + wg.Add(1) + go func() { + defer wg.Done() + suite.Nil(op.Check(region)) + }() + } + wg.Wait() +}