[
https://issues.apache.org/jira/browse/YUNIKORN-3360?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18106476#comment-18106476
]
Dale Richardson commented on YUNIKORN-3360:
-------------------------------------------
[~wilfreds]
The new guard surfaced a semantic conflict with tests merged two days later in
YUNIKORN-3352 (tracked in YUNIKORN-3398, test-only fix). While looking at that
I think two aspects of this change deserve a follow-up look.
1. ** The state where {{sa.allocations}} holds the key but {{sa.requests}} does
not is reachable, because ask removal and allocation removal are deliberately
decoupled: {{removeAsksInternal}} drops the asks immediately, while
{{sa.allocations}} entries survive until the shim confirms the releases. Any
{{TerminationType_SCHEDULING_FAILED_ON_RM}} release arriving in that window
reaches {{RollbackAllocation}} with the ask gone. One concrete path: a
soft-gang app in Running with zero allocated placeholders falls into
{{timeoutPlaceholderProcessing}} case 2, fires {{{}ResumeApplication{}}}, which
is only valid from New/Accepted - the transition fails, the app stays Running
(still rollback-eligible), and {{removeAsksInternal("")}} has wiped the asks.
{{RemoveAllocationAsk}} on an allocated key opens the same window.
2. ** {{SCHEDULING_FAILED_ON_RM}} bypasses the normal remove-and-destroy path
entirely ({{{}partition.removeAllocation{}}} routes it straight to
{{{}rollbackAllocation{}}}), and on error the caller only logs a warning. So
the allocation stays in {{sa.allocations}} and {{{}sa.allocatedResource{}}},
the node allocation is not removed, the queue's allocated resource is not
decremented, and the partition allocation count is not adjusted - while the
shim considers the release processed (the rollback path intentionally sends no
confirmation back). Cleanup then depends on some later release arriving for the
same key, e.g. from a pod delete.
To be clear, I don't think reverting the current fix is the answer as before
the current guard was added a rollback in that window had its own accounting
error: {{deallocateAsk}} unconditionally adds the ask's resource back to app
and queue pending, so an ask nothing tracks any more permanently inflated
pending (terminal cleanup walks {{sa.requests}} and never finds it). Neither
before nor after is the ghost case handled correctly.
The correct handling for a rollback whose ask is no longer tracked is probably
to proceed with the allocated-side cleanup (remove from {{{}sa.allocations{}}},
decrement allocated resources, node, queue, partition count) but skip returning
the ask to pending - there is nothing left to re-schedule. That could live in
{{RollbackAllocation}} itself, or the caller could fall through to the normal
remove path when the ask is untracked. Happy to put up a patch if there's
agreement on the direction.
One judgement call to flag: the last sentence offers a patch — leave it in only
if you actually want to pick that work up; otherwise end at "handled correctly"
and let the 3360 participants propose the direction. Also, if you'd rather
split concerns, paragraph 1–2 (reachability + dropped release) stand alone as
the bug report and paragraphs 3–4 could go in a second comment proposing the
fix — but as one comment it reads fine and avoids a fragmented thread.
> RollbackAllocation should safety check request existence
> ---------------------------------------------------------
>
> Key: YUNIKORN-3360
> URL: https://issues.apache.org/jira/browse/YUNIKORN-3360
> Project: Apache YuniKorn
> Issue Type: Task
> Components: core - scheduler
> Reporter: Wilfred Spiegelenburg
> Assignee: Aditya Maheshwari
> Priority: Major
> Labels: pull-request-available
> Fix For: 1.10.0
>
>
> When RollbackAllocation is called the only check that is made is for the
> existence in the allocations map. There should be a check against the
> requests map also to make sure the allocation is looked at.
> The ask should always be there but we need to be consistent with other code
> paths which do this check.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
