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.

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

Add page for displaying compaction jobs computed from bucket-index. #7381

Merged
merged 9 commits into from
Feb 14, 2024

Conversation

pstibrany
Copy link
Member

@pstibrany pstibrany commented Feb 14, 2024

What this PR does

This PR adds admin page that displays compaction jobs computed from bucket index. This is same output as produced by tools/compaction-planner tool, without the need to provide all necessary configuration via CLI flags, since compactor already has all the required configuration.

Page allows to experiment with different merge shards and split counts, and it can also show which compactor is expected to own (and run) the job based on current state of the ring. (Screenshot was created with only one compactor in the ring)

screenshot

Checklist

  • Tests updated.
  • Documentation added.
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
  • [na] about-versioning.md updated with experimental features.

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
@pstibrany pstibrany requested review from jdbaldry and a team as code owners February 14, 2024 12:45
Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
Copy link
Member

@jdbaldry jdbaldry left a comment

Choose a reason for hiding this comment

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

Some nits for the docs changes

docs/sources/mimir/references/http-api/index.md Outdated Show resolved Hide resolved
docs/sources/mimir/references/http-api/index.md Outdated Show resolved Hide resolved
pstibrany and others added 2 commits February 14, 2024 14:46
Co-authored-by: Jack Baldry <jack.baldry@grafana.com>
Co-authored-by: Jack Baldry <jack.baldry@grafana.com>
Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

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

I believe I found a typo in one of the pages.

pkg/api/api.go Outdated Show resolved Hide resolved
pkg/compactor/compactor_tenants.gohtml Outdated Show resolved Hide resolved
pkg/compactor/planned_jobs.gohtml Show resolved Hide resolved
pkg/compactor/planned_jobs.gohtml Outdated Show resolved Hide resolved
Copy link
Member

@jdbaldry jdbaldry left a comment

Choose a reason for hiding this comment

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

Docs changes LGTM, I'll leave the rest of the files for maintainers to review :)

pstibrany and others added 2 commits February 14, 2024 15:25
Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>
Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>
@pstibrany
Copy link
Member Author

I believe I found a typo in one of the pages.

Congrats, but that's not that difficult in my texts :)

Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

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

Generally LGTM, see comments.

<table border="1" cellpadding="5" style="border-collapse: collapse;">
<thead>
<tr>
<th>Job Number</th>
Copy link
Contributor

Choose a reason for hiding this comment

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

[Nit] Don't we use sentence case also in our UI titles?

pkg/compactor/planned_jobs.gohtml Show resolved Hide resolved
pkg/compactor/planned_jobs.gohtml Show resolved Hide resolved
pkg/compactor/planned_jobs_http.go Show resolved Hide resolved
pkg/compactor/planned_jobs_http.go Show resolved Hide resolved
pkg/compactor/planned_jobs_http.go Show resolved Hide resolved
pkg/compactor/planned_jobs_http.go Show resolved Hide resolved
pkg/compactor/planned_jobs_http.go Show resolved Hide resolved
pkg/compactor/planned_jobs_http.go Show resolved Hide resolved
Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>
Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
@pstibrany pstibrany enabled auto-merge (squash) February 14, 2024 15:28
@pstibrany pstibrany merged commit ca35211 into main Feb 14, 2024
28 checks passed
@pstibrany pstibrany deleted the planned-compactor-jobs-page branch February 14, 2024 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants