[GitHub] [yunikorn-k8shim] craigcondit commented on a diff in pull request #277: [YUNIKORN-641]: Add events for placeholder timeout to pod

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

craigcondit commented on code in PR #277:
URL: https://github.com/apache/yunikorn-k8shim/pull/277#discussion_r875836155


##########
pkg/appmgmt/general/general.go:
##########
@@ -225,7 +225,7 @@ func (os *Manager) filterPods(obj interface{}) bool {
 	}
 }
 
-func (os *Manager) addPod(obj interface{}) {

Review Comment:
   Can we document that this is visible for testing only? Normally this should not be accessed outside this package.



##########
pkg/cache/application.go:
##########
@@ -743,6 +752,18 @@ func (app *Application) handleAppTaskCompletedEvent(event *fsm.Event) {
 	dispatcher.Dispatch(NewRunApplicationEvent(app.applicationID))
 }
 
+func (app *Application) publishPlaceholderTimeoutEvents(task *Task) {
+	if app.requestOriginatingTask != nil && task.IsPlaceholder() && task.terminationType == si.TerminationType_name[int32(si.TerminationType_TIMEOUT)] {
+		log.Logger().Info("trying to send placeholder timeout events to the original pod from application",

Review Comment:
   This should probably be logged at debug level.



##########
pkg/cache/application.go:
##########
@@ -280,8 +281,8 @@ func (app *Application) getTaskGroups() []v1alpha1.TaskGroup {
 }
 
 func (app *Application) setOwnReferences(ref []metav1.OwnerReference) {
-	app.lock.RLock()
-	defer app.lock.RUnlock()
+	app.lock.Lock()

Review Comment:
   Good catch.



##########
pkg/cache/application.go:
##########
@@ -60,6 +60,7 @@ type Application struct {
 	placeholderAsk             *si.Resource // total placeholder request for the app (all task groups)
 	placeholderTimeoutInSec    int64
 	schedulingStyle            string
+	requestOriginatingTask     interfaces.ManagedTask // Original Pod which creates the requests

Review Comment:
   Can we simplify this name to just `originatingTask`? The current name is a mouthful...



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