wilfred-s commented on code in PR #1058:
URL: https://github.com/apache/yunikorn-core/pull/1058#discussion_r2668152651
##########
pkg/scheduler/objects/application.go:
##########
@@ -395,18 +410,27 @@ func (sa *Application) timeoutPlaceholderProcessing() {
// Case 1: if all app's placeholders are allocated, only part
of them gets replaced, just delete the remaining placeholders
var toRelease []*Allocation
replacing := 0
+ preempted := 0
for _, alloc := range sa.getPlaceholderAllocations() {
// skip over the allocations that are already marked
for release, they will be replaced soon
if alloc.IsReleased() {
replacing++
continue
}
- alloc.SetReleased(true)
+ err := alloc.SetReleased(true)
+ if err != nil {
+ log.Log(log.SchedApplication).Warn("allocation
is already preempted, so skipping release process",
+ zap.String("applicationID",
sa.ApplicationID),
+ zap.String("allocationKey",
alloc.GetAllocationKey()))
+ preempted++
+ continue
+ }
toRelease = append(toRelease, alloc)
}
log.Log(log.SchedApplication).Info("Placeholder timeout,
releasing allocated placeholders",
zap.String("AppID", sa.ApplicationID),
zap.Int("placeholders being replaced", replacing),
+ zap.Int("placeholders already preempted while tried to
release", preempted),
Review Comment:
shorten text to `replaced` and `preempted`
##########
pkg/scheduler/objects/application.go:
##########
@@ -441,6 +480,7 @@ func (sa *Application) timeoutPlaceholderProcessing() {
zap.String("AppID", sa.ApplicationID),
zap.Int("releasing placeholders", len(toRelease)),
zap.Int("pending placeholders", len(pendingRelease)),
+ zap.Int("placeholders already preempted while tried to
release", preempted),
Review Comment:
shorten text to `preempted` we should not repeat `placeholders in the two
lines above either
##########
pkg/scheduler/objects/application.go:
##########
@@ -330,15 +330,30 @@ func (sa *Application) timeoutStateTimer(expectedState
string, event application
zap.String("state", sa.stateMachine.Current()))
// if the app is completing, but there are placeholders
left, first do the cleanup
if sa.IsCompleting() &&
!resources.IsZero(sa.allocatedPlaceholder) {
+ replacing := 0
+ preempted := 0
var toRelease []*Allocation
for _, alloc := range
sa.getPlaceholderAllocations() {
// skip over the allocations that are
already marked for release
if alloc.IsReleased() {
+ replacing++
+ continue
+ }
+ err := alloc.SetReleased(true)
+ if err != nil {
+
log.Log(log.SchedApplication).Warn("allocation is already preempted, so
skipping release process",
+
zap.String("applicationID", sa.ApplicationID),
+
zap.String("allocationKey", alloc.GetAllocationKey()))
+ preempted++
continue
}
- alloc.SetReleased(true)
toRelease = append(toRelease, alloc)
}
+ log.Log(log.SchedApplication).Info("application
is getting timed out, releasing allocated placeholders",
+ zap.String("AppID", sa.ApplicationID),
+ zap.Int("placeholders being replaced",
replacing),
+ zap.Int("placeholders already preempted
while tried to release", preempted),
Review Comment:
shorten text in the log: `replaced`, `preempted`
--
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]
