Skip to content
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

chore(block-scheduler): planner improvements to limit jobs and consider retention when planning #15432

Merged
merged 5 commits into from
Dec 17, 2024
Merged

chore(block-scheduler): planner improvements to limit jobs and consider retention when planning #15432

merged 5 commits into from
Dec 17, 2024
+81 −30

Conversation

ashwanthgoli
Copy link
Contributor

@ashwanthgoli ashwanthgoli commented Dec 17, 2024 •
edited
Loading

What this PR does / why we need it:

  • Adds config to limit the number of jobs that can be returned every time we call plan. Without this, the planner could overwhelm the job queue by adding a large number of jobs if the partition is lagging.
  • Last committed offset could be lower than the start offset if we are failing to catch-up with retention. Updates the planner to consider this when planning jobs. This is very unlikely to happen if the builders are actively running.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Checklist

  • Reviewed the CONTRIBUTING.md guide (required)
  • Documentation added
  • Tests updated
  • Title matches the required conventional commits format, see here
    • Note that Promtail is considered to be feature complete, and future development for logs collection will be in Grafana Alloy. As such, feat PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
  • If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

@pull-request-size pull-request-size bot added size/L and removed size/M labels Dec 17, 2024
@ashwanthgoli ashwanthgoli marked this pull request as ready for review December 17, 2024 05:47
@ashwanthgoli ashwanthgoli requested a review from a team as a code owner December 17, 2024 05:47
@github-actions github-actions bot added the type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories label Dec 17, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 17, 2024 •
edited
Loading

💻 Deploy preview deleted.

benclive
benclive approved these changes Dec 17, 2024
View reviewed changes
pkg/blockbuilder/scheduler/strategy.go
@@ -84,6 +90,7 @@ func (p *RecordCountPlanner) Plan(ctx context.Context) ([]*JobWithMetadata, erro
jobs = append(jobs, job)

currentStart = currentEnd
Copy link
Contributor

Choose a reason for hiding this comment

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

Not part of this PR but just wanted to check: Does this overlap one record because end of one job == start of next job?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, but Max offset of the job is exclusive

@ashwanthgoli ashwanthgoli merged commit fe50c72 into main Dec 17, 2024
61 checks passed
@ashwanthgoli ashwanthgoli deleted the scheduler-improvements branch December 17, 2024 13:03
mveitas pushed a commit to mveitas/loki that referenced this pull request Jan 6, 2025
@ashwanthgoli @mveitas
chore(block-scheduler): planner improvements to limit jobs and consid…
ab70b9b 
…er retention when planning (grafana#15432)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benclive benclive benclive approved these changes

Assignees
No one assigned
Labels
size/L type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ashwanthgoli @benclive