[GitHub] [incubator-yunikorn-core] yangwwei commented on a change in pull request #230: [YUNIKORN-484] Added waiting state timeout

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

yangwwei commented on a change in pull request #230:
URL: https://github.com/apache/incubator-yunikorn-core/pull/230#discussion_r557021327



##########
File path: pkg/scheduler/objects/application_state.go
##########
@@ -61,10 +63,11 @@ const (
 	Rejected
 	Completed
 	killed
+	Deleting
 )
 
 func (as applicationState) String() string {
-	return [...]string{"New", "Accepted", "Starting", "Running", "Waiting", "Rejected", "Completed", "killed"}[as]
+	return [...]string{"New", "Accepted", "Starting", "Running", "Waiting", "Rejected", "Completed", "killed", "Deleting"}[as]

Review comment:
       Suggest to rename `Deleting` to `Expired`.
   And corresponding event rename to `expireApplication`

##########
File path: pkg/scheduler/partition.go
##########
@@ -49,6 +49,7 @@ type PartitionContext struct {
 	// Private fields need protection
 	root                   *objects.Queue                  // start of the queue hierarchy
 	applications           map[string]*objects.Application // applications assigned to this partition
+	completedApplications  map[string]*objects.Application // completed applications from this partition

Review comment:
       let's remove this map, such overhead seems not to be necessary
   because the app cleanup happens in the background, we can iterate the application's map and delete apps that has expired, it won't cause any perf issue




----------------------------------------------------------------
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.

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