Merge pull request #3500 from jayantjain93/read-request-saturation

Decrease the number of GCE Read Requests when node deletion.
kubernetes · Sep 10, 2020 · 63259fb · 63259fb
2 parents a80841e + 821f42d
commit 63259fb
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 20 deletions.
diff --git a/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go b/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client.go
@@ -34,9 +34,9 @@ import (
 )
 
 const (
-	defaultOperationWaitTimeout  = 20 * time.Second
-	defaultOperationPollInterval = 100 * time.Millisecond
-
+	defaultOperationWaitTimeout          = 20 * time.Second
+	defaultOperationPollInterval         = 100 * time.Millisecond
+	defaultOperationDeletionPollInterval = 1 * time.Second
 	// ErrorCodeQuotaExceeded is error code used in InstanceErrorInfo if quota exceeded error occurs.
 	ErrorCodeQuotaExceeded = "QUOTA_EXCEEDED"
 
@@ -73,8 +73,9 @@ type autoscalingGceClientV1 struct {
 	projectId string
 
 	// These can be overridden, e.g. for testing.
-	operationWaitTimeout  time.Duration
-	operationPollInterval time.Duration
+	operationWaitTimeout          time.Duration
+	operationPollInterval         time.Duration
+	operationDeletionPollInterval time.Duration
 }
 
 // NewAutoscalingGceClientV1 creates a new client for communicating with GCE v1 API.
@@ -85,28 +86,30 @@ func NewAutoscalingGceClientV1(client *http.Client, projectId string) (*autoscal
 	}
 
 	return &autoscalingGceClientV1{
-		projectId:             projectId,
-		gceService:            gceService,
-		operationWaitTimeout:  defaultOperationWaitTimeout,
-		operationPollInterval: defaultOperationPollInterval,
+		projectId:                     projectId,
+		gceService:                    gceService,
+		operationWaitTimeout:          defaultOperationWaitTimeout,
+		operationPollInterval:         defaultOperationPollInterval,
+		operationDeletionPollInterval: defaultOperationDeletionPollInterval,
 	}, nil
 }
 
 // NewCustomAutoscalingGceClientV1 creates a new client using custom server url and timeouts
 // for communicating with GCE v1 API.
 func NewCustomAutoscalingGceClientV1(client *http.Client, projectId, serverUrl string,
-	waitTimeout, pollInterval time.Duration) (*autoscalingGceClientV1, error) {
+	waitTimeout, pollInterval time.Duration, deletionPollInterval time.Duration) (*autoscalingGceClientV1, error) {
 	gceService, err := gce.New(client)
 	if err != nil {
 		return nil, err
 	}
 	gceService.BasePath = serverUrl
 
 	return &autoscalingGceClientV1{
-		projectId:             projectId,
-		gceService:            gceService,
-		operationWaitTimeout:  waitTimeout,
-		operationPollInterval: pollInterval,
+		projectId:                     projectId,
+		gceService:                    gceService,
+		operationWaitTimeout:          waitTimeout,
+		operationPollInterval:         pollInterval,
+		operationDeletionPollInterval: deletionPollInterval,
 	}, nil
 }
 
@@ -169,11 +172,15 @@ func (client *autoscalingGceClientV1) ResizeMig(migRef GceRef, size int64) error
 	if err != nil {
 		return err
 	}
-	return client.waitForOp(op, migRef.Project, migRef.Zone)
+	return client.waitForOp(op, migRef.Project, migRef.Zone, false)
 }
 
-func (client *autoscalingGceClientV1) waitForOp(operation *gce.Operation, project, zone string) error {
-	for start := time.Now(); time.Since(start) < client.operationWaitTimeout; time.Sleep(client.operationPollInterval) {
+func (client *autoscalingGceClientV1) waitForOp(operation *gce.Operation, project, zone string, isDeletion bool) error {
+	pollInterval := client.operationPollInterval
+	if isDeletion {
+		pollInterval = client.operationDeletionPollInterval
+	}
+	for start := time.Now(); time.Since(start) < client.operationWaitTimeout; time.Sleep(pollInterval) {
 		klog.V(4).Infof("Waiting for operation %s %s %s", project, zone, operation.Name)
 		registerRequest("zone_operations", "get")
 		if op, err := client.gceService.ZoneOperations.Get(project, zone, operation.Name).Do(); err == nil {
@@ -200,7 +207,7 @@ func (client *autoscalingGceClientV1) DeleteInstances(migRef GceRef, instances [
 	if err != nil {
 		return err
 	}
-	return client.waitForOp(op, migRef.Project, migRef.Zone)
+	return client.waitForOp(op, migRef.Project, migRef.Zone, true)
 }
 
 func (client *autoscalingGceClientV1) FetchMigInstances(migRef GceRef) ([]cloudprovider.Instance, error) {

diff --git a/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client_test.go b/cluster-autoscaler/cloudprovider/gce/autoscaling_gce_client_test.go
@@ -73,7 +73,7 @@ func TestWaitForOp(t *testing.T) {
 
 	operation := &gce_api.Operation{Name: "operation-1505728466148-d16f5197"}
 
-	err := g.waitForOp(operation, projectId, zoneB)
+	err := g.waitForOp(operation, projectId, zoneB, false)
 	assert.NoError(t, err)
 	mock.AssertExpectationsForObjects(t, server)
 }
@@ -94,6 +94,6 @@ func TestWaitForOpTimeout(t *testing.T) {
 
 	operation := &gce_api.Operation{Name: "operation-1505728466148-d16f5197"}
 
-	err := g.waitForOp(operation, projectId, zoneB)
+	err := g.waitForOp(operation, projectId, zoneB, false)
 	assert.Error(t, err)
 }