[GitHub] [yunikorn-core] craigcondit commented on a diff in pull request #458: [YUNIKORN-1434] Mark allocation as being preempted

[GitBox <gi...@apache.org>]

craigcondit commented on code in PR #458:
URL: https://github.com/apache/yunikorn-core/pull/458#discussion_r1037504089


##########
pkg/scheduler/objects/allocation_ask.go:
##########
@@ -262,16 +261,9 @@ func (aa *AllocationAsk) HasTriggeredPreemption() bool {
 	return aa.preemptionTriggered
 }
 
-// SetLastPreemptionAttempt sets the time which tells when the last preemption attempt was performed for this ask.
-func (aa *AllocationAsk) SetLastPreemptionAttempt(attempt time.Time) {
+// test only

Review Comment:
   This is only called from the objects pkg, so can be moved to a *_test.go file. No need to pollute the main package with test code.



##########
pkg/scheduler/objects/application.go:
##########
@@ -1147,13 +1140,13 @@ func (sa *Application) tryPreemption(reserve *reservation, ask *AllocationAsk) b
 	// Are there any victims/asks to preempt?
 	victims := preemptor.GetVictims()
 	if len(victims) > 0 {
+		log.Logger().Info("Found victims for daemon set ask preemption ",

Review Comment:
   Given we've renamed this feature to 'required node preemption', can we change this message to reflect that? Maybe "Found victims for required node preemption "



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org