chore(blooms)!: Remove bloom compactor component #13969
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
type/docs
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
force-pushed
the
chaudum/remove-bloom-compactor
branch
from
194f560 to
a4b4018
Compare
Signed-off-by: Christian Haudum <christian.haudum@gmail.com>
chaudum
changed the title
salvacorts
approved these changes
Aug 29, 2024
| 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"` |
There was a problem hiding this comment.
Thanks for making these changes!
Signed-off-by: Christian Haudum <christian.haudum@gmail.com>
7 tasks
pascal-sochacki
pushed a commit
to pascal-sochacki/loki
that referenced
this pull request
Aug 29, 2024
**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 of per-tenant settings changed their prefix from `-bloom-compactor.*` to `-bloom-build.*`. Other settings for the compactor component itself, also prefixed with `-bloom-compactor.*` were removed. See upgrade docs for further information. **Special notes for your reviewer**: :heavy_check_mark: Part of grafana#13957 📔 Documentation update grafana#13965⚠️ Integrating the bloom planner and builder into the `backend` target is done in a follow-up PR. --- Signed-off-by: Christian Haudum <christian.haudum@gmail.com>
pascal-sochacki
pushed a commit
to pascal-sochacki/loki
that referenced
this pull request
Aug 29, 2024
**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 of per-tenant settings changed their prefix from `-bloom-compactor.*` to `-bloom-build.*`. Other settings for the compactor component itself, also prefixed with `-bloom-compactor.*` were removed. See upgrade docs for further information. **Special notes for your reviewer**: :heavy_check_mark: Part of grafana#13957 📔 Documentation update grafana#13965⚠️ Integrating the bloom planner and builder into the `backend` target is done in a follow-up PR. --- Signed-off-by: Christian Haudum <christian.haudum@gmail.com>
pascal-sochacki
pushed a commit
to pascal-sochacki/loki
that referenced
this pull request
Aug 29, 2024
**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 of per-tenant settings changed their prefix from `-bloom-compactor.*` to `-bloom-build.*`. Other settings for the compactor component itself, also prefixed with `-bloom-compactor.*` were removed. See upgrade docs for further information. **Special notes for your reviewer**: :heavy_check_mark: Part of grafana#13957 📔 Documentation update grafana#13965⚠️ Integrating the bloom planner and builder into the `backend` target is done in a follow-up PR. --- Signed-off-by: Christian Haudum <christian.haudum@gmail.com>
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)
7 tasks
This was referenced Oct 21, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
backendtarget of the simple scalable deployment.It does not mean that adding planner/builder to the
backendtarget 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
CONTRIBUTING.mdguide (required)featPRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.docs/sources/setup/upgrade/_index.mdproduction/helm/loki/Chart.yamland updateproduction/helm/loki/CHANGELOG.mdandproduction/helm/loki/README.md. Example PRdeprecated-config.yamlanddeleted-config.yamlfiles respectively in thetools/deprecated-config-checkerdirectory. Example PR