Skip to content

workloadmanager: prevent inactive session GC starvation - #502

Open
ANAMASGARD wants to merge 1 commit into
volcano-sh:mainfrom
ANAMASGARD:fix/454-gc-candidate-pagination
Open

ANAMASGARD wants to merge 1 commit into
volcano-sh:mainfrom
ANAMASGARD:fix/454-gc-candidate-pagination

Conversation

@ANAMASGARD

Copy link
Copy Markdown

What type of PR is this?

/kind bug

What this PR does / why we need it:

GC currently filters only the first 100 inactive sessions by their individual idle timeouts. When those sessions have long timeouts, an eligible session behind them is never examined.

This change adds Redis and Valkey ZSCAN traversal, retaining the cursor and pending session IDs across GC runs. Each candidate's session record and activity score are refreshed before evaluating its timeout. Empty pages, duplicate results, cancellation, and individual failures do not prevent traversal of later candidates. Expiry processing runs first with a bounded budget inside the existing two-minute GC deadline.

Existing listing methods and storage formats are preserved. Orphan index removal remains separate from #495.

Validation:

  • Confirmed the 101-session regression fails before the fix and passes afterward.
  • Passed go test -mod=readonly ./pkg/... and go test -mod=readonly -race ./pkg/....
  • Passed go vet -mod=readonly ./..., go build -mod=readonly ./cmd/..., make fmt-check, and make lint.
  • Passed native Redis 7.2.7 and Valkey 8.0.2 integration tests with race detection, including deletion of 512 sessions with identical activity timestamps across scan pages.

Which issue(s) this PR fixes:

Fixes #454

Special notes for your reviewer:

Prepared with AI assistance using Codex. The commit includes a DCO sign-off.

Progress is in memory and resets on restart. ZSCAN traverses active index entries too. Fresh activity reads do not make Kubernetes deletion atomic with concurrent activity updates. Native-store tests use a fake Kubernetes client; full cluster E2E was not run because the Docker daemon was unavailable. Reproduction commands and design tradeoffs are documented in docs/research/gc-candidate-pagination.md.

Does this PR introduce a user-facing change?:

Fix inactive-session garbage collection starvation behind the first 100 candidates by resuming Redis and Valkey scans across GC runs.

Signed-off-by: Gaurav Chaudhary <chaudharygaurav2004@gmail.com>
Copilot AI balanced review requested due to automatic review settings November 3, 2026 12:21
@volcano-sh-bot volcano-sh-bot added the kind/bug Something isn't working label Nov 3, 2026
@volcano-sh-bot

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign hzxuzhonghu, yaozengzeng 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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 86.04651% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.41%. Comparing base (f521ae9) to head (9cae48b).
⚠️ Report is 25 commits behind head on main.

Files with missing lines Patch % Lines
pkg/workloadmanager/garbage_collection.go 86.84% 5 Missing and 5 partials ⚠️
pkg/store/gc_scan.go 63.63% 2 Missing and 2 partials ⚠️
pkg/store/store_redis.go 90.47% 1 Missing and 1 partial ⚠️
pkg/store/store_valkey.go 90.47% 1 Missing and 1 partial ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #502      +/-   ##
==========================================
+ Coverage   60.45%   63.41%   +2.96%     
==========================================
  Files          35       36       +1     
  Lines        3616     3742     +126     
==========================================
+ Hits         2186     2373     +187     
+ Misses       1211     1139      -72     
- Partials      219      230      +11     
Flag Coverage Δ
unittests 63.41% <86.04%> (+2.96%) ⬆️

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

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

# GC candidate pagination

Research date: 2026-11-03. Tracking issue: [#454](https://github.com/volcano-sh/agentcube/issues/454).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please remove this file from the PR. As I understand it, it’s for agent analysis and shouldn’t be maintained in this repository.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Something isn't working size/XXL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GC can starve eligible sessions behind the 100-item candidate limit

5 participants