Skip to content

Commit

Permalink
gce: rm unnecessary GetMigInstanceTemplate locking
Browse files Browse the repository at this point in the history
The lock held from (and covering most of) `GetMigInstanceTemplate()` prevents
`InstanceTemplates.Get()` concurrent executions, while it's not really needed
as all caches access functions used here already take care of locking cache
accesses (and it's reading and filling one single cache entry per call).

`InstanceTemplates.Get()` isn't fast (at about 0.5s/call in my test env),
running all calls sequentially slows down startup significantly:

```
forceRefresh()
  fetchAutoMigs()
    workqueue.ParallelizeUntil(..., func() { <- can launch concurrent registerMigs
      registerMig()
        GetMigTemplateNode()
          GetMigInstanceTemplate() <- lock held
            FetchMigTemplate()
              client.gceService.InstanceTemplates.Get() <- slow API call
```

When running this change on a cluster having 800 MIGs, startup went from 6min to 1.5min.
Also tested with a `-race` build.
  • Loading branch information
bpineau committed Feb 7, 2022
1 parent ea565af commit 432db09
Showing 1 changed file with 0 additions and 3 deletions.
3 changes: 0 additions & 3 deletions cluster-autoscaler/cloudprovider/gce/mig_info_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,9 +244,6 @@ func (c *cachingMigInfoProvider) GetMigInstanceTemplate(migRef GceRef) (*gce.Ins
return nil, err
}

c.migInfoMutex.Lock()
defer c.migInfoMutex.Unlock()

template, found := c.cache.GetMigInstanceTemplate(migRef)
if found && template.Name == templateName {
return template, nil
Expand Down

0 comments on commit 432db09

Please sign in to comment.