Fixed lint warnings * Code clean up

apache · Nov 11, 2022 · 1d009bd · 1d009bd
1 parent 88daab9
commit 1d009bd
Show file tree

Hide file tree

Showing 4 changed files with 2 additions and 31 deletions.
diff --git a/pkg/scheduler/objects/application.go b/pkg/scheduler/objects/application.go
@@ -542,8 +542,6 @@ func (sa *Application) removeAsksInternal(allocKey string) int {
 	// When the resource trackers are zero we should not expect anything to come in later.
 	hasPlaceHolderAllocations := len(sa.getPlaceholderAllocations()) > 0
 	if resources.IsZero(sa.pending) && resources.IsZero(sa.allocatedResource) && !sa.IsFailing() && !sa.IsCompleting() && !hasPlaceHolderAllocations {
-		//sa.decUserResourceUsage(sa.allocatedResource, true)
-		log.Logger().Debug("step 1", zap.String("dd", sa.allocatedResource.String()))
 		if err := sa.HandleApplicationEvent(CompleteApplication); err != nil {
 			log.Logger().Warn("Application state not changed to Completing while updating ask(s)",
 				zap.String("currentState", sa.CurrentState()),
@@ -1548,17 +1546,14 @@ func (sa *Application) removeAllocationInternal(uuid string, releaseType si.Term
 		sa.allocatedPlaceholder = resources.Sub(sa.allocatedPlaceholder, alloc.GetAllocatedResource())
 		// if all the placeholders are replaced, clear the placeholder timer
 		if resources.IsZero(sa.allocatedPlaceholder) {
-			log.Logger().Debug("step 3")
 			sa.clearPlaceholderTimer()
 			if (sa.IsCompleting() && sa.stateTimer == nil) || sa.IsFailing() || sa.IsResuming() || sa.hasZeroAllocations() {
 				removeApp = true
-				log.Logger().Debug("step 7")
 				event = CompleteApplication
 				if sa.IsFailing() {
 					event = FailApplication
 				}
 				if sa.IsResuming() {
-					log.Logger().Debug("step 6")
 					event = RunApplication
 					removeApp = false
 				}
@@ -1567,11 +1562,9 @@ func (sa *Application) removeAllocationInternal(uuid string, releaseType si.Term
 		}
 		sa.decUserResourceUsage(alloc.GetAllocatedResource(), removeApp)
 	} else {
-		log.Logger().Debug("step 4")
 		sa.allocatedResource = resources.Sub(sa.allocatedResource, alloc.GetAllocatedResource())
 		// When the resource trackers are zero we should not expect anything to come in later.
 		if sa.hasZeroAllocations() {
-			log.Logger().Debug("step 5")
 			removeApp = true
 			event = CompleteApplication
 			eventWarning = "Application state not changed to Waiting while removing an allocation"

diff --git a/pkg/scheduler/objects/application_state.go b/pkg/scheduler/objects/application_state.go
@@ -152,31 +152,12 @@ func NewAppState() *fsm.FSM {
 			},
 			fmt.Sprintf("enter_%s", Resuming.String()): func(event *fsm.Event) {
 				app := event.Args[0].(*Application) //nolint:errcheck
-				//app.decUserResourceUsage(app.allocatedPlaceholder, true)
 				metrics.GetQueueMetrics(app.queuePath).DecQueueApplicationsRunning()
 				metrics.GetSchedulerMetrics().DecTotalApplicationsRunning()
 			},
 			fmt.Sprintf("enter_%s", Completing.String()): func(event *fsm.Event) {
 				app := event.Args[0].(*Application) //nolint:errcheck
 				app.setStateTimer(completingTimeout, app.stateMachine.Current(), CompleteApplication)
-				log.Logger().Debug("step 1", zap.Int("ss", len(app.placeholderData)))
-				//	log.Logger().Debug("step 2", zap.String("ss", resources.Zero)
-				//if app.placeholderData == nil {
-				// Usage needs to be tracked (decreased) only for the app which crossed starting state.
-				/*
-					starting := false
-					for _, stateLog := range app.stateLog {
-						if stateLog.ApplicationState == Starting.String() {
-							starting = true
-							break
-						}
-					}
-					if starting {
-						app.decUserResourceUsage(app.allocatedResource, true)
-					}*/
-				//} else {
-				//app.decUserResourceUsage(app.allocatedPlaceholder, true)
-				//}
 				metrics.GetQueueMetrics(app.queuePath).DecQueueApplicationsRunning()
 				metrics.GetSchedulerMetrics().DecTotalApplicationsRunning()
 			},
@@ -206,7 +187,6 @@ func NewAppState() *fsm.FSM {
 			},
 			fmt.Sprintf("enter_%s", Failing.String()): func(event *fsm.Event) {
 				app := event.Args[0].(*Application) //nolint:errcheck
-				//app.decUserResourceUsage(app.allocatedPlaceholder, true)
 				metrics.GetQueueMetrics(app.queuePath).DecQueueApplicationsRunning()
 				metrics.GetQueueMetrics(app.queuePath).IncQueueApplicationsFailed()
 				metrics.GetSchedulerMetrics().DecTotalApplicationsRunning()

diff --git a/pkg/scheduler/objects/application_test.go b/pkg/scheduler/objects/application_test.go
@@ -813,8 +813,7 @@ func TestStateChangeOnPlaceholderAdd(t *testing.T) {
 	released = app.RemoveAllocationAsk(askID)
 	assert.Equal(t, released, 0, "allocation ask should not have been reserved")
 	assert.Assert(t, app.IsCompleting(), "Application should have stayed same, changed unexpectedly: %s", app.CurrentState())
-	//assertUserGroupNilResourceWithError(t)
-	//assertUserGroupResource(t, res)
+	assertUserGroupResource(t, resources.NewResourceFromMap(map[string]resources.Quantity{"first": 0}))
 
 	log := app.GetStateLog()
 	assert.Equal(t, len(log), 2, "wrong number of app events")

diff --git a/pkg/scheduler/partition_test.go b/pkg/scheduler/partition_test.go
@@ -20,7 +20,6 @@ package scheduler
 
 import (
 	"fmt"
-	"github.com/apache/yunikorn-core/pkg/scheduler/ugm"
 	"strconv"
 	"strings"
 	"testing"
@@ -37,6 +36,7 @@ import (
 	"github.com/apache/yunikorn-core/pkg/rmproxy/rmevent"
 	"github.com/apache/yunikorn-core/pkg/scheduler/objects"
 	"github.com/apache/yunikorn-core/pkg/scheduler/policies"
+	"github.com/apache/yunikorn-core/pkg/scheduler/ugm"
 	siCommon "github.com/apache/yunikorn-scheduler-interface/lib/go/common"
 	"github.com/apache/yunikorn-scheduler-interface/lib/go/si"
 )
@@ -1159,7 +1159,6 @@ func TestRequiredNodeNotExist(t *testing.T) {
 		t.Fatal("allocation should not have worked on unknown node")
 	}
 	assertUserGroupNilResourceWithError(t)
-	//assertUserGroupResource(t, resources.Multiply(res, 2))
 }
 
 // basic ds scheduling on specific node in first allocate run itself (without any need for reservation)