Skip to content

Commit

Permalink
volume controller: Speed up binding by not sorting volumes
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
jsafrane committed May 20, 2016
1 parent ab10484 commit c7da3ab
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 42 deletions.
72 changes: 31 additions & 41 deletions pkg/controller/persistentvolume/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -56,7 +56,6 @@ func (pvIndex *persistentVolumeOrderedIndex) listByAccessModes(modes []api.Persi
volumes[i] = obj.(*api.PersistentVolume)
}

sort.Sort(byCapacity{volumes})
return volumes, nil
}

Expand All @@ -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
}
}
}

Expand All @@ -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
Expand All @@ -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]
Expand Down
22 changes: 21 additions & 1 deletion pkg/controller/persistentvolume/index_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package persistentvolume

import (
"sort"
"testing"

"k8s.io/kubernetes/pkg/api"
Expand Down Expand Up @@ -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)
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
}

0 comments on commit c7da3ab

Please sign in to comment.