-
Notifications
You must be signed in to change notification settings - Fork 5.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
disttask: refactor task step and batch subtask dispatching #46957
Conversation
/ok-to-test |
Hi @D3Hunter. Thanks for your PR. PRs from untrusted users cannot be marked as trusted with I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #46957 +/- ##
================================================
- Coverage 73.2442% 73.0326% -0.2117%
================================================
Files 1332 1357 +25
Lines 397646 406299 +8653
================================================
+ Hits 291253 296731 +5478
- Misses 87747 90955 +3208
+ Partials 18646 18613 -33
Flags with carried forward coverage won't be shown. Click here to find out more.
|
/retest |
@@ -94,6 +91,7 @@ func TestFrameworkDynamicBasic(t *testing.T) { | |||
} | |||
|
|||
func TestFrameworkDynamicHA(t *testing.T) { | |||
t.Skip("skip this test because the function is not implemented yet") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why skip it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reverted e573f2d
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: wjhuang2016, ywqzzy The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
[LGTM Timeline notifier]Timeline:
|
/retest |
/retest |
What problem does this PR solve?
Issue Number: ref #46258
Problem Summary:
since subtask metas might be too large to save in one transaction,
we plan to save subtasks in batch later, so add this:
invariant: task.Step always means the most recent step that all
corresponding subtasks have been saved to system table.
when all subtasks of task.Step is finished, we call OnNextSubtasksBatch
to generate subtasks of next step. after all subtasks of next step are
saved to system table, we will update task.Step to next step, so the
invariant hold.
What is changed and how it works?
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.