Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: enhanced maxSurge performance #1929

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Feature: enhanced maxSurge performance #1929

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 17 additions & 18 deletions pkg/controller/cloneset/sync/cloneset_scale.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,17 +70,16 @@ func (r *realControl) Scale(
// 2. calculate scale numbers
diffRes := calculateDiffsWithExpectation(updateCS, pods, currentRevision, updateRevision, revision.IsPodUpdate)
updatedPods, notUpdatedPods := clonesetutils.GroupUpdateAndNotUpdatePods(pods, updateRevision)

if diffRes.scaleUpNum > diffRes.scaleUpLimit {
r.recorder.Event(updateCS, v1.EventTypeWarning, "ScaleUpLimited", fmt.Sprintf("scaleUp is limited because of scaleStrategy.maxUnavailable, limit: %d", diffRes.scaleUpLimit))
if diffRes.ScaleUpNum > diffRes.ScaleUpLimit {
r.recorder.Event(updateCS, v1.EventTypeWarning, "ScaleUpLimited", fmt.Sprintf("scaleUp is limited because of scaleStrategy.maxUnavailable, limit: %d", diffRes.ScaleUpLimit))
}

// 3. scale out
if diffRes.scaleUpNum > 0 {
if diffRes.ScaleUpNum > 0 {
// total number of this creation
expectedCreations := diffRes.scaleUpLimit
expectedCreations := diffRes.ScaleUpLimit
// lack number of current version
expectedCurrentCreations := diffRes.scaleUpNumOldRevision
expectedCurrentCreations := diffRes.ScaleUpNumOldRevision

klog.V(3).InfoS("CloneSet began to scale out pods, including current revision",
"cloneSet", klog.KObj(updateCS), "expectedCreations", expectedCreations, "expectedCurrentCreations", expectedCurrentCreations)
Expand Down Expand Up @@ -108,16 +107,16 @@ func (r *realControl) Scale(
// 5. specified delete
if podsToDelete := util.DiffPods(podsSpecifiedToDelete, podsInPreDelete); len(podsToDelete) > 0 {
newPodsToDelete, oldPodsToDelete := clonesetutils.GroupUpdateAndNotUpdatePods(podsToDelete, updateRevision)
klog.V(3).InfoS("CloneSet tried to delete pods specified", "cloneSet", klog.KObj(updateCS), "deleteReadyLimit", diffRes.deleteReadyLimit,
klog.V(3).InfoS("CloneSet tried to delete pods specified", "cloneSet", klog.KObj(updateCS), "DeleteReadyLimit", diffRes.DeleteReadyLimit,
"newPods", util.GetPodNames(newPodsToDelete).List(), "oldPods", util.GetPodNames(oldPodsToDelete).List())

podsCanDelete := make([]*v1.Pod, 0, len(podsToDelete))
for _, pod := range podsToDelete {
if !isPodReady(coreControl, pod) {
if !isPodReady(coreControl, pod, updateCS.Spec.MinReadySeconds) {
podsCanDelete = append(podsCanDelete, pod)
} else if diffRes.deleteReadyLimit > 0 {
} else if diffRes.DeleteReadyLimit > 0 {
podsCanDelete = append(podsCanDelete, pod)
diffRes.deleteReadyLimit--
diffRes.DeleteReadyLimit--
}
}

Expand All @@ -127,24 +126,24 @@ func (r *realControl) Scale(
}

// 6. scale in
if diffRes.scaleDownNum > 0 {
if diffRes.ScaleDownNum > 0 {
if numToDelete > 0 {
klog.V(3).InfoS("CloneSet skipped to scale in for deletion", "cloneSet", klog.KObj(updateCS), "scaleDownNum", diffRes.scaleDownNum,
klog.V(3).InfoS("CloneSet skipped to scale in for deletion", "cloneSet", klog.KObj(updateCS), "ScaleDownNum", diffRes.ScaleDownNum,
"numToDelete", numToDelete, "specifiedToDelete", len(podsSpecifiedToDelete), "preDelete", len(podsInPreDelete))
return false, nil
}

klog.V(3).InfoS("CloneSet began to scale in", "cloneSet", klog.KObj(updateCS), "scaleDownNum", diffRes.scaleDownNum,
"oldRevision", diffRes.scaleDownNumOldRevision, "deleteReadyLimit", diffRes.deleteReadyLimit)
klog.V(3).InfoS("CloneSet began to scale in", "cloneSet", klog.KObj(updateCS), "ScaleDownNum", diffRes.ScaleDownNum,
"oldRevision", diffRes.ScaleDownNumOldRevision, "DeleteReadyLimit", diffRes.DeleteReadyLimit)

podsPreparingToDelete := r.choosePodsToDelete(updateCS, diffRes.scaleDownNum, diffRes.scaleDownNumOldRevision, notUpdatedPods, updatedPods)
podsPreparingToDelete := r.choosePodsToDelete(updateCS, diffRes.ScaleDownNum, diffRes.ScaleDownNumOldRevision, notUpdatedPods, updatedPods)
podsToDelete := make([]*v1.Pod, 0, len(podsPreparingToDelete))
for _, pod := range podsPreparingToDelete {
if !isPodReady(coreControl, pod) {
if !isPodReady(coreControl, pod, updateCS.Spec.MinReadySeconds) {
podsToDelete = append(podsToDelete, pod)
} else if diffRes.deleteReadyLimit > 0 {
} else if diffRes.DeleteReadyLimit > 0 {
podsToDelete = append(podsToDelete, pod)
diffRes.deleteReadyLimit--
diffRes.DeleteReadyLimit--
}
}

Expand Down
94 changes: 51 additions & 43 deletions pkg/controller/cloneset/sync/cloneset_sync_utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,39 +49,39 @@ var (
)

type expectationDiffs struct {
// scaleUpNum is a non-negative integer, which indicates the number that should scale up.
scaleUpNum int
// ScaleUpNum is a non-negative integer, which indicates the number that should scale up.
ScaleUpNum int `json:"scaleUpNum"`
// scaleNumOldRevision is a non-negative integer, which indicates the number of old revision Pods that should scale up.
// It might be bigger than scaleUpNum, but controller will scale up at most scaleUpNum number of Pods.
scaleUpNumOldRevision int
// scaleDownNum is a non-negative integer, which indicates the number that should scale down.
// It might be bigger than ScaleUpNum, but controller will scale up at most ScaleUpNum number of Pods.
ScaleUpNumOldRevision int `json:"scaleUpNumOldRevision"`
// ScaleDownNum is a non-negative integer, which indicates the number that should scale down.
// It has excluded the number of Pods that are already specified to delete.
scaleDownNum int
// scaleDownNumOldRevision is a non-negative integer, which indicates the number of old revision Pods that should scale down.
// It might be bigger than scaleDownNum, but controller will scale down at most scaleDownNum number of Pods.
ScaleDownNum int `json:"scaleDownNum"`
// ScaleDownNumOldRevision is a non-negative integer, which indicates the number of old revision Pods that should scale down.
// It might be bigger than ScaleDownNum, but controller will scale down at most ScaleDownNum number of Pods.
// It has excluded the number of old Pods that are already specified to delete.
scaleDownNumOldRevision int
ScaleDownNumOldRevision int `json:"scaleDownNumOldRevision"`

// scaleUpLimit is the limit number of creating Pods when scaling up
// ScaleUpLimit is the limit number of creating Pods when scaling up
// it is limited by scaleStrategy.maxUnavailable
scaleUpLimit int
// deleteReadyLimit is the limit number of ready Pods that can be deleted
ScaleUpLimit int `json:"scaleUpLimit"`
// DeleteReadyLimit is the limit number of ready Pods that can be deleted
// it is limited by UpdateStrategy.maxUnavailable
deleteReadyLimit int
DeleteReadyLimit int `json:"deleteReadyLimit"`

// useSurge is the number that temporarily expect to be above the desired replicas
useSurge int
// useSurgeOldRevision is part of the useSurge number
// UseSurge is the number that temporarily expect to be above the desired replicas
UseSurge int `json:"useSurge"`
// UseSurgeOldRevision is part of the UseSurge number
// it indicates the above number of old revision Pods
useSurgeOldRevision int
UseSurgeOldRevision int `json:"useSurgeOldRevision"`

// updateNum is the diff number that should update
// UpdateNum is the diff number that should update
// '0' means no need to update
// positive number means need to update more Pods to updateRevision
// negative number means need to update more Pods to currentRevision (rollback)
updateNum int
// updateMaxUnavailable is the maximum number of ready Pods that can be updating
updateMaxUnavailable int
UpdateNum int `json:"updateNum"`
// UpdateMaxUnavailable is the maximum number of ready Pods that can be updating
UpdateMaxUnavailable int `json:"updateMaxUnavailable"`
}

func (e expectationDiffs) isEmpty() bool {
Expand Down Expand Up @@ -131,7 +131,7 @@ func calculateDiffsWithExpectation(cs *appsv1alpha1.CloneSet, pods []*v1.Pod, cu
}
klog.V(1).InfoS("Calculate diffs for CloneSet", "cloneSet", klog.KObj(cs), "replicas", replicas, "partition", partition,
"maxSurge", maxSurge, "maxUnavailable", maxUnavailable, "allPodCount", len(pods), "newRevisionCount", newRevisionCount,
"newRevisionActiveCount", newRevisionActiveCount, "oldrevisionCount", oldRevisionCount, "oldRevisionActiveCount", oldRevisionActiveCount,
"newRevisionActiveCount", newRevisionActiveCount, "oldRevisionCount", oldRevisionCount, "oldRevisionActiveCount", oldRevisionActiveCount,
"unavailableNewRevisionCount", unavailableNewRevisionCount, "unavailableOldRevisionCount", unavailableOldRevisionCount,
"preDeletingNewRevisionCount", preDeletingNewRevisionCount, "preDeletingOldRevisionCount", preDeletingOldRevisionCount,
"toDeleteNewRevisionCount", toDeleteNewRevisionCount, "toDeleteOldRevisionCount", toDeleteOldRevisionCount,
Expand Down Expand Up @@ -210,27 +210,35 @@ func calculateDiffsWithExpectation(cs *appsv1alpha1.CloneSet, pods []*v1.Pod, cu

// Use surge for old and new revision updating
var updateSurge, updateOldRevisionSurge int
if util.IsIntPlusAndMinus(updateOldDiff, updateNewDiff) {
// TODO delete the comments when released
if util.IsIntPlusAndMinus(updateOldDiff, updateNewDiff) || (updateNewDiff == 0 && unavailableNewRevisionCount > 0) {
if util.IntAbs(updateOldDiff) <= util.IntAbs(updateNewDiff) {
// 要删的旧 Pod <= 要建的新 Pod，允许建最多要删除旧 Pod 数量的新 Pod（100%，后面会用 maxSurge 限制）
updateSurge = util.IntAbs(updateOldDiff)
if updateOldDiff < 0 {
// 如果要建旧 Pod，允许新增要新建数量的旧 Pod
updateOldRevisionSurge = updateSurge
}
} else {
updateSurge = util.IntAbs(updateNewDiff)
// 要删的旧 Pod > 要建的新 Pod，不允许再新建 Pod，等待后面删除旧 Pod
//updateSurge = util.IntAbs(updateNewDiff)
// => 改为无论如何都允许新建旧 Pod 数量的新 Pod
updateSurge = util.IntAbs(updateOldDiff)
if updateNewDiff > 0 {
// 如果要删新 Pod，保留相等数量的旧 Pod
updateOldRevisionSurge = updateSurge
}
}
}

// It is because the controller is designed not to do scale and update in once reconcile
if scaleSurge >= updateSurge {
res.useSurge = integer.IntMin(maxSurge, scaleSurge)
res.useSurgeOldRevision = integer.IntMin(res.useSurge, scaleOldRevisionSurge)
res.UseSurge = integer.IntMin(maxSurge, scaleSurge)
res.UseSurgeOldRevision = integer.IntMin(res.UseSurge, scaleOldRevisionSurge)
} else {
res.useSurge = integer.IntMin(maxSurge, updateSurge)
res.useSurgeOldRevision = integer.IntMin(res.useSurge, updateOldRevisionSurge)
res.UseSurge = integer.IntMin(maxSurge, updateSurge)
res.UseSurgeOldRevision = integer.IntMin(res.UseSurge, updateOldRevisionSurge)
res.UseSurgeOldRevision = integer.IntMin(res.UseSurgeOldRevision+unavailableNewRevisionCount, oldRevisionActiveCount-partition)
}
}

Expand All @@ -241,35 +249,35 @@ func calculateDiffsWithExpectation(cs *appsv1alpha1.CloneSet, pods []*v1.Pod, cu
currentTotalCount = currentTotalCount - preDeletingOldRevisionCount - preDeletingNewRevisionCount
currentTotalOldCount = currentTotalOldCount - preDeletingOldRevisionCount
}
expectedTotalCount := replicas + res.useSurge
expectedTotalOldCount := partition + res.useSurgeOldRevision
expectedTotalCount := replicas + res.UseSurge
expectedTotalOldCount := partition + res.UseSurgeOldRevision

// scale up
if num := expectedTotalCount - currentTotalCount; num > 0 {
res.scaleUpNum = num
res.scaleUpNumOldRevision = integer.IntMax(expectedTotalOldCount-currentTotalOldCount, 0)
res.ScaleUpNum = num
res.ScaleUpNumOldRevision = integer.IntMax(expectedTotalOldCount-currentTotalOldCount, 0)

res.scaleUpLimit = integer.IntMin(res.scaleUpNum, integer.IntMax(scaleMaxUnavailable-totalUnavailable, 0))
res.ScaleUpLimit = integer.IntMin(res.ScaleUpNum, integer.IntMax(scaleMaxUnavailable-totalUnavailable, 0))
}

// scale down
// Note that this should exclude the number of Pods that are already specified to delete.
if num := currentTotalCount - toDeleteOldRevisionCount - toDeleteNewRevisionCount - expectedTotalCount; num > 0 {
res.scaleDownNum = num
res.scaleDownNumOldRevision = integer.IntMax(currentTotalOldCount-toDeleteOldRevisionCount-expectedTotalOldCount, 0)
res.ScaleDownNum = num
res.ScaleDownNumOldRevision = integer.IntMax(currentTotalOldCount-toDeleteOldRevisionCount-expectedTotalOldCount, 0)
}
if toDeleteNewRevisionCount > 0 || toDeleteOldRevisionCount > 0 || res.scaleDownNum > 0 {
res.deleteReadyLimit = integer.IntMax(maxUnavailable+(len(pods)-replicas)-totalUnavailable, 0)
if toDeleteNewRevisionCount > 0 || toDeleteOldRevisionCount > 0 || res.ScaleDownNum > 0 {
res.DeleteReadyLimit = integer.IntMax(maxUnavailable+(len(pods)-replicas)-totalUnavailable, 0)
}

// The consistency between scale and update will be guaranteed by syncCloneSet and expectations
if util.IntAbs(updateOldDiff) <= util.IntAbs(updateNewDiff) {
res.updateNum = updateOldDiff
res.UpdateNum = updateOldDiff
} else {
res.updateNum = 0 - updateNewDiff
res.UpdateNum = 0 - updateNewDiff
}
if res.updateNum != 0 {
res.updateMaxUnavailable = maxUnavailable + len(pods) - replicas
if res.UpdateNum != 0 {
res.UpdateMaxUnavailable = maxUnavailable + len(pods) - replicas
}

return
Expand All @@ -287,8 +295,8 @@ func isSpecifiedDelete(cs *appsv1alpha1.CloneSet, pod *v1.Pod) bool {
return false
}

func isPodReady(coreControl clonesetcore.Control, pod *v1.Pod) bool {
return IsPodAvailable(coreControl, pod, 0)
func isPodReady(coreControl clonesetcore.Control, pod *v1.Pod, minReadySeconds int32) bool {
return IsPodAvailable(coreControl, pod, minReadySeconds)
}

func IsPodAvailable(coreControl clonesetcore.Control, pod *v1.Pod, minReadySeconds int32) bool {
Expand Down
Loading
Loading