Skip to content

Configurable assigned task limit per Network #873

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.

Sign up for GitHub

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

Jump to bottom
Merged
4meta5 merged 11 commits into from
May 17, 2024
Merged

Configurable assigned task limit per Network #873

4meta5 merged 11 commits into from
May 17, 2024

Conversation

@4meta5
Copy link
Contributor

@4meta5 4meta5 commented Apr 29, 2024 •
edited
Loading

Closes #871

  • configurable per network via root gated extrinsic set_shard_task_limit
  • if new_limit < current_limit, the shard will not be assigned any more tasks until assigned less than new_limit
  • number of assigned tasks per shard does not include completed tasks
  • testing

@4meta5 4meta5 self-assigned this Apr 29, 2024
@4meta5 4meta5 mentioned this pull request May 1, 2024
Closed
@4meta5 4meta5 added ready-for-review PR author(s) done with coding and reviewer[s] can start and removed WIP labels May 1, 2024
@4meta5 4meta5 requested a review from dvc94ch May 1, 2024 18:26
@4meta5 4meta5 added this to the Milestone Q2 2024 milestone May 1, 2024
dvc94ch
dvc94ch approved these changes May 16, 2024
View reviewed changes
Copy link
Contributor

@dvc94ch dvc94ch left a comment

Choose a reason for hiding this comment

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

lgtm

pallets/tasks/src/lib.rs
if capacity.is_zero() {
// no new tasks assigned if capacity reached or exceeded
return;
}
Copy link
Contributor

@dvc94ch dvc94ch May 16, 2024

Choose a reason for hiding this comment

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

we already handle the zero case later no?

Copy link
Contributor Author

@4meta5 4meta5 May 17, 2024

Choose a reason for hiding this comment

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

We take(capacity) below after filtering for tasks that are assignable. I think returning early is better if capacity is 0 rather than going through all the tasks just to take 0 of them afterwards.

@4meta5 4meta5 merged commit c9eb3e2 into development May 17, 2024
@4meta5 4meta5 deleted the amar-configurable-assigned-task-limit-per-shard branch May 17, 2024 04:59
@penumbra23 penumbra23 mentioned this pull request May 17, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@dvc94ch dvc94ch dvc94ch approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@4meta5 4meta5

Labels

enhancement New feature or request priority: high ready-for-review PR author(s) done with coding and reviewer[s] can start status: confirmed

Projects

None yet

Milestone

Milestone Q2 2024

Development

Successfully merging this pull request may close these issues.

Make number of assigned tasks configurable

3 participants

@4meta5 @dvc94ch