[GitHub] [incubator-yunikorn-k8shim] wilfred-s commented on a change in pull request #375: [YUNIKORN-1100] Fix scheduler cache inconsistencies

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

wilfred-s commented on a change in pull request #375:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/375#discussion_r816383879



##########
File path: pkg/cache/context.go
##########
@@ -472,25 +457,26 @@ func (ctx *Context) AssumePod(name string, node string) error {
 			}
 			// assign the node name for pod
 			assumedPod.Spec.NodeName = node
-			return ctx.schedulerCache.AssumePod(assumedPod, allBound)
+			ctx.schedulerCache.AssumePod(assumedPod, allBound)
+			return nil
 		}
 	}
 	return nil
 }
 
 // forget pod must be called when a pod is assumed to be running on a node,
 // but then for some reason it is failed to bind or released.
-func (ctx *Context) ForgetPod(name string) error {
+func (ctx *Context) ForgetPod(name string) {
 	ctx.lock.Lock()
 	defer ctx.lock.Unlock()
 
 	if pod, ok := ctx.schedulerCache.GetPod(name); ok {
 		log.Logger().Debug("forget pod", zap.String("pod", pod.Name))
-		return ctx.schedulerCache.ForgetPod(pod)
+		ctx.schedulerCache.ForgetPod(pod)
+		return
 	}
 	log.Logger().Debug("unable to forget pod",
 		zap.String("reason", fmt.Sprintf("pod %s not found in scheduler cache", name)))

Review comment:
       NIT: Can we clean up this logging to just be:
   ```
   log.Logger().Debug("unable to forget pod: not found in cache", zap.String("pod", 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.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

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