github/kubernetes
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #35194 from wojtek-t/efficient_selector

Browse Source 
Automatic merge from submit-queue

Optimize label selector

The number of values for a given label is generally pretty small (in huge majority of cases it is exactly one value).
Currently computing selectors is up to 50% of CPU usage in both apiserver and scheduler.

Changing the structure in which those values are stored from map to slice improves the performance of typical usecase for computing selectors.

Early results:
- scheduler throughput it ~15% higher
- apiserver cpu-usage is also lower (seems to be also ~10-15%)
This commit is contained in:
Kubernetes Submit Queue
2016-10-20 20:45:22 -07:00
committed by GitHub
parent 90c4a13312 9c91da7a22
commit c6df88991d
5 changed files with 45 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
pkg/api/helpers.go
View File

@@ -417,7 +417,7 @@ func NodeSelectorRequirementsAsSelector(nsm []NodeSelectorRequirement) (labels.S
default:
return nil, fmt.Errorf("%q is not a valid node selector operator", expr.Operator)
}
r, err := labels.NewRequirement(expr.Key, op, sets.NewString(expr.Values...))
r, err := labels.NewRequirement(expr.Key, op, expr.Values)
if err != nil {
return nil, err
}

5
pkg/api/unversioned/helpers.go
View File

@@ -21,7 +21,6 @@ import (
"k8s.io/kubernetes/pkg/labels"
"k8s.io/kubernetes/pkg/selection"
"k8s.io/kubernetes/pkg/util/sets"
)
// LabelSelectorAsSelector converts the LabelSelector api type into a struct that implements
@@ -36,7 +35,7 @@ func LabelSelectorAsSelector(ps *LabelSelector) (labels.Selector, error) {
}
selector := labels.NewSelector()
for k, v := range ps.MatchLabels {
r, err := labels.NewRequirement(k, selection.Equals, sets.NewString(v))
r, err := labels.NewRequirement(k, selection.Equals, []string{v})
if err != nil {
return nil, err
}
@@ -56,7 +55,7 @@ func LabelSelectorAsSelector(ps *LabelSelector) (labels.Selector, error) {
default:
return nil, fmt.Errorf("%q is not a valid pod selector operator", expr.Operator)
}
r, err := labels.NewRequirement(expr.Key, op, sets.NewString(expr.Values...))
r, err := labels.NewRequirement(expr.Key, op, append([]string(nil), expr.Values...))
if err != nil {
return nil, err
}

55
pkg/labels/selector.go
View File

@@ -91,9 +91,12 @@ func (a ByKey) Less(i, j int) bool { return a[i].key < a[j].key }
// Requirement implements both set based match and exact match
// Requirement should be initialized via NewRequirement constructor for creating a valid Requirement.
type Requirement struct {
key string
operator selection.Operator
strValues sets.String
key string
operator selection.Operator
// In huge majority of cases we have at most one value here.
// It is generally faster to operate on a single-element slice
// than on a single-element map, so we have a slice here.
strValues []string
}
// NewRequirement is the constructor for a Requirement.
@@ -107,7 +110,7 @@ type Requirement struct {
// of characters. See validateLabelKey for more details.
//
// The empty string is a valid value in the input values set.
func NewRequirement(key string, op selection.Operator, vals sets.String) (*Requirement, error) {
func NewRequirement(key string, op selection.Operator, vals []string) (*Requirement, error) {
if err := validateLabelKey(key); err != nil {
return nil, err
}
@@ -128,8 +131,8 @@ func NewRequirement(key string, op selection.Operator, vals sets.String) (*Requi
if len(vals) != 1 {
return nil, fmt.Errorf("for 'Gt', 'Lt' operators, exactly one value is required")
}
for val := range vals {
if _, err := strconv.ParseInt(val, 10, 64); err != nil {
for i := range vals {
if _, err := strconv.ParseInt(vals[i], 10, 64); err != nil {
return nil, fmt.Errorf("for 'Gt', 'Lt' operators, the value must be an integer")
}
}
@@ -137,14 +140,24 @@ func NewRequirement(key string, op selection.Operator, vals sets.String) (*Requi
return nil, fmt.Errorf("operator '%v' is not recognized", op)
}
for v := range vals {
if err := validateLabelValue(v); err != nil {
for i := range vals {
if err := validateLabelValue(vals[i]); err != nil {
return nil, err
}
}
sort.Strings(vals)
return &Requirement{key: key, operator: op, strValues: vals}, nil
}
func (r *Requirement) hasValue(value string) bool {
for i := range r.strValues {
if r.strValues[i] == value {
return true
}
}
return false
}
// Matches returns true if the Requirement matches the input Labels.
// There is a match in the following cases:
// (1) The operator is Exists and Labels has the Requirement's key.
@@ -162,12 +175,12 @@ func (r *Requirement) Matches(ls Labels) bool {
if !ls.Has(r.key) {
return false
}
return r.strValues.Has(ls.Get(r.key))
return r.hasValue(ls.Get(r.key))
case selection.NotIn, selection.NotEquals:
if !ls.Has(r.key) {
return true
}
return !r.strValues.Has(ls.Get(r.key))
return !r.hasValue(ls.Get(r.key))
case selection.Exists:
return ls.Has(r.key)
case selection.DoesNotExist:
@@ -189,10 +202,10 @@ func (r *Requirement) Matches(ls Labels) bool {
}
var rValue int64
for strValue := range r.strValues {
rValue, err = strconv.ParseInt(strValue, 10, 64)
for i := range r.strValues {
rValue, err = strconv.ParseInt(r.strValues[i], 10, 64)
if err != nil {
glog.V(10).Infof("ParseInt failed for value %+v in requirement %#v, for 'Gt', 'Lt' operators, the value must be an integer", strValue, r)
glog.V(10).Infof("ParseInt failed for value %+v in requirement %#v, for 'Gt', 'Lt' operators, the value must be an integer", r.strValues[i], r)
return false
}
}
@@ -210,8 +223,8 @@ func (r *Requirement) Operator() selection.Operator {
}
func (r *Requirement) Values() sets.String {
ret := sets.String{}
for k := range r.strValues {
ret.Insert(k)
for i := range r.strValues {
ret.Insert(r.strValues[i])
}
return ret
}
@@ -258,9 +271,9 @@ func (r *Requirement) String() string {
buffer.WriteString("(")
}
if len(r.strValues) == 1 {
buffer.WriteString(r.strValues.List()[0])
buffer.WriteString(r.strValues[0])
} else { // only > 1 since == 0 prohibited by NewRequirement
buffer.WriteString(strings.Join(r.strValues.List(), ","))
buffer.WriteString(strings.Join(r.strValues, ","))
}
switch r.operator {
@@ -561,7 +574,7 @@ func (p *Parser) parseRequirement() (*Requirement, error) {
return nil, err
}
if operator == selection.Exists || operator == selection.DoesNotExist { // operator found lookahead set checked
return NewRequirement(key, operator, nil)
return NewRequirement(key, operator, []string{})
}
operator, err = p.parseOperator()
if err != nil {
@@ -577,7 +590,7 @@ func (p *Parser) parseRequirement() (*Requirement, error) {
if err != nil {
return nil, err
}
return NewRequirement(key, operator, values)
return NewRequirement(key, operator, values.List())
}
@@ -784,7 +797,7 @@ func SelectorFromSet(ls Set) Selector {
}
var requirements internalSelector
for label, value := range ls {
if r, err := NewRequirement(label, selection.Equals, sets.NewString(value)); err != nil {
if r, err := NewRequirement(label, selection.Equals, []string{value}); err != nil {
//TODO: double check errors when input comes from serialization?
return internalSelector{}
} else {
@@ -805,7 +818,7 @@ func SelectorFromValidatedSet(ls Set) Selector {
}
var requirements internalSelector
for label, value := range ls {
requirements = append(requirements, Requirement{key: label, operator: selection.Equals, strValues: sets.NewString(value)})
requirements = append(requirements, Requirement{key: label, operator: selection.Equals, strValues: []string{value}})
}
// sort to have deterministic string representation
sort.Sort(ByKey(requirements))

14
pkg/labels/selector_test.go
View File

@@ -320,7 +320,7 @@ func TestRequirementConstructor(t *testing.T) {
{strings.Repeat("a", 254), selection.Exists, nil, false}, //breaks DNS rule that len(key) <= 253
}
for _, rc := range requirementConstructorTests {
if _, err := NewRequirement(rc.Key, rc.Op, rc.Vals); err == nil && !rc.Success {
if _, err := NewRequirement(rc.Key, rc.Op, rc.Vals.List()); err == nil && !rc.Success {
t.Errorf("expected error with key:%#v op:%v vals:%v, got no error", rc.Key, rc.Op, rc.Vals)
} else if err != nil && rc.Success {
t.Errorf("expected no error with key:%#v op:%v vals:%v, got:%v", rc.Key, rc.Op, rc.Vals, err)
@@ -525,7 +525,7 @@ func TestSetSelectorParser(t *testing.T) {
}
func getRequirement(key string, op selection.Operator, vals sets.String, t *testing.T) Requirement {
req, err := NewRequirement(key, op, vals)
req, err := NewRequirement(key, op, vals.List())
if err != nil {
t.Errorf("NewRequirement(%v, %v, %v) resulted in error:%v", key, op, vals, err)
return Requirement{}
@@ -548,22 +548,22 @@ func TestAdd(t *testing.T) {
"key",
selection.In,
[]string{"value"},
internalSelector{Requirement{"key", selection.In, sets.NewString("value")}},
internalSelector{Requirement{"key", selection.In, []string{"value"}}},
},
{
"keyEqualsOperator",
internalSelector{Requirement{"key", selection.In, sets.NewString("value")}},
internalSelector{Requirement{"key", selection.In, []string{"value"}}},
"key2",
selection.Equals,
[]string{"value2"},
internalSelector{
Requirement{"key", selection.In, sets.NewString("value")},
Requirement{"key2", selection.Equals, sets.NewString("value2")},
Requirement{"key", selection.In, []string{"value"}},
Requirement{"key2", selection.Equals, []string{"value2"}},
},
},
}
for _, ts := range testCases {
req, err := NewRequirement(ts.key, ts.operator, sets.NewString(ts.values...))
req, err := NewRequirement(ts.key, ts.operator, ts.values)
if err != nil {
t.Errorf("%s - Unable to create labels.Requirement", ts.name)
}

3
pkg/registry/generic/registry/store_test.go
View File

@@ -43,7 +43,6 @@ import (
etcdtesting "k8s.io/kubernetes/pkg/storage/etcd/testing"
"k8s.io/kubernetes/pkg/storage/storagebackend/factory"
storagetesting "k8s.io/kubernetes/pkg/storage/testing"
"k8s.io/kubernetes/pkg/util/sets"
"k8s.io/kubernetes/pkg/util/validation/field"
"k8s.io/kubernetes/pkg/util/wait"
)
@@ -109,7 +108,7 @@ func NewTestGenericStoreRegistry(t *testing.T) (factory.DestroyFunc, *Store) {
func matchPodName(names ...string) storage.SelectionPredicate {
// Note: even if pod name is a field, we have to use labels,
// because field selector doesn't support "IN" operator.
l, err := labels.NewRequirement("name", selection.In, sets.NewString(names...))
l, err := labels.NewRequirement("name", selection.In, names)
if err != nil {
panic("Labels requirement must validate successfully")
}