[GitHub] [incubator-yunikorn-core] manirajv06 commented on a change in pull request #305: [YUNIKORN-800] ensure consistent response between /ws/v1/partition/{par…

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

manirajv06 commented on a change in pull request #305:
URL: https://github.com/apache/incubator-yunikorn-core/pull/305#discussion_r690882418



##########
File path: pkg/scheduler/objects/queue.go
##########
@@ -544,19 +546,31 @@ func (sq *Queue) RemoveApplication(app *Application) {
 	defer sq.Unlock()
 
 	delete(sq.applications, appID)
+	sq.completedApplications[appID] = app
 }
 
-// Get a copy of all apps holding the lock
+// GetCopyOfApps Get a copy of all non-complated apps holding the lock
 func (sq *Queue) GetCopyOfApps() map[string]*Application {
 	sq.RLock()
 	defer sq.RUnlock()
-	appsCopy := make(map[string]*Application)
+	appsCopy := make(map[string]*Application, len(sq.applications))
 	for appID, app := range sq.applications {
 		appsCopy[appID] = app
 	}
 	return appsCopy
 }
 
+// GetCopyOfCompletedApps Get a copy of all completed apps holding the lock
+func (sq *Queue) GetCopyOfCompletedApps() map[string]*Application {
+	sq.RLock()
+	defer sq.RUnlock()
+	appsCopy := make(map[string]*Application, len(sq.completedApplications))

Review comment:
       Can we call it as completedAppsCopy or something similar?

##########
File path: pkg/scheduler/objects/queue_test.go
##########
@@ -324,8 +324,14 @@ func TestRemoveApplication(t *testing.T) {
 	assert.Assert(t, resources.IsZero(leaf.pending), "leaf queue pending resource not zero")
 	leaf.RemoveApplication(nonExist)
 	assert.Equal(t, len(leaf.applications), 1, "Non existing application was removed from the queue")
+	assert.Equal(t, len(leaf.GetCopyOfApps()), 1, "Application was not removed from the queue as expected")

Review comment:
       Message is bit confusing. I think this assert ensures Non existing application removal. Message should be other way around similar to previous line?




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