DRA CEL: skip estimating the cost in the scheduler

Compiling a CEL expression used to do the cost estimation, whether the caller
needed the result or not. Now callers can skip it. The scheduler does that,
through the CEL cache.

The main advantage is that failures in the estimator (like panics) are limited
to the apiserver. Performance in the scheduler is not expected to benefit much
because compilation results are cached.
This commit is contained in:
Patrick Ohly 2025-01-22 08:45:32 +01:00
parent f89e4c08cf
commit 00faa5e7ae
3 changed files with 26 additions and 10 deletions

View File

@ -43,6 +43,8 @@ func NewCache(maxCacheEntries int) *Cache {
// GetOrCompile checks whether the cache already has a compilation result
// and returns that if available. Otherwise it compiles, stores successful
// results and returns the new result.
//
// Cost estimation is disabled.
func (c *Cache) GetOrCompile(expression string) CompilationResult {
// Compiling a CEL expression is expensive enough that it is cheaper
// to lock a mutex than doing it several times in parallel.
@ -55,7 +57,7 @@ func (c *Cache) GetOrCompile(expression string) CompilationResult {
return *cached
}
expr := GetCompiler().CompileCELExpression(expression, Options{})
expr := GetCompiler().CompileCELExpression(expression, Options{DisableCostEstimation: true})
if expr.Error == nil {
c.add(expression, &expr)
}

View File

@ -18,6 +18,7 @@ package cel
import (
"fmt"
"math"
"sync"
"testing"
@ -73,6 +74,11 @@ func TestCacheSemantic(t *testing.T) {
if resultFalse == resultFalseAgain {
t.Fatal("result of compiling `false` should have been evicted from the cache")
}
// Cost estimation must be off (not needed by scheduler).
if resultFalseAgain.MaxCost != math.MaxUint64 {
t.Error("cost estimation should have been disabled, was enabled")
}
}
func TestCacheConcurrency(t *testing.T) {

View File

@ -20,6 +20,7 @@ import (
"context"
"errors"
"fmt"
"math"
"reflect"
"strings"
"sync"
@ -120,6 +121,10 @@ type Options struct {
// CostLimit allows overriding the default runtime cost limit [resourceapi.CELSelectorExpressionMaxCost].
CostLimit *uint64
// DisableCostEstimation can be set to skip estimating the worst-case CEL cost.
// If disabled or after an error, [CompilationResult.MaxCost] will be set to [math.Uint64].
DisableCostEstimation bool
}
// CompileCELExpression returns a compiled CEL expression. It evaluates to bool.
@ -133,6 +138,7 @@ func (c compiler) CompileCELExpression(expression string, options Options) Compi
Detail: errorString,
},
Expression: expression,
MaxCost: math.MaxUint64,
}
}
@ -141,10 +147,6 @@ func (c compiler) CompileCELExpression(expression string, options Options) Compi
return resultError(fmt.Sprintf("unexpected error loading CEL environment: %v", err), apiservercel.ErrorTypeInternal)
}
// We don't have a SizeEstimator. The potential size of the input (= a
// device) is already declared in the definition of the environment.
estimator := c.newCostEstimator()
ast, issues := env.Compile(expression)
if issues != nil {
return resultError("compilation failed: "+issues.String(), apiservercel.ErrorTypeInvalid)
@ -176,15 +178,21 @@ func (c compiler) CompileCELExpression(expression string, options Options) Compi
OutputType: ast.OutputType(),
Environment: env,
emptyMapVal: env.CELTypeAdapter().NativeToValue(map[string]any{}),
MaxCost: math.MaxUint64,
}
costEst, err := env.EstimateCost(ast, estimator)
if err != nil {
compilationResult.Error = &apiservercel.Error{Type: apiservercel.ErrorTypeInternal, Detail: "cost estimation failed: " + err.Error()}
return compilationResult
if !options.DisableCostEstimation {
// We don't have a SizeEstimator. The potential size of the input (= a
// device) is already declared in the definition of the environment.
estimator := c.newCostEstimator()
costEst, err := env.EstimateCost(ast, estimator)
if err != nil {
compilationResult.Error = &apiservercel.Error{Type: apiservercel.ErrorTypeInternal, Detail: "cost estimation failed: " + err.Error()}
return compilationResult
}
compilationResult.MaxCost = costEst.Max
}
compilationResult.MaxCost = costEst.Max
return compilationResult
}