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.

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

why RemoveNodeFromTracker clean the passed utilization map? #6889

Open
duyawen8 opened this issue Jun 4, 2024 · 1 comment
Open

why RemoveNodeFromTracker clean the passed utilization map? #6889

duyawen8 opened this issue Jun 4, 2024 · 1 comment
Labels
area/cluster-autoscaler kind/bug Categorizes issue or PR as related to a bug.

Comments

@duyawen8
Copy link

duyawen8 commented Jun 4, 2024

Which component are you using?: cluster-autoscaler

What version of the component are you using?: cluster-autoscaler-1.23.0

Component version: cluster-autoscaler-1.23.0

What k8s version are you using (kubectl version)?:

kubectl version Output
$ kubectl version

What environment is this in?:
self-build kubernetes cluster, not cloud provider cluster

What did you expect to happen?: don't reset unneed time

What happened instead?: reset unneed time

How to reproduce it (as minimally and precisely as possible):

Anything else we need to know?:

  1. Why clean unneededNodes when the number of using or used node exceed 50? What is the purpose? It makes our cluster scale-down slow because clean means reset unneeded time.

cluster-autoscaler/core/scale_down.go

// Nothing super-bad should happen if the node is removed from tracker prematurely.
simulator.RemoveNodeFromTracker(sd.usageTracker, toRemove.Node.Name, sd.unneededNodes)
nodeDeletionStart := time.Now()

cluster-autoscaler/simulator/tracker.go

// RemoveNodeFromTracker removes node from tracker and also cleans the passed utilization map.
func RemoveNodeFromTracker(tracker *UsageTracker, node string, utilization map[string]time.Time) {
	klog.V(4).Infof("Removing node %s from utilization map", node)
	keysToRemove := make([]string, 0)
	if mainRecord, found := tracker.Get(node); found {
		if mainRecord.usingTooMany {
			klog.V(4).Infof("Node %s is using too many nodes, removing all keys from utilization map", node)
			keysToRemove = getAllKeys(utilization)
		} else {
		usingloop:
			for usedNode := range mainRecord.using {
				if usedNodeRecord, found := tracker.Get(usedNode); found {
					if usedNodeRecord.usedByTooMany {
						klog.V(4).Infof("Node %s is used by too many nodes, removing all keys from utilization map", usedNode)
						keysToRemove = getAllKeys(utilization)
						break usingloop
					} else {
						for anotherNode := range usedNodeRecord.usedBy {
							keysToRemove = append(keysToRemove, anotherNode)
						}
					}
				}
			}
		}
	}
	tracker.Unregister(node)
	delete(utilization, node)
	for _, key := range keysToRemove {
		delete(utilization, key)
	}
}
@duyawen8 duyawen8 added the kind/bug Categorizes issue or PR as related to a bug. label Jun 4, 2024
@adrianmoisey
Copy link
Contributor

/area cluster-autoscaler

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cluster-autoscaler kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

No branches or pull requests

3 participants