wilfred-s commented on a change in pull request #230:
URL:
https://github.com/apache/incubator-yunikorn-core/pull/230#discussion_r553098103
##########
File path: pkg/scheduler/partition.go
##########
@@ -919,6 +921,9 @@ func (pc *PartitionContext) GetApplications()
[]*objects.Application {
for _, app := range pc.applications {
appList = append(appList, app)
}
+ for _, app := range pc.completedApplications {
+ appList = append(appList, app)
+ }
Review comment:
This will look strange in the web UI. The completed applications will
now be shown as part of the queues.
If we keep old apps for a long period of time this will have a huge impact
on the web UI performance.
I think we need to split this up.
##########
File path: pkg/scheduler/partition.go
##########
@@ -1112,3 +1117,16 @@ func (pc *PartitionContext) removeAllocationAsk(appID
string, allocationKey stri
}
}
}
+
+// Move all the completed apps into the completedApp list
+// Delete all the applications marked for removal
+func (pc *PartitionContext) cleanupApps() {
+ for _, app := range pc.GetApplications() {
Review comment:
I would expect that this works directly on the applications list and
does a range over the apps moving them into the completed map:
```
pc.Lock
defer pc.Unlock
for app := range pc.applications {
if app.IsCompleted() {
delete(pc.applications, app)
pc.completedApplications[app.appID] = app
}
```
The completedApplication cleanup can be run separately at a far lower
interval.
##########
File path: pkg/scheduler/partition.go
##########
@@ -1112,3 +1117,16 @@ func (pc *PartitionContext) removeAllocationAsk(appID
string, allocationKey stri
}
}
}
+
+// Move all the completed apps into the completedApp list
+// Delete all the applications marked for removal
+func (pc *PartitionContext) cleanupApps() {
+ for _, app := range pc.GetApplications() {
+ if app.IsCompleted() {
+ pc.removeApplication(app.ApplicationID)
Review comment:
An application can only be `completed` if there is nothing left running
reserved etc as it comes after `waiting`.
Not sure why we would need to call `removeApplication` on it.
I would expect that the state change of the app from waiting to completed
removes the app from the queue, sets the queue link in the app to `nil` and
just leaves the queue name.
----------------------------------------------------------------
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:
us...@infra.apache.org
