When a lot of persistentvolumes are created together, POST persistentvolume request latency grows significantly

[mborsz]

What happened:
I was running a scale test where I created ~8 PV/s for some period of time. I observed that POST persistentvolume latency started growing

I debugged this already:

• kube-controller-manager created PD objects in GCE
• due to rate limited error, it failed to fetch labels for new disk: https://github.com/kubernetes/kubernetes/blob/master/pkg/volume/gcepd/gce_util.go#L191
• PersistentVolumeLabel admission controller tried to add missing GCE labels, but failed due to the same rate limited error (this also caused significant delay in request processing)

What you expected to happen:

POST persistentvolume latency to be ~constant during the test.

This can be achieved by:

1. removing cloud.GetAutoLabelsForZonalPD call from https://github.com/kubernetes/kubernetes/blob/master/pkg/volume/gcepd/gce_util.go#L188
   • this way, there is no API call that can fail after object is created
   • since we just successfully created a PD, we know all parameters for that PD (zone, region, potentially in future some other disk's properties as well) and we can use this information for labels computation and there is no point in validating if it still exists.
2. Implementing proper retry strategy for cloud.GetAutoLabelsForPD calls so that we do not move load to kube-apiserver when kube-controller-manager is overloaded.
3. (partially) by simply passing zone argument in https://github.com/kubernetes/kubernetes/blob/master/pkg/volume/gcepd/gce_util.go#L188 we can reduce number of calls there 3-4x times in case of zonal disks, but this is a mitigation only

For me, 1 is the most reasonable thing to do (and I have some draft PR for that), but I wanted to hear your opinion on this before I start cleaning the PR.

Potential drawbacks of this approach are:

• Right now, if disk with given name already exists, we simply reuse that disk (https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/legacy-cloud-providers/gce/gce_disks.go#L732). In this case, we in fact don't know what replicationZones have been used for creation of that PD, so we do need to get them in that case. We can limit get calls to only this case.

How to reproduce it (as minimally and precisely as possible):
Just create a lot of pvs together.
Anything else we need to know?:

Environment:

• Kubernetes version (use kubectl version):
• Cloud provider or hardware configuration:
• OS (e.g: cat /etc/os-release):
• Kernel (e.g. uname -a):
• Install tools:
• Network plugin and version (if this is a network-related bug):
• Others:

/assign @saad-ali
/cc @wojtek-t

[wojtek-t]

/sig storage

@kubernetes/sig-storage-bugs @msau42

[mborsz]

Some draft PR that implements 1: #87811

The overall idea is to modify CreateDisk methods to return Disk object on success (CreateDisk* methods have all necessary data to do that).
If we will need to add some other label in the future that can be deduced based on the input data this approach will still work just fine. Otherwise (e.g. if we want to add labels that cannot be easily deduced based on the input, e.g. expected IOPs rate or creationTimestamp), we will need to add GCE call back.

Please let me know what you think about this approach.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[wojtek-t]

/remove-lifecycle stale

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[wojtek-t]

/remove-lifecycle stale

This has been partially addressed by #87811 but we need to verify if we need more.
/lifecycle frozen