Skip to content

chore(blooms)!: Remove bloom compactor component - #13969

Merged
chaudum merged 8 commits into
mainfrom
chaudum/remove-bloom-compactor
Aug 29, 2024
Merged

chore(blooms)!: Remove bloom compactor component#13969
chaudum merged 8 commits into
mainfrom
chaudum/remove-bloom-compactor

Conversation

@chaudum

@chaudum chaudum commented Aug 27, 2024

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

This commit removes the code related to the bloom compactor which is superseded by the bloom planner and builders.

A handful of CLI arguments changed their prefixes from -bloom-compactor.* to -bloom-build.*.

Special notes for your reviewer:

✔️ Part of #13957

📔 Documentation update #13965

⚠️ Merging this PR is currently blocked by the fact that removing the bloom compactor is removing the blooms write path from the backend target of the simple scalable deployment.
It does not mean that adding planner/builder to the backend target needs to be part of this PR, but there should be a clear path how this is going to be implemented to have a path forward (PoC)

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
  • For Helm chart changes bump the Helm chart version in production/helm/loki/Chart.yaml and update production/helm/loki/CHANGELOG.md and production/helm/loki/README.md. Example PR
  • 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
@github-actions github-actions Bot added the type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories label Aug 27, 2024
This commit removes the code related to the bloom compactor which is
superseded by the bloom planner and builders.

A handful of CLI arguments changed their prefixes from
`-bloom-compactor.*` to `-bloom-build.*`.

Part of #13957

Documentation update #13965

Signed-off-by: Christian Haudum <christian.haudum@gmail.com>
Signed-off-by: Christian Haudum <christian.haudum@gmail.com>
Signed-off-by: Christian Haudum <christian.haudum@gmail.com>
Signed-off-by: Christian Haudum <christian.haudum@gmail.com>
…ponent

Signed-off-by: Christian Haudum <christian.haudum@gmail.com>
@chaudum
chaudum force-pushed the chaudum/remove-bloom-compactor branch from 194f560 to a4b4018 Compare August 27, 2024 14:45
Signed-off-by: Christian Haudum <christian.haudum@gmail.com>
@chaudum
chaudum marked this pull request as ready for review August 29, 2024 09:13
@chaudum
chaudum requested a review from a team as a code owner August 29, 2024 09:13
@chaudum chaudum changed the title chore(blooms): Remove bloom compactor component Aug 29, 2024
Comment thread pkg/validation/limits.go
BloomCompactorEnabled bool `yaml:"bloom_compactor_enable_compaction" json:"bloom_compactor_enable_compaction" category:"experimental"`
BloomCompactorMaxBlockSize flagext.ByteSize `yaml:"bloom_compactor_max_block_size" json:"bloom_compactor_max_block_size" category:"experimental"`
BloomCompactorMaxBloomSize flagext.ByteSize `yaml:"bloom_compactor_max_bloom_size" json:"bloom_compactor_max_bloom_size" category:"experimental"`
BloomBuildMaxBuilders int `yaml:"bloom_build_max_builders" json:"bloom_build_max_builders" category:"experimental"`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for making these changes!

@chaudum
chaudum merged commit b75eacc into main Aug 29, 2024
@chaudum
chaudum deleted the chaudum/remove-bloom-compactor branch August 29, 2024 11:28
chaudum added a commit that referenced this pull request Sep 2, 2024
…#13997)

Previously, the bloom compactor component was part of the `backend` target in the Simple Scalable Deployment (SSD) mode. However, the bloom compactor was removed (#13969) in favour of planner and builder, and therefore also removed from the backend target.

This PR adds the planner and builder components to the backend target so it can continue building blooms if enabled.

The planner needs to be run as singleton, therefore there must only be one instance that creates tasks for the builders, even if multiple replicas of the backend target are deployed.
This is achieved by leader election through the already existing index gateway ring in the backend target. The planner leader is determined by the ownership of the leader key. Builders connect to the planner leader to pull tasks.

----

Signed-off-by: Christian Haudum <christian.haudum@gmail.com>
grafanabot pushed a commit that referenced this pull request Sep 10, 2024
…#13997)

Previously, the bloom compactor component was part of the `backend` target in the Simple Scalable Deployment (SSD) mode. However, the bloom compactor was removed (#13969) in favour of planner and builder, and therefore also removed from the backend target.

This PR adds the planner and builder components to the backend target so it can continue building blooms if enabled.

The planner needs to be run as singleton, therefore there must only be one instance that creates tasks for the builders, even if multiple replicas of the backend target are deployed.
This is achieved by leader election through the already existing index gateway ring in the backend target. The planner leader is determined by the ownership of the leader key. Builders connect to the planner leader to pull tasks.

----

Signed-off-by: Christian Haudum <christian.haudum@gmail.com>
(cherry picked from commit bf60455)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature/blooms size/XXL type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories

2 participants