Skip to content

fix: retain terminating pod in cache to prevent premature eviction#1719

Open
maishivamhoo123 wants to merge 2 commits intoProject-HAMi:masterfrom
maishivamhoo123:fix/retain-terminating-pod-cache
Open

fix: retain terminating pod in cache to prevent premature eviction#1719
maishivamhoo123 wants to merge 2 commits intoProject-HAMi:masterfrom
maishivamhoo123:fix/retain-terminating-pod-cache

Conversation

@maishivamhoo123
Copy link
Copy Markdown
Contributor

The Fix:
This PR updates the scheduler to retain pods in the device cache while they are in the Terminating state. The cache will now only evict the pod once the Kubelet fully reports it as terminated (reaching a Succeeded or Failed phase).

Fixes #1368

Verification / Testing Performed

  • Ran standard unit tests via make test (Tests for scheduler and util passed).
  • Verified full pod lifecycle state transitions in scheduler_test.go.
  • Ran linter via make verify .

Signed-off-by: maishivamhoo123 <maishivamhoo@gmail.com>
@hami-robot
Copy link
Copy Markdown
Contributor

hami-robot bot commented Mar 29, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: maishivamhoo123
Once this PR has been reviewed and has the lgtm label, please assign wawa0210 for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@github-actions github-actions bot added the kind/bug Something isn't working label Mar 29, 2026
@hami-robot hami-robot bot added the size/M label Mar 29, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request addresses issue #1368 by preventing terminating pods from being prematurely removed from the scheduler's cache. It introduces a new IsPodTerminating utility function and includes tests to verify the behavior. Feedback highlights that while the early return preserves the cache entry, it leaves the cached pod object in a stale state because the DeletionTimestamp is not updated, which may cause issues for logic expecting current pod metadata.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 29, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

Flag Coverage Δ
unittests 52.09% <100.00%> (+0.16%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
pkg/device/pods.go 47.87% <100.00%> (+5.51%) ⬆️
pkg/scheduler/scheduler.go 42.49% <100.00%> (+2.02%) ⬆️
pkg/util/util.go 75.17% <100.00%> (+0.35%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: maishivamhoo123 <maishivamhoo@gmail.com>
@maishivamhoo123
Copy link
Copy Markdown
Contributor Author

@archlitchi @Shouren @FouoF and @team can you please review this PR?

@archlitchi
Copy link
Copy Markdown
Member

@maishivamhoo123 could you tell me how this PR fixes the issue #1368?

That issue seems to address that pod in pending state will not try to be scheduled again before around 5 minutes even after pod terminated and release some resources

@maishivamhoo123
Copy link
Copy Markdown
Contributor Author

@archlitchi @Shouren Thank you for the review.

Before this PR
func onAddPod
Pod A starts terminating and receives a DeletionTimestamp.
HAMi immediately removes Pod A from its internal device cache.
Because the cache is empty, HAMi incorrectly reports to the kube-scheduler that the Node's resources are free.
The kube-scheduler attempts to schedule and bind Pending Pod B to that Node.
The Kubelet has not finished tearing down Pod A, so the physical resources remain locked and the binding fails.
Due to the binding failure, the kube-scheduler places Pod B into its exponential backoff queue, causing the 5-minute delay.

After this PR

Pod A starts terminating and receives a DeletionTimestamp.
The util.IsPodTerminating(pod) logic detects this. HAMi retains Pod A in the cache and updates its state instead of deleting it.
HAMi accurately reports to the kube-scheduler that the Node's resources are still in use.
Pending Pod B waits safely in the active scheduling queue without triggering any errors.
Pod A fully terminates (reaching Succeeded or Failed phase), and the Kubelet releases the physical resources.
HAMi evicts Pod A from the cache. The kube-scheduler successfully binds Pod B immediately, bypassing the 5-minute backoff penalty.

@maishivamhoo123 maishivamhoo123 requested a review from Shouren March 30, 2026 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HAMi Scheduler Not Trying to Schedule Previously Pending Workload for 5mins

3 participants