wilfred-s commented on code in PR #950:
URL: https://github.com/apache/yunikorn-core/pull/950#discussion_r1722927979
##########
pkg/scheduler/objects/queue.go:
##########
@@ -1609,20 +1612,38 @@ func (sq *Queue) updateMaxResourceMetrics() {
// updateAllocatedResourceMetrics updates allocated resource metrics for all
queue types.
func (sq *Queue) updateAllocatedResourceMetrics() {
+ // Special case when we prune the allocated resources to zero, we need
to update the metrics
+ // here only keep basic types memory and vcores to make sure we have
updated metrics
+ if sq.allocatedResource.IsEmpty() {
+
metrics.GetQueueMetrics(sq.QueuePath).SetQueueAllocatedResourceMetrics("memory",
float64(0))
+
metrics.GetQueueMetrics(sq.QueuePath).SetQueueAllocatedResourceMetrics("vcores",
float64(0))
+ }
Review Comment:
If we prune after we call the update for the metrics we keep the metrics
clean and correct. We do not have to make a guess to what was zero. The queue
also might not have used memory or vcores ever. Setting them here to zero in
the metrics could throw things off.
This would need a comment in the code to why we first call the metric update
and then prune.
##########
pkg/scheduler/objects/node.go:
##########
@@ -312,6 +313,7 @@ func (sn *Node) RemoveAllocation(allocationKey string)
*Allocation {
delete(sn.allocations, allocationKey)
sn.allocatedResource.SubFrom(alloc.GetAllocatedResource())
sn.availableResource.AddTo(alloc.GetAllocatedResource())
+ sn.allocatedResource.Prune()
Review Comment:
NIT should move this one line up for clarity
##########
pkg/scheduler/objects/queue.go:
##########
@@ -1609,20 +1612,38 @@ func (sq *Queue) updateMaxResourceMetrics() {
// updateAllocatedResourceMetrics updates allocated resource metrics for all
queue types.
func (sq *Queue) updateAllocatedResourceMetrics() {
+ // Special case when we prune the allocated resources to zero, we need
to update the metrics
+ // here only keep basic types memory and vcores to make sure we have
updated metrics
+ if sq.allocatedResource.IsEmpty() {
+
metrics.GetQueueMetrics(sq.QueuePath).SetQueueAllocatedResourceMetrics("memory",
float64(0))
+
metrics.GetQueueMetrics(sq.QueuePath).SetQueueAllocatedResourceMetrics("vcores",
float64(0))
+ }
for k, v := range sq.allocatedResource.Resources {
metrics.GetQueueMetrics(sq.QueuePath).SetQueueAllocatedResourceMetrics(k,
float64(v))
}
}
// updatePendingResourceMetrics updates pending resource metrics for all queue
types.
func (sq *Queue) updatePendingResourceMetrics() {
+ // Special case when we prune the pending resources to zero, we need to
update the metrics
+ // here only keep basic types memory and vcores to make sure we have
updated metrics
+ if sq.pending.IsEmpty() {
+
metrics.GetQueueMetrics(sq.QueuePath).SetQueuePendingResourceMetrics("memory",
float64(0))
+
metrics.GetQueueMetrics(sq.QueuePath).SetQueuePendingResourceMetrics("vcores",
float64(0))
+ }
for k, v := range sq.pending.Resources {
metrics.GetQueueMetrics(sq.QueuePath).SetQueuePendingResourceMetrics(k,
float64(v))
}
}
// updatePreemptingResourceMetrics updates preempting resource metrics for all
queue types.
func (sq *Queue) updatePreemptingResourceMetrics() {
+ // Special case when we prune the preempting resources to zero, we need
to update the metrics
+ // here only keep basic types memory and vcores to make sure we have
updated metrics
+ if sq.preemptingResource.IsEmpty() {
+
metrics.GetQueueMetrics(sq.QueuePath).SetQueuePreemptingResourceMetrics("memory",
float64(0))
+
metrics.GetQueueMetrics(sq.QueuePath).SetQueuePreemptingResourceMetrics("vcores",
float64(0))
+ }
Review Comment:
Same as for the allocated metrics update.
##########
pkg/scheduler/objects/queue.go:
##########
@@ -1609,20 +1612,38 @@ func (sq *Queue) updateMaxResourceMetrics() {
// updateAllocatedResourceMetrics updates allocated resource metrics for all
queue types.
func (sq *Queue) updateAllocatedResourceMetrics() {
+ // Special case when we prune the allocated resources to zero, we need
to update the metrics
+ // here only keep basic types memory and vcores to make sure we have
updated metrics
+ if sq.allocatedResource.IsEmpty() {
+
metrics.GetQueueMetrics(sq.QueuePath).SetQueueAllocatedResourceMetrics("memory",
float64(0))
+
metrics.GetQueueMetrics(sq.QueuePath).SetQueueAllocatedResourceMetrics("vcores",
float64(0))
+ }
for k, v := range sq.allocatedResource.Resources {
metrics.GetQueueMetrics(sq.QueuePath).SetQueueAllocatedResourceMetrics(k,
float64(v))
}
}
// updatePendingResourceMetrics updates pending resource metrics for all queue
types.
func (sq *Queue) updatePendingResourceMetrics() {
+ // Special case when we prune the pending resources to zero, we need to
update the metrics
+ // here only keep basic types memory and vcores to make sure we have
updated metrics
+ if sq.pending.IsEmpty() {
+
metrics.GetQueueMetrics(sq.QueuePath).SetQueuePendingResourceMetrics("memory",
float64(0))
+
metrics.GetQueueMetrics(sq.QueuePath).SetQueuePendingResourceMetrics("vcores",
float64(0))
+ }
Review Comment:
Same as for the allocated metrics update.
--
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]
