From c7da3abd5b35c4615d1b18f2ad38afa2dfa8712d Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Thu, 19 May 2016 14:25:52 +0200 Subject: [PATCH] volume controller: Speed up binding by not sorting volumes The binder sorts all available volumes first, then it filters out volumes that cannot be bound by processing each volume in a loop and then finds the smallest matching volume by binary search. So, if we process every available volume in a loop, we can also remember the smallest matching one and save us potentially long sorting (and quick binary search). --- pkg/controller/persistentvolume/index.go | 72 ++++++++----------- pkg/controller/persistentvolume/index_test.go | 22 +++++- 2 files changed, 52 insertions(+), 42 deletions(-) diff --git a/pkg/controller/persistentvolume/index.go b/pkg/controller/persistentvolume/index.go index 8da72389bdbae..39c85d939ed14 100644 --- a/pkg/controller/persistentvolume/index.go +++ b/pkg/controller/persistentvolume/index.go @@ -38,7 +38,7 @@ func accessModesIndexFunc(obj interface{}) ([]string, error) { return []string{""}, fmt.Errorf("object is not a persistent volume: %v", obj) } -// listByAccessModes returns all volumes with the given set of AccessModeTypes *in order* of their storage capacity (low to high) +// listByAccessModes returns all volumes with the given set of AccessModeTypes. The list is unsorted! func (pvIndex *persistentVolumeOrderedIndex) listByAccessModes(modes []api.PersistentVolumeAccessMode) ([]*api.PersistentVolume, error) { pv := &api.PersistentVolume{ Spec: api.PersistentVolumeSpec{ @@ -56,7 +56,6 @@ func (pvIndex *persistentVolumeOrderedIndex) listByAccessModes(modes []api.Persi volumes[i] = obj.(*api.PersistentVolume) } - sort.Sort(byCapacity{volumes}) return volumes, nil } @@ -75,26 +74,43 @@ func (pvIndex *persistentVolumeOrderedIndex) findByClaim(claim *api.PersistentVo // potential matches (the GCEPD example above). allPossibleModes := pvIndex.allPossibleMatchingAccessModes(claim.Spec.AccessModes) + var smallestVolume *api.PersistentVolume + var smallestVolumeSize int64 + requestedQty := claim.Spec.Resources.Requests[api.ResourceName(api.ResourceStorage)] + requestedSize := requestedQty.Value() + for _, modes := range allPossibleModes { volumes, err := pvIndex.listByAccessModes(modes) if err != nil { return nil, err } - // volumes are sorted by size but some may be bound or earmarked for a specific claim. - // filter those volumes for easy binary search by size - // return the exact pre-binding match, if found - unboundVolumes := []*api.PersistentVolume{} + // Go through all available volumes with two goals: + // - find a volume that is either pre-bound by user or dynamically + // provisioned for this claim. Because of this we need to loop through + // all volumes. + // - find the smallest matching one if there is no volume pre-bound to + // the claim. for _, volume := range volumes { - // volume isn't currently bound or pre-bound. - if volume.Spec.ClaimRef == nil { - unboundVolumes = append(unboundVolumes, volume) + if isVolumeBoundToClaim(volume, claim) { + // Exact match! No search required. This catches both volumes + // pre-bound by user and volumes dynamically provisioned by the + // controller. + return volume, nil + } + + if volume.Spec.ClaimRef != nil { + // This volume waits for exact claim or is alredy bound. continue } - if isVolumeBoundToClaim(volume, claim) { - // Exact match! No search required. - return volume, nil + volumeQty := volume.Spec.Capacity[api.ResourceStorage] + volumeSize := volumeQty.Value() + if volumeSize >= requestedSize { + if smallestVolume == nil || smallestVolumeSize > volumeSize { + smallestVolume = volume + smallestVolumeSize = volumeSize + } } } @@ -109,18 +125,9 @@ func (pvIndex *persistentVolumeOrderedIndex) findByClaim(claim *api.PersistentVo return nil, nil } - searchPV := &api.PersistentVolume{ - Spec: api.PersistentVolumeSpec{ - AccessModes: claim.Spec.AccessModes, - Capacity: api.ResourceList{ - api.ResourceName(api.ResourceStorage): claim.Spec.Resources.Requests[api.ResourceName(api.ResourceStorage)], - }, - }, - } - - i := sort.Search(len(unboundVolumes), func(i int) bool { return matchPredicate(searchPV, unboundVolumes[i]) }) - if i < len(unboundVolumes) { - return unboundVolumes[i], nil + if smallestVolume != nil { + // Found a matching volume + return smallestVolume, nil } } return nil, nil @@ -131,23 +138,6 @@ func (pvIndex *persistentVolumeOrderedIndex) findBestMatchForClaim(claim *api.Pe return pvIndex.findByClaim(claim, matchStorageCapacity) } -// byCapacity is used to order volumes by ascending storage size -type byCapacity struct { - volumes []*api.PersistentVolume -} - -func (c byCapacity) Less(i, j int) bool { - return matchStorageCapacity(c.volumes[i], c.volumes[j]) -} - -func (c byCapacity) Swap(i, j int) { - c.volumes[i], c.volumes[j] = c.volumes[j], c.volumes[i] -} - -func (c byCapacity) Len() int { - return len(c.volumes) -} - // matchStorageCapacity is a matchPredicate used to sort and find volumes func matchStorageCapacity(pvA, pvB *api.PersistentVolume) bool { aQty := pvA.Spec.Capacity[api.ResourceStorage] diff --git a/pkg/controller/persistentvolume/index_test.go b/pkg/controller/persistentvolume/index_test.go index 61134f13700c8..c80984649cd74 100644 --- a/pkg/controller/persistentvolume/index_test.go +++ b/pkg/controller/persistentvolume/index_test.go @@ -17,6 +17,7 @@ limitations under the License. package persistentvolume import ( + "sort" "testing" "k8s.io/kubernetes/pkg/api" @@ -193,7 +194,7 @@ func TestMatchingWithBoundVolumes(t *testing.T) { } } -func TestSort(t *testing.T) { +func TestListByAccessModes(t *testing.T) { volList := newPersistentVolumeOrderedIndex() for _, pv := range createTestVolumes() { volList.store.Add(pv) @@ -203,6 +204,7 @@ func TestSort(t *testing.T) { if err != nil { t.Error("Unexpected error retrieving volumes by access modes:", err) } + sort.Sort(byCapacity{volumes}) for i, expected := range []string{"gce-pd-1", "gce-pd-5", "gce-pd-10"} { if string(volumes[i].UID) != expected { @@ -214,6 +216,7 @@ func TestSort(t *testing.T) { if err != nil { t.Error("Unexpected error retrieving volumes by access modes:", err) } + sort.Sort(byCapacity{volumes}) for i, expected := range []string{"nfs-1", "nfs-5", "nfs-10"} { if string(volumes[i].UID) != expected { @@ -552,3 +555,20 @@ func TestFindingPreboundVolumes(t *testing.T) { t.Errorf("Expected %s but got volume %s instead", pv8.Name, volume.Name) } } + +// byCapacity is used to order volumes by ascending storage size +type byCapacity struct { + volumes []*api.PersistentVolume +} + +func (c byCapacity) Less(i, j int) bool { + return matchStorageCapacity(c.volumes[i], c.volumes[j]) +} + +func (c byCapacity) Swap(i, j int) { + c.volumes[i], c.volumes[j] = c.volumes[j], c.volumes[i] +} + +func (c byCapacity) Len() int { + return len(c.volumes) +}