chore(qrm): include checkAllNUMABindingResult to WithCheckAndExecutio…

…nTimeLogging
kubewharf · Nov 22, 2024 · 7cd8c68 · 7cd8c68
1 parent 191f2c9
commit 7cd8c68
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 15 deletions.
diff --git a/pkg/agent/qrm-plugins/util/numabinding/calculator/dryrun_calculator.go b/pkg/agent/qrm-plugins/util/numabinding/calculator/dryrun_calculator.go
@@ -19,11 +19,9 @@ package calculator
 import (
 	"context"
 
-	"github.com/kubewharf/katalyst-core/pkg/metrics"
-	"github.com/kubewharf/katalyst-core/pkg/util/general"
-
 	"github.com/kubewharf/katalyst-core/pkg/agent/qrm-plugins/util/allocation"
 	"github.com/kubewharf/katalyst-core/pkg/agent/qrm-plugins/util/state"
+	"github.com/kubewharf/katalyst-core/pkg/metrics"
 )
 
 const (
@@ -38,7 +36,7 @@ type dryRunCalculator struct {
 func NewDryRunCalculator(emitter metrics.MetricEmitter, calculators ...NUMABindingCalculator) NUMABindingCalculator {
 	withLoggingCalculators := make([]NUMABindingCalculator, 0, len(calculators))
 	for _, calculator := range calculators {
-		withLoggingCalculators = append(withLoggingCalculators, WithExecutionTimeLogging(calculator, emitter))
+		withLoggingCalculators = append(withLoggingCalculators, WithCheckAndExecutionTimeLogging(calculator, emitter))
 	}
 	return &dryRunCalculator{
 		calculators: withLoggingCalculators,
@@ -54,9 +52,7 @@ func (d *dryRunCalculator) Run(ctx context.Context) {
 
 func (d *dryRunCalculator) CalculateNUMABindingResult(current allocation.PodAllocations, numaAllocatable state.NUMAResource) (allocation.PodAllocations, bool, error) {
 	for _, calc := range d.calculators {
-		result, success, err := calc.CalculateNUMABindingResult(current, numaAllocatable)
-		general.Infof("dry run calculator %s result: %v, success: %v, err: %v", calc.Name(), result, success, err)
-		CheckAllNUMABindingResult(d.emitter, calc.Name(), success, result)
+		_, _, _ = calc.CalculateNUMABindingResult(current, numaAllocatable)
 	}
 	return current, true, nil
 }

diff --git a/pkg/agent/qrm-plugins/util/numabinding/calculator/util.go b/pkg/agent/qrm-plugins/util/numabinding/calculator/util.go
@@ -36,7 +36,7 @@ const (
 	metricNameCalculateTimeCost = "calculate_time_cost"
 )
 
-func CheckAllNUMABindingResult(emitter metrics.MetricEmitter, calculator string, success bool, result allocation.PodAllocations) bool {
+func checkAllNUMABindingResult(emitter metrics.MetricEmitter, calculator string, success bool, result allocation.PodAllocations) bool {
 	unSuccessPods := make(map[string]types.NamespacedName)
 	for podUID, alloc := range result {
 		if alloc.BindingNUMA == -1 {
@@ -63,12 +63,12 @@ func CheckAllNUMABindingResult(emitter metrics.MetricEmitter, calculator string,
 	return true
 }
 
-type withExecutionTimeLogging struct {
+type withCheckAndExecutionTimeLogging struct {
 	emitter metrics.MetricEmitter
 	NUMABindingCalculator
 }
 
-func (w *withExecutionTimeLogging) CalculateNUMABindingResult(current allocation.PodAllocations,
+func (w *withCheckAndExecutionTimeLogging) CalculateNUMABindingResult(current allocation.PodAllocations,
 	numaAllocatable state.NUMAResource,
 ) (allocation.PodAllocations, bool, error) {
 	begin := time.Now()
@@ -86,11 +86,11 @@ func (w *withExecutionTimeLogging) CalculateNUMABindingResult(current allocation
 
 	general.InfoS("calculate numa result", "calculator", w.Name(), "result", result, "success", success)
 
-	return result, success, nil
+	return result, checkAllNUMABindingResult(w.emitter, w.Name(), success, result), nil
 }
 
-func WithExecutionTimeLogging(calculator NUMABindingCalculator, emitter metrics.MetricEmitter) NUMABindingCalculator {
-	return &withExecutionTimeLogging{
+func WithCheckAndExecutionTimeLogging(calculator NUMABindingCalculator, emitter metrics.MetricEmitter) NUMABindingCalculator {
+	return &withCheckAndExecutionTimeLogging{
 		emitter:               emitter,
 		NUMABindingCalculator: calculator,
 	}

diff --git a/pkg/agent/qrm-plugins/util/numabinding/manager.go b/pkg/agent/qrm-plugins/util/numabinding/manager.go
@@ -86,7 +86,7 @@ func NewSharedNUMABindingManager(
 		metaServer:            metaServer,
 		reservedCPUs:          reservedCPUs,
 		allocationUpdater:     updater,
-		numaBindingCalculator: calculator.WithExecutionTimeLogging(numaBindingCalculator, emitter),
+		numaBindingCalculator: calculator.WithCheckAndExecutionTimeLogging(numaBindingCalculator, emitter),
 	}, nil
 }
 
@@ -130,7 +130,7 @@ func (m *managerImpl) sync(ctx context.Context) {
 		return
 	}
 
-	if !calculator.CheckAllNUMABindingResult(m.emitter, m.numaBindingCalculator.Name(), success, result) {
+	if !success {
 		return
 	}