Skip to content
This repository has been archived by the owner on Nov 16, 2023. It is now read-only.

Split higher level cell when allocated bad cells #27

Merged
merged 11 commits into from
Jul 15, 2020
1 change: 1 addition & 0 deletions pkg/algorithm/cell.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ package algorithm

import (
"fmt"

"github.com/microsoft/hivedscheduler/pkg/api"
"k8s.io/klog"
)
yqwang-ms marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
45 changes: 42 additions & 3 deletions pkg/algorithm/cell_allocation.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,11 @@ package algorithm

import (
"fmt"
"sort"

"github.com/microsoft/hivedscheduler/pkg/api"
"github.com/microsoft/hivedscheduler/pkg/common"
"k8s.io/klog"
"sort"
)

// buddyAlloc is used for allocating a free physical cell to a preassigned virtual cell.
Expand Down Expand Up @@ -89,21 +90,59 @@ func getLowestFreeCellLevel(freeList ChainCellList, l CellLevel) CellLevel {
"even split to the highest level %v", l-1))
}

// when buddyAlloc allocates bad cells,
Copy link
Member

@yqwang-ms yqwang-ms Jul 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when buddyAlloc allocates bad cells, [](start = 3, length = 36)

after buddyAlloc failed to allocate cells, ... #Closed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

// check whether it is safe to split a higher level cell to get current level cells.
func checkSplitSafety(freeList ChainCellList, freeCellNum map[CellLevel]int32, l CellLevel) bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// check whether it is safe to split a higher level cell to get current level cells.
func checkSplitSafety(freeList ChainCellList, freeCellNum map[CellLevel]int32, l CellLevel) bool {
// check whether it is safe to split a higher level cell to get a cell at the checking level.
func checkSplitSafety(freeList ChainCellList, freeCellNum map[CellLevel]int32, checkingLevel CellLevel) bool {

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change to currentLevel as the same in buddyAlloc

var splitableCell Cell
Copy link
Member

@yqwang-ms yqwang-ms Jul 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

splitable [](start = 5, length = 9)

typo: splitable -> splittable #Closed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

splitableNum := map[CellLevel]int32{}
for i := CellLevel(len(freeList)); i >= CellLevel(1); i-- {
// calculate splitable number
Copy link
Member

@yqwang-ms yqwang-ms Jul 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

calculate splitable number [](start = 5, length = 26)

Seems we can early stop the "calculate splitable number" to l+1 #Closed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check safety to panic

Copy link
Member

@yqwang-ms yqwang-ms Jul 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Background: The panic will only fail current pod schedule, instead of exit the whole process)
Lower level safety problems seems does not necessary to fail current allocation, do we need to panic here?
@zhypku any suggestion? #Closed

Copy link
Member

@yqwang-ms yqwang-ms Jul 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems current behaiour is: even if there are safety broken, but if the broken does not impact current pod scheduling, the pod can still be scheduled.
If so, we can insist this behaiour. #Closed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

early stop at current level + 1

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

splitable -> splittable

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

splitableNum[i] = int32(len(freeList[i])) - freeCellNum[i]
if i < CellLevel(len(freeList)) && splitableCell != nil {
splitableNum[i] += splitableNum[i+1] * int32(len(splitableCell.GetChildren()))
}
// iterate higher level cell
if splitableCell == nil && len(freeList[i]) > 0 {
splitableCell = freeList[i][0]
} else if splitableCell != nil {
splitableCell = splitableCell.GetChildren()[0]
}
// check safety
if splitableNum[i] < 0 {
return false
}
Copy link
Member

@yqwang-ms yqwang-ms Jul 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When will splitableNum[i] < 0?
Seems we already safety break, and the whole scheduler should crash. #Closed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in a test case, will change to panic later

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

print more debug info when VC Safety Broken


In reply to: 451561941 [](ancestors = 451561941)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added

}
// if there exists a higher level cell with splitable num > 0
for l++; l <= CellLevel(len(freeList)); l++ {
Copy link
Contributor

@hzhua hzhua Jul 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for l++; l <= CellLevel(len(freeList)); l++ {
for l := CellLevel(checkingLevel)+1 ; l <= CellLevel(len(freeList)); l++ {

if splitableNum[l] > 0 {
return true
}
}
return false
}

// mapVirtualPlacementToPhysical maps cells in a VC placement to the physical cluster.
// For the preassigned cells, it will call buddy alloc to map them;
// For the nonPreassigned cells, it will map them following the topology inside the corresponding preassigned cells.
func mapVirtualPlacementToPhysical(
preassignedCells []*cellBindingPathVertex,
nonPreassignedCells [][]*cellBindingPathVertex,
freeList ChainCellList,
freeCellNum map[CellLevel]int32,
suggestedNodes common.Set,
ignoreSuggestedNodes bool,
bindings map[api.CellAddress]*PhysicalCell) bool {

for _, c := range preassignedCells {
if !buddyAlloc(c, freeList, getLowestFreeCellLevel(
for !buddyAlloc(c, freeList, getLowestFreeCellLevel(
freeList, c.cell.GetLevel()), suggestedNodes, ignoreSuggestedNodes, bindings) {
return false
klog.Info("Buddy allocation failed due to bad cells, try to split higher level cells")
if checkSplitSafety(freeList, freeCellNum, c.cell.GetLevel()) {
klog.Infof("Remove level %v free list %v", c.cell.GetLevel(), freeList[c.cell.GetLevel()])
freeList[c.cell.GetLevel()] = CellList{}
} else {
return false
}
}
}
for _, cells := range nonPreassignedCells {
Expand Down
4 changes: 3 additions & 1 deletion pkg/algorithm/hived_algorithm.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,14 @@ package algorithm

import (
"fmt"
"sync"

"github.com/microsoft/hivedscheduler/pkg/api"
"github.com/microsoft/hivedscheduler/pkg/common"
"github.com/microsoft/hivedscheduler/pkg/internal"
core "k8s.io/api/core/v1"
meta "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/klog"
"sync"
)

// HivedAlgorithm implements an internal.SchedulerAlgorithm. It schedules affinity groups using the algorithm of HiveD.
Expand Down Expand Up @@ -917,6 +918,7 @@ func (h *HivedAlgorithm) scheduleGuaranteedAffinityGroup(
preassignedCells,
nonPreassignedCells,
h.freeCellList[sr.chain].shallowCopy(),
h.allVCFreeCellNum[sr.chain],
sr.suggestedNodes,
sr.ignoreSuggestedNodes,
bindings); ok {
Expand Down
3 changes: 2 additions & 1 deletion pkg/algorithm/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,12 @@ package algorithm

import (
"fmt"
"strings"

"github.com/microsoft/hivedscheduler/pkg/api"
"github.com/microsoft/hivedscheduler/pkg/common"
core "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types"
"strings"
)

type (
Expand Down
5 changes: 3 additions & 2 deletions pkg/scheduler/scheduler.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ package scheduler

import (
"fmt"
"sync"
"time"

"github.com/microsoft/hivedscheduler/pkg/algorithm"
si "github.com/microsoft/hivedscheduler/pkg/api"
"github.com/microsoft/hivedscheduler/pkg/common"
Expand All @@ -39,8 +42,6 @@ import (
"k8s.io/client-go/tools/cache"
"k8s.io/klog"
ei "k8s.io/kubernetes/pkg/scheduler/api"
"sync"
"time"
)

// HivedScheduler is the scheduling framework which serves as the bridge between
Expand Down