Skip to content

Commit

Permalink
enhance: Tidy compaction executor (milvus-io#33778)
Browse files Browse the repository at this point in the history 
Move compaction executor to compaction pacakge.

issue: milvus-io#32451

Signed-off-by: bigsheeper <yihao.dai@zilliz.com>
  • Loading branch information
@bigsheeper @yellow-shine
bigsheeper authored and yellow-shine committed Jul 2, 2024
1 parent 6a44829 commit b21e63c
Show file tree
Hide file tree
Showing 8 changed files with 157 additions and 133 deletions.
134 changes: 78 additions & 56 deletions internal/datanode/compaction_executor.go → internal/datanode/compaction/executor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

package datanode
package compaction

import (
"context"
Expand All @@ -24,9 +24,9 @@ import (
"go.uber.org/zap"
"golang.org/x/sync/semaphore"

"github.com/milvus-io/milvus/internal/datanode/compaction"
"github.com/milvus-io/milvus/internal/proto/datapb"
"github.com/milvus-io/milvus/pkg/log"
"github.com/milvus-io/milvus/pkg/util/paramtable"
"github.com/milvus-io/milvus/pkg/util/typeutil"
)

Expand All @@ -35,11 +35,21 @@ const (
maxParallelTaskNum = 10
)

type compactionExecutor struct {
executing *typeutil.ConcurrentMap[int64, compaction.Compactor] // planID to compactor
completedCompactor *typeutil.ConcurrentMap[int64, compaction.Compactor] // planID to compactor
type Executor interface {
Start(ctx context.Context)
Execute(task Compactor)
Slots() int64
RemoveTask(planID int64)
GetResults(planID int64) []*datapb.CompactionPlanResult
DiscardByDroppedChannel(channel string)
DiscardPlan(channel string)
}

type executor struct {
executing *typeutil.ConcurrentMap[int64, Compactor] // planID to compactor
completedCompactor *typeutil.ConcurrentMap[int64, Compactor] // planID to compactor
completed *typeutil.ConcurrentMap[int64, *datapb.CompactionPlanResult] // planID to CompactionPlanResult
taskCh chan compaction.Compactor
taskCh chan Compactor
taskSem *semaphore.Weighted
dropped *typeutil.ConcurrentSet[string] // vchannel dropped

Expand All @@ -48,66 +58,70 @@ type compactionExecutor struct {
resultGuard sync.RWMutex
}

func newCompactionExecutor() *compactionExecutor {
return &compactionExecutor{
executing: typeutil.NewConcurrentMap[int64, compaction.Compactor](),
completedCompactor: typeutil.NewConcurrentMap[int64, compaction.Compactor](),
func NewExecutor() *executor {
return &executor{
executing: typeutil.NewConcurrentMap[int64, Compactor](),
completedCompactor: typeutil.NewConcurrentMap[int64, Compactor](),
completed: typeutil.NewConcurrentMap[int64, *datapb.CompactionPlanResult](),
taskCh: make(chan compaction.Compactor, maxTaskQueueNum),
taskCh: make(chan Compactor, maxTaskQueueNum),
taskSem: semaphore.NewWeighted(maxParallelTaskNum),
dropped: typeutil.NewConcurrentSet[string](),
}
}

func (c *compactionExecutor) execute(task compaction.Compactor) {
c.taskCh <- task
c.toExecutingState(task)
func (e *executor) Execute(task Compactor) {
e.taskCh <- task
e.toExecutingState(task)
}

func (e *executor) Slots() int64 {
return paramtable.Get().DataNodeCfg.SlotCap.GetAsInt64() - int64(e.executing.Len())
}

func (c *compactionExecutor) toExecutingState(task compaction.Compactor) {
c.executing.Insert(task.GetPlanID(), task)
func (e *executor) toExecutingState(task Compactor) {
e.executing.Insert(task.GetPlanID(), task)
}

func (c *compactionExecutor) toCompleteState(task compaction.Compactor) {
func (e *executor) toCompleteState(task Compactor) {
task.Complete()
c.executing.GetAndRemove(task.GetPlanID())
e.executing.GetAndRemove(task.GetPlanID())
}

func (c *compactionExecutor) removeTask(planID UniqueID) {
c.completed.GetAndRemove(planID)
task, loaded := c.completedCompactor.GetAndRemove(planID)
func (e *executor) RemoveTask(planID int64) {
e.completed.GetAndRemove(planID)
task, loaded := e.completedCompactor.GetAndRemove(planID)
if loaded {
log.Info("Compaction task removed", zap.Int64("planID", planID), zap.String("channel", task.GetChannelName()))
}
}

func (c *compactionExecutor) start(ctx context.Context) {
func (e *executor) Start(ctx context.Context) {
for {
select {
case <-ctx.Done():
return
case task := <-c.taskCh:
err := c.taskSem.Acquire(ctx, 1)
case task := <-e.taskCh:
err := e.taskSem.Acquire(ctx, 1)
if err != nil {
return
}
go func() {
defer c.taskSem.Release(1)
c.executeTask(task)
defer e.taskSem.Release(1)
e.executeTask(task)
}()
}
}
}

func (c *compactionExecutor) executeTask(task compaction.Compactor) {
func (e *executor) executeTask(task Compactor) {
log := log.With(
zap.Int64("planID", task.GetPlanID()),
zap.Int64("Collection", task.GetCollection()),
zap.String("channel", task.GetChannelName()),
)

defer func() {
c.toCompleteState(task)
e.toCompleteState(task)
}()

log.Info("start to execute compaction")
Expand All @@ -117,45 +131,45 @@ func (c *compactionExecutor) executeTask(task compaction.Compactor) {
log.Warn("compaction task failed", zap.Error(err))
return
}
c.completed.Insert(result.GetPlanID(), result)
c.completedCompactor.Insert(result.GetPlanID(), task)
e.completed.Insert(result.GetPlanID(), result)
e.completedCompactor.Insert(result.GetPlanID(), task)

log.Info("end to execute compaction")
}

func (c *compactionExecutor) stopTask(planID UniqueID) {
task, loaded := c.executing.GetAndRemove(planID)
func (e *executor) stopTask(planID int64) {
task, loaded := e.executing.GetAndRemove(planID)
if loaded {
log.Warn("compaction executor stop task", zap.Int64("planID", planID), zap.String("vChannelName", task.GetChannelName()))
task.Stop()
}
}

func (c *compactionExecutor) isValidChannel(channel string) bool {
func (e *executor) isValidChannel(channel string) bool {
// if vchannel marked dropped, compaction should not proceed
return !c.dropped.Contain(channel)
return !e.dropped.Contain(channel)
}

func (c *compactionExecutor) discardByDroppedChannel(channel string) {
c.dropped.Insert(channel)
c.discardPlan(channel)
func (e *executor) DiscardByDroppedChannel(channel string) {
e.dropped.Insert(channel)
e.DiscardPlan(channel)
}

func (c *compactionExecutor) discardPlan(channel string) {
c.resultGuard.Lock()
defer c.resultGuard.Unlock()
func (e *executor) DiscardPlan(channel string) {
e.resultGuard.Lock()
defer e.resultGuard.Unlock()

c.executing.Range(func(planID int64, task compaction.Compactor) bool {
e.executing.Range(func(planID int64, task Compactor) bool {
if task.GetChannelName() == channel {
c.stopTask(planID)
e.stopTask(planID)
}
return true
})

// remove all completed plans of channel
c.completed.Range(func(planID int64, result *datapb.CompactionPlanResult) bool {
e.completed.Range(func(planID int64, result *datapb.CompactionPlanResult) bool {
if result.GetChannel() == channel {
c.removeTask(planID)
e.RemoveTask(planID)
log.Info("remove compaction plan and results",
zap.String("channel", channel),
zap.Int64("planID", planID))
Expand All @@ -164,18 +178,26 @@ func (c *compactionExecutor) discardPlan(channel string) {
})
}

func (c *compactionExecutor) getCompactionResult(planID int64) *datapb.CompactionPlanResult {
c.resultGuard.RLock()
defer c.resultGuard.RUnlock()
_, ok := c.executing.Get(planID)
func (e *executor) GetResults(planID int64) []*datapb.CompactionPlanResult {
if planID != 0 {
result := e.getCompactionResult(planID)
return []*datapb.CompactionPlanResult{result}
}
return e.getAllCompactionResults()
}

func (e *executor) getCompactionResult(planID int64) *datapb.CompactionPlanResult {
e.resultGuard.RLock()
defer e.resultGuard.RUnlock()
_, ok := e.executing.Get(planID)
if ok {
result := &datapb.CompactionPlanResult{
State: datapb.CompactionTaskState_executing,
PlanID: planID,
}
return result
}
result, ok2 := c.completed.Get(planID)
result, ok2 := e.completed.Get(planID)
if !ok2 {
return &datapb.CompactionPlanResult{
PlanID: planID,
Expand All @@ -185,17 +207,17 @@ func (c *compactionExecutor) getCompactionResult(planID int64) *datapb.Compactio
return result
}

func (c *compactionExecutor) getAllCompactionResults() []*datapb.CompactionPlanResult {
c.resultGuard.RLock()
defer c.resultGuard.RUnlock()
func (e *executor) getAllCompactionResults() []*datapb.CompactionPlanResult {
e.resultGuard.RLock()
defer e.resultGuard.RUnlock()
var (
executing []int64
completed []int64
completedLevelZero []int64
)
results := make([]*datapb.CompactionPlanResult, 0)
// get executing results
c.executing.Range(func(planID int64, task compaction.Compactor) bool {
e.executing.Range(func(planID int64, task Compactor) bool {
executing = append(executing, planID)
results = append(results, &datapb.CompactionPlanResult{
State: datapb.CompactionTaskState_executing,
Expand All @@ -205,7 +227,7 @@ func (c *compactionExecutor) getAllCompactionResults() []*datapb.CompactionPlanR
})

// get completed results
c.completed.Range(func(planID int64, result *datapb.CompactionPlanResult) bool {
e.completed.Range(func(planID int64, result *datapb.CompactionPlanResult) bool {
completed = append(completed, planID)
results = append(results, result)

Expand All @@ -217,8 +239,8 @@ func (c *compactionExecutor) getAllCompactionResults() []*datapb.CompactionPlanR

// remove level zero results
lo.ForEach(completedLevelZero, func(planID int64, _ int) {
c.completed.Remove(planID)
c.completedCompactor.Remove(planID)
e.completed.Remove(planID)
e.completedCompactor.Remove(planID)
})

if len(results) > 0 {
Expand Down
39 changes: 19 additions & 20 deletions ...rnal/datanode/compaction_executor_test.go → ...rnal/datanode/compaction/executor_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

package datanode
package compaction

import (
"context"
Expand All @@ -24,18 +24,17 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/milvus-io/milvus/internal/datanode/compaction"
"github.com/milvus-io/milvus/internal/proto/datapb"
)

func TestCompactionExecutor(t *testing.T) {
t.Run("Test execute", func(t *testing.T) {
planID := int64(1)
mockC := compaction.NewMockCompactor(t)
mockC := NewMockCompactor(t)
mockC.EXPECT().GetPlanID().Return(planID).Once()
mockC.EXPECT().GetChannelName().Return("ch1").Once()
executor := newCompactionExecutor()
executor.execute(mockC)
executor := NewExecutor()
executor.Execute(mockC)

assert.EqualValues(t, 1, len(executor.taskCh))
assert.EqualValues(t, 1, executor.executing.Len())
Expand All @@ -44,11 +43,11 @@ func TestCompactionExecutor(t *testing.T) {
executor.stopTask(planID)
})

t.Run("Test start", func(t *testing.T) {
ex := newCompactionExecutor()
t.Run("Test Start", func(t *testing.T) {
ex := NewExecutor()
ctx, cancel := context.WithCancel(context.TODO())
cancel()
go ex.start(ctx)
go ex.Start(ctx)
})

t.Run("Test executeTask", func(t *testing.T) {
Expand All @@ -61,10 +60,10 @@ func TestCompactionExecutor(t *testing.T) {
{false, "compact return error"},
}

ex := newCompactionExecutor()
ex := NewExecutor()
for _, test := range tests {
t.Run(test.description, func(t *testing.T) {
mockC := compaction.NewMockCompactor(t)
mockC := NewMockCompactor(t)
mockC.EXPECT().GetPlanID().Return(int64(1))
mockC.EXPECT().GetCollection().Return(int64(1))
mockC.EXPECT().GetChannelName().Return("ch1")
Expand Down Expand Up @@ -100,8 +99,8 @@ func TestCompactionExecutor(t *testing.T) {
{expected: true, channel: "ch1", desc: "no in dropped"},
{expected: false, channel: "ch2", desc: "in dropped"},
}
ex := newCompactionExecutor()
ex.discardByDroppedChannel("ch2")
ex := NewExecutor()
ex.DiscardByDroppedChannel("ch2")
for _, test := range tests {
t.Run(test.desc, func(t *testing.T) {
assert.Equal(t, test.expected, ex.isValidChannel(test.channel))
Expand All @@ -110,26 +109,26 @@ func TestCompactionExecutor(t *testing.T) {
})

t.Run("test stop vchannel tasks", func(t *testing.T) {
ex := newCompactionExecutor()
mc := compaction.NewMockCompactor(t)
ex := NewExecutor()
mc := NewMockCompactor(t)
mc.EXPECT().GetPlanID().Return(int64(1))
mc.EXPECT().GetChannelName().Return("mock")
mc.EXPECT().Compact().Return(&datapb.CompactionPlanResult{PlanID: 1}, nil).Maybe()
mc.EXPECT().Stop().Return().Once()

ex.execute(mc)
ex.Execute(mc)

require.True(t, ex.executing.Contain(int64(1)))

ex.discardByDroppedChannel("mock")
ex.DiscardByDroppedChannel("mock")
assert.True(t, ex.dropped.Contain("mock"))
assert.False(t, ex.executing.Contain(int64(1)))
})

t.Run("test getAllCompactionResults", func(t *testing.T) {
ex := newCompactionExecutor()
t.Run("test GetAllCompactionResults", func(t *testing.T) {
ex := NewExecutor()

mockC := compaction.NewMockCompactor(t)
mockC := NewMockCompactor(t)
ex.executing.Insert(int64(1), mockC)

ex.completedCompactor.Insert(int64(2), mockC)
Expand All @@ -150,7 +149,7 @@ func TestCompactionExecutor(t *testing.T) {
require.Equal(t, 2, ex.completedCompactor.Len())
require.Equal(t, 1, ex.executing.Len())

result := ex.getAllCompactionResults()
result := ex.GetResults(0)
assert.Equal(t, 3, len(result))

for _, res := range result {
Expand Down
Loading

0 comments on commit b21e63c

Please sign in to comment.