[GitHub] [yunikorn-core] wilfred-s commented on a diff in pull request #463: [YUNIKORN-1440] Cleanup terminated applications once expired

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

wilfred-s commented on code in PR #463:
URL: https://github.com/apache/yunikorn-core/pull/463#discussion_r1037754703


##########
pkg/scheduler/partition.go:
##########
@@ -1433,6 +1433,13 @@ func (pc *PartitionContext) cleanupExpiredApps() {
 		delete(pc.rejectedApplications, app.ApplicationID)
 		pc.Unlock()
 	}
+	for k, app := range pc.completedApplications {
+		pc.Lock()
+		if app.IsExpired() {
+			delete(pc.completedApplications, k)
+		}
+		pc.Unlock()
+	}

Review Comment:
   This will cause a data race. The completedApplications map can be manipulated from a different go routine. Maps are not go routine safe. You will need to introduce a `pc.GetCompletedAppsByState()` method to do the filtering under a lock like we do for rejected apps.
   The delete will still need to be within the lock.



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