Skip to content

scheduler: abstract nextPod and gang inplements it#2417

Merged
koordinator-bot[bot] merged 1 commit intokoordinator-sh:mainfrom
ZiMengSheng:gangnextpod_upstream
May 19, 2025
Merged

scheduler: abstract nextPod and gang inplements it#2417
koordinator-bot[bot] merged 1 commit intokoordinator-sh:mainfrom
ZiMengSheng:gangnextpod_upstream

Conversation

@ZiMengSheng
Copy link
Copy Markdown
Member

@ZiMengSheng ZiMengSheng commented Apr 22, 2025 •
edited
Loading

Ⅰ. Describe what this PR does

Provide strong guarantee for gang departure.

Ⅱ. Does this pull request fix one issue?

Ⅲ. Describe how to verify it

After this PR,

  1. Coscheduling need to be removed from QueueSort Extension Point.
  2. SchedulerQueueingHints Featuregate can be enabled safely.
    In addision,
  3. Coscheduling need to be add to PreEnqueue Extension Point.
  4. The Coscheduling plugin needs to be placed before the plugin that may successfully preempt the event in PostFilter to ensure that it will be executed if scheduling fails.

Ⅳ. Special notes for reviews

V. Checklist

  • I have written necessary docs and comments
  • I have added necessary unit tests and integration tests
  • All checks passed in make test

@koordinator-bot koordinator-bot Bot requested review from buptcozy and eahydra April 22, 2025 01:56
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 22, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 36.55352% with 243 lines in your changes missing coverage. Please review.

Project coverage is 65.85%. Comparing base (44319ca) to head (99cbf7c).
Report is 57 commits behind head on main.

Files with missing lines Patch % Lines
pkg/scheduler/plugins/coscheduling/core/core.go 26.66% 73 Missing and 4 partials ⚠️
...heduler/frameworkext/framework_extender_factory.go 47.12% 44 Missing and 2 partials ⚠️
pkg/scheduler/plugins/coscheduling/core/gang.go 47.05% 35 Missing and 1 partial ⚠️
pkg/scheduler/plugins/coscheduling/coscheduling.go 5.55% 34 Missing ⚠️
...cheduler/plugins/coscheduling/core/gang_context.go 0.00% 21 Missing ⚠️
...g/scheduler/plugins/coscheduling/core/ganggroup.go 65.11% 13 Missing and 2 partials ⚠️
.../scheduler/plugins/coscheduling/core/gang_cache.go 53.33% 7 Missing ⚠️
...cheduler/plugins/coscheduling/core/gang_summary.go 0.00% 7 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2417      +/-   ##
==========================================
- Coverage   66.07%   65.85%   -0.23%     
==========================================
  Files         470      471       +1     
  Lines       55562    55534      -28     
==========================================
- Hits        36715    36570     -145     
- Misses      16198    16321     +123     
+ Partials     2649     2643       -6
Flag Coverage Δ
unittests 65.85% <36.55%> (-0.23%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 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.

saintube
saintube reviewed Apr 22, 2025
View reviewed changes
Comment thread pkg/scheduler/frameworkext/framework_extender_factory.go
Comment thread pkg/scheduler/frameworkext/framework_extender_factory.go
@ZiMengSheng ZiMengSheng force-pushed the gangnextpod_upstream branch 7 times, most recently from 766d123 to d7c7429 Compare April 24, 2025 03:39
@ZiMengSheng ZiMengSheng changed the title scheduler: abstract nextPod and make it obey gang scheduler: abstract nextPod and gang inplements it Apr 24, 2025
@ZiMengSheng ZiMengSheng force-pushed the gangnextpod_upstream branch 4 times, most recently from 277bda0 to 994b76d Compare April 24, 2025 09:24
KunWuLuan
KunWuLuan reviewed Apr 25, 2025
View reviewed changes
Comment thread pkg/scheduler/frameworkext/framework_extender_factory.go
saintube
saintube reviewed Apr 27, 2025
View reviewed changes
Comment thread config/manager/scheduler-config.yaml
saintube
saintube reviewed Apr 27, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@saintube saintube left a comment

Choose a reason for hiding this comment

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

lgtm

KunWuLuan
KunWuLuan reviewed Apr 27, 2025
View reviewed changes
Comment thread pkg/scheduler/plugins/coscheduling/core/gang_context.go Outdated
@ZiMengSheng ZiMengSheng force-pushed the gangnextpod_upstream branch 7 times, most recently from 1e31080 to 60e2ab6 Compare April 29, 2025 03:40
@KunWuLuan
Copy link
Copy Markdown
Member

KunWuLuan commented May 6, 2025

lgtm

@saintube
Copy link
Copy Markdown
Member

saintube commented May 9, 2025

/lgtm

@ZiMengSheng ZiMengSheng force-pushed the gangnextpod_upstream branch from 60e2ab6 to c4714f4 Compare May 14, 2025 07:56
@koordinator-bot koordinator-bot Bot removed the lgtm label May 14, 2025
@ZiMengSheng ZiMengSheng force-pushed the gangnextpod_upstream branch 6 times, most recently from 6549076 to af8e277 Compare May 16, 2025 16:00
@ZiMengSheng
Copy link
Copy Markdown
Member Author

ZiMengSheng commented May 16, 2025

@saintube PTAL

@ZiMengSheng ZiMengSheng force-pushed the gangnextpod_upstream branch 2 times, most recently from 06cf4fe to c161ad6 Compare May 19, 2025 02:03
saintube
saintube reviewed May 19, 2025
View reviewed changes
Comment thread pkg/scheduler/metrics/metrics.go Outdated
@ZiMengSheng ZiMengSheng force-pushed the gangnextpod_upstream branch from c161ad6 to 511f995 Compare May 19, 2025 03:17
saintube
saintube approved these changes May 19, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@saintube saintube left a comment

Choose a reason for hiding this comment

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

/lgtm

@koordinator-bot koordinator-bot Bot added the lgtm label May 19, 2025
@ZiMengSheng ZiMengSheng force-pushed the gangnextpod_upstream branch from 511f995 to 99cbf7c Compare May 19, 2025 05:55
@koordinator-bot koordinator-bot Bot removed the lgtm label May 19, 2025
@saintube
Copy link
Copy Markdown
Member

saintube commented May 19, 2025

/lgtm

@koordinator-bot koordinator-bot Bot added the lgtm label May 19, 2025
@ZiMengSheng
Copy link
Copy Markdown
Member Author

ZiMengSheng commented May 19, 2025

/approve

@koordinator-bot
Copy link
Copy Markdown

koordinator-bot Bot commented May 19, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: saintube, ZiMengSheng

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

The pull request process is described 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

@koordinator-bot koordinator-bot Bot merged commit 13f4065 into koordinator-sh:main May 19, 2025
22 checks passed
@ZiMengSheng ZiMengSheng mentioned this pull request May 23, 2025
Closed
3 tasks
@ZiMengSheng ZiMengSheng mentioned this pull request Jun 25, 2025
Closed
@ZiMengSheng ZiMengSheng added this to the v1.7 milestone Jul 14, 2025
This was referenced Dec 19, 2025
Open
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@saintube saintube saintube approved these changes

@buptcozy buptcozy Awaiting requested review from buptcozy

@eahydra eahydra Awaiting requested review from eahydra

+1 more reviewer

@KunWuLuan KunWuLuan KunWuLuan left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@saintube saintube

Labels

approved area/koord-scheduler lgtm size/XXL

Projects

None yet

Milestone

v1.7

Development

Successfully merging this pull request may close these issues.

5 participants

@ZiMengSheng @KunWuLuan @saintube @songtao98 @wangjianyu-qiaopu