pbacsko commented on code in PR #1096:
URL: https://github.com/apache/yunikorn-core/pull/1096#discussion_r3564601776
##########
pkg/scheduler/objects/application.go:
##########
@@ -1321,10 +1332,22 @@ func (sa *Application)
tryPlaceholderAllocate(nodeIterator func() NodeIterator,
if
!node.preAllocateCheck(reqFit.GetAllocatedResource(), resKey) {
return true
}
+
+ // Is this node suitable to run the pod?
+ if len(feasibleNodes) > 0 {
+ if _, ok := feasibleNodes[node.NodeID]; !ok {
+
log.Log(log.SchedApplication).Debug("skipping node as it is not feasible to run
the pod",
+ zap.String("allocationKey",
resKey),
+ zap.String("node", node.NodeID))
Review Comment:
I wouldn't log this even on DEBUG level. It can generate too much output on
a bigger cluster.
##########
pkg/scheduler/objects/application.go:
##########
@@ -1525,6 +1558,17 @@ func (sa *Application) tryNodesNoReserve(ask
*Allocation, iterator NodeIterator,
if !node.FitInNode(ask.GetAllocatedResource()) || node.NodeID
== reservedNode {
return true
}
+
+ // Is this node suitable to run the pod?
+ if len(feasibleNodes) > 0 {
+ if _, ok := feasibleNodes[node.NodeID]; !ok {
+ log.Log(log.SchedApplication).Debug("skipping
node as it is not feasible to run the pod",
+ zap.String("allocationKey",
ask.GetAllocationKey()),
+ zap.String("node", node.NodeID))
Review Comment:
I wouldn't log this even on DEBUG level. It can generate too much output on
a bigger cluster.
##########
pkg/scheduler/partition_test.go:
##########
@@ -1803,6 +1803,7 @@ func TestRequiredNodeCancelOtherReservations(t
*testing.T) {
// the second one should be reserved as the 2nd node is not scheduling
result = partition.tryAllocate()
+ println(result.String())
Review Comment:
debugging statement, delete
##########
pkg/scheduler/objects/node.go:
##########
@@ -487,6 +488,23 @@ func (sn *Node) preAllocateConditions(ask *Allocation)
error {
// Checking pre-conditions in the shim for a reservation.
func (sn *Node) preReserveConditions(ask *Allocation) error {
+ // run predicates for this pod before in hand and fetch feasible nodes
+ feasibleNodes, err := ask.preAllocateConditions(false)
+ if err != nil {
+ preErrors := make(map[string]int, 1)
+ preErrors[err.Error()]++
+ ask.SendPredicatesFailedEvent(preErrors)
+ return err
+ }
+ // Is this node suitable to run the pod?
+ if len(feasibleNodes) > 0 {
+ if _, ok := feasibleNodes[sn.NodeID]; !ok {
+ log.Log(log.SchedApplication).Debug("skipping node as
it is not feasible to run the pod",
Review Comment:
I have the same concern like in `application.go`. If this is called from a
loop (node iterator), it can generate way too much output.
##########
pkg/scheduler/objects/application.go:
##########
@@ -1564,41 +1618,56 @@ func (sa *Application) tryNodes(ask *Allocation,
iterator NodeIterator) *Allocat
if !node.FitInNode(ask.GetAllocatedResource()) {
return true
}
- tryNodeStart := time.Now()
- result, err := sa.tryNode(node, ask)
- if err != nil {
- if predicateErrors == nil {
- predicateErrors = make(map[string]int)
+
+ // Is there any pod predicate errors? No node would be picked
up for allocation in case of any errors
+ // and better to get into process of picking up a node for
reservation right away
+ if len(podPredicateErrors) == 0 {
+ // Is this node suitable to run the pod?
+ if len(feasibleNodes) > 0 {
+ if _, ok := feasibleNodes[node.NodeID]; !ok {
+
log.Log(log.SchedApplication).Debug("skipping node as it is not feasible to run
the pod",
+ zap.String("allocationKey",
allocKey),
+ zap.String("node", node.NodeID))
Review Comment:
I wouldn't log this even on DEBUG level. It can generate too much output on
a bigger cluster.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
